fix(workflows): report accepted label outcomes in activation summaries (#1963) - #1967
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The work is explicitly owned by squad:procedures, so it should be reviewed and approved by the Procedures agent/persona per the repository routing guard.
Review tier: Lite
Findings: None
What changed in this PR
This PR aligns Squad activation summaries and activation-binding artifacts with what the run can actually know: accepted add_labels operations (not create-issue), and binding references that remain valid JSON after gh-aw temporary-ID substitution.
Changes:
- Updates activation-path prose to report labels only when an
add_labelsoperation carrying that label was accepted for the same target, and forbids “verified/confirmed” language the runtime can’t support. - Fixes
Activation bindings:to use quoted, resolvable issue references ("#aw_..."/"#123") and defines failure-closed behavior for unresolved temporary IDs. - Extends the post-activation checker to resolve
issue/epic_issuereferences and adds/updates tests; raises the source-growth budget to 175 KB.
| File | Description |
|---|---|
| workflows/squad.md | Tightens label-reporting contract (accepted add_labels only) and updates Activation bindings: reference format + disclosure rules. |
| workflows/shared/squad-planning-ontology.md | Updates the artifact contract to define quoted issue references and “reported labels mean accepted label ops.” |
| scripts/check-agent-binding.mjs | Adds reference resolution ("#42" / legacy 42) and fails closed on unresolved #aw_... IDs. |
| test/gh-aw-quality.test.ts | Raises SOURCE_GROWTH_BUDGET_KB from 170 → 175 with rationale. |
| test/gh-aw-agent-binding-correspondence.test.ts | Updates assertions to require the stronger “accepted add_labels” evidence and explicitly rule out create-issue. |
| test/gh-aw-activation-summary-outcomes.test.ts | Adds a new suite covering label-outcome semantics, bindings quoting, unresolved-ID failure, and strict compile gates. |
| test/gh-aw-activate-fast-path-label-provisioning.test.ts | Updates fast-path assertion text to match the new “accepted add_labels” wording. |
| test/check-agent-binding.test.ts | Updates the bindings-shape assertion to expect quoted epic_issue reference. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot code review — addressedResult: review tier Lite, Findings: None, zero inline comments, zero review threads. The one item raised was ownership, not a code finding:
Response. Correct on the routing guard, and it is satisfied on the authoring side: issue #1963 carries That covers authorship, not approval. Approval is deliberately not self-served here:
Why a prose-heavy diff still warrants close reading. Most of this change is workflow instruction text, which no compiler checks. The safeguard is that the new suite's assertions were verified to fail against the pre-fix prose (20 of 28), so they encode the contract rather than restate the diff. The two pre-existing assertions I modified are the ones that pinned the old One judgment call worth a reviewer's attention: the source-growth guard moves 170 → 175 KB. I compressed prose first and only raised it after; the 40 KB ambient-prompt budget is unaffected and still passing, which is the condition that guard's own comment names as making a raise legitimate. Ready for independent review. |
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 2 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | 0 active Copilot thread(s) resolved (2 outdated skipped) |
| ✅ | CI passing | All checks passing |
Files Changed (8 files, +619 −41)
| File | +/− |
|---|---|
scripts/check-agent-binding.mjs |
+56 −22 |
test/check-agent-binding.test.ts |
+1 −1 |
test/gh-aw-activate-fast-path-label-provisioning.test.ts |
+6 −1 |
test/gh-aw-activation-summary-outcomes.test.ts |
+416 −0 |
test/gh-aw-agent-binding-correspondence.test.ts |
+15 −1 |
test/gh-aw-quality.test.ts |
+16 −1 |
workflows/shared/squad-planning-ontology.md |
+22 −4 |
workflows/squad.md |
+87 −11 |
Total: +619 −41
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Both activation paths could report labels that were never applied, and the
`Activation bindings:` block specified JSON the agent could not produce.
Over-claim by attribution. `squad-plan-activate` Label Pre-flight Step 8 let a
summary name a `squad:{agent}` label once "that issue's `create-issue` call
returned successfully carrying it". `create-issue`'s `labels:` field cannot land
a label the repository lacks — the failure #1959 fixed — so that sentence
licensed reporting labels that never existed. Both paths now condition a
reported label on an accepted `add_labels` call carrying it for that same
issue, targeted by the item's own temporary ID or, for a reused item, its
verified real number. A successful `create-issue` is stated to be no evidence.
Invalid bindings JSON. The block required bare `{created task issue number}`.
The agent never learns a created issue's real number during its turn, and
gh-aw's temporary-ID substitution is a plain text replacement over the whole
comment body that does not skip fenced code blocks and keeps the `#`. A bare
`"issue":#aw_task1` becomes `"issue":#42` — invalid JSON that fails the entire
block. Verified against the pinned runtime (github/gh-aw-actions@v0.87.2,
setup/js/temporary_id.cjs). Quoting is the narrowest correct fix:
`"issue":"#aw_task1"` becomes `"issue":"#42"`, which parses. No new safe job,
no temporary_id_map plumbing, and no inferred numbers.
Runtime limitation, stated rather than worked around: safe outputs are applied
in a post-agent job, so a run has evidence only that a call was accepted for a
specific target, never the GitHub API result. "Accepted" is the strongest
honest claim; the prose forbids saying a label was verified, confirmed, or
checked. Under-claiming is rejected too — an accepted operation may not be
reported as an omission — and the required `Non-roster agent values` handling
from the base stack is preserved and made two-directional.
The post-activation checker resolves `"#42"` and bare integers, and fails
closed on an unresolved `#aw_...` rather than skipping or repairing it.
Tests: new focused suite covering both paths; 20 of its 28 assertions fail
against the pre-fix prose. Two existing assertions that pinned the old
create-issue attribution are updated to the strictly stronger condition, with
their original intent (#1860, #1959) preserved.
Source growth guard raised 170 -> 173 KB. Measured against dev with #1966
already squash-merged, so this counts only #1963's bytes: 175 485 B = 171.4 KB.
172 KB would leave 643 B, reproducing the near-zero-margin failure the guard's
own comment warns about; 173 KB leaves 1 667 B. Not set higher, because #1964
is unmerged and pre-raising for it would hide growth that has not happened. The
ambient prompt budget is unaffected at 32 795 B = 32.0 KB against 40 KB, which
is the condition that guard names as making a raise legitimate.
Closes #1963
Parent #1957
Previously stacked on #1959 (PR #1966); that merged to dev as 9e3f6b3, so this
now targets dev directly and contains #1963's changes only.
Working as Procedures (Prompt Engineer)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
8009774 to
2361a3f
Compare
🟡 Impact Analysis — PR #1967Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (2 files)
scripts (1 file)
tests (5 files)
This report is generated automatically for every PR. See #733 for details. |
Rebased onto
|
| before | after | |
|---|---|---|
| mergeable | CONFLICTING / DIRTY |
MERGEABLE / CLEAN |
| changedFiles | 9 | 8 |
| commits vs base | 4 | 1 |
git diff origin/dev...HEAD contains no #1966 fast-path prose — every hunk already in dev was dropped, not reapplied.
Size guard: measured, not projected
Re-measured after the rebase, so it counts only #1963's bytes:
| bytes | KB | |
|---|---|---|
| Combined authored source | 175 485 | 171.4 |
| 172 KB budget | 176 128 | passes, only 643 B headroom |
| 173 KB budget (chosen) | 177 152 | passes, 1 667 B headroom |
| Ambient prompt | 32 795 | 32.0 / 40 KB — passing |
Set to 173, not 175. 172 leaves 643 B, which reproduces the near-zero-margin failure the guard's own comment warns about. Deliberately not raised further for #1964: it hasn't merged, and pre-raising would hide growth that hasn't happened. #1964 raises it against its own measurement when it lands. The ambient budget passing at 32.0 KB is the guard's stated precondition for a legitimate raise.
Verification
- All four workflows strict-compile (
squad,squad-review,squad-deps-worker,squad-implement-worker) - 276 passed across the 6 affected suites, including this PR's 28
- Full CI: 16 pass, 2 skip, 0 fail — first run ever on this PR, since
squad-ci.ymlonly fires on PRs targetingdev/preview/mainand this was previously stacked on a feature branch
Locally 9 tests fail in test/cli/*, test/acceptance/*, test/squad-observer. These are pre-existing and local-only: clean origin/dev fails the same six files with 51 failures, and CI's test job is green on both. The 51-vs-9 variance across runs is order-dependent shared-fixture state, unrelated to this change.
Do not merge.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A couple of small but important consistency issues remain (ontology wording implies “applied” vs accepted semantics, and the new compile-gate install hint should match the repo’s pinned gh-aw version).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
test/gh-aw-activation-summary-outcomes.test.ts — The local install hint for gh aw isn’t consistent with the repo’s pinned version (CI uses `gh… |
|
workflows/shared/squad-planning-ontology.md — The ontology still says bindings record labels "reported as applied", but this PR’s contract… |
Review on #1967 caught the exact defect this PR exists to remove, in a file the PR already touches. The `Activation bindings:` paragraph in the shared ontology said bindings record "epic labels reported as applied", while the paragraph directly below it correctly defines a reported label as an accepted `add_labels` operation that the agent never observes landing. squad.md stated the accurate contract and the ontology stated the strong one. That split matters because the ontology is the artifact contract the deterministic checker consumes and the file most likely to be read standalone: a consumer reading only that paragraph gets the claim the runtime cannot justify. Left as prose-only drift it would have turned one consistently-wrong place into two inconsistently-wrong ones. Reworded to the accepted-operation semantics used everywhere else, pointing a standalone reader at the definition below it. Guarded against re-drift with a negative assertion over the ontology, mirroring the guard the suite already applies to squad.md. The match is shape-targeted rather than a ban on the word "applied": the same file legitimately says safe outputs "are applied after the agent turn", which is the justification for the weaker claim, so a blanket match would forbid the correct prose too. Verified by mutation — restoring the old wording fails the new test, and only that test. Also pins the local `gh aw` install hint to v0.87.10, matching the pin in .github/workflows/squad-ci.yml, so local strict-compile and lock output match CI. The separate `gh-aw-actions@v0.87.2` citation is left as-is: that is the pinned runtime action whose temporary_id.cjs this change was verified against, which is a different artifact from the CLI extension, and v0.87.2 is still what .github/workflows references. Source growth re-measured after the edit: 175 519 B = 171.4 KB, 1 633 B under the 173 KB guard, so the guard is unchanged. Ambient prompt unaffected. Closes #1963 Parent #1957 Working as Procedures (Prompt Engineer) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes core activation workflow contracts (summary semantics + bindings format) and the fail-closed checker behavior, and the PR description explicitly indicates it should not be merged yet.
Review tier: Lite
Findings: None
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
workflows/shared/squad-planning-ontology.md — The ontology still says bindings record labels "reported as applied", but this PR’s contract… View resolved comment |
|
test/gh-aw-activation-summary-outcomes.test.ts — The local install hint for gh aw isn’t consistent with the repo’s pinned version (CI uses `gh… View resolved comment |


Closes #1963. Parent #1957.
Working as Procedures (Prompt Engineer) — issue carries
squad:procedures.Stack status — now unstacked, targets
devdirectlyThe stack this PR was built on has landed:
devas9e3f6b34, branch deleteddev, single commitBecause #1966 was squash-merged, its original commits and
dev's squashedequivalent were two different histories, and this branch briefly showed
CONFLICTINGwithchangedFiles=9. Resolved by resetting toorigin/devandcherry-picking only #1963's commit.
changedFilesis now 8, andgit diff origin/dev...HEADcontains no #1966 fast-path prose — every hunk of#1966's that is already in
devwas dropped rather than reapplied.No longer stacked: this merges into
devon its own.The problem
#1962 and #1959 made the label operations correct. They did not make the
summary correct. Two defects remained, one per activation path, plus a latent
data-contract bug.
1. Over-claim by attribution.
squad-plan-activateLabel Pre-flight Step 8allowed naming a
squad:{agent}label once "that issue'screate-issuecallreturned successfully carrying it." But
create-issue'slabels:field cannotland a label the repository does not already have — the exact failure #1959
fixed. That sentence licensed reporting labels that were never applied. The fast
path had the softer form of the same problem.
2.
Activation bindings:specified JSON the agent cannot produce. The blockrequired bare
{created task issue number}. The agent never learns a createdissue's real number during its turn. The disclosed workaround — a bare temporary
ID — produces invalid JSON.
3. Unresolved references were undefined behavior. An
#aw_…survivingsubstitution means that
create-issuenever landed, but nothing said so.What changed
Label reporting — accepted operations only (both paths)
A label reaches an activated issue through exactly one route: an accepted
add_labelsoperation targeting that issue. Both paths now require that areported
squad:{owner}trace to anadd_labelscall this run made, carryingthat label, targeting that same issue — by its own
temporary_id, or by itsverified real number for a reused issue.
A successful
create-issueis stated explicitly to be not evidence. Nosummary may say a label was carried by, applied by, or included in issue
creation.
Both directions are closed:
deferred, rejected, or borrowed from another item's call.
A silent under-claim manufactures a defect that did not occur.
The required
Non-roster agent valueshandling from the base stack is preservedand made two-directional: required when an accepted owner did not become a
label, and forbidden for an owner that did.
Bindings carry quoted, resolvable references
issueandepic_issueare now JSON strings:"issue":"#aw_task1""issue":"#42""issue":"#123"The quoting is load-bearing, not style. Verified empirically against the
pinned runtime (
github/gh-aw-actions@v0.87.2,setup/js/temporary_id.cjs):substitution is a plain-text replacement over the whole comment body that does
not skip fenced code blocks, and it keeps the
#.This is the narrowest correct design. It needs no new safe job, no
temporary_id_mapplumbing, and no inferred numbers. One uniform shapecovers created and reused items, and an unresolved reference survives as data
rather than corrupting the block.
Checker resolves references and fails closed
scripts/check-agent-binding.mjsresolves"#42"and bare integers (sopre-contract artifacts still validate) and throws on an unresolved
#aw_…rather than skipping, coercing, or repairing it — a surviving temporary ID means
that issue does not exist. Cross-repo
owner/repo#42is rejected; Squadactivates same-repo only.
Runtime limitation — disclosed, not worked around
Safe outputs are applied in a post-agent job. An activation run has evidence only
that a call was accepted for a specific target, never the GitHub API result.
There is no supported surface for the agent to observe downstream application
during its turn.
So "accepted" is the strongest honest claim, and the prose forbids writing that a
label was verified, confirmed on the issue, or checked — nothing here reads
labels back. Verifying bindings against labels actually present remains the
deterministic post-activation checker's job. A compiled-artifact test pins the
agent job to
issues: read, which is why this boundary exists.I did not invent a success-shaped workaround for the observability gap.
Tests
New
test/gh-aw-activation-summary-outcomes.test.ts(28 tests). 20 of themfail against the pre-fix prose — verified by reverting the source and
re-running, so they are load-bearing rather than restating the diff.
Coverage: intended-vs-actual over-claims,
create_issueattribution, invalidJSON on substitution, unresolved temporary IDs, under-claiming, and the required
Non-roster agent valuesreporting — asserted per path viait.each, so afix to one path cannot mask the other.
Two existing assertions pinned the old
create-issueattribution and now assertthe strictly stronger condition, with their original intent preserved and
documented inline:
gh-aw-agent-binding-correspondence.test.ts(gh-aw:plan activatesummary reportssquad:{agent}labels it did not apply, and omits the requiredNon-roster agent valuesdisclosure #1860) — still catches gh-aw:plan activatesummary reportssquad:{agent}labels it did not apply, and omits the requiredNon-roster agent valuesdisclosure #1860'soriginal defect, now for the right reason.
gh-aw-activate-fast-path-label-provisioning.test.ts(Provision labels correctly in the recommended Squad activation fast path #1959) — "actuallyapplied" was itself the over-claim; Provision labels correctly in the recommended Squad activation fast path #1959's never-skipped/deferred/intended
guarantee is retained verbatim inside the tightened sentence.
Size guard 170 → 173 KB (measured post-rebase, not projected)
Measured against
devafter the rebase, so this counts only #1963's bytes:172 KB is rejected because 643 B reproduces the near-zero-margin failure mode
the guard's own comment warns about. It is deliberately not set higher:
#1964 is projected to take the combined total to ~180 002 B ≈ 175.8 KB, but
#1964 has not merged, and pre-raising for an unmerged branch would hide growth
that has not happened. #1964 raises this guard against its own measurement when
it lands.
Nearly all growth is inside the
squad-plan-activate/squad-plan-acceptinline
## skill:blocks, which gh-aw strips from the always-loaded prompt.The 40 KB ambient budget is unaffected and still passing at 32.0 KB — the
condition that guard's own comment names as making a raise legitimate (same
rationale as #1959's 160 → 170). I compressed prose first and only then raised
it.
Verification
squad,squad-review,squad-deps-worker,squad-implement-workernpm run buildclean (build-generated version stamps and regeneratedtemplates were reverted, not committed)
cli-packaging-smoke,squad-observer,acceptance,hostile,cli/*) reproduce on theuntouched base — pre-existing, not from this change
workflows/,scripts/,test/, nonematching the changelog gate's SDK/CLI/templates regex
Scope
Activation bindings:stays plan-activate-only. The fast path emitsplan-accepted/phases-accepted, which the checker'sACTIVATION_ARTIFACTSset does not consume; #1963 says bindings remain complete, which is
preservation, not expansion. Only the label-reporting semantics are shared
across both paths.
Out of scope: capacity policy (#1961) beyond compatibility, the broad contract
suite (#1960), E4 (#1958), checker distribution, post-activation implementation,
unrelated cleanup.
Merge status: the stack is resolved — #1965 and #1966 are merged and this PR now
targets
devdirectly. Copilot reviewed the current head (Findings: None), all reviewthreads are resolved, and full CI is green. The earlier "do not merge" hold referred to
the stack ordering and no longer applies.