Skip to content

feat(annotate): add strict atomic result output#1091

Open
rNoz wants to merge 2 commits into
backnotprop:mainfrom
rNoz:rnoz/feat-annotate-strict-result
Open

feat(annotate): add strict atomic result output#1091
rNoz wants to merge 2 commits into
backnotprop:mainfrom
rNoz:rnoz/feat-annotate-strict-result

Conversation

@rNoz

@rNoz rNoz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Important/Disclaimer (same note in all my PRs): This is not spam (I created 4 PRs after the weekend of hard work + 2 from previous weeks), just my willing to help: I have been stress testing this tool (thank you for Plannotator!), finding some enhancements, proposing a couple of improvements and fixing some issues. I keep all these features/fixes in topic branches (to be able to publish them, now here as PRs), but I have been using all them in a local integration branch that I use to run plannotator with all these contributions (I use it in all projects and CLI harnesses I own). Therefore, apart from the tests per branch and ad hoc manual tests, I have been using it flawlessly. I decided to create the PRs, in case they are useful for you. Feel free to drop, update, extend.... anything you want. I am willing to contribute, so if you prefer that I tackle something or rebase/solve merge conflicts in the future (At 2026-07-20: all is ready!), please, don't hesitate to ping me. I am sharing this amount of work in case it is useful for you all.
The plannotator "binary" that I use many times per day, it is based on the integration branch that has these PRs (branches cherrypicked) in order:

image

Summary

Direct plannotator annotate gates now support optional fail-closed approval status and atomic structured-result files.

--require-approval makes annotated and dismissed decisions exit nonzero after their JSON is published. --result-file <path> writes the exact newline-terminated stdout record through a private same-directory temporary file and atomically publishes it without overwriting an existing destination.

Existing plaintext, JSON, and hook-native paths still use the legacy emitter and retain their current output and zero exit status.

Implements #1090

Why

Structured stdout identifies each annotate decision, but every legacy decision exits successfully. Automation must parse stdout before it can distinguish approval from requested changes.

Detached callers also need an atomic readiness boundary. Shell redirection can expose an empty or partial file, while a pre-existing or concurrently created destination can be mistaken for the current result. Atomic no-clobber publication makes one complete JSON record the only successful file outcome.

This is not a crash-durability guarantee: file data is synced before hard-link publication, but the parent directory is not synced.

Out of all the PRs I am creating, this is the one that while solving real cases for me, I opted to offer it totally optional in terms of not affecting default behaviors (and more importantly, hooks, etc). However, please let me know if you want that I include some other test or CLI Harness in the validation.

Changes

  • Add direct-only parsing and help for --require-approval and --result-file <path>.
  • Require both strict options to use annotate --gate --json, reject --hook, reject non-annotate commands, and reject missing or duplicate values.
  • Resolve result paths from the invocation working directory and reject missing parents, existing destinations, and dangling destination symlinks before server startup.
  • Add strict serialization for approved, annotated, and dismissed decisions, including approved feedback when a capable server supplies it.
  • Add a strict exit policy that preserves legacy zero exits and returns nonzero only for non-approval decisions under --require-approval.
  • Add an injectable annotate completion coordinator that waits for the decision, stops the server, publishes the result file, waits for stdout completion, and exits in order.
  • Publish files through a 0600 same-directory temporary file, write, file sync, close, atomic hard link, and temporary unlink.
  • Fail closed without copy or overwrite fallback when writing or hard-link publication fails.
  • Keep the existing plaintext, JSON, and hook emitter unchanged for all non-strict invocations.
  • Document strict direct gates in CLI help and the public annotate-gate guide.

Automated validation

At rnoz/feat-annotate-strict-result 5097c557, based on upstream/main 2594b374:

  • bun test apps/hook/server/cli.test.ts apps/hook/server/annotate-command.test.ts apps/hook/server/strict-annotate-result.test.ts: 35 passed, 0 failed, 120 assertions.
  • bun run typecheck: passed.
  • bun run build:marketing: passed; Astro built 40 pages.
  • bun run --cwd apps/review build && bun run build:hook: passed; Vite transformed 3,511 review modules and 5,581 hook modules.
  • git diff --check: passed.

Evaluated along with other in-flight PRs I am pushing, for the final E2E tests in real sessions (see disclaimer note for the PRs mentioned):

  • Serialized full suite: 2,221 passed, 99 skipped, 0 failed, 6,125 assertions across 193 files.
  • Typecheck passed.
  • Marketing built 40 pages.
  • Review and hook builds transformed 3,511 and 5,582 modules respectively.
  • OpenCode build passed, bundling 690 embedded modules and 90 Node modules.
  • Pi build passed.

Live strict direct-gate validation

These are ad hoc E2E tests I have manually performed for this branch, but remember that I am using the integration branch with all these contributions in real projects. The ad hoc tests where specifically done with low reasoning models more like a threshold "check". I am using all this on a daily basis, flawlessly.

All file-review runs used stable project paths so the reviewed source could be revised and reopened:

  • Factory Kimi K2.7 Code: submitted three feedback items and exited 1. The exact same source path was revised and reopened with history/diff, then approved with exit 0 and note Thanks. I confirm I see the diff.
  • Claude Haiku 4.5: a native browser close produced dismissed and exit 1. The same path was reopened and then approved with exit 0. This run used the then-current local integration, which included the now-deferred close-signal prototype; it proves strict dismissed serialization and exit pairing, not ownership or correctness of browser-close detection.
  • Copilot Haiku 4.5: completed one detached same-shell gate, approved with exit 0 and note Some fake note, and consumed the authoritative result. Earlier Copilot attempts exposed private adapter instructions and were harness failures, not product failures.
  • Missing result parent: exited 1, emitted Result file parent does not exist on stderr, and created no result file.
  • Final rebuilt UI gate: retained validation/live/approval-ui-v2.stdout.log and validation/live/approval-ui-v2.result.json contain matching JSON lines and feedback. No final-UI process exit code was captured. Automated coordinator tests verify approved exit 0 and exact stdout/result byte equality. Approval-copy details belong to the separate structured-approval UX change.

Manual testing gaps

Compatibility

Strict behavior is opt-in. Without --require-approval or --result-file, annotate still delegates to the existing plaintext, JSON, or hook-native emitter and exits 0.

The new flags are parsed only by the direct binary. They are not added to the shared OpenCode or Pi slash-command parser. --hook remains unchanged and rejects either strict flag.

Result-file publication never copies, truncates, or overwrites the requested destination. The hard link is the only publication point, so a destination created after startup validation causes the command to fail closed.

Adopt the grep convention for the strict annotate gate's exit codes:
0 = approved, 1 = negative human outcome (annotated/dismissed under
--require-approval), 2 = the gate itself was misconfigured or could not
start/deliver a decision. Previously all usage/startup/validation
failures shared exit 1 with "reviewer did not approve", so callers could
not tell a denied review from a broken gate.

- parseStrictAnnotateOptions failures (bad flag combos, strict flags
  outside annotate --gate --json) now exit 2
- --result-file preflight failures (missing parent, pre-existing or
  dangling-symlink destination) now exit 2
- post-decision publication failures (destination raced into existence,
  hard links unavailable, stdout write failure) now exit 2: they deliver
  no decision record at all, so the code's own fail-closed handling
  presents them as environment errors, never as a reviewer outcome --
  and never approval, since only 0 means approved
- decision outcomes keep 0/1 exactly as before; signal deaths keep 128+n
- document the contract in AGENTS.md and the annotate-gates guide

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants