fix: BUG-LOOPS-002: worker-verifier workflow can succeed without task comments or completion

[andrei-hasna]

Objective

BUG-LOOPS-002: worker-verifier workflow can succeed without task comments or completion

PersonalNotes worker/verifier workflow personalnotes:platform-wv:2a2d8602:b437a49817fa:workflow run e52a5ad23f2e succeeded all steps (prepare-worktree, worker, verifier) at 2026-06-30T15:47:53Z, but todos inspect for task 2a2d8602 still showed status pending and zero comments. This matches Socializer evidence writeback risk. Expected: worker/verifier template records workflow id, run id, worker evidence, verifier evidence, and marks the task done only when verifier passes, or records a failure/follow-up. Current state makes successful agent work invisible to the task queue and causes repeated routing risk. Do not expose secrets in captured output.

Verification

• policy source: base 35c9ec6 (immutable commit — agent-proof)
• install: pass
• typecheck: pass
• build: pass
• test: pass

---

Run run_cc89364f683a · backend codewith · task 86146f11-3247-4c74-a1a7-a28832ea0266
🏭 Generated by @hasnaxyz/factory

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

Adversarial review — REQUEST CHANGES (hold, do not merge as-is)

Reviewed alongside #136, which targets the same defect class. #136 is now merged (868bcc1). This PR conflicts with it and, on its own acceptance criterion, has a P1 defect. The dependency-status plumbing here is genuinely valuable and should be salvaged — please rebase rather than abandon.

Evidence gathered

Gates on head 08ec2389 (measured unpiped, cmd > log 2>&1; rc=$?):

• bunx tsc --noEmit -p tsconfig.json → rc=0
• bun test src/lib/templates.test.ts src/lib/workflow-runner.test.ts → rc=0, 81 pass
• CI bun (ubuntu/macos) + runner image security are red — but they are equally red on main (35c9ec66, failing step Supply-chain checks / Scan runner image). Pre-existing, not attributable to this PR.

I rendered the writeback script and executed it against a stubbed todos on PATH. It fails closed on every plumbing break, which is good work:

input	rc	result
{"worker":"succeeded"}	0	evidence comment written
{"worker":"failed"}	1	worker step did not pass: failed
{"worker":"timed_out"}	1	worker step did not pass: timed_out
statuses env absent	1	missing worker dependency status
LOOPS_WORKFLOW_RUN_ID absent	1	LOOPS_WORKFLOW_RUN_ID is required
statuses not JSON	1	... is not valid JSON
{"verifier":"failed"}	1	verifier step did not pass: failed

P1 (blocking) — the writeback marks the task done on exit code alone, which is a regression in the exact scenario this PR exists to fix

verifier-writeback runs todos done whenever the verifier step's dependency status is succeeded. There is no check on what the verifier actually did. Demonstrated: LOOPS_WORKFLOW_DEPENDENCY_STATUSES='{"verifier":"succeeded"}' → rc=0 and TODOS_CALLED: --project /proj done task-123 --notes ....

Compare the premise of BUG-LOOPS-002 — an agent that exits 0 without doing the work:

• main today: agent no-ops, exits 0. The agent never runs todos done, so the task stays open — a residual signal a human can catch. The workflow is wrongly green (the bug), but the task record is honest.
• with this PR: agent no-ops, exits 0. OpenLoops marks the task done and writes the comment "Verifier step passed after worker evidence; OpenLoops is marking the source task done". No worker evidence was examined. That sentence is not true, and it is now durable provenance in the task record.

So for the no-op agent this PR converts a visible open task into a closed one with a false attestation. Exit code 0 is being treated as proof of work; it is only proof of exit. Per our house rule, an inference must be marked as an inference — the comment asserts a check that did not happen.

Fix direction: gate todos done on evidence content, not just dependency status. #136's taskEvidenceGateCommand already does exactly this and is now on main — reuse it.

P2 (non-blocking, but note it)

Scope gap. todosWritebackStep is wired only into renderTodosTaskWorkerVerifierWorkflow (templates.ts:835,845). renderTaskLifecycleWorkflow (templates.ts:850) gets nothing. #136 covers both templates. A rebase should close this.

Marker format will not satisfy #136's gate. #136 matches the comment's first line by exact equality. The formats differ:

• fix: BUG-001: workflow success can hide missing task evidence and failed agent execution #136 expects: openloops:worker=evidence task=task-123 event=evt-9
• this PR writes: openloops:worker=evidence task=task-123 event=evt-9 workflow=wf1 workflow_run=run1

So the deterministic writeback contributes nothing toward satisfying the merged gate — the agent would still have to emit #136's marker itself, and both gates would fire independently. If the intent is for runtime-written evidence to satisfy the gate (it should be — that is strictly stronger than trusting the agent), align the formats or have the gate match a prefix.

Conflicts

Merging #136 first was deliberate. This PR now conflicts in 4 files (measured with git merge --no-commit --no-ff, conflicts in both orderings):
src/lib/templates.ts, src/lib/templates.test.ts, src/lib/__snapshots__/templates.test.ts.snap, src/cli/index.test.ts

The conflict is semantic, not just textual: this PR replaces the workerVerifierSteps(...) call in that template with an explicit step graph, while #136 appends a gate step after it. A rebase needs a human decision, not a merge driver.

What I'd like to see

1. Rebase on current main (has fix: BUG-001: workflow success can hide missing task evidence and failed agent execution #136).
2. Keep the dependency-status plumbing (workflowDependencyStatuses through executor + workflow-runner) — that is the durable, reusable contribution here and it is well built.
3. Keep the failure writeback (worker/verifier failed/timed_out → comment + red run). That half is a genuine improvement over main.
4. Make todos done conditional on the evidence check, not on exit status alone.
5. Extend to renderTaskLifecycleWorkflow.
6. Add a test that a successful-but-empty verifier does NOT complete the task. Note the three tests added here assert structure and plumbing; none asserts that a no-op success fails — which is consistent with the P1 above.

Not blocking, unrelated: workflow-runner.test.ts "cursor pattern (worker opens PR, no artifact)" is load-sensitive and times out at 5000ms under a loaded box. It fails on pre-merge main 35c9ec6 too, so it is pre-existing — filing separately.

[andrei-hasna]

Closing as superseded — the diagnosis was right, the remedy belongs in its own change

Tracked as todos task b2cd48cf — "loops: gate task completion on evidence content, not step exit status (supersedes PR #138)" — which carries the full design, the salvage list, and the trap below.

Closing rather than holding, for one reason: the fix this needs is a change to the completion contract (what entitles the runtime to mark a task done), not a repair to this diff. Leaving it open would park it in the queue looking nearly-ready when it actually needs a design decision first.

What was right, and it was the harder half

You found the real defect and you built the right primitive for it. The LOOPS_WORKFLOW_DEPENDENCY_STATUSES plumbing through executor.ts and workflow-runner.ts is the durable contribution here, and it is well built — I rendered the writeback script and ran it against a stubbed todos, and it fails closed on every plumbing break:

input	rc	result
{"worker":"succeeded"}	0	evidence comment written
{"worker":"failed"} / {"worker":"timed_out"}	1	worker step did not pass: …
statuses env absent	1	missing worker dependency status
LOOPS_WORKFLOW_RUN_ID absent	1	LOOPS_WORKFLOW_RUN_ID is required
statuses not JSON	1	… is not valid JSON
{"verifier":"failed"}	1	verifier step did not pass: failed

Gates were green too: tsc --noEmit rc=0, bun test templates+workflow-runner rc=0 (81 pass).

Why it could not merge as written

verifier-writeback runs todos done whenever the verifier step's dependency status is succeeded, with no check on what the verifier did. Against the premise of this very bug — an agent that exits 0 having done nothing:

• main today: the agent never runs todos done, so the task stays open. The workflow is wrongly green (the bug), but the task record is honest.
• with this PR: OpenLoops marks the task done and writes "Verifier step passed after worker evidence; OpenLoops is marking the source task done". No worker evidence was examined. That sentence becomes durable, false provenance.

So in the one scenario it exists to fix, it turned a visible open task into a closed one carrying a false attestation. Exit 0 is proof of exit, not proof of work. Your own tests reflect this honestly — the three added here assert structure and plumbing; none asserts that a successful-but-empty verifier fails, because under this design it does not.

Why #136 was merged instead

#136 checks evidence content (status, both markers, non-placeholder bodies, ordering) and is demonstrably discriminating — five planted silent successes all rc=1, genuine rc=0, and reverting its source fails 10 tests including "workflow fails when successful agents leave no task completion evidence". Its weakness is the mirror of this PR's strength: the evidence it checks is agent-authored, so it verifies compliance rather than work.

Neither half closes the hole alone. They are complementary, which is what b2cd48cf is for: the runtime writes the evidence (your half) and the gate verifies it (#136's half). Note the two do not currently compose — #136 matches the comment's first line by exact equality on openloops:worker=evidence task=<id> event=<eid>, while this PR writes … workflow=<id> workflow_run=<id> appended, so runtime-written evidence would not satisfy the merged gate.

Trap to carry forward — this one is easy to lose

workflow-runner.ts blockedExitCodesForStep() silently downgrades a failing step to status skipped (never failure) when the step id/name matches /gate/i and the exit code is in DEFAULT_BLOCKED_EXIT_CODES = [12]. #136 avoided this deliberately, twice over: it named its step task-evidence-check (no "gate" match) and set blockedExitCodes: [] explicitly. Any future evidence gate that puts "gate" in its id or name, without that override, will silently neuter itself — a guard against silent success that fails silently.

Also worth carrying over

todosWritebackStep was wired only into renderTodosTaskWorkerVerifierWorkflow (templates.ts:835,845); renderTaskLifecycleWorkflow got nothing, while #136's gate covers both.

Not blocking and not yours: this branch now conflicts with main in 4 files after #136 landed, semantically rather than textually (this PR replaces the workerVerifierSteps(...) call; #136 appends after it). b2cd48cf starts from current main rather than rebasing this.