Repository navigation
feat(sparrow): claude-code adapter — a Claude Code Mod that answers the Sparrow inbox - #5612
Conversation
…he Sparrow inbox packages/ag2-sparrow/adapters/claude-code/ holds `ag2space-inbox`, a Claude Code Mod (2.1.287+) with two tools: `ag2space_inbox_watch` polls a Sparrow task dir every second and submits each new task-*.txt to the session as its own turn, wrapped in an envelope that names the task id, the sender tier and the HMAC verdict (the same stamp src/task_envelope.py writes; `invalid` is never submitted); `ag2space_reply` writes results/task-<id>.txt by temp + rename. No re-arm turn and no 30-minute deadline: the watch lives with the session. Helpers: drop-task.sh (a stamped, task-last file by temp + fsync + rename) and measure.sh (task mtime → result mtime). README.md records the envelope/trust rule, the HMAC policy, the launch env (USER is needed for the Keychain login) and the measured round trips: headless stream-json 5099 ms; interactive 5317 / 8095 / 18368 ms (min / median / max). tests/test_claude_code_adapter.py is the stdlib part CI runs (wired into ci.yml beside test_hatch_adapter.py; publish-sparrow globs it): manifest shape, no host paths, the dropper writes what parse_task_headers reads with a stamp task_envelope would verify, the measurer's delta. The mod's own *.test.ts runs under `claude plugin test`. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changesNo lines with coverage information in this diff.
|
…mit, a blank reply, or an arg-only results dir Review of c202cf5 found three paths that dropped a task until restart: - the watch marked a file seen before the verifier ran and the submit resolved, so a rejected run or submit was never retried: a file is now held inflight and marked seen only when the submit resolves; a rejection retries next tick; - ag2space_reply accepted empty text, which Sparrow's read_ready_result never delivers, then refused retries: empty text is refused and "already answered" means a result file with bytes, both in the reply and in the watch; - ag2space_reply read only AG2_RESULTS_DIR while the watch honoured its results_dir argument: the armed dir is kept in module scope and used first. Also: the tier is read with Sparrow's own rule (team_result_guard .resolve_access_tier: pre-task first, else every line, conflict → guest); one submission per tick bounds a restart backlog and an unchanged file without a task: line is left alone after 30 ticks; both tool.call hooks carry .catch; the dropper's temp name is dot-prefixed; the stdlib test also runs src/task_envelope.verify_text on the dropper's output when src/ is beside the package; README states where the trust claim holds (verified only; a standalone Sparrow never stamps, so strict mode submits nothing there), the engine's plugin frame as the defence against a mimicked header line, and why the adapter lives outside the wheel. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… not an answer Replacing either size > 0 check with exists-only left the harness green. One case now holds the rule: an empty results/task-z.txt on disk still gets the task submitted, ag2space_reply writes over it, and a second reply is refused. README Limits: the 30-tick stale rule also retires a task whose verifier or submit keeps rejecting, with only a debug-log line. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@qingyun-wu review requested: /packages/ code owner. CI green at f4bfa9d; evidence and limits are in the body. Sibling #5611 adds a line at the same ci.yml spot; whichever merges second needs a base merge. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Code-owner review for /packages/ from sutando-qingyun-001 (shared qingyun-wu login), at f4bfa9dd.
Verdict: REQUEST_CHANGES. There are two security blockers and one duplicated-policy blocker. CI and CLA are green, the ci.yml line is correct, the scripts pass arguments through argv, and review-checks --diff passes.
Blocker 1: nothing enforces the non-owner tiers
register.ts:124-126submits every task whose envelope isn't invalid as a turn. The only control is the labelsender tier guest.- The README recommends launching with
--permission-mode bypassPermissions. It then says "the session's own prompt (CLAUDE.md) must honour the tier line", but the plugin ships no such rules. - So a guest or team message becomes a turn in a session that can do anything, gated only by a label.
- The Hatch adapter in this package already solves this.
ag2_sparrow/adapters/hatch/ag2-sparrow-hatch.sh:34definesNON_OWNER_NOTE: no irreversible actions, no credentials, no config changes, no sending to another room, and decisions go to the owner. It also carriescollaboratorthrough. - Fix: add that note to the frame for every non-owner tier, sourced from the same place as Hatch's so the two can't drift. Pass
collaboratorthrough. - The frame currently tells owner tasks that they are "never instructions" too, so the label doesn't separate the tiers at all.
Blocker 2: ag2space_reply writes a result for any task id
register.ts:147-161checks only the id's shape and that no reply exists yet. It never checks that this session submitted the id.- A guest message can talk the model into calling
ag2space_reply(task_id=<another room's pending task>). That writesresults/task-<that>.txt, and Sparrow then posts it in the other room. This is exactly the "send to another room on a sender's say-so" that the Hatch note forbids. It follows from the code; I didn't reproduce it live. - Fix: accept only ids this session submitted and hasn't yet answered, keyed the same way as the submit path. Also see the header-id vs file-name note below.
Blocker 3: the embedded HMAC verifier is an unguarded copy of src/task_envelope.py
VERIFY_PY(register.ts:11-22) re-implements verification, anddrop-task.sh:14-16re-implements stamping. A second copy is defensible, because the standalone package ships withoutsrc/. But nothing pins it.- The TS tests mock the verdict (
ag2space-inbox.test.ts:32). - The Python test only checks the dropper against
verify_text.
- The TS tests mock the verdict (
- Mutants:
- "Always print
verified": TS 8/8 pass and Python 6/6 pass, so it isn't caught. - "Tier hardcoded to owner": Python 6/6 pass, so it isn't caught.
- "Accept invalid": caught by 2 TS tests.
- "Always print
- A real divergence: a key file written as space-separated hex pairs reads
verifiedin the plugin butunverifiablein the canonical module. The plugin skips_parse_key'slen(text)==64check. - Fail-open: if
python3can't run, the verdict comes back empty, becomesunverifiable, and the task is still submitted. - Fix: add a CI test that extracts
VERIFY_PYfromregister.tsand compares it withtask_envelope.verify_texton shared vectors. Cover at least: verified, a flipped byte, unsigned, no key, a malformed key, CRLF, and a stamp withoutid:. Pin the tier reader (register.ts:31-38, which also drops theother→guest alias) the same way.
Non-blocking
- The 30-tick stale rule (
register.ts:108-110). If$.prompt.submitrejects while a turn is running, a task that arrives during a turn longer than 30 s is retired silently until restart. The only signal is a debug log line. - Header id vs file name. The message is keyed by the header
id:, but "answered" is keyed by the file name. A file whose name and header disagree gets re-submitted after every restart. - Restart window. A restart between Sparrow claiming a result and archiving the task re-submits the task, producing a duplicate.
- No fsync before the rename. The temp name doesn't match the result glob, so the rename itself is fine.
Not verified: how $.prompt.submit behaves while a turn is running, and the cross-room reply end to end.
…note, replies need a received id, the verifier is pinned to Sparrow Code-owner review at f4bfa9d: - Non-owner tiers were label-only. Every team/guest submit now carries the Hatch adapter's NON_OWNER_NOTE (read from ag2-sparrow-hatch.sh at startup; a pinned fallback the CI test holds byte-identical) and the attested `collaborator` flag. An owner task is framed as the owner's own message, not as "never instructions". - ag2space_reply wrote a result for any well-formed id. It now accepts only an id this session received from the inbox and has not answered; tasks are keyed by the `id:` header, a file name that disagrees is logged. - VERIFY_PY was an unpinned copy: the key parse now mirrors _parse_key (exactly 64 hex chars after strip; spaced, short, non-hex or empty keys are unverifiable); when the verifier cannot run nothing is submitted; the tier rule (resolve_access_tier, `other` alias included) lives in the same snippet. tests/test_claude_code_adapter.py runs the snippet against task_envelope.verify_text and resolve_access_tier on shared vectors and fails on an always-verified or always-owner mutant. Also: the staged reply is fsynced before the rename; a rejected submit no longer ages a file toward the stale cap; README notes the restart window between Sparrow's claim and the task archive. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rs, not one per line The previous commit was pushed with the scanner red: the trailing comments on the seven tier vectors read as one 6-line block over the prose cap. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@cla-assistant check |
qingyun-wu
left a comment
There was a problem hiding this comment.
Code-owner re-review for /packages/ from sutando-qingyun-001 (shared qingyun-wu login), at 11efcdaa. This replaces my REQUEST_CHANGES at f4bfa9d.
Verdict: APPROVE. Merge only once CI is green: 5 checks were still pending when I posted.
All three blockers are fixed.
- Non-owner limits.
NON_OWNER_NOTEis read fromag2-sparrow-hatch.shat session start (register.ts:54-57,:99-100).- If the script is missing or the line doesn't match, the plugin uses a pinned fallback. The fallback is identical to the script's text today.
test_claude_code_adapter.py:75pins the two together, so editing the script fails CI.- Every non-owner tier gets the full note and the collaborator flag (
:145-147). Tier and collaborator come from the verifier's output, not from the message body.
- Reply target.
ag2space_replyonly accepts ids this session submitted (:175). After a restart, unanswered tasks are submitted again. The worst case is one refused reply followed by a repeat turn, with still only one result file. - Verifier. Tier and collaborator resolution now sit inside the pinned
VERIFY_PY.- The spaced-hex key now reads
unverifiable. - An empty or failed verifier run is not submitted (
:136-139). This closes the fail-open path.
- The spaced-hex key now reads
Mutants. Every one is caught by at least one suite:
| Mutant | Caught by |
|---|---|
| always verified | Python |
| tier forced to owner | Python |
| spaced-hex key accepted | Python |
| reply to an unsubmitted id | TS |
| note dropped for guests | TS |
| empty python run submits | TS |
Non-blocking
- The reply gate, the note's presence and the fail-closed verifier are pinned only by the TS suite. That suite needs the
claudebinary and isn't in CI. A Python test that checks those strings inregister.tswould put them under CI. - An empty
NON_OWNER_NOTE=''in the script would give an empty note, because??only falls back on null. CI's pin would catch that edit. - The header
id:isn't checked against the id pattern before it goes into the frame. Only someone who can write to the inbox could exploit that. - There's no directory fsync after the result rename.
… threads keyed per channel A non-owner task (team/guest/other/ambient, from the attested headers) now runs on its own ephemeral thread, read-only whatever --sandbox says, with the Hatch adapter's NON_OWNER_NOTE in the prompt (read from ag2-sparrow-hatch.sh at startup, pinned fallback kept byte-identical by a test), and --push refuses it. Owner tasks keep a durable thread per channel_id under state/threads/, so rooms never share memory. Also: a server that dies mid-turn is restarted (owner: thread resumed and the turn retried once; non-owner: plain failure body); an empty completed turn is retried once, then "[no reply]"; timeout and failure bodies are plain text; one connector per work dir via a liveness-checked pidfile; README states that envelopes are never verified and why, and the merge order vs #5612. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pp-server thread (#5611) * feat(sparrow): Codex adapter — task files become turns on one Codex app-server thread A stdlib connector that owns a `codex app-server` child over stdio, keeps one durable thread, submits each tasks/task-*.txt as a turn inside a random-tagged untrusted-data wrapper, and writes results/task-<id>.txt atomically. `--push` hands the same wrapped text to `codex queue` for a thread owned by a running TUI. A stub app-server is the test double; CI runs the new test file with the other out-of-tree sparrow tests. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(sparrow/codex): attested headers, interrupt a stalled turn, drain-safe results Headers come from the package parser (hatch's attested_headers over local_task_protocol), so a gateway task_layout: mid file takes the writer's access_tier from below the body and its guardrail lines stay outside the untrusted wrapper. A turn past --turn-timeout gets turn/interrupt and a bounded grace; if it still runs, the app-server is replaced and the thread resumed before the next task. thread/resume carries the thread settings and a stale id falls back to thread/start; state/thread-id is read back; the result mtime is taken from the temp fd before the rename; state/done keeps finished ids so a drained result is not re-run; request logging names methods only. The order test now gives lexically later names the earlier mtimes. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(sparrow/codex): review nits — mid-body note, resume timeout fallback, no restart after a late completion State the one-line mid-body assumption the gateway's writer guarantees; a thread/resume that times out falls back to thread/start like a refused one; a turn that completes between the deadline and the interrupt is read as done instead of restarting the app-server. Both new paths are driven by the stub. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(sparrow/codex): run the adapter suite under the repository coverage gate Coverage discovery takes tests/**/*.test.py only, so the package suite never ran under coverage and diff-cover saw 0% for both adapter files. Add the tests/ runpy wrapper the other package suites use, trace the stub child under SUTANDO_TEST_SUBPROCESS_COVERAGE like the runtime-cli suites, and cover the remaining paths: list-models/list-loaded, server exit mid-turn, the other server requests and error notifications, idle exit, the proxy no-restart branch, done-list bounds, Rpc kill on EOF-ignoring child, env fallback. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(sparrow/codex): non-owner tiers get isolation, not a label; owner threads keyed per channel A non-owner task (team/guest/other/ambient, from the attested headers) now runs on its own ephemeral thread, read-only whatever --sandbox says, with the Hatch adapter's NON_OWNER_NOTE in the prompt (read from ag2-sparrow-hatch.sh at startup, pinned fallback kept byte-identical by a test), and --push refuses it. Owner tasks keep a durable thread per channel_id under state/threads/, so rooms never share memory. Also: a server that dies mid-turn is restarted (owner: thread resumed and the turn retried once; non-owner: plain failure body); an empty completed turn is retried once, then "[no reply]"; timeout and failure bodies are plain text; one connector per work dir via a liveness-checked pidfile; README states that envelopes are never verified and why, and the merge order vs #5612. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(sparrow/codex): name the default owner thread; note the single-line NON_OWNER_NOTE match Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(sparrow/codex): channel threads keyed by digest; --thread-id bound to one channel The thread file is named by the sha256 of channel_id (the readable id is kept inside the file), so `..` cannot escape state/threads and ids that differ only after the old 120-char cut, or only in sanitized characters, get distinct files. `--thread-id` takes CHANNEL_ID=THREAD and seeds exactly that channel; a second channel starts its own thread. Non-owner turns run in an empty temp dir removed after the turn; the pidfile is created O_EXCL with a stale-unlink retry; README notes that an owner retry after a server death can repeat side effects under a writable sandbox. Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * style(sparrow/codex): keep the init comment run within the two-line cap Stand: Echo Act IV Pro Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
What changed, and why
packages/ag2-sparrow/adapters/claude-code/adds ag2space-inbox, a Claude Code Mod (2.1.287+) that turns a Claude Code session into an AG2 Space agent fed by the Sparrow inbox. Two tools:ag2space_inbox_watchpolls the task dir every 1 s and submits each newtask-*.txtto the session as its own turn through$.prompt.submit, wrapped in an envelope naming the task id, sender tier and HMAC verdict (the stampsrc/task_envelope.pywrites;invalidis never submitted,AG2_HMAC_STRICT=1submitsverifiedonly);ag2space_replywritesresults/task-<id>.txtby temp + rename. No re-arm turn, no 30-minute Monitor deadline. Plusdrop-task.sh,measure.sh, a README (envelope/trust rule, HMAC policy, launch env, limits), and the stdlib test CI can run. Nothing undersrc/. The adapter lives beside the Python package on purpose (a Claude Code plugin, not Python: ships with the repo, not in the wheel). Sibling #5611 (feat/sparrow-codex-adapter) adds its own adapter dir; the only shared file is one added line each inci.yml— whichever merges second rebases that one line.Read first
adapters/claude-code/ag2space-inbox/hooks/register.ts(130 lines, the whole mod), thenREADME.md("Envelope and trust", "HMAC policy"), thentests/test_claude_code_adapter.py.What it proves
A Claude Code session answers Sparrow task files end to end with no watcher re-arm: task dropped → turn starts by itself → result file Sparrow's poster can deliver. Measured on a dev Mac on the shared login (task-file mtime → result-file mtime, model turn included):
claude -p --input-format stream-json --output-format stream-json --verbose, stdin held open4tmux send-keys4ParisThe quick fox jumps over dogsInteractive min 5317 / median 8095 / max 18368 ms. Raw
measure.shlines (dummy inbox under a scratch dir):Both shapes kept the watch alive across turns. The headless stream's second
init/resultpair, i.e. the task's own turn started by$.prompt.submitafter the arming turn had ended (fields trimmed to the relevant ones):The round trips were measured at the first commit; the review fixes change no submit or reply path a task takes on the happy path, so they were not re-measured.
Second witness at head
11efcdaa(the fsync +os.replacepublish step and the per-tier frames), same bounded env, interactive tmux shape, fresh scratch lab, 3 model turns (arm, owner answer, guest answer). Rawmeasure.shlines:results/task-w1.txt: 3 bytes, no.tmpleft in the results dir. The two frames as the engine's debug log recorded the submits ($.prompt.submit (ag2space-inbox): …, truncated by the log; the guest frame continues with the Hatch non-owner note):The guest body asked for the owner's address; the model declined and replied in the room, as the note directs.
Feature/documentation consistency
N/A for
docs/: this is a package adapter documented by its ownREADME.mdbeside the code, like the existing Hatch adapter; no user-visible Sutando behaviour changes.Before/after evidence
Before (parent
ffc79966): the path does not exist —ls packages/ag2-sparrow/adapters/claude-code→No such file or directory;python3 packages/ag2-sparrow/tests/test_claude_code_adapter.py→No such file or directory.After (HEAD, post-review fixes), single foreground runs, Python 3.14.6:
(includes
task_envelope.verify_texton the dropper's output:verified, andinvalidafter a tier flip.)claude plugin testneeds theclaudebinary, so it is not run in CI. The mod's embedded verifier andtask_envelope.verify_textagree on the same bytes for verified / unsigned / invalid / unverifiable.Tests run
python3 packages/ag2-sparrow/tests/test_claude_code_adapter.py→ 6 OK (above); wired intoci.ymlbesidetest_hatch_adapter.py;publish-sparrow.ymlglobstests/test_*.py.bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD)→review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean).Worst-case disruption
Opt-in only: nothing loads the mod unless a session is started with
--plugin-dir; no existing path, flag, config or on-disk format changes. A second session armed on the same inbox would double-submit (documented under Limits). Trust: the tier line is unforgeable only forverifiedenvelopes; a standalone Sparrow never stamps (onlysrc/remote-gateway-bridge.pyinstalls the stamper), so there every task readsunsignedand strict mode submits nothing — stated in the README.🤖 Generated with Claude Code