Skip to content

feat(advisor-board): --reply-format json, a verified machine-consumable reply per seat - #1443

Open
ViperJuice wants to merge 3 commits into
mainfrom
claude/reply-format-json
Open

ViperJuice wants to merge 3 commits into
mainfrom
claude/reply-format-json

Conversation

@ViperJuice

@ViperJuice ViperJuice commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Wires the strict panel_reply verifier (agent-harness#1286, shipped in 0.7.25 with no caller) into advisor-board, so a board can return one machine-consumable, verified reply per seat. Opt-in; the default path is unchanged.

What it does

phase-loop advisor-board <artifact> --reply-format json asks every seat for one JSON object (verdict, summary, findings[] with severity, title, body, optional location) followed by the usual last line (the verdict word), and verifies the reply.

  • Verified: attached to the seat (PanelLegResult.panel_reply, a non-field attachment so asdict and the goldens never see it), under reply in --json, and in the seat's streamed leg-*.verdict.json.
  • Not verified: the seat is DEGRADED with a typed panel_reply_<kind> detail and its text kept, so the governed classifier holds it as it holds any non-conforming review (panel_nonconforming). Kinds: no_json, ambiguous_reply, schema_mismatch, verdict_missing, verdict_forbidden, verdict_inconsistent, terminal_mismatch, recommendation_missing, too_large; all exact literals in the closed detail vocabulary.
  • --json gains a top-level reply_format and a per-seat reply only when the flag is used.

Design decisions (and the two I changed my mind on)

  1. The JSON is followed by the legacy last line. I first planned "JSON and nothing else". But _completion_ok is applied at nine places inside the seat transports (TUI, broker, gemini) to decide a reply is complete, so a JSON-only reply would never read as complete. Keeping the last line means no transport changes; the verifier requires it to agree with the JSON (terminal_mismatch), so a seat cannot satisfy the transport with one verdict and the consumer with another.
  2. The format reaches seats through the brief, not the prompt, and the CLI builds it. I first planned for the runtime to append the instructions. It cannot: the sealed prompt is a rigid envelope that verify_harden_evidence.py::sealed_prompt_parts validates end to end (preamble, digest-bound instruction frame, digest-bound bundle frame), the staged review-instructions.md must hash to the digest minted from _resolve_brief(mode, brief_ref), and about ten call sites (governed review, runner, train packet, the digest minting) each resolve the brief independently. So the CLI writes review brief + format instructions as one file, passes it as brief_ref everywhere, and mints the digest from exactly those bytes: the mechanism --advisory already uses for its contract. _default_spawn's signature and the sealed envelope are untouched.
  3. --advisory is refused with the flag. An advisory review is recognised by the exact sha256 of its contract (is_advisory_brief, "never landing evidence"). Appending the format would change the digest and a non-landing review would stop looking like one. A test pins that the extended contract is no longer recognised, which is why the CLI refuses to build it. Agy canary capture is also refused (it records the exact default brief).
  4. Verification point. In invoke_board it runs inside the per-seat worker, before _run_legs_ordered hands the result to on_leg_complete and the verdict file, so a streaming consumer never sees an unverified reply as OK. Native fills are bound at two sites (the early all-deferred path and the per-seat path); both verify. My first version covered only one of them: the end-to-end native-fill test I wrote caught it (a legacy-format fill stayed OK), and a mutation on each site is now killed separately.
  5. Mutation in place via object.__setattr__ (the _finalize_research_result precedent), so attachments the seat already carries survive.

Verification

  • New tests: 38 (tests/test_panel_reply_board.py, 35; three added to tests/test_panel_reply.py), Python 3.10 and 3.14. They cover the flag and its refusals, the derived brief and the digest, every failure kind, a real invoke_board run over the four-seat default board with mixed replies, streaming ordering, the default path unchanged, an advisory-mode board, and native fills on both binding paths.
  • Mutations: 13 / 13 killed, each separately: _run_seat not verifying; the early path not verifying fills; the per-seat path not verifying fills; the last-line check dropped; the recommendation check dropped; a failure no longer degrading; the streamed file and the CLI payload dropping reply; the CLI not passing the flag; --advisory allowed; the digest minted from the base brief; no idempotence marker; always-verify when the flag is off. (My first mutation run silently ran nothing under zsh; I reran it with real counts. Two initially survived: the CLI payload reply key and one fill site. Both now have tests.)
  • Regression: 1,508 passed across the launch guards (scratch-inventory, reference-inventory, launch-guard, helpers, audit-inventory), test_agent_runtime_provider_cs08.py (the _default_spawn signature guard), the detail-vocabulary tests, the goldens, the advisor-board CLI/advisory/integration suites, both native-fill suites, session labels, placement and login-wait. Python 3.10: 574 passed on the changed-code suites. ruff and docs-audit clean. Layout identity still 805042ce…; jail profile digest still 5ccd7e86….
  • Full suite vs main: running now against a clean origin/main checkout; I will post the failing-set comparison here.

Not verified

  • A live board. Nothing here runs a real seat. Whether real models follow "one JSON object, then the verdict word" reliably is open; the failure mode is a typed DEGRADED, never a silent pass, but a model that wraps the JSON in prose or adds a second object will be held. A short real board on ai is the check, and I would rather run it with you than spend seat quota unasked.
  • Under --landing-tier, the president sees seat text that now contains JSON; its prompt is unchanged. The extraction of findings from seat prose (agent-harness#1110) is not touched.
  • main's own CI for the agent-harness#1404 merge (d6d02f61) was still running when I opened this, and the run for cd16aa48 had one failed job (test_mutation_definitions_are_frozen_but_not_executed_preimplementation, the known local-only flake of agent-harness#1127, here in CI for the first time).

Overlap

git merge-tree against the open PRs that touch the same files: agent-harness#1432 (cli.py) no conflict; agent-harness#1438 (panel_invoker.py) conflicts only in CHANGELOG.md, which whichever merges second resolves.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…le reply per seat

advisor-board --reply-format json asks every seat for ONE JSON object (verdict, summary,
findings[]) then the usual last line, and verifies it with the strict panel_reply verifier that
shipped in 0.7.25 with no caller. A verified reply is attached to the seat (PanelLegResult.panel_reply)
and appears under reply in --json and in the streamed per-seat verdict file; one that does not
verify is DEGRADED with a typed panel_reply_<kind> detail and its text kept, so the governed
classifier holds it like any non-conforming review.

The JSON is followed by the legacy last line on purpose: every seat transport decides a reply is
complete from its last line, so no transport changes, and the verifier requires the line to agree
with the JSON.

The format reaches the seats through the brief, not the prompt: the brief is bound by the HARDEN
instruction digest and the sealed prompt envelope and is resolved at about ten call sites, so the
CLI extends the review brief itself, stages it as one file and passes it as brief_ref, with the
digest minted from exactly those bytes (the advisory contract's mechanism). --advisory is refused
with it (an advisory review is recognised by the exact digest of its contract) and so is agy
canary capture.

invoke_board(reply_format=...) verifies each seat before any on_leg_complete callback sees it and
again after native fills are bound (both binding sites). The default is byte for byte unchanged and
_default_spawn's signature is untouched.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-10T02:24:56.509780Z c799038 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7990383b4

ℹ️ 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".

artifact_text = artifact_path.read_text(encoding="utf-8")
payload = native_fill_request_payload(
board, artifact_text, **({"brief_ref": advisory_brief_ref} if advisory else {}),
board, artifact_text, **({"brief_ref": advisory_brief_ref} if advisory_brief_ref is not None else {}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve reply format in native-fill resume output

When --emit-native-request --reply-format json is used, this binds the request to the extended structured-reply brief, but the early-return JSON record does not include reply_format and the printed re-run command omits --reply-format json. Following that generated command reconstructs the ordinary brief, so preflight_native_leg_fills rejects the completed fill with a brief-digest mismatch before any seat launches. Include the reply format in both the record and suggested resume command.

Useful? React with 👍 / 👎.

@ViperJuice

ViperJuice commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

Full suite vs main (the item I left open in the description)

Same selection as CI's PR job (-m "not dotfiles_integration", the chronology-scope node deselected, the two legible-* modules ignored), -n 12 --dist loadfile, in a scratch venv, this branch against a detached checkout of origin/main (d6d02f61).

passed failed errors distinct failing/erroring tests
this branch 12,145 72 320 387
main 12,103 75 320 390
  • Failing on the branch but not on main: none.
  • Failing on main but not on the branch: 3 (test_contract_floor_guard_378::test_real_dist_owns_imported_module, test_sandbox_placement::test_local_equivalence_matches_the_base_golden, test_seat_executor_review::test_convergence_review_waits_for_completion_without_a_model_deadline). This change touches none of them and I do not know why they differ; I read it as run-to-run variance in the scratch environment, not as an effect of this PR.
  • The 320 errors and ~70 failures are the same on both sides. They are environmental to the scratch venv (agy canary and similar suites that need the CI image), not caused by this change; CI's pytest jobs are the authority.
  • Passed counts differ by 42: the 38 tests this PR adds, plus the 3 above that pass on the branch, plus one test that was skipped on main and ran on the branch (skipped 667 vs 668; I did not identify it).

Correction to the earlier comparison: my first run of this comparison silently ran nothing (pytest-xdist missing from the scratch venv, pytest exited on a usage error, both result files empty, and the diff printed "no differences"). The numbers above are from the rerun, where both runs completed and produced summaries.

🤖 Generated with Claude Code

ViperJuice and others added 2 commits October 10, 2026 18:12
CHANGELOG.md only: keep both the --reply-format json entry and the grok
seat tree entry (agent-harness#1438).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
CHANGELOG.md only: keep the --reply-format json entry alongside the entries
main gained since the last merge.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant