v6.7 Part 2: reviewer-field verification (#29, #31) - #12
Merged
Conversation
4 tasks
jarvis-stark-ops
force-pushed
the
kaipo/v6.7-reviewer-fields
branch
from
June 9, 2026 23:22
7be631b to
9c0bbc0
Compare
jarvis-stark-ops
force-pushed
the
kaipo/v6.7-reviewer-fields
branch
from
June 9, 2026 23:40
9c0bbc0 to
de76994
Compare
This was referenced Jun 10, 2026
jarvis-stark-ops
changed the base branch from
kaipo/v6.7-completion-gates
to
main
June 10, 2026 15:58
…ding
Adds `verify_reviewer_fields` and wires it into `complete_task`. The
self-review of the original Part 2 surfaced four P0/P1 regex bugs —
this implementation switches to a line-by-line YAML-ish parser that
sidesteps all of them.
Closes hermes-jarvis#29 (Tony test_quality field at verdict)
Closes hermes-jarvis#31 (Tony adversarial-review lens)
Context: hermes-jarvis#61 (bootstrap-paradox case study)
## What's required
test_quality:
imports_match_deliverable_entrypoints: <true|false|not_applicable: ≥8-char reason>
evidence:
- <bullet item with tests-path AND :N line reference>
When the task body mentions HTTP/server surfaces in a path-like
context (`app/api/<file>`, `server/<file>`, `route.ts`,
`openapi.<ext>`, etc.):
adversarial_pass:
env_vars: <enumeration or honest [] / none>
request_inputs: ditto
file_paths: ditto
external_io: ditto
## Self-review fixes baked into this commit (not a follow-up)
The first version of Part 2 used regex with lazy `[\s\S]{1,N}?` captures
plus lookahead for the next field key. Four ways that broke:
1. P0 — Field leech: greedy/lazy captures crossed section boundaries,
so a reviewer who put real content in `external_io` made empty
`env_vars`/`request_inputs`/`file_paths` appear satisfied.
2. P0 — Prose evidence: `evidence:\n a paragraph` passed the
`len >= 20` check because the substantive bar was character count,
not structured citations.
3. P1 — Empty not_applicable: `imports_match_deliverable_entrypoints:
not_applicable:` (no reason) was accepted because the unanchored
regex captured the next field's name as the reason.
4. P1 — Docs-mention adversarial trigger: bodies that mentioned
`openapi` or `public-api` in prose force-required adversarial_pass
even on docs-only review tasks.
Fix: switch to a `_parse_field` line walker that respects YAML
indentation. A field's body is the consecutive following lines
indented STRICTLY DEEPER than the field line; a sibling at equal or
shallower indent ends the body. Same-line ("inline") and multi-line
("block") values are both supported. Adversarial-trigger patterns now
require path-like context (extension or path separator), not prose.
Evidence-citation check: at least one bullet-list line containing
both a `tests/` or `*.test.*`/`*_test.*` path token AND a `:N` line
reference; explicit `[]`/`none`/`n/a` honest-empty markers are
accepted.
## Tests
16 new tests in `TestReviewerFields` and 2 in
`TestReviewerFieldsIntegration` covering:
- Bare-approve rejection
- Baseline + full verdicts pass
- Field leech rejected (the four sibling fields evaluated independently)
- Prose evidence rejected
- Empty not_applicable rejected; not_applicable with ≥8-char reason
passes
- Docs-only OpenAPI mention does NOT force adversarial_pass
- OpenAPI with path-like context DOES trigger
- Non-review roles skipped; case-insensitive role match
- x_no_reviewer_fields opt-out via metadata
- Integration: Tony bare-approve raises CompletionGateError, emits
`completion_blocked_v6_7_gates` event, task state preserved
79 passed in tests/cli/test_kanban_completion_gates.py +
adjacent regression set — zero failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1/N6 fixes A second-pass independent review of the reviewer-fields PR found three new findings that this commit addresses: ## N1 — Empty body crashed splitlines()[0] Same shape as the workspace-diff crash bug fixed on PR #11. When `body` was empty or None, `verify_reviewer_fields` raised IndexError before constructing the violation. Real tasks with empty body columns hit this. Fix: guard splitlines result. ## N2 — Inline prose ≥20 chars bypassed adversarial_pass gates `adversarial_pass.env_vars: see above explanation about general env safety` (51 chars of prose, zero structure) passed the gate because the substantive check was character count. Same laundering shape as the prose-evidence bypass we fixed in the previous self-review. Fix: introduce `_has_adversarial_structure` requiring at least one of: bullet item line, UPPER_SNAKE env-var token followed by colon, path-like substring with a slash (e.g. `lib/ingest.ts`), or a source file extension (.ts, .py, .yaml, etc.). Honest empty markers ([], none, n/a) still accepted. Pure prose now correctly rejected. ## N6 — `evidence: none` bypassed citation discipline on code reviews When the body indicated code-touching review (`app/api/...`, `route.ts`, etc.), the previous gate accepted `evidence: none` as an honest-empty declaration. But for a code review, "no test evidence" is exactly the problem — Tony's job is to verify tests cover the deliverable. Fix: `_field_present` now takes `code_change_context` and disallows the honest-empty escape on `test_quality.evidence` when the body triggers adversarial requirements. Non-code reviews (docs, design) still accept `evidence: none`. ## Tests 7 new tests in `TestReviewerFieldsSelfReviewFixes`: - empty body doesn't crash - None body doesn't crash - adversarial inline prose bypass rejected (the N2 exploit) - structured inline values (ENV: bound, file.ts, path/like) pass - evidence:none blocked when body triggers adversarial - evidence:none allowed for pure docs review - bullet list under adversarial fields is the canonical happy path 64 passed in tests/cli/test_kanban_completion_gates.py — up from 57. 87 passed across PR #12 + adjacent regression set; zero failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rsarial structure The second self-review of PR #12's N2 fix found a remaining bypass: prose that name-dropped ``README.md`` / ``config.yaml`` / ``notes.md`` still satisfied ``_has_adversarial_structure`` because the source-file regex matched those extensions. Workers could write \`\`adversarial_pass.env_vars: see README.md elsewhere\`\` and pass — exactly the marker-padding bypass shape N2 was supposed to close. Fix: drop ``md``/``yaml``/``yml``/``json``/``txt`` from ``_ADVERSARIAL_SOURCE_FILE_RE``. Only code-bearing extensions count as structural cues. Honest yaml/json/md mentions inside a real PATH still satisfy ``_ADVERSARIAL_PATH_RE`` (e.g. ``configs/app.yaml`` matches the path-with-slash regex), so legitimate references aren't lost. Tests: - ``test_prose_padded_with_readme_md_rejected`` — verdict that drops README.md / config.yaml / notes.md / docs.json into prose fails the gate on all four adversarial fields. - ``test_real_yaml_path_with_slash_still_passes`` — verdict that cites ``configs/env.example`` / ``schemas/inputs.yaml`` still passes via the path regex. 66 passed in tests/cli/test_kanban_completion_gates.py — up from 64. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jarvis-stark-ops
force-pushed
the
kaipo/v6.7-reviewer-fields
branch
from
June 10, 2026 15:59
1ed0884 to
f6aaa8e
Compare
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.
Stacks on top of #11 — base is `kaipo/v6.7-completion-gates` so the diff shows only Part 2's changes. After PR #11 lands and this PR's base swaps to `main`, merge cleanly.
Summary
Adds `verify_reviewer_fields` gate that fires when a review-role worker (tony, tchalla, vision, reviewer) calls `kanban_complete`. The verdict text must include structured discipline fields proving the review actually happened.
Closes hermes-jarvis#29 (Tony `test_quality` field) and hermes-jarvis#31 (Tony adversarial-review lens).
Required fields (always)
```
test_quality:
imports_match_deliverable_entrypoints: <true|false>
evidence:
- file:line cites where tests actually exercise the deliverable
```
Additionally required when body mentions HTTP/server surfaces
When the task body matches `app/api/`, `server/`, `route.ts`, `openapi`, "request handler", "public-api", or "http endpoint":
```
adversarial_pass:
env_vars: []
request_inputs: []
file_paths: []
external_io: [<enumeration with timeout/auth per call>]
```
Each section accepts explicit `[]` / `none` for honest empty answers — vs missing field which rejects.
Opt-outs and graceful paths
Why this is a separate PR from Part 1
Part 1 (#11) shipped gates that catch FABRICATED implementation (empty diff + too-fast + stray files). Part 2 ships gates that catch FABRICATED reviews (rubber-stamp "approve" with no structured evidence). They share a module but address orthogonal failure modes — splitting makes review cleaner.
Test plan
🤖 Generated with Claude Code