feat(chains): verify dev-loop pr handoff - #1009
Conversation
| gh pr list -R "$repo" --state open --limit 100 --json number,author --jq '.[] | "\(.number)\t\(.author.login)"' | sort -n > "$after" | ||
| actor_prs=$(awk -F '\t' -v actor="$actor" 'FILENAME == ARGV[1] { before[$1] = 1; next } !($1 in before) && $2 == actor { print $1 }' "$before" "$after") | ||
| actor_pr_count=$(printf '%s\n' "$actor_prs" | sed '/^$/d' | wc -l | tr -d ' ') | ||
| if [ "$actor_pr_count" -eq 1 ]; then |
There was a problem hiding this comment.
[ISSUE] Accepting any single new same-actor open PR can hand off a concurrent impostor — if feature created none but another same-actor PR appeared during the run (manual create, overlapping skill), pr-review runs on the wrong PR and the chain still reports success. Why it matters: the chain’s purpose is exact feature→review handoff; set-difference alone is necessary but not sufficient under same-actor concurrency (you already reject count>1 and wrong-actor). Bind the candidate with createdAt >= snapshot time (or the feature dispatch timestamp), and add a concurrent-same-actor-impostor test beside concurrent-other.
Svector-anu
left a comment
Svector-anu
left a comment
There was a problem hiding this comment.
Verdict: discussion-needed — same-actor concurrent impostor can false-handoff
Findings (mirrored as inline comments):
- [ISSUE] scripts/dev-loop-pr.sh:45 — accepting the sole new same-actor open PR without a createdAt/snapshot-time bind can attribute a concurrent impostor PR to feature when feature created none; pr-review then runs on the wrong PR while the chain reports success
b863b41 to
870d62c
Compare
|
fixed in the handoff no longer relies on author + set difference alone. chain-runner now passes its existing unique dispatch id into the feature prompt; feature records the exact added the requested |
| after=$(mktemp) | ||
| gh pr list -R "$repo" --state open --limit 100 --json number,author,body --jq '.[] | "\(.number)\t\(.author.login)\t\(.body // "")"' | sort -n > "$after" | ||
| actor_prs=$(awk -F '\t' -v actor="$actor" -v marker="<!-- aeon-dispatch:$dispatch_id -->" 'FILENAME == ARGV[1] { before[$1] = 1; next } !($1 in before) && $2 == actor && index($0, marker) { print $1 }' "$before" "$after") | ||
| actor_pr_count=$(printf '%s\n' "$actor_prs" | sed '/^$/d' | wc -l | tr -d ' ') |
There was a problem hiding this comment.
[CRITICAL] Raw PR body is written into a line-oriented TSV — markdown bodies always contain newlines, so <!-- aeon-dispatch:… --> lands on a continuation line that fails $2 == actor, verify-new-pr exits 3, and chain-runner soft-succeeds with CHAIN_NO_ACTION while skipping review. Why it matters: the happy-path handoff this PR adds will silently no-op on every real feature PR (tests only use single-line bodies). Collapse newlines in jq (e.g. gsub("\n";" ")) or match the marker with jq/contains before line-splitting.
Svector-anu
left a comment
Svector-anu
left a comment
There was a problem hiding this comment.
Verdict: blocked: multiline PR bodies break dispatch-marker verify
Findings (mirrored as inline comments):
- [CRITICAL] scripts/dev-loop-pr.sh:46 — serializing raw multi-line PR bodies into line-oriented TSV makes the dispatch-marker check miss every real feature PR, so verify exits 3 and the chain soft-succeeds without running pr-review
870d62c to
fff82f3
Compare
|
fixed in
the happy-path fixtures now use real multiline markdown bodies with the marker below other text. the full handoff suite passes, including the multiline marker, empty baseline, same-actor impostor, wrong-actor, ambiguous, and concurrent-other cases. |
Svector-anu
left a comment
Svector-anu
left a comment
There was a problem hiding this comment.
Verdict: discussion-needed — feature PR templates omit the dispatch receipt the handoff now requires
Findings (mirrored as inline comments):
- [ISSUE] skills/feature/SKILL.md:326 — B7
gh pr create --bodytemplate has no<!-- aeon-dispatch:<ID> -->placeholder (same gap in A8); agents that copy the nearby template skip the receipt,verify-new-prsoft-exits 3, and the chain reports success while leaving an unreviewed PR
Prior CRITICAL on SHA 870d62c (multiline TSV body break) is fixed in fff82f3 via jq-native JSON checks; marker bind still covers the earlier same-actor impostor case.
| - build | ||
| - growth | ||
| --- | ||
| When the run prompt supplies a `Workflow correlation ID`, include the exact marker `<!-- aeon-dispatch:<ID> -->` in every PR body you create. This is a machine-checked chain receipt: do not alter, omit, or place it only in the final response. |
There was a problem hiding this comment.
[ISSUE] This receipt requirement is not mirrored in the B7/A8 gh pr create --body templates (B7 at skills/feature/SKILL.md:326, A8 at :165) — under a long feature run the agent copies the nearby template, omits <!-- aeon-dispatch:<ID> -->, and verify-new-pr exits 3. Why it matters: the chain reports clean no-action success while leaving a real unreviewed PR, which is the handoff gap this change exists to close. Bake the marker into those templates when a Workflow correlation ID is present.
fff82f3 to
1986685
Compare
|
fixed in the dispatch receipt is now embedded directly in all three the handoff test also asserts that every feature pr template carries the conditional marker; all handoff and existing chain-correlation tests pass. |
Svector-anu
left a comment
Svector-anu
left a comment
There was a problem hiding this comment.
Verdict: approve-ready — no blockers.
|
The machine-checked feature -> PR bind is good. Do not merge yet: the review half of the loop can no-op the only PR the chain exists to review. Blocker: Also: the dispatch receipt in CI is green. The GitHub-state bind (snapshot, marker, same-actor, fail-closed on ambiguous) is the right shape. The two items above are what make the loop actually review the PR it just opened. |
Co-Authored-By: Claude <noreply@anthropic.com>
…d-handoff # Conflicts: # catalog/packs.json # catalog/skills.json # eyebrowlock.json
2 participants
what
dev-loopchain:featurethenpr-reviewexternal:owner/repo[#issue]target<!-- aeon-dispatch:<id> -->body markerowner/repo#numbertopr-review, which now supports exact-pr scopewhy
a successful feature harness exit is not proof that a pr exists. empty chain vars also let both skills fall back to broad watched-repository behavior, so the reviewer could inspect unrelated prs instead of the feature result.
the handoff now trusts github state plus a run-specific receipt, not the feature's closing prose: no pr skips review cleanly; multiple, unmarked, wrong-actor, missing, or malformed results fail closed.
verification
bash scripts/tests/test_dev_loop_handoff.sh— passed, including empty baseline, one, none, ambiguous, wrong-actor, concurrent-other, and unmarked concurrent-same-actor casesbash scripts/tests/test_chain_runner.sh— 5 passed, 0 failedbash scripts/tests/test_chain_when.sh— 18 passed, 0 failednode --test scripts/validate-config.test.js— 14 passednode scripts/validate-config.js— cleaneyebrow verify --ci— passed after canonical lock regenerationgit diff --check— cleanmutation proof for the empty-baseline regression: replacing the filename-based first-input check with
NR == FNRmade the empty-list case fail withfeature created no new open PR; restoring the fix made the suite pass.real end-to-end proof on the fork:
33520525541completed successfully33520550958createdSvector-anu/skopos#106Verified feature PR: Svector-anu/skopos#10633521199398reviewed only that pr and completed successfullythe chain is workflow-dispatch only. it does not enable either skill, pin a harness, choose a target, or merge a pr for the operator.
generated artifacts
current main contained pre-existing catalog and eyebrow drift from recently merged skill changes. the committed catalog refresh records those current skill revisions, and the canonical eyebrow v0.4.1 regeneration records all current-tree drift plus this pr-review change so the integrity gate evaluates the actual tree.