fix(quad-cli): transport-integrity nonce + PR #16 review follow-ups - #17
Merged
Merged
Conversation
…x docs (closes #15) Implements the remaining scope of issue #15 that PR #16 explicitly deferred, plus three non-blocking findings surfaced by cross-model review (Opus/Sonnet/Codex) of PR #16. Transport-integrity nonce: - Each run issues a random per-run token and instructs every reviewer to echo it back as a top-level transport_nonce field, so a schema-valid response can be distinguished from one that silently worked from a truncated/stale/altered prompt. - reviewers.nonce_status records confirmed/not-echoed/mismatch/unavailable per declared tool (only under --artifact, alongside the rest of reviewers/environment). - A mismatch (present but wrong echo) drops that reviewer from consensus even though its JSON was otherwise schema-valid. A missing echo is NOT penalized -- it is indistinguishable from an older CLI/model that never learned the field, per issue #15's own framing. - QUAD_CLI_TRANSPORT_NONCE env var overrides the generated token for deterministic test fixtures only. --min-reviewers revisit: already resolved by PR #16's documented decision to keep the default at 2 with recorded rationale; no code change needed, closing out the remaining issue #15 item. Review follow-ups: - observed_failure diagnostics now redact embedded absolute paths (home-dir and otherwise), not just resolved_path, closing the leak Codex's review of PR #16 found. - Documented that reviewers/environment are only populated under --artifact (doc previously implied they were unconditional like status/reviewers_effective). - Documented the ~40s, --gate-timeout-exempt cost of --artifact's CLI version re-probing. Refs #15
drvoss
force-pushed
the
fix/quad-cli-nonce-and-review-followups
branch
from
August 27, 2026 09:45
77e344a to
6cc8e00
Compare
…fore Sonnet's cross-review of this PR found that placing the nonce instruction before <diff> only proves the reviewer received the instructions header, not the diff body -- the realistic failure mode (tail truncation of the usually-large diff) would still let a reviewer see and correctly echo the nonce while never receiving the diff, producing a false 'confirmed'. Moving the instruction after </diff> means confirmed now requires having received the entire prompt, diff included; any truncation before that point yields not-echoed instead. Also verified (and rejecting as a false positive) Sonnet's other finding: the local 'main' branch ref used during review was stale (pre-dating PR #16's squash-merge), which made PR #16's already-merged --min-reviewers rationale text look new in a diff against local main. origin/main already contains it; no change needed there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #15's remaining scope (transport-integrity nonce; --min-reviewers revisit already resolved by PR #16's documented decision) and addresses the three non-blocking findings from the Opus/Sonnet/Codex cross-review of PR #16 (path-leak in observed_failure, doc/behavior mismatch for reviewers/environment gating, undocumented --artifact perf cost).
Changes
eviewers.nonce_status\ records confirmed/not-echoed/mismatch/unavailable per declared tool. Mismatch drops the reviewer from consensus; missing echo does not (ambiguous per design).
edactPathTokensInText\ fixes the path-leak in \observed_failure.
Testing
pm test: 1116/1116 pass (was 1110; +6 new)
pm run validate,
pm run lint:md: pass
pm run smoke:quad:available: ran live against installed codex CLI, no crash, nonce_status populated correctly
Refs #15