fix(ploeg): never read a current Run's verification from agent prose #204

Closed

ryangr0 wants to merge 4 commits from ryangr0/ploeg-verification-provenance into development AGit

pull from: ryangr0/ploeg-verification-provenance

merge into: webgrip:development

webgrip:development

webgrip:renovate/all-non-major

webgrip:agent/vik-1897

webgrip:main

webgrip:renovate/lock-file-maintenance

webgrip:renovate/go-1.25.x

webgrip:renovate/pin-dependencies

webgrip:agent/vik-1316

webgrip:agent/vik-1343

Conversation 0 Commits 4 Files changed 29 +885 -145

ryangr0 commented 2026-10-03 13:04:23 +00:00

Owner

Copy link

Stacked on #195. It touches the same RoundReports select and RunReport fields. The three commits from #195 come first; this PR's own change is the last commit (4d79debc). Merge #195 first. Implements VIK-1780 (handoff EXEC-03), the remainder of VIK-1733 / #152.

Problem

Since #152, a structured verification record beats prose. But parseEvidence still treats every writing Run without a record as a Run from an older worker, and parses its summary marker [Ploeg verification passed] and its ### Ploeg verification section.

A current Run with no configured checks, or with checks skipped because it was cancelled or opened no pull request, also has no record. Its summary and findings are the agent's to write. So the usage report on the pull request showed Verification: passed and Commit verified: \``.

TestACurrentRunWithoutAWorkerRecordNeverShowsAPass reproduces this through the real store. On development it renders passed with the agent's invented commit.

Invariant

• For a Run ploegd stores now, the worker's structured record is the only verification evidence. Prose never supplies a result or a commit, and no record means "not recorded".
• Prose counts only for a Run stored before ploegd kept this distinction, and then only as visibly unverified history.
• A record that contradicts itself is refused at the API.

Change

• Migration 0036. It adds agent_runs.evidence_version (nullable SMALLINT, ≥ 1). ReportOutcome sets it to store.CurrentEvidenceVersion (1) for every reported outcome. Existing rows stay NULL; nothing infers provenance from their text.

• parseEvidence. A record always wins. With no record, a stamped Run is "not recorded". Only an unstamped Run goes through legacyEvidence, which now marks its result Historical.

• Usage report. A historical result reads Verification (historical prose, not a worker record): passed, unverified and Commit named in that prose (unverified): …, never Commit verified.

• Verification.Validate (applied by handleOutcome to every record; it refuses the report) now also refuses:

  • a passed record with zero checks;
  • a passed check with a nonzero exit code;
  • a failed check with exit code 0;
  • missing or out-of-order start and finish times;
  • a check that ran outside the record's time window;
  • a not_run check that carries an exit code or times.

  This is the chosen contract: these records are rejected, not normalised. Only a broken or forged worker sends one. It is documented in the schema description and on Validate.

• Docs. docs/how-to/review-an-agent-pr.md and the outcome schema describe "not recorded" and the historical label.

Acceptance criteria → tests

• Zero checks configured, spoofed marker, heading and hash → not recorded: TestACurrentRunWithoutAWorkerRecordNeverShowsAPass. It fails on development.
• Checks skipped by cancellation or a non-delivery outcome → same path, because no record means not recorded. The agent's own verification was already discarded by the worker (TestAnAgentCannotClaimTheWorkersVerification).
• A structured failure survives earlier or later narrative, code fences, duplicate markers and a different heading: TestAStructuredFailureSurvivesAnyNarrative.
• Contradictory records are rejected: 9 new cases in TestVerificationValidateRejectsInconsistentRecords, all of which validated on development, plus 2 API cases in TestValidateOutcomeReport.
• Old rows stay readable and labelled: TestProseOfARunStoredBeforeTheDistinctionIsUnverifiedHistory; the legacy fixture in TestUsageReportEvidenceParsesVerificationAndCommit now expects the historical label.
• Readers cannot store writing-run evidence: unchanged, covered by TestRoundReportsCarryTheWorkersVerification.

Test fixtures with zero timestamps were updated to real ones: the worker's incomplete record and the API's verification passed case.

Commands (at 4d79debc, Go 1.27.1, Node 24.21.0)

• mise exec -- go test ./pkg/harness/ ./pkg/shiftengine/ ./pkg/store/ ./pkg/httpapi/ ./pkg/worker/ -count=1 -v: 600 passed, 0 failed, 4 skipped (the operator qualification tests, which mise run integration runs)
• mise exec -- go test ./... -count=1 in apps/ploeg: 34 packages ok
• mise run verify: all gates passed (vloer 7, demo-replay 1, vloer-extension 4, ploeg 7, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1)

Migration and rollback

Additive column. An old binary ignores it. Rolling back the binary leaves the column in place and harmless. Migration number 0036: if another branch also adds 0036, renumber whichever merges second.

Not in scope

• Running checks on the exact pushed candidate (VIK-1738).
• Delivery identity (VIK-1732).

Refs VIK-1780

🤖 Generated with Claude Code

**Stacked on #195.** It touches the same `RoundReports` select and `RunReport` fields. The three commits from #195 come first; this PR's own change is the last commit (`4d79debc`). Merge #195 first. Implements VIK-1780 (handoff EXEC-03), the remainder of VIK-1733 / #152. ## Problem Since #152, a structured verification record beats prose. But `parseEvidence` still treats every writing Run *without* a record as a Run from an older worker, and parses its summary marker `[Ploeg verification passed]` and its `### Ploeg verification` section. A current Run with no configured checks, or with checks skipped because it was cancelled or opened no pull request, also has no record. Its summary and findings are the agent's to write. So the usage report on the pull request showed `Verification: passed` and `Commit verified: \`<anything>\``. `TestACurrentRunWithoutAWorkerRecordNeverShowsAPass` reproduces this through the real store. On `development` it renders `passed` with the agent's invented commit. ## Invariant - For a Run ploegd stores now, the worker's structured record is the only verification evidence. Prose never supplies a result or a commit, and no record means "not recorded". - Prose counts only for a Run stored before ploegd kept this distinction, and then only as visibly unverified history. - A record that contradicts itself is refused at the API. ## Change - **Migration 0036.** It adds `agent_runs.evidence_version` (nullable `SMALLINT`, ≥ 1). `ReportOutcome` sets it to `store.CurrentEvidenceVersion` (1) for every reported outcome. Existing rows stay `NULL`; nothing infers provenance from their text. - **`parseEvidence`.** A record always wins. With no record, a stamped Run is "not recorded". Only an unstamped Run goes through `legacyEvidence`, which now marks its result `Historical`. - **Usage report.** A historical result reads `Verification (historical prose, not a worker record): passed, unverified` and `Commit named in that prose (unverified): …`, never `Commit verified`. - **`Verification.Validate`** (applied by `handleOutcome` to every record; it refuses the report) now also refuses: - a `passed` record with zero checks; - a passed check with a nonzero exit code; - a failed check with exit code 0; - missing or out-of-order start and finish times; - a check that ran outside the record's time window; - a `not_run` check that carries an exit code or times. This is the chosen contract: these records are **rejected**, not normalised. Only a broken or forged worker sends one. It is documented in the schema description and on `Validate`. - **Docs.** `docs/how-to/review-an-agent-pr.md` and the outcome schema describe "not recorded" and the historical label. ## Acceptance criteria → tests - **Zero checks configured, spoofed marker, heading and hash** → not recorded: `TestACurrentRunWithoutAWorkerRecordNeverShowsAPass`. It fails on `development`. - **Checks skipped by cancellation or a non-delivery outcome** → same path, because no record means not recorded. The agent's own verification was already discarded by the worker (`TestAnAgentCannotClaimTheWorkersVerification`). - **A structured failure survives earlier or later narrative, code fences, duplicate markers and a different heading**: `TestAStructuredFailureSurvivesAnyNarrative`. - **Contradictory records are rejected**: 9 new cases in `TestVerificationValidateRejectsInconsistentRecords`, all of which validated on `development`, plus 2 API cases in `TestValidateOutcomeReport`. - **Old rows stay readable and labelled**: `TestProseOfARunStoredBeforeTheDistinctionIsUnverifiedHistory`; the legacy fixture in `TestUsageReportEvidenceParsesVerificationAndCommit` now expects the historical label. - **Readers cannot store writing-run evidence**: unchanged, covered by `TestRoundReportsCarryTheWorkersVerification`. Test fixtures with zero timestamps were updated to real ones: the worker's `incomplete` record and the API's `verification passed` case. ## Commands (at 4d79debc, Go 1.27.1, Node 24.21.0) - `mise exec -- go test ./pkg/harness/ ./pkg/shiftengine/ ./pkg/store/ ./pkg/httpapi/ ./pkg/worker/ -count=1 -v`: 600 passed, 0 failed, 4 skipped (the operator qualification tests, which `mise run integration` runs) - `mise exec -- go test ./... -count=1` in apps/ploeg: 34 packages ok - `mise run verify`: all gates passed (vloer 7, demo-replay 1, vloer-extension 4, ploeg 7, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1) ## Migration and rollback Additive column. An old binary ignores it. Rolling back the binary leaves the column in place and harmless. Migration number 0036: if another branch also adds 0036, renumber whichever merges second. ## Not in scope - Running checks on the exact pushed candidate (VIK-1738). - Delivery identity (VIK-1732). Refs VIK-1780 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ryangr0 added 4 commits 2026-10-03 13:04:24 +00:00

fix(ploeg): retry a failed reviewer and close review_failed when no review came ... bb9b3861fa

A reading Run that failed was logged and stepped over. When it was the
reviewer of a writer's pull request, the Shift closed plan_exhausted,
the item settled awaiting_review and the tracker said "plan complete"
for a review that never happened (Shift 90, Shift 118).

ADR-0043 (accepted) decides the fix, implemented here:

- A Round whose reading Role's Runs all failed re-opens in place for
  that Role only, through store.ReopenRound, without advancing the round
  counter. Readers that reported are not re-run. A failed writer keeps
  precedence.
- The retry uses the writer's two budgets: MaxRunAttempts for agent
  failures (agent_error, idle, timeout: an ACP watchdog kill stays an
  agent failure) and MaxInfraFailures for infrastructure ones. The pool
  is checked when the retry is claimed, so an empty pool opens no Run
  and the sweep parks the Shift naming the spend.
- When the budgets are spent the Round completes and the plan advances.
  If the plan then ends plan_exhausted and the last reading Round after
  the last writer has a Role with no non-failed Outcome, the Shift
  closes review_failed. With a writer's pull request the item still
  settles awaiting_review. The close message and the tracker comment say
  "not reviewed by an agent: <role> Run failed (<failure_reason>)" and
  "Agent review unavailable", never "plan complete" or an approval.
- A reader that reported without a verdict still closes plan_exhausted,
  and a stuck reader still freezes the plan.

store.RunReport now carries the Run's failure reason. The
shift-orchestration spec's swept-reader scenario is reworded and a
requirement for the reader retry is added. The review how-to lists
review_failed.

VIK-1304

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(vloer): mark a pull request whose reviewer kept failing as unreviewed ... 146cf69fee

Ploeg now closes a Shift review_failed when its reviewer Run failed and
its retries ran out. The pull request still waits for review, so the
item sits in Ready for review, where its row read "No agent verdict".

The review lane chip now reads "Agent review unavailable" with the
reason in its title, the Work Item page says no agent reviewed the pull
request, and the close reason reads "No agent reviewed it: the reviewer
kept failing" wherever Shift close reasons are listed.

VIK-1304

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

chore(site): re-record the demo replay with the review_failed wording ... c2ced7378a

VIK-1304

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

fix(ploeg): never read a current Run's verification from agent prose ... 4d79debc13

parseEvidence treated every writing Run without a structured
verification record as a Run from an older worker and parsed the
"[Ploeg verification passed]" marker and the "### Ploeg verification"
section out of its summary and findings. A current Run with no checks
configured, or whose checks were skipped, also has no record, and both
texts are the agent's to write, so the usage report on the pull request
showed "Verification: passed" and "Commit verified" for whatever the
agent typed.

ploegd now stamps agent_runs.evidence_version = 1 when it stores a
reported outcome (migration 0036; existing rows stay NULL and no text is
read to decide otherwise). For a stamped Run the structured record is
the only evidence: without one the report says "not recorded". Only an
unstamped Run falls back to the prose of its time, and the report labels
it "historical prose, not a worker record ... unverified" and names its
commit as unverified instead of "Commit verified".

Verification.Validate, which ploegd applies to every reported record,
now also refuses a passed record with no check, a passed check with a
nonzero exit code, a failed check with exit code 0, missing or
out-of-order start and finish times, a check that ran outside the
record's time, and a check that did not run but has an exit code or
times. The outcome schema documents the contract.

VIK-1780

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ryangr0 referenced this pull request 2026-10-03 22:13:47 +00:00

build(ploeg): consume Ploeg from ploeg-hq/ploeg and stop publishing it from Unfold #206

ryangr0 referenced this pull request 2026-10-03 22:42:24 +00:00

docs: say a Run's verification comes only from the worker's record #208

ryangr0 referenced this pull request from a commit 2026-10-04 08:39:25 +00:00

docs: say a Run's verification comes only from the worker's record

ryangr0 closed this pull request 2026-10-04 08:41:49 +00:00

Some checks failed

Hide all checks

[Workflow] On Pull Request / checks (pull_request) Has been cancelled

Details

[Workflow] On Pull Request / warnings (pull_request) Has been cancelled

Details

[Workflow] On Pull Request / release-policy (pull_request) Has been cancelled

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

webgrip/unfold!204

No description provided.