The verification pass cannot read the Verification section a real plan wrote, and runs a line of its prose as a command

Closed
#336 0 comments 0 reactions 1 assignee View on GitHub

Maintainers usually reply within 1 day

@ibolton336 is already working on this.

Since Nov 4, 2026.

  • #337 by @ibolton336 — open

Assessment

This issue has not been assessed yet.

Description

bug stream/controller

#300 landed in #308: the harness re-runs the commands in docs/plan.md's ## Verification section itself instead of trusting the handoff's Status. I went back to the artifact that motivated #300 and measured what the merged harness does with it. It fails the run — but it never runs a single one of the plan's commands, and one word of English prose is the difference between failing and passing.

The artifact

ibolton336/tackle2-ui@f31caea66, the verify stage's own commit from workflow run ui-5nlrx. It is the tree that reported "Build: passed, remaining errors: none" while tsc failed with 45 errors — the case #300 was filed for. Still exactly 45 error TS, all _isPending renames.

Its plan's Verification section, written by the plan agent, looks like this:

## Verification

Run from the repository root:

```bash
npm ci
npm run build -w common
npm run tsc -w client
npm run lint -w client
npm run format:check
npm run test -w client -- --maxWorkers=2
```

`tsc` is the authoritative check: ...

**Runtime verification**: skipped — this migration changes only the query library API surface, not application behavior. The type system enforces correctness.

What the merged harness does with it

parsePlanVerification drops fenced blocks first, via handoffProse — deliberately, so a plan that quotes the plan template back cannot have its <build command> placeholders run. That is the right call for the fence, but it means every command this plan names is discarded before parsing begins.

What survives is the bold prose line. verificationItem takes the leading ** as its list marker, so the line parses as label Runtime verification with value skipped — this migration changes only the query library API surface, not application behavior. The type system enforces correctness. That value is 132 runes, under verifyCommandMaxLen (300), and it is not one of verifyEmptyValues (the map holds the bare word skipped, not a sentence beginning with it). So classifyVerifyItem falls through to default: and sets it as a command.

Driving runPlanVerification from main against a clone of that commit:

verification: running Runtime verification — skipped — this migration changes only the query library API surface, not application behavior. The type system enforces correctness. (timeout 5m0s)
verification: Runtime verification FAILED — exit 127 after 0s
verification: Runtime verification output (tail of 36 captured bytes):
/bin/sh: skipped: command not found
{
  "status": "failed",
  "reason": "Runtime verification failed: skipped — this migration changes only the query library API surface, not application behavior. The type system enforces…",
  "commit": "f31caea6659f10d04f844f56f8a749944cf55776",
  "commands": [
    {
      "label": "Runtime verification",
      "command": "skipped — this migration changes only the query library API surface, not application behavior. The type system enforces…",
      "status": "failed",
      "exitCode": 127,
      "seconds": 0.056,
      "output": "/bin/sh: skipped: command not found"
    }
  ]
}

tsc, lint, format:check and the tests never ran. The verdict is red and the evidence is command not found on a sentence of English. A reviewer reading it learns nothing about the 45 type errors, and the recorded commands array actively misleads about what was measured.

One word flips it to green

Shorten that prose line to the bare word the empty-value map covers:

**Runtime verification**: skipped

Same tree, same 45 errors, and now:

{
  "status": "notRun",
  "reason": "the ## Verification section of docs/plan.md names no command to run"
}

notRun never fails the stage, including under HARNESS_VERIFY_ENFORCE=on — correctly, by its own definition, since notRun means "nothing to measure". So on this plan the original false green survives #308 intact. Red and green are one word of prose apart and neither is a measurement.

The mechanism itself is fine

The same commands in the form catalog/skills/plan/SKILL.md specifies:

## Verification
- Build: npm run build -w common
- Typecheck: npm run tsc -w client
cmd="npm run build -w common"  status=passed
cmd="npm run tsc -w client"    status=failed
reason: "Typecheck failed: npm run tsc -w client"

Caught exactly as designed, with the real compiler errors in the output. The gap is the plan format, not the verification logic. The skill does specify the - Build: list form, but nothing enforces it, and the plan agent on a real run did not follow it. verify_test.go's planFromSkill fixture is commented "as the plan skill specifies it and as a real plan writes it" — that second half is the assumption that fails here.

Suggested fixes

Two separable defects, in priority order.

  1. A prose line must never become a command. The **Label**: sentence shape is the hole: the ** satisfies the list-marker class and a sentence under 300 runes satisfies the command test. Options include requiring a real -/* marker (not **), treating a value containing sentence punctuation followed by a space as prose, or treating a value whose first word is one of the verifyEmptyValues keys as that value — any of which turns this case into a skip with a reason.

  2. "I could not read it" is not "there was nothing to measure." A ## Verification section that exists and yields no runnable command currently reports notRun, which by design never fails a stage. But a section full of commands in a fence is not an absent section — there was something to measure and the harness could not attribute a result to it. incomplete is the status that already means exactly that, and it does fail under enforcement. Reserve notRun for no plan, no section, HARNESS_VERIFY=off, and a stage that never claimed success.

Worth considering alongside: having the plan skill's Verification contract be the parser's contract, so a plan whose commands are unreadable is a plan-stage failure rather than something discovered one stage later.

Reproducing

git clone -b dogfood/react-query-v5 https://github.com/ibolton336/tackle2-ui && git checkout f31caea66

then call runPlanVerification(ctx, repo, dir, branch, 5*time.Minute, 20*time.Minute, newRedactor()) against it from harness/cmd/migration-harness. npm ci first if you want case 3 to reach tsc; cases 1 and 2 need no toolchain.

Dominant language
Go
Stars
5
Forks
13
Avg merge
3d 7h
Merged PRs (30d)
44

Getting set up

  • Ships a Dockerfile or Docker Compose file
  • Has a pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from konveyor/agentic-controller

All issues in konveyor/agentic-controller

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.