fix(workflows): fail fast on a malformed args carrier instead of misreporting it
Both workflow scripts destructured object-form `args` behind `args || {}`
only. A named-workflow invocation can deliver `args` as a string; a
non-empty string is truthy, so every carrier field silently read
`undefined` and the failure surfaced far from the cause: implement-loop
dispatched its plan-index agent against "Read the plan at undefined" and
stopped only via a mislabelled NEEDS_CONTEXT after burning agent calls;
compiler-driven-edit briefed its edit agent with "EDIT: undefined" and
mapped the agent's refusal onto the straddle-rule BOUNCE to specify —
a mechanics failure dressed as a design finding.
Guards now run before any agent dispatch, mirroring the existing
malformed-input pattern (the task_range guard):
- A string carrier is first JSON-parsed: a JSON-serialized object
carrier (a stringifying caller or substrate layer that still authored
the documented object form) is accepted; free text does not parse to
an object and is rejected.
- A carrier that is not an object (string, number, boolean, array) is a
distinct infra BLOCKED naming the received type (compiler-driven-edit:
kind bad-carrier) — never a bounce, never NEEDS_CONTEXT.
- Missing or blank required fields fail fast by name, per mode:
standard iter_id+plan_path, mini iter_id+red_test_path+cause_summary,
compiler-driven-edit edit_description+def_site. implement-loop also
rejects an unknown mode, since the required-field set keys off it.
String-form args are NOT a documented interface — the only invocation
examples in the plugin are object-form (implement/SKILL.md); the issue's
free-text-passthrough fix part was dropped on that ground (triage
verification on the issue). implement/SKILL.md now also documents the
infra early-exit end-report shape (minimal {status, iter_id, reason},
no BLOCKED.md, no blocked_detail) in the Iron Law exceptions, Step 2,
and Step 4's no-file special case.
Verified: both scripts pass node --check (async-wrapped, as the Workflow
substrate runs them) and a 17-case stub-agent harness — malformed
carriers block with zero agent dispatches; valid and JSON-serialized
object carriers reach the first agent unchanged.
closes #24
This commit is contained in:
+13
-3
@@ -74,7 +74,9 @@ Its executor is **observe-then-bounce**, shipped as
|
|||||||
would otherwise read as DONE. A clean working tree after the edit
|
would otherwise read as DONE. A clean working tree after the edit
|
||||||
phase is a no-op, not a success → distinct `BLOCKED` (no-op-edit),
|
phase is a no-op, not a success → distinct `BLOCKED` (no-op-edit),
|
||||||
never DONE. Positive evidence (tree differs from HEAD) is required,
|
never DONE. Positive evidence (tree differs from HEAD) is required,
|
||||||
checked both at the edit phase and independently at verify.
|
checked both at the edit phase and independently at verify. (A
|
||||||
|
malformed or missing carrier is likewise a distinct `BLOCKED`
|
||||||
|
(bad-carrier) before any agent runs — never a bounce.)
|
||||||
4. **Bounce** otherwise — the change was not purely behaviour-preserving
|
4. **Bounce** otherwise — the change was not purely behaviour-preserving
|
||||||
after all, so route up per the straddle rule: a site that forces a
|
after all, so route up per the straddle rule: a site that forces a
|
||||||
design decision → `specify`; a site that turns out to **encode new
|
design decision → `specify`; a site that turns out to **encode new
|
||||||
@@ -102,7 +104,7 @@ WITHIN THIS LOOP, MAIN HEAD IS SACROSANCT — NO RESET, NO REVERT; THE LOOP ONLY
|
|||||||
PER-TASK PHASES ARE SEPARATE AGENT CALLS IN THE WORKFLOW SCRIPT — implementer, then spec-compliance, then quality. SPEC COMPLIANCE IS GATED BEFORE QUALITY.
|
PER-TASK PHASES ARE SEPARATE AGENT CALLS IN THE WORKFLOW SCRIPT — implementer, then spec-compliance, then quality. SPEC COMPLIANCE IS GATED BEFORE QUALITY.
|
||||||
TASKS RUN SEQUENTIALLY; A BLOCKED TASK STOPS THE LOOP — NO SKIP-AHEAD (TASK ORDERING DEPENDENCIES ARE UNKNOWN).
|
TASKS RUN SEQUENTIALLY; A BLOCKED TASK STOPS THE LOOP — NO SKIP-AHEAD (TASK ORDERING DEPENDENCIES ARE UNKNOWN).
|
||||||
NEVER PUSH PAST `BLOCKED` BY HAND.
|
NEVER PUSH PAST `BLOCKED` BY HAND.
|
||||||
ON `PARTIAL` OR `BLOCKED`, THE SCRIPT WRITES `BLOCKED.md` AT THE REPO ROOT — UNCOMMITTED BY CONVENTION. ON `DONE`, NO SEPARATE FILE. (ONE EXCEPTION: A NO-OP ITERATION IS `BLOCKED` WITH NO `BLOCKED.md` — ITS TREE IS ALREADY CLEAN, SO THE STATUS + REASON RIDE THE END-REPORT AND THERE IS NOTHING TO CLEAN UP.)
|
ON `PARTIAL` OR `BLOCKED`, THE SCRIPT WRITES `BLOCKED.md` AT THE REPO ROOT — UNCOMMITTED BY CONVENTION. ON `DONE`, NO SEPARATE FILE. (EXCEPTIONS THAT WRITE NO `BLOCKED.md`: A NO-OP ITERATION — ITS TREE IS ALREADY CLEAN — AND ANY PRE-LOOP EARLY EXIT (MALFORMED CARRIER, DIRTY-TREE PREFLIGHT, PLAN-EXTRACT GUARD): NOTHING RAN AND NOTHING WAS WRITTEN, SO THE STATUS + `infra:`-PREFIXED REASON RIDE THE END-REPORT AND THERE IS NOTHING TO CLEAN UP OR EXPLAIN.)
|
||||||
`DONE` REQUIRES POSITIVE EVIDENCE THAT AN EDIT LANDED — A NO-OP ITERATION (`git status --porcelain` EMPTY, untracked files INCLUDED) IS NEVER `DONE`, IT IS `BLOCKED` (the issue #11 vacuous-green shape, guarded on the main path). THE VERDICT IS THE GIT TREE COUNT, NEVER A SELF-REPORT.
|
`DONE` REQUIRES POSITIVE EVIDENCE THAT AN EDIT LANDED — A NO-OP ITERATION (`git status --porcelain` EMPTY, untracked files INCLUDED) IS NEVER `DONE`, IT IS `BLOCKED` (the issue #11 vacuous-green shape, guarded on the main path). THE VERDICT IS THE GIT TREE COUNT, NEVER A SELF-REPORT.
|
||||||
IN MINI MODE THE RED->GREEN IS OBSERVED, NOT ASSERTED: AN INDEPENDENT VERIFY RE-RUNS THE HANDED-OFF RED TEST AND THE SUITE; A STILL-RED TEST OR A REGRESSION IS `BLOCKED` (route back to debug, RED-first).
|
IN MINI MODE THE RED->GREEN IS OBSERVED, NOT ASSERTED: AN INDEPENDENT VERIFY RE-RUNS THE HANDED-OFF RED TEST AND THE SUITE; A STILL-RED TEST OR A REGRESSION IS `BLOCKED` (route back to debug, RED-first).
|
||||||
THE COMPILER-DRIVEN ARM COMMITS ONLY ON CLEAN BUILD + SUITE GREEN UNCHANGED; ELSE IT BOUNCES (specify for a design hole, tdd for discovered new behaviour, debug for a regression).
|
THE COMPILER-DRIVEN ARM COMMITS ONLY ON CLEAN BUILD + SUITE GREEN UNCHANGED; ELSE IT BOUNCES (specify for a design hole, tdd for discovered new behaviour, debug for a regression).
|
||||||
@@ -197,7 +199,10 @@ per-task summaries, concerns, E2E fixtures, the stats path, and — on
|
|||||||
PARTIAL/BLOCKED — the blocked detail. Per-task chatter stayed inside
|
PARTIAL/BLOCKED — the blocked detail. Per-task chatter stayed inside
|
||||||
the workflow's agent contexts and never reached the orchestrator. Read
|
the workflow's agent contexts and never reached the orchestrator. Read
|
||||||
the end-report; it is the per-task summary you build the commit body
|
the end-report; it is the per-task summary you build the commit body
|
||||||
from.
|
from. An **infra early-exit** (malformed or missing carrier, dirty-tree
|
||||||
|
preflight, a plan-extract guard) returns a minimal report instead:
|
||||||
|
`status: BLOCKED` plus an `infra:`-prefixed `reason`, no `blocked_detail`
|
||||||
|
and none of the per-task fields — nothing ran.
|
||||||
|
|
||||||
### Step 3 — Orchestrator inspect + commit step (on DONE)
|
### Step 3 — Orchestrator inspect + commit step (on DONE)
|
||||||
|
|
||||||
@@ -243,6 +248,11 @@ tree:
|
|||||||
already clean. The diagnostic is in the end-report's `blocked_detail`, not
|
already clean. The diagnostic is in the end-report's `blocked_detail`, not
|
||||||
a file. Nothing to clean up or stash; re-plan or re-run from the same
|
a file. Nothing to clean up or stash; re-plan or re-run from the same
|
||||||
clean tree.
|
clean tree.
|
||||||
|
The same no-file shape occurs for an **infra early-exit** (`reason`
|
||||||
|
prefixed `infra:` — malformed/missing carrier, dirty preflight tree, a
|
||||||
|
plan-extract guard): no `BLOCKED.md`, no `blocked_detail`, and the tree
|
||||||
|
exactly as the loop found it. Fix the invocation (or clean the tree) and
|
||||||
|
re-invoke.
|
||||||
1. Read `BLOCKED.md` — `## What did not` names the failure mode and the
|
1. Read `BLOCKED.md` — `## What did not` names the failure mode and the
|
||||||
blocked task's reason. Read `git diff` to see what was attempted.
|
blocked task's reason. Read `git diff` to see what was attempted.
|
||||||
2. Decide:
|
2. Decide:
|
||||||
|
|||||||
@@ -50,6 +50,8 @@
|
|||||||
// Carrier — Workflow `args`:
|
// Carrier — Workflow `args`:
|
||||||
// edit_description: what type/signature change to make
|
// edit_description: what type/signature change to make
|
||||||
// def_site: the definition site (file + symbol) it originates from
|
// def_site: the definition site (file + symbol) it originates from
|
||||||
|
// A non-object carrier or a missing required field fails fast as an infra
|
||||||
|
// BLOCKED before any agent is dispatched (issue #24).
|
||||||
|
|
||||||
export const meta = {
|
export const meta = {
|
||||||
name: 'compiler-driven-edit',
|
name: 'compiler-driven-edit',
|
||||||
@@ -58,7 +60,47 @@ export const meta = {
|
|||||||
phases: [{ title: 'Edit + propagate' }, { title: 'Verify' }, { title: 'Verdict' }],
|
phases: [{ title: 'Edit + propagate' }, { title: 'Verify' }, { title: 'Verdict' }],
|
||||||
}
|
}
|
||||||
|
|
||||||
const { edit_description, def_site } = args || {}
|
// Carrier guard (issue #24): a named-workflow invocation can deliver `args`
|
||||||
|
// as a STRING; `args || {}` alone does not catch it (a non-empty string is
|
||||||
|
// truthy), so both fields would read undefined and the edit agent would be
|
||||||
|
// briefed with the literal "EDIT: undefined" — its refusal would then ride
|
||||||
|
// the straddle-rule branch below as a BOUNCE to specify, dressing a
|
||||||
|
// mechanics failure as a design finding. Normalize, then reject a malformed
|
||||||
|
// carrier HERE, before any agent is dispatched — a distinct infra verdict,
|
||||||
|
// never a bounce.
|
||||||
|
let carrier = args
|
||||||
|
if (typeof carrier === 'string') {
|
||||||
|
// Tolerate a JSON-SERIALIZED object carrier (a stringifying caller or
|
||||||
|
// substrate layer still authored the documented object form); free text
|
||||||
|
// does not parse to an object and falls through to the rejection below.
|
||||||
|
try {
|
||||||
|
carrier = JSON.parse(carrier)
|
||||||
|
} catch {}
|
||||||
|
}
|
||||||
|
if ((carrier !== undefined && carrier !== null && typeof carrier !== 'object') || Array.isArray(carrier)) {
|
||||||
|
return {
|
||||||
|
status: 'BLOCKED',
|
||||||
|
kind: 'bad-carrier',
|
||||||
|
reason:
|
||||||
|
`infra: args arrived as ${Array.isArray(carrier) ? 'an array' : `a ${typeof carrier}`}, not the documented ` +
|
||||||
|
'object carrier — nothing was dispatched; re-invoke with args: { edit_description, def_site }',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const { edit_description, def_site } = carrier || {}
|
||||||
|
|
||||||
|
const missingFields = Object.entries({ edit_description, def_site })
|
||||||
|
.filter(([, v]) => typeof v !== 'string' || !v.trim())
|
||||||
|
.map(([k]) => k)
|
||||||
|
if (missingFields.length) {
|
||||||
|
return {
|
||||||
|
status: 'BLOCKED',
|
||||||
|
kind: 'bad-carrier',
|
||||||
|
reason:
|
||||||
|
`infra: missing required carrier field(s): ${missingFields.join(', ')} — nothing was dispatched; ` +
|
||||||
|
'the edit brief never reached the edit agent',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const STANDING =
|
const STANDING =
|
||||||
"Standing reading first (the plugin convention): read the project's CLAUDE.md " +
|
"Standing reading first (the plugin convention): read the project's CLAUDE.md " +
|
||||||
|
|||||||
@@ -61,6 +61,8 @@
|
|||||||
// red_test_path: (mini) absolute path to the RED test from debug / tdd
|
// red_test_path: (mini) absolute path to the RED test from debug / tdd
|
||||||
// cause_summary: (mini) 1–2 sentences (debugger cause, or tdd spec_summary)
|
// cause_summary: (mini) 1–2 sentences (debugger cause, or tdd spec_summary)
|
||||||
// constraint: (mini) e.g. 'minimal fix, no surrounding cleanup'
|
// constraint: (mini) e.g. 'minimal fix, no surrounding cleanup'
|
||||||
|
// A non-object carrier, an unknown mode, or a missing required field fails
|
||||||
|
// fast as an infra BLOCKED before any agent is dispatched (issue #24).
|
||||||
|
|
||||||
export const meta = {
|
export const meta = {
|
||||||
name: 'implement-loop',
|
name: 'implement-loop',
|
||||||
@@ -75,6 +77,33 @@ export const meta = {
|
|||||||
],
|
],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Carrier guard (issue #24): a named-workflow invocation can deliver `args`
|
||||||
|
// as a STRING. `args || {}` alone does not catch that — a non-empty string is
|
||||||
|
// truthy, so every field would destructure to undefined and the failure
|
||||||
|
// would surface far downstream (a plan-index dispatch against "Read the plan
|
||||||
|
// at undefined"), mislabelled as a plan-content problem after burning agent
|
||||||
|
// calls. Normalize, then reject a malformed carrier HERE, before any agent
|
||||||
|
// is dispatched — a distinct infra verdict, never a substantive one.
|
||||||
|
let carrier = args
|
||||||
|
if (typeof carrier === 'string') {
|
||||||
|
// Tolerate a JSON-SERIALIZED object carrier (a stringifying caller or
|
||||||
|
// substrate layer still authored the documented object form); free text
|
||||||
|
// does not parse to an object and falls through to the rejection below.
|
||||||
|
try {
|
||||||
|
carrier = JSON.parse(carrier)
|
||||||
|
} catch {}
|
||||||
|
}
|
||||||
|
if ((carrier !== undefined && carrier !== null && typeof carrier !== 'object') || Array.isArray(carrier)) {
|
||||||
|
return {
|
||||||
|
status: 'BLOCKED',
|
||||||
|
iter_id: null,
|
||||||
|
reason:
|
||||||
|
`infra: args arrived as ${Array.isArray(carrier) ? 'an array' : `a ${typeof carrier}`}, not the documented ` +
|
||||||
|
'object carrier — nothing was dispatched; re-invoke with args: { mode, iter_id, ' +
|
||||||
|
'plan_path | red_test_path + cause_summary, ... } (invocation examples in implement/SKILL.md)',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const {
|
const {
|
||||||
mode = 'standard',
|
mode = 'standard',
|
||||||
iter_id,
|
iter_id,
|
||||||
@@ -83,7 +112,34 @@ const {
|
|||||||
red_test_path,
|
red_test_path,
|
||||||
cause_summary,
|
cause_summary,
|
||||||
constraint = 'minimal change, no surrounding cleanup',
|
constraint = 'minimal change, no surrounding cleanup',
|
||||||
} = args || {}
|
} = carrier || {}
|
||||||
|
// Blank-safe echo for the guard verdicts below: never mirror a blank/non-string
|
||||||
|
// iter_id into the end-report while also naming it missing.
|
||||||
|
const iterIdEcho = typeof iter_id === 'string' && iter_id.trim() ? iter_id : null
|
||||||
|
|
||||||
|
if (mode !== 'standard' && mode !== 'mini') {
|
||||||
|
return {
|
||||||
|
status: 'BLOCKED',
|
||||||
|
iter_id: iterIdEcho,
|
||||||
|
reason: `infra: unknown mode '${mode}' — expected 'standard' or 'mini'; nothing was dispatched`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// The required carrier fields per mode (the header comment above is the
|
||||||
|
// contract). Missing or blank fields fail fast by name — the loop must never
|
||||||
|
// glob for a plan or brief an implementer with the literal string "undefined".
|
||||||
|
const requiredByMode = mode === 'mini' ? { iter_id, red_test_path, cause_summary } : { iter_id, plan_path }
|
||||||
|
const missingFields = Object.entries(requiredByMode)
|
||||||
|
.filter(([, v]) => typeof v !== 'string' || !v.trim())
|
||||||
|
.map(([k]) => k)
|
||||||
|
if (missingFields.length) {
|
||||||
|
return {
|
||||||
|
status: 'BLOCKED',
|
||||||
|
iter_id: iterIdEcho,
|
||||||
|
reason:
|
||||||
|
`infra: missing required carrier field(s) for mode '${mode}': ${missingFields.join(', ')} — ` +
|
||||||
|
'nothing was dispatched (carrier contract in the header comment and implement/SKILL.md)',
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const STANDING =
|
const STANDING =
|
||||||
"Standing reading first (the plugin convention): read the project's CLAUDE.md " +
|
"Standing reading first (the plugin convention): read the project's CLAUDE.md " +
|
||||||
|
|||||||
Reference in New Issue
Block a user