Repository navigation
feat: add ZCode session hooks integration - #160
qinghuidaqzp wants to merge 3 commits into
Conversation
|
Thanks for this! It is in our review queue, and we will come back here with findings. |
|
Sorry for the churn: main moved quite a bit in the last two days (v0.12.14 to v0.12.17, with changes in |
585f4df to
ea0fc6e
Compare
|
Thanks for the rebase, and for the care that went into this. We reviewed head The good news first. Nothing here changes behaviour for anyone who doesn't use ZCode. There are no new dependencies. The framing holds: PostToolUse and Before we can merge, a few things need changing. Items 1, 3, 4 and 5 we reproduced on an isolated daemon. Item 2 comes from reading the code. 1. Unanswered mail is re-injected forever. (
2. A reply to a teammate's terminal never confirms. (
3. The ZCode hook sends
4. MCP fails outright without
5. Setup and removal regress for existing ZCode users. (
Smaller points. We'd like these too, but they don't block:
Tests we'd like added:
Happy to talk through any of these, especially the confirmation design in item 1. |
|
Thanks for the detailed reproduction and review. Addressed in
The smaller points are addressed too: empty managed containers are removed; disabled declarations are not reported hook-wired; README, directory text and sender notes say after successful tools with no end-of-turn delivery; output follows the user's lower cap and an oversized head blocks later hook delivery with a bounded inbox-recovery reminder; confirmation bookkeeping is best-effort; non-UTF-8 paths skip optional hook installation without a panic. Local Windows validation: fmt and strict Clippy pass, 269 Rust tests pass, build passes, and 53 isolated-daemon ZCode checks pass. Unix-specific regressions are included for CI. I have not repeated the genuine Desktop GUI test or your full two-machine suite on this head; those are not claimed as current passes. The PR description now reflects the final behavior and validation. |
|
Thank you, that was fast and thorough. We re-reviewed
The store migration also holds up:
Nothing changes for non-ZCode users. Tests: 279 unit tests; two_machines, reconnect, codex_busy and gemini all match main; mixed_versions matches main. One thing left before we can merge: After an engine rollback, the installed hooks lose mail silently. (
Two small things while you're there:
|
|
Thanks for the rollback reproduction. Fixed in
Local validation: fmt/strict Clippy/build pass, 270 Rust tests, 53 ZCode end-to-end checks, and 2 real rollback scenarios pass. Genuine Desktop GUI validation and macOS end-to-end execution were not repeated locally; new cross-platform CI results are in Checks. |
|
Thanks! The rollback guard works. We ran the setup-written argv against the official v0.12.17 and v0.10.8 binaries, with both default and custom state dirs and with paths containing spaces and non-ASCII characters. Both engines exit 2 in about 10 ms, print nothing, and leave every row untouched, and Sorry, though: the way we suggested doing it has two side effects. Baking the default state path into the hook means:
Also, if the default state dir ever moves, these hooks would be the only integration still pointing at the old one. A smaller variant keeps the guard without these effects: pass That is the last thing from the review. |
ZCode receives Reason messages after successful tools through native SessionStart/PostToolUse hooks, with the existing MCP inbox as recovery. Setup adds optional hooks beside the merged MCP integration; it preserves native/
.agentsprecedence and user settings.Delivery is bounded: each message can be offered by hooks at most twice (initial attempt plus one retry after its lease expires). The attempt counter is persisted with the message, including across daemon restarts. Hook stdout never marks it read, and exhausted messages remain recoverable through
check_inbox. Agents are not asked to send acknowledgement-only replies.A normal correlated reply confirms only the exact sender/session pair, including a terminal's machine-qualified address. Explicit inbox reads record the upstream
handed_to_agentreceipt. Confirmation bookkeeping cannot fail an already-stored send. Hooks connect only to an existing daemon and have a bounded request budget.Every installed hook carries
--network-home <resolved state directory>, including the default state directory. Older engines such as v0.12.17 reject that argument before entering their generic hook handler, keeping pending mail unread after a binary rollback. Re-running setup adds the guard to existing managed entries while preserving disabled flags.Messages use the existing framing and attachment confinement. Hooks honor the smaller of the user
hooks.maxOutputBytesand the 24 KiB safety cap, including the final newline. An oversized message at the head of the queue intentionally holds later hook mail untilcheck_inboxreads it; later messages do not overtake it. A bounded recovery reminder is returned when it fits. If even that reminder cannot fit, stdout is empty and mail remains unread. There is no Stop hook, end-of-turn delivery or idle wake-up.Per-call ZCode metadata routes the exact session/workspace. Older clients with no metadata retain an isolated per-process inbox. Partial metadata is rejected rather than guessing a workspace or another conversation.
Hook configuration is best-effort: disabled, malformed or unknown hooks do not gate MCP installation, path updates or removal. Global and individual disable flags are preserved, recognized managed hooks are removed around unknown groups, and empty managed containers are cleaned up. Non-UTF-8 executable paths cannot panic hook JSON construction.
Validation
Windows head
56fe7cc, based on upstreambcf045c(0.12.17):cargo fmt --checkcargo clippy --all-targets --locked -- -D warningscargo test --locked: 270 passedcargo build --lockedpython tests/e2e/zcode_hooks.py: 53 checks passed against an isolated real daemon. The configured-executable comparison now resolves both paths, including macOS symlinked temporary roots.python tests/e2e/zcode_rollback.py --old-bin <v0.12.17 executable>: 2 actual rollback scenarios passed, for default and custom state directories. Used the official Windows v0.12.17 release after SHA-256 verification. Each scenario installs hooks with this build, queues one previously offered and one never-offered message, replaces only the isolated fixture executable, starts the old daemon on the migrated store, and runs both installed hook commands. Both fail with exit 2 and empty stdout, all message rows stay unchanged, and the old MCPcheck_inboxrecovers both messages.Unit regressions cover exact machine-qualified terminal confirmation, repeated lease expiry, database reopen, preserved disable flags, cleanup, rollback guards and upgrading unguarded managed hooks. Unix-only path/write tests are exercised by CI. No new dependencies or peer wire identifier changes.
Historical GUI validation applies only to the original PR2 commit
adb7568: ZCode Desktop 3.15.3 received PostToolUse messages across two conversations and replied through MCP; suspended-daemon recovery was exercised. GUI validation and a release build have not been repeated on this head. The current cross-platform CI result is in Checks. This remains Draft for maintainer review.