feat(kanban): standalone card-drop + comment receiver (goal cards, dedupe, fail-closed auth) - #84
Conversation
…dupe, fail-closed auth)
The single inbound HTTP write surface connecting off-box producers (Render CPE
website / diligence inbox, Modal cron producers) to the local Kanban board.
Standalone stdlib-only launchd daemon on loopback :8646 (not api_server.py, to
avoid widening the multiplex gateway port); shells out to `hermes kanban
create/comment`.
- POST /kanban/card-drop {assignee,title,body,dedupe_key?,priority?,goal?,
goal_max_turns?} -> {id}; goal:true -> --goal; dedupe_key -> --idempotency-key.
- POST /kanban/comment {card_id,text} -> {id,commented}.
- X-Cron-Secret auth, FAIL CLOSED when secret unset (inverts research-agent's
fail-open _check_cron_auth; public hostname makes the gate the boundary).
- launchd plist + launcher (sources secret from ~/.hermes/.env) + README.
- 20 tests pass; live e2e proved create(goal)/dedupe/comment/403 on a real board.
Patch note: ~/.hermes/plans/hermes-patches/kanban-card-drop-receiver.md
|
Claude finished @exiao's task in 2m 22s —— View job Review: PR #84 — kanban card-drop receiver
LGTM. Re-reviewed the branch at Latest round lands clean:
Verified against the real CLI (
Architecture / footprint: Correct rung — a stdlib-only, CLI-backed launchd daemon, not a new core model tool and not a widening of the always-on gateway port. No prompt-cache or role-alternation surface (it's an out-of-band HTTP daemon, not in the agent loop). Auth is fail-closed ( Minor, non-blocking (not new to this commit): the daemon reads a few env keys ( 0 blocking, 1 minor. |
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
1 |
First entries
scripts/kanban_receiver/test_kanban_receiver.py:22: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
✅ Fixed issues: none
Unchanged: 6194 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a standalone, standard-library-only HTTP receiver (kanban_receiver.py) to serve as an inbound write surface for dropping cards and comments onto a local Kanban board via the hermes kanban CLI. The changes include a launchd service configuration, a bash launcher script, documentation, and comprehensive unit and integration tests. Feedback suggests improving the validation of the user-supplied goal_max_turns parameter to ensure it is strictly positive and falls back to a safe default rather than failing with a 400 error.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — clean, well-scoped receiver on the right rung of the Footprint Ladder (CLI-backed standalone daemon, not a new core tool or gateway-port widening). Verified the hermes kanban CLI contract it shells out to: create flags, JSON {id} output, idempotency returning the existing id, and none as a legal unassigned sentinel. Auth is correctly fail-closed with hmac.compare_digest, auth-before-body-parse, loopback bind, and -- argv guards. One minor non-blocking hardening: non-string JSON field values (e.g. title:5 or body:{}) raise an uncaught AttributeError/TypeError and reset the connection instead of returning 400 — coerce with str() or validate before building argv. Full notes in the PR comment.
A zero/negative goal_max_turns forwarded a stall value to the worker. Clamp: >=1 passes through, non-positive drops the flag (CLI applies its own default), malformed still 400s. Adds coverage for both paths. Addresses gemini-code-assist review on #84.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8360cb2114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2s: - The exec-log filter dropped only the --body flag token, leaking the following body value (diligence/inbox content) into the launchd stderr log. _redact_args() now strips the flag AND its value. - assignee \"none\" was forwarded as --assignee none, which hermes kanban create stores as a literal lane (create does not canonicalize the sentinel like assign/reassign do), stranding the card in ready. Omit the flag so the card lands genuinely unassigned. Adds coverage for both.
There was a problem hiding this comment.
Reviewed the receiver against the hermes kanban CLI contract, plist/launcher, and tests. Well-scoped: correct footprint rung (stdlib-only, CLI-backed launchd daemon — no new core tool, no gateway-port widening), fail-closed auth (compare_digest, auth-before-parse, loopback bind, -- argv guards), profile-safe by delegation, and the goal_max_turns fix already lands Gemini's point.
3 minor, non-blocking items (details in the PR comment):
- Card body leaks into /tmp stderr log — the L113 redaction filter drops only the
--bodyflag token, not the body value that follows it. - Non-string JSON field values (title:5, assignee:5, body:{...}) raise uncaught AttributeError/TypeError → connection reset instead of a clean 400.
assignee: "none"stores a literal "none" lane rather than a NULL/unassigned card, so the dispatcher's default_assignee (NULL-only) never triages it — mismatch with the documented 'unassigned' intent.
0 blocking, 3 minor.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ca32f0c01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Reviewed against the hermes kanban CLI contract, plist/launcher, and tests. Well-scoped: correct footprint rung (stdlib-only, CLI-backed launchd daemon — no new core tool, no gateway-port widening), fail-closed auth (compare_digest, auth-before-parse, loopback bind, -- argv guards), profile-safe by delegation, and all three prior-round fixes (body redaction, none-assignee omission, non-positive goal_max_turns) land with tests.
2 minor, non-blocking:
- Comment text (and card-drop title) still leak into /tmp/hermes/kanban-receiver-stderr.log — the redaction fix is flag-based (
_REDACT_FLAGS={'--body'}), but comment_card passes text as a positional after--, so it's logged verbatim. Redact positionals too / log only flag names. - Non-string JSON field values (assignee:5, title:{}, body:5, card_id:5) call .strip()/hit subprocess and raise AttributeError/TypeError; do_POST has no try/except, so the client gets a connection reset instead of a 400. Coerce with str() or validate before building argv.
0 blocking, 2 minor. Full details in the PR comment.
Addresses claude review item #2 on PR #84: title/assignee/body/card_id/text that arrive as int/object/list previously hit an uncaught AttributeError on .strip() and reset the connection. Now rejected up front with a clean 400. +3 tests (26 total). Items #1 (log redaction) and #3 (omit 'none') already landed in f1c4671/6ca32f0c.
|
Addressed the 3 review items:
26 tests pass, ruff clean. |
There was a problem hiding this comment.
1 blocking, 1 minor — details in the Claude review comment.
-
(blocking) Unknown-card comment returns 502, not the promised 404.
kb.add_comment()raisesValueError('unknown task <id>')which the CLI prints askanban: unknown task <id>, but comment_card (kanban_receiver.py:263) only matches 'not found'/'no such'. The test_comment_unknown_card_is_404 test passes only because it mocks stderr='task not found: t_x' — a string the real CLI never emits (green mock). Fix: add 'unknown task' to the substring set and use the real stderr in the test. -
(minor) Card title and comment text still leak into /tmp/hermes/kanban-receiver-stderr.log — _redact_args strips only --body, but the positional title and comment text after -- are logged verbatim at line 130.
Codex P2s:
- comment_card mapped unknown cards to 502: the CLI emits
"kanban: unknown task <id>" (from add_comment ValueError), which the
404 match ("not found"/"no such") missed. Match "unknown task" too;
the fabricated test string is replaced with the real CLI output.
- _redact_args only stripped --body, so the comment text positional
(comment <id> <text>) leaked user content into the exec log. Redact
the trailing comment positional while keeping the card id for
debuggability.
Adds coverage for both plus a 502 real-fault case.
There was a problem hiding this comment.
Re-reviewed the synchronized branch against the real hermes kanban CLI, plist/launcher, and tests. Right footprint rung (stdlib-only, CLI-backed launchd daemon), fail-closed auth verified (compare_digest, auth-before-parse, loopback bind, -- argv guards). All four prior-round fixes land with tests, and the unknown-card 404 now matches the real CLI output (kanban: unknown task <id>), not a fabricated mock string.
1 minor, non-blocking: the card title still reaches /tmp/hermes/kanban-receiver-stderr.log verbatim — _redact_args only redacts positionals for the comment subcommand and the --body flag, so the create positional title (inbox/diligence-derived, same sensitivity class as the already-redacted body/comment text) is logged as-is. Fix: redact the trailing create positional too, or log only flag names.
0 blocking, 1 minor. Full details in the Claude review comment.
…eview nit) Addresses the final claude review item on PR #84: the create positional title (inbox/diligence-derived, same sensitivity class as the already-redacted body) reached /tmp stderr verbatim. _redact_args now redacts every trailing positional after -- except the comment card id, and keeps flag names with a <redacted> value marker. 28 tests pass.
|
Addressed the final review nit (title in stderr log): |
There was a problem hiding this comment.
LGTM — re-reviewed the synchronized branch. The last outstanding item (create title leaking into the exec log) is fixed: _redact_args now redacts every trailing positional after -- except the comment card id, covered by tests. Footprint (stdlib-only CLI-backed launchd daemon), fail-closed auth, profile-safety, none-sentinel omission, unknown-card 404 against the real CLI string, and non-string-field 400s all verified. 0 blocking.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e4cd0f4d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
launchd from a source checkout where hermes is installed only in the repo venv (not /usr/local/bin or /opt/homebrew/bin) execd system python3, so _hermes_bin()'s `which hermes` failed and fell back to `sys.executable -m hermes_cli.main` under the wrong interpreter, breaking authenticated card writes. Resolve $REPO_ROOT/.venv (or venv) first: prepend its bin to PATH and exec its python3 so both the CLI resolution and dependency imports use the venv. (codex P2)
There was a problem hiding this comment.
LGTM — re-reviewed the synchronized branch (98453f2 venv-preference fix) against the real hermes kanban CLI, launcher, plist, and tests. Correct footprint rung (stdlib-only CLI-backed launchd daemon, no core tool / no gateway-port widening), fail-closed auth (compare_digest, auth-before-parse, loopback bind, -- argv guards), and the CLI contract holds: create flags + none-omission = unassigned, and unknown-card comment maps to 404 against the real 'kanban: unknown task ' string (tests assert the real string, not a mock). All prior review threads land with tests. 0 blocking.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98453f2903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except (TypeError, ValueError): | ||
| return 400, {"error": "priority must be an integer"} | ||
|
|
||
| if payload.get("goal"): |
There was a problem hiding this comment.
When a producer serializes goal as a string such as "false" or "0", this truthiness check still appends --goal, so an ordinary card is dispatched as a multi-turn goal loop and can burn the default goal budget. Please accept only a JSON boolean (or explicitly parse string forms) before enabling goal mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7140991: added _is_goal() which accepts only a JSON boolean and treats falsey string tokens ("", false, 0, no, off) as False, so a producer sending goal:"false" no longer enables goal mode. Regression test test_create_card_goal_false_string_does_not_enable_goal proves red-before/green-after (29 pass with the fix; the goal-string case fails without it).
| <string>/tmp/hermes/kanban-receiver-stdout.log</string> | ||
| <key>StandardErrorPath</key> | ||
| <string>/tmp/hermes/kanban-receiver-stderr.log</string> |
There was a problem hiding this comment.
On a clean macOS account where /tmp/hermes does not already exist, launchd has to open these Standard*Path files before run_receiver.sh runs; launchd.plist(5) documents that these keys map stdout/stderr to the given file, created if the file is missing, but the missing parent directory is not guaranteed. The install path here does not create /tmp/hermes, so launchctl load can fail before the receiver ever starts; log to an existing location or add a pre-install step that creates the directory.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7140991: StandardOut/ErrorPath now point at __HOME__/.hermes/logs/ (always present via the __HOME__ substitution) instead of /tmp/hermes, whose parent dir isn't guaranteed on a clean account. README log path updated to match.
Address two Codex P2s on PR #84: - kanban_receiver.py: a producer serializing `goal` as the string "false"/"0" still passed the bare truthiness check, dispatching an ordinary card as a multi-turn goal loop and burning the goal budget. Add `_is_goal()` that accepts a JSON boolean and treats falsey string tokens ("", false, 0, no, off) as False. Regression test proves red-before/green-after. - plist: StandardOut/ErrorPath pointed at /tmp/hermes, whose parent dir is not guaranteed to exist on a clean account, so `launchctl load` could fail before the receiver starts. Point at __HOME__/.hermes/logs (always present via the __HOME__ substitution). README log path updated to match.
There was a problem hiding this comment.
LGTM — reviewed 7140991 (goal-boolean validation + log-dir move) against the real hermes kanban CLI, launcher, plist, and tests. _is_goal() correctly rejects falsey goal strings; StandardOut/ErrorPath now target ~/.hermes/logs (created by standard Hermes setup, config.py:860) instead of the ungaranteed /tmp/hermes. CLI contract holds (create flags + positional title, comment positionals, unknown-card 404 vs real 'kanban: unknown task '). Correct footprint rung (stdlib-only CLI-backed launchd daemon), fail-closed auth. 0 blocking, 1 minor (optional env overrides — worth a README note). Full notes in the PR comment.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7140991f4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if assignee != "none": | ||
| args += ["--assignee", assignee] |
There was a problem hiding this comment.
none drops before dispatch
When a producer sends assignee: "none", this only omits --assignee, leaving the new task as a parent-free ready card. In installations with kanban.default_assignee configured, the dispatcher auto-assigns unassigned ready tasks and spawns them (hermes_cli/kanban_db.py:7870-7907), so these intended “triage later” drops will be executed by the default lane instead of remaining unassigned. Pass a non-dispatching state such as --triage/blocked for the none sentinel, or add another opt-out from default assignment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Noting this is a separate concern from the two P2s this fix pass addressed (goal-boolean validation + plist log dir). The none-sentinel handling here is the PR's original code (blame: 6ca32f0, unchanged by my fix 7140991). The finding is valid though: with kanban.default_assignee set, the dispatcher auto-assigns unassigned ready cards, so a none drop meant as 'triage later' would get executed by the default lane. Flagging for @exiao as a follow-up decision (pass --triage/blocked for the none sentinel) rather than expanding this merge.
1 participant
What
The single inbound HTTP write surface connecting off-box producers (the Render CPE website / diligence inbox, Modal cron producers) to this machine's local Kanban board (
$HERMES_HOME/kanban.db). Closes the deferred receiver linchpin from both plans:~/.hermes/plans/cpe-chat-route-to-research-lead.md(Card A)~/.hermes/plans/diligence-inbox-production.md(section A+)Host decision (spike)
Standalone launchd daemon, not
gateway/platforms/api_server.py. api_server registers all routes in one hardcoded block with no auth'd custom-route hook; extending it widens the always-on multiplex gateway port (secondary profiles must not bind ports → crash-loop risk). A tiny stdlib-only daemon on loopback :8646 has a far smaller blast radius and shells out to the installedhermes kanbanCLI.Endpoints
GET /health(no auth; liveness).POST /kanban/card-drop {assignee,title,body,dedupe_key?,priority?,goal?,goal_max_turns?}→{id}.goal:true→--goal [--goal-max-turns N];dedupe_key→--idempotency-key(repeat collapses to existing id).POST /kanban/comment {card_id,text}→{id,commented:true}; unknown card → 404.Auth: fail CLOSED
X-Cron-Secretviahmac.compare_digestagainstKANBAN_RECEIVER_SECRET/CRON_SECRET. No secret configured → 403 on ALL writes (deliberate inversion of research-agent's fail-open_check_cron_auth; the public hostnamekanban.getbloom.appmakes the gate the security boundary). Loopback-only bind; public path is the existinghermes-webhookscloudflared tunnel.Verification
{"id":"t_c59d46e6"}, card showsgoal_mode: True; same dedupe_key returned the SAME id; comment landed; no-secret → 403; bad assignee → 400.Deploy (separate gated ops, NOT in this PR)
Cloudflared ingress + DNS route for
kanban.getbloom.app, set the secret + install the plist, and flip the CPE dark producers (KANBAN_PRODUCERS_ENABLED+KANBAN_CARD_DROP_URLon theresearch-configModal secret). Tracked as review-required prod ops on the board.Patch note:
~/.hermes/plans/hermes-patches/kanban-card-drop-receiver.md