The verification pass cannot read the Verification section a real plan wrote, and runs a line of its prose as a command
Maintainers usually reply within 1 day
Assessment
This issue has not been assessed yet.
Description
#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.
-
A prose line must never become a command. The
**Label**: sentenceshape 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 theverifyEmptyValueskeys as that value — any of which turns this case into a skip with a reason. -
"I could not read it" is not "there was nothing to measure." A
## Verificationsection that exists and yields no runnable command currently reportsnotRun, 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.incompleteis the status that already means exactly that, and it does fail under enforcement. ReservenotRunfor 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from konveyor/agentic-controller
-
bug stream/controller
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
konveyor/agentic-controller#164 ·
Maintainers usually reply within 1 day
-
documentation good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
konveyor/agentic-controller#124 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
konveyor/agentic-controller#323 ·
Maintainers usually reply within 1 day
-
Provider-aware Gateway verification for aws-bedrock / gcp-vertex-ai, so bad credentials fail at verify rather than mid-runPossibly taken @ibolton336 claimed this 3 days ago. Openenhancement stream/controller
konveyor/agentic-controller#304 · 1 assignee ·
Maintainers usually reply within 1 day
-
verify skill: require the whole Verification block to pass on the final tree before committingPossibly taken @ibolton336 claimed this 3 days ago. Openenhancement stream/controller
konveyor/agentic-controller#301 · 1 assignee ·
Maintainers usually reply within 1 day
All issues in konveyor/agentic-controller
Similar issues
-
agent-butler-finding bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
jordansmall/spindrift#6414 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Icinga/icinga-kubernetes#294 ·
Maintainers usually reply within 4 days
-
Cluster details: the Control Plane version field has a different border color than its icon boxOpenkind/design sig/ui
Difficulty 1/5 Under an hour Newbie friendliness 92/100
kubermatic/dashboard#8449 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
filecoin-project/curio#1632 ·
Maintainers usually reply within 1 day