add inbox-watch plugin - #363
Open
javimosch wants to merge 1 commit into
Open
Conversation
inbox-watch polls GitHub, IMAP and Resend inbound and reports only items new since the last run, classifying DMARC reports and out-of-office autoreplies apart from actual human replies. https://github.com/javimosch/inbox-watch Two design points worth the reviewer's attention: 1. inbox/check and inbox/peek are separate commands on purpose. The cursor is consumed on read, so a normal run eats the alert a cron would have sent; peek reports identically without committing state. The destructive one should never be the one reached for while debugging. 2. All inbox commands pass --exit-zero. inbox-watch exits 10 for "new items", which is a SUCCESS signal for a shell caller, but the process adapter treats any non-zero exit as failure -- so routed naively the one case that matters, mail arrived, would surface as an error. The flag was added upstream for exactly this; JSON consumers read data.count instead. The shell adapter would also have solved (2) by remapping the exit code, but it requires adapterConfig.unsafe=true, which is too high a price for a plugin's primary commands. Tests cover the output contract (versioned envelope, structured disabled entries, exit_codes documenting 10 as success) and that check/peek route distinctly. Verified live against the real binary through supercli; plugins doctor reports the binary check ok. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds the Changesinbox-watch plugin
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/inbox-watch-plugin.test.js`:
- Around line 78-85: Update the fake command response used by the “peek is a
distinct command from check” test to include its calculated peek value, then
parse the JSON output and assert data.data.peek is true so the test verifies
--peek reaches the binary rather than only checking the wrapper command name.
In `@plugins/inbox-watch/install-guidance.json`:
- Around line 5-10: Update the install_steps sequence in install-guidance.json
to add an instruction after copying config.example.json that tells users to edit
~/.inbox-watch/config.json and configure monitoring targets before running the
existing verification step.
In `@plugins/inbox-watch/plugin.json`:
- Around line 110-121: Update the exposed passthrough route’s process adapter
configuration for the “inbox-watch” command to include the same baseArgs
exit-code normalization used by the non-passthrough route, including
--exit-zero. Add or extend a passthrough test to verify that a poll returning
exit code 10 is normalized successfully rather than reported as an adapter
failure.
- Line 22: Update the installation command in the inbox-watch plugin
configuration to use the native sc client instead of supercli, preserving the
existing plugins install arguments and path.
In `@plugins/inbox-watch/skills/quickstart/SKILL.md`:
- Line 90: Update the cron example’s fenced code block in the quickstart
documentation to use the sh language identifier instead of an untyped fence,
preserving the snippet content unchanged.
- Around line 8-10: Adopt the contract that both human replies and labelled
autoreplies trigger alerts, then align every public description accordingly:
update plugins/inbox-watch/skills/quickstart/SKILL.md lines 8-10, 19-23, and 3;
plugins/inbox-watch/meta.json line 2; and plugins/inbox-watch/plugin.json line
4. Remove wording that limits alerts to human replies and ensure the autoreply
row and all metadata consistently describe the selected behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5614723e-b8de-4e93-8f9c-f5048e68ce21
📒 Files selected for processing (5)
__tests__/inbox-watch-plugin.test.jsplugins/inbox-watch/install-guidance.jsonplugins/inbox-watch/meta.jsonplugins/inbox-watch/plugin.jsonplugins/inbox-watch/skills/quickstart/SKILL.md
Comment on lines
+78
to
+85
| test("peek is a distinct command from check", () => { | ||
| // They are separate on purpose: the destructive one must never be the one | ||
| // reached for while debugging, since a normal run consumes the cursor and | ||
| // eats the alert the cron would have sent. | ||
| const r = runNoServer("inbox-watch inbox peek --json", { env: ctx.env }) | ||
| expect(r.ok).toBe(true) | ||
| expect(JSON.parse(r.output).command).toBe("inbox-watch.inbox.peek") | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Assert that --peek reaches the binary.
This only checks the wrapper command name. The fake already calculates peek at Line 24, but never returns it, so removing --peek from the manifest still passes. Include peek in the fake JSON and assert data.data.peek === true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@__tests__/inbox-watch-plugin.test.js` around lines 78 - 85, Update the fake
command response used by the “peek is a distinct command from check” test to
include its calculated peek value, then parse the JSON output and assert
data.data.peek is true so the test verifies --peek reaches the binary rather
than only checking the wrapper command name.
Comment on lines
+5
to
+10
| "install_steps": [ | ||
| "git clone https://github.com/javimosch/inbox-watch /tmp/inbox-watch", | ||
| "install -m755 /tmp/inbox-watch/inbox-watch ~/.local/bin/inbox-watch", | ||
| "mkdir -p ~/.inbox-watch && cp /tmp/inbox-watch/config.example.json ~/.inbox-watch/config.json", | ||
| "Verify: inbox-watch help-json" | ||
| ], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the configuration step to the standalone flow.
After Line 8, instruct users to edit ~/.inbox-watch/config.json before verification; the inline guidance does this, but this standalone path does not. Otherwise a successful install can monitor nothing.
Proposed fix
"mkdir -p ~/.inbox-watch && cp /tmp/inbox-watch/config.example.json ~/.inbox-watch/config.json",
+ "Edit ~/.inbox-watch/config.json (what to watch); secrets go in env vars",
"Verify: inbox-watch help-json"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "install_steps": [ | |
| "git clone https://github.com/javimosch/inbox-watch /tmp/inbox-watch", | |
| "install -m755 /tmp/inbox-watch/inbox-watch ~/.local/bin/inbox-watch", | |
| "mkdir -p ~/.inbox-watch && cp /tmp/inbox-watch/config.example.json ~/.inbox-watch/config.json", | |
| "Verify: inbox-watch help-json" | |
| ], | |
| "install_steps": [ | |
| "git clone https://github.com/javimosch/inbox-watch /tmp/inbox-watch", | |
| "install -m755 /tmp/inbox-watch/inbox-watch ~/.local/bin/inbox-watch", | |
| "mkdir -p ~/.inbox-watch && cp /tmp/inbox-watch/config.example.json ~/.inbox-watch/config.json", | |
| "Edit ~/.inbox-watch/config.json (what to watch); secrets go in env vars", | |
| "Verify: inbox-watch help-json" | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/inbox-watch/install-guidance.json` around lines 5 - 10, Update the
install_steps sequence in install-guidance.json to add an instruction after
copying config.example.json that tells users to edit ~/.inbox-watch/config.json
and configure monitoring targets before running the existing verification step.
| "mkdir -p ~/.inbox-watch && cp /tmp/inbox-watch/config.example.json ~/.inbox-watch/config.json", | ||
| "Edit ~/.inbox-watch/config.json (what to watch); secrets go in env vars", | ||
| "Verify: inbox-watch help-json", | ||
| "supercli plugins install ./plugins/inbox-watch --on-conflict replace --json" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file context =="
sed -n '1,140p' plugins/inbox-watch/plugin.json
echo
echo "== occurrences of native install command syntax =="
rg -n "plugins install|supercli plugins install|sc plugins install|SuperCLI|native" -S plugins README.md . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200
echo
echo "== sc availability =="
command -v sc || true
command -v sc-zig || true
if command -v sc >/tmp/sc_bin 2>/tmp/sc_err; then
bin="$(cat /tmp/sc_bin)"
echo "SC_BIN=$bin"
sc --version 2>/tmp/sc_ver || true
echo "--- sc inspect available via CLI help? ---"
sc --help 2>&1 | rg -n "inspect|install|Native|native|plugins" || true
fi
if command -v sc-zig >/tmp/sc_zig_bin 2>/tmp/sc_zig_err; then
bin="$(cat /tmp/sc_zig_bin)"
echo "SC_ZIG_BIN=$bin"
sc-zig --version 2>/tmp/sc_zig_ver || true
sc-zig --help 2>&1 | rg -n "inspect|install|Native|native|plugins" || true
fi
echo
echo "== repo source inspection for sc supercli aliases =="
rg -n "function sc|alias sc|bin[[:space:]]*=|supercli|SC_CLI|super-cli" -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: javimosch/supercli
Length of output: 32023
Use sc for native plugin installation.
plugins/inbox-watch/plugin.json is a new bundled plugin, and the installation step still uses supercli plugins install. Node.js sc is the native plugin-install client, so update this to sc plugins install ./plugins/inbox-watch --on-conflict replace --json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/inbox-watch/plugin.json` at line 22, Update the installation command
in the inbox-watch plugin configuration to use the native sc client instead of
supercli, preserving the existing plugins install arguments and path.
Source: Coding guidelines
Comment on lines
+110
to
+121
| { | ||
| "namespace": "inbox-watch", | ||
| "resource": "_", | ||
| "action": "_", | ||
| "description": "Passthrough to the inbox-watch CLI", | ||
| "adapter": "process", | ||
| "adapterConfig": { | ||
| "command": "inbox-watch", | ||
| "passthrough": true, | ||
| "missingDependencyHelp": "Install: see https://github.com/javimosch/inbox-watch" | ||
| }, | ||
| "args": [] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize exit 10 for passthrough too.
This exposed route omits --exit-zero; a passthrough poll that finds new items exits 10 and is surfaced as a process-adapter failure. Add the same baseArgs normalization and cover it with a passthrough test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/inbox-watch/plugin.json` around lines 110 - 121, Update the exposed
passthrough route’s process adapter configuration for the “inbox-watch” command
to include the same baseArgs exit-code normalization used by the non-passthrough
route, including --exit-zero. Add or extend a passthrough test to verify that a
poll returning exit code 10 is normalized successfully rather than reported as
an adapter failure.
Comment on lines
+8
to
+10
| **Tells you only when a human actually replied.** Polls GitHub notifications, IMAP | ||
| and Resend inbound; prints what is new since the last run; **exits 10** if there | ||
| was anything — so a cron alerts only when something real arrived. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the public alert contract for autoreplies.
The descriptions promise alerts only for human replies, while the quickstart table says labelled autoreplies also alert. Choose the intended behavior and make every public surface match.
plugins/inbox-watch/skills/quickstart/SKILL.md#L8-L10: revise the “only when a human replied” claim.plugins/inbox-watch/skills/quickstart/SKILL.md#L19-L23: align the autoreply alert row with the chosen contract.plugins/inbox-watch/skills/quickstart/SKILL.md#L3-L3: align front-matter description.plugins/inbox-watch/meta.json#L2-L2: align plugin metadata description.plugins/inbox-watch/plugin.json#L4-L4: align manifest description.
📍 Affects 3 files
plugins/inbox-watch/skills/quickstart/SKILL.md#L8-L10(this comment)plugins/inbox-watch/skills/quickstart/SKILL.md#L19-L23plugins/inbox-watch/skills/quickstart/SKILL.md#L3-L3plugins/inbox-watch/meta.json#L2-L2plugins/inbox-watch/plugin.json#L4-L4
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/inbox-watch/skills/quickstart/SKILL.md` around lines 8 - 10, Adopt
the contract that both human replies and labelled autoreplies trigger alerts,
then align every public description accordingly: update
plugins/inbox-watch/skills/quickstart/SKILL.md lines 8-10, 19-23, and 3;
plugins/inbox-watch/meta.json line 2; and plugins/inbox-watch/plugin.json line
4. Remove wording that limits alerts to human replies and ensure the autoreply
row and all metadata consistently describe the selected behavior.
|
|
||
| ## Wire it up | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare the cron snippet as shell.
Use ```sh so Markdown tooling and renderers classify the example correctly.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/inbox-watch/skills/quickstart/SKILL.md` at line 90, Update the cron
example’s fenced code block in the quickstart documentation to use the sh
language identifier instead of an untyped fence, preserving the snippet content
unchanged.
Source: Linters/SAST tools
Owner
Author
|
❌ automaintainer verify gate: Output |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a plugin for inbox-watch — polls GitHub, IMAP and Resend inbound, reports only items new since the last run, and classifies DMARC reports and out-of-office autoreplies apart from actual human replies.
Files are all new (
plugins/inbox-watch/*,__tests__/inbox-watch-plugin.test.js) — nothing existing is touched.Two design points worth a look
1.
inbox checkandinbox peekare separate commands on purpose. The cursor is consumed on read, so a normal run eats the alert a cron would have sent;peekreports identically without committing state. The destructive one should never be the one reached for while debugging.2. All inbox commands pass
--exit-zero. inbox-watch exits10for "new items" — a success signal for a shell caller — butcli/adapters/process.jstreats any non-zero exit as failure (if (code !== 0)). Routed naively, the one case that matters (mail arrived) surfaces as an error. The flag was added upstream for exactly this; JSON consumers readdata.count.The
shelladapter would also have solved (2) by remapping the exit code, but it requiresadapterConfig.unsafe=true, which is too high a price for a plugin's primary commands.Verification
npx jest __tests__/inbox-watch-plugin.test.js— 5/5 pass; covers the output contract (versioned envelope, structureddisabledentries,exit_codesdocumenting 10 as success) and that check/peek route distinctly.{"ok":true,"installed_commands":6,"conflicts":[]}, commands executed end to end against the real binary.supercli plugins doctorreports the binary checkok: true.Branched from
48fd82a9(master had moved 29 commits; no upstream file matchesinbox-watch, so this should merge cleanly).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
inbox-watchplugin for monitoring GitHub, IMAP, Resend, and mailbox sources.--peek, seeding, CLI passthrough, and structured JSON output.Documentation