fix(gh-aw): bound activation label capacity and make truncation explicit (#1961) - #1964
Conversation
Working as FIDO (Quality Owner).
A 50-issue activation could lose `add_labels` operations while the run still
reported success. Two causes:
1. `add-labels: max: 80` was below the worst case under the reading gh-aw's own
injected prose invites. The compiler emits "Maximum {max} label(s) can be
added", which reads as a budget of label NAMES; a full 50-issue activation
applying `squad` + `squad:{agent}` needs 100. An agent taking that phrasing
literally could conclude it had overrun and stop labeling early or batch
issues together.
2. gh-aw v0.87.10 drops an over-limit item rather than failing. Its collector
(`collect_ndjson_output.cjs`) rejects the item and `continue`s, pushing a
string into `errors`; those are emitted with `core.warning`, never
`core.setFailed`. The run finishes green with label operations missing and
nothing announces it.
Verified against the pinned runtime rather than inferred: `max` caps safe-output
ITEMS (tool calls) per type, not label names inside a call. A two-label call
costs one item.
Capacity: largest supported activation is 50 issues (`enterprise` profile
`max_issues: 50`, the highest documented profile limit, and the same threshold
`squad-plan-activate` uses to force phased activation). Worst case at that size
is 50 `create-issue` items, 50 `add_labels` calls, <=2 labels per call, 100
label names across the run.
Changes:
- `add-labels: max` 80 -> 110, sized to cover the worst case under BOTH readings
(50 calls, 100 names) so no interpretation of the cap can justify dropping a
label operation. `create-issue` stays at 75.
- Record the derivation and the item-vs-label-name semantics in the activation
skill, plus the fact that an over-limit item is dropped, not failed.
- New Step 2e Label-Operation Reconciliation: track `activated` vs `labeled`,
count a never-made/rejected/errored call as unlabeled, and on shortfall call
`report_incomplete` naming the affected work items. gh-aw treats that as a
failure signal even on successful exit, so a truncated activation can no
longer be recorded as clean. Cap exhaustion becomes a nameable cause when
observed; #1683's rule against guessing at caps is preserved and narrowed.
Tests: `test/gh-aw-activation-capacity.test.ts` (27) covers the derived maximum,
capacity under both readings, at-maximum and one-over boundaries, and the
reconciliation contract. Compiled-artifact assertions prefer runtime evidence
over prose: the declared cap reaching `GH_AW_SAFE_OUTPUTS_CONFIG`, the injected
constraint carrying the same number, `report_incomplete` being present (and so
callable), and agent-job permissions staying `issues: read`. Fails closed if
`gh aw` is absent, per #1834. Mutation-checked: reverting the cap to 80 fails 4.
No changes to temporary-ID linkage (#1962), fast-path parity (#1959), label
result reporting (#1963), broad contract coverage (#1960), or E4 (#1958).
Step 2e is deliberately neutral about how `add_labels` identifies its target so
it does not conflict with #1962.
Closes #1961
Parent #1957
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🟡 Impact Analysis — PR #1964Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
tests (2 files)
This report is generated automatically for every PR. See #733 for details. |
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 9 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 (3 outdated skipped) |
| ✅ | CI passing | All checks passing |
Files Changed (3 files, +636 −12)
| File | +/− |
|---|---|
test/gh-aw-activation-capacity.test.ts |
+576 −0 |
test/gh-aw-quality.test.ts |
+17 −10 |
workflows/squad.md |
+43 −2 |
Total: +636 −12
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new test file uses an extensionless ESM helper import (./helpers/gh-aw-lock) that is inconsistent with other suites and can break under Node ESM/NodeNext resolution after transpilation.
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 (1)
| Severity | Finding |
|---|---|
test/gh-aw-activation-capacity.test.ts — This test imports the helper without the .js extension, but other Vitest suites import the same… |
What changed in this PR
This PR hardens the gh-aw activation workflow contract to prevent “green” runs that silently drop add_labels operations under safe-output caps, by increasing the add-labels cap and requiring a reconciliation step that converts any label-operation shortfall into an explicit report_incomplete failure.
Changes:
- Increase
safe-outputs.add-labels.maxfrom 80 → 110 and document the capacity rationale (50-issue max, item-vs-label semantics, drop-not-fail behavior). - Add Step 2e “Label-Operation Reconciliation” guidance to detect and fail on
labeled < activated. - Add a new compiled-artifact contract test suite to lock in capacity math and prove the compiled workflow exposes the required runtime config/tools.
| File | Description |
|---|---|
| workflows/squad.md | Raises add-labels cap to 110 and documents/mandates reconciliation to prevent silent truncation being reported as success. |
| test/gh-aw-activation-capacity.test.ts | Adds capacity + compilation contract tests to validate max sizing, compiled safe-output config, and callable tooling (including report_incomplete). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Independent review found three factual defects in the prose and tests
shipped by the first commit. All three are corrected against the pinned
gh-aw v0.87.10 sources rather than gh-aw's own injected descriptions.
1. `report_incomplete` does NOT make the run conclude non-successfully.
`report_incomplete_handler.cjs` emits `core.warning` only, and
`handle_agent_failure.cjs` contains zero `setFailed`/`process.exit`
calls in 4453 lines. What it actually does is open or update a durable
`[aw] {workflow} reported incomplete result` tracking issue. Step 2e
now states that, says the run still reports success, and tells the
agent never to rely on a red run to carry the signal. gh-aw's own tool
description ("treated as a failure signal even when the agent exits
successfully") is misleading; trusting it was exactly the injected-prose
mistake #1961 exists to prevent.
Limitation, stated honestly: no narrow mechanism in the pinned runtime
makes an incomplete activation conclude red. Forcing one would require
a custom safe-job, which is outside this issue's scope. The durable
tracking record plus the "never report a clean activation you did not
perform" rule are what carry the signal.
2. Cap enforcement is dual, not collection-only. Per Safe Outputs MCE4,
`enforcePerTypeMax` in `safe_outputs_handlers.cjs` throws JSON-RPC
`E002: {type} limit reached` at invocation time, which the agent DOES
see; the collector then drops surplus items with a warning. Removed the
claim that an over-limit item "never appears as an error to the agent".
Reconciliation still keys on absence-of-success, not presence-of-error,
because a call can also simply never be made.
3. The old `max: 80` did not cause runtime truncation — 50 calls never
reached it. The real hazard is gh-aw's injected "Maximum 80 label(s)
can be added" wording against a 100-label worst case, which invites
agent self-truncation. 110 is justified by that hazard plus bounded
margin, and the docblock now explicitly records what is NOT claimed.
Also, per the PR #1965 review: Step 2e no longer demands a real issue
number for items created during the run. Issue creation is deferred to
the post-agent job, so it names the stable temporary ID plus title and
intended labels, and quotes a real number only for a reused issue —
avoiding reintroduction of #1962's invalid-number assumption.
Tests: renamed the two overclaiming tests and gave each a negative guard
that fails if the retracted wording returns; added coverage for the
temporary-ID rule. Trimmed prose to stay under the existing 160 KB
source-growth guard rather than raising another test's budget.
Verified: 221/221 across the three gh-aw suites; mutation-checked (cap
-> 80 fails 5 tests incl. the compiled-runtime one, reintroducing either
retracted claim fails its guard); strict compile clean on all four
workflows; npm run build passes.
Closes #1961
Parent #1957
Working as FIDO (Quality Owner)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2471face-3b27-4419-bd48-0becd2642d7f
Stacks this branch explicitly on `bradygaster-plan-activate-temporary-ids` (PR #1965 head 528ba9b) so #1964 reviews as a clean delta on top of the temporary-ID contract instead of colliding with it at merge time. Conflict resolution (workflows/squad.md, Step 2d — the only conflict): - Took #1965's Self-Validation sentence wholesale. It is strictly stronger than the version on this branch ("requested/recognized", "the last task's temporary ID", "never substitute a guessed issue number"). - Reapplied only this branch's minimal trailing clause, which is #1961's concern and not addressed by #1965: never surface a safe-output cap as a guessed reason for a partial run — name a cap only when Step 2e observed one actually being reached. Step 2e concretized against the now-present mechanism: - The report identifier for an item created this run is the `temporary_id` minted under #1965's Temporary-ID Contract, not a GitHub issue number, because creation is deferred to the safe-output job. - A real number may be quoted only where independently verified (dedup-by-title match, or Step 1's idempotent-rerun path). This mirrors #1965's own "Existing and reused issues" rule rather than restating a parallel contract. Source-growth guard raised 160 -> 168 KB (test/gh-aw-quality.test.ts), per that guard's own documented criterion rather than to turn a red test green: - Ambient prompt (the canonical budget) measures 32.0 KB against 40 KB. - All growth is inside the `squad-plan-activate` inline skill, which the extractor strips from the ambient prompt and loads on demand. - Neither PR was individually over: #1965 alone 163812 B, #1961 alone 163819 B, against the old 163840 B ceiling — 28 and 21 bytes of headroom. Two independently-compliant PRs could not coexist under a threshold that tight. Validation: 239/239 across the four gh-aw suites; three mutation probes (cap 110->80, Step 2e temp-ID rule, retracted dual-enforcement claim) each fail their intended tests; all four workflows compile with `--strict` showing only pre-existing warnings; `npm run build` passes. Compiled lock confirms both contracts coexist: add_labels max 110, create_issue max 75 + require_temporary_id. Refs #1961, #1965. Parent #1957. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2471face-3b27-4419-bd48-0becd2642d7f
Addresses Copilot review on PR #1964. The reviewer is correct on both counts. `tsc --module nodenext --moduleResolution nodenext` reports exactly one error on this file, and TypeScript names the fix itself: test/gh-aw-activation-capacity.test.ts(94,46): error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './helpers/gh-aw-lock.js'? The repo is `"type": "module"` with `module`/`moduleResolution: NodeNext` in the root tsconfig, and the four other suites importing this helper already use the `.js` specifier. This file was the lone outlier; it now matches. Verified: 293/293 across all eight suites that import the helper, all four workflows compile with `--strict` (pre-existing warnings only), `npm run build` passes, and the NodeNext type-check on this file is now clean. Refs #1961. Parent #1957. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2471face-3b27-4419-bd48-0becd2642d7f
Keeps the stack on `bradygaster-plan-activate-temporary-ids` current after #1965 advanced past 528ba9b. GitHub had flagged the PR CONFLICTING/DIRTY. One conflict, again at Step 2d, resolved by the same rule used for 528ba9b: take #1965's sentence wholesale and reapply only this branch's trailing clause. #1965 c95d43a tightens 2d by defining the created count explicitly ("the number of `create-issue` calls this run emitted") and binding `created={N}`/`expected={M}` to it. That definition is adopted verbatim. This branch's delta — never surface a safe-output cap as a *guessed* reason for a partial run; name one only when Step 2e observed it being reached — is reapplied to the trailing sentence. Step 2e is unchanged and still follows 2d. #1965's other two hunks in this commit (Uniqueness wording, pre-existing-epic parent rule) auto-merged and are present verbatim. Validation: 240/240 across the four gh-aw suites (count rises from 239 because c95d43a adds a test to #1965's own suite, which passes untouched); all four workflows compile with `--strict` showing only pre-existing warnings; `npm run build` passes. Refs #1961, #1965. Parent #1957. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2471face-3b27-4419-bd48-0becd2642d7f
Resolves #1964 against dev head 9e3f6b3, which now carries both #1965 (temporary-ID contract, fa5cc26) and #1966 (fast-path label provisioning, squash 9e3f6b3). workflows/squad.md — auto-merged, no conflict. #1966's edits land in the `/squad activate` fast path and Step 4; #1961's are in squad-plan-activate Step 2d/2e. Different regions, so the recurring Step 2d conflict did not recur this round. Verified by anchor grep that all three contracts survive unduplicated: #1966's Fast-Path Label Provisioning / non-roster reporting, #1965's Temporary-ID Contract and created-count wording, and #1961's capacity budget / max 110 / Step 2e. test/gh-aw-quality.test.ts — SOURCE_GROWTH_BUDGET_KB conflict resolved to 172 (was 168 here, 170 on dev). Measured, not guessed: combined source on the resolved branch is 173241 B (169.2 KB). - 168 now FAILS by 1209 B, so keeping it would land red. - 170 passes by only 839 B. That is the same stale-threshold failure this guard already hit once at 160, where 21-28 bytes of headroom meant two independently-compliant PRs could not coexist. - 172 leaves 2887 B (~2.8 KB) of real margin, so the guard still bites on genuine growth. dev's #1959/#1962 rationale comment is preserved verbatim; the #1961 rationale is appended beneath it rather than replacing it. Guard precondition confirmed: the canonical ambient prompt budget is still 32.0 KB (32795 B) against 40 KB, unchanged by either PR, because all growth sits inside inline `## skill:` blocks that gh-aw strips from the always-loaded prompt. That is this guard's own documented condition for a legitimate raise. Validation: 320/320 across 9 targeted suites including #1966's new fast-path suite; all four workflows `gh aw compile --strict` clean (pre-existing discriminator/slash_command warnings only); npm run build exit 0. Closes #1961 Parent #1957 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2471face-3b27-4419-bd48-0becd2642d7f
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>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Step 2e’s report_incomplete reason currently implies labels were actually received/applied during the agent turn, which isn’t guaranteed by an accepted add_labels safe-output and could misreport outcomes.
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 (1)
| Severity | Finding |
|---|---|
workflows/squad.md — Step 2e’s reconciliation message currently says issues “received their labels”, but at agent time… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
test/gh-aw-activation-capacity.test.ts — This test imports the helper without the .js extension, but other Vitest suites import the same… View resolved comment |
#1963) (#1967) * fix(workflows): report accepted label outcomes in activation summaries 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> * fix(workflows): remove applied-label over-claim from the shared ontology 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> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…-label-capacity-safeguards # Conflicts: # test/gh-aw-quality.test.ts
|
Resolved the merge with Only one conflict, as expected: The budget is measured, not projected. I merged both branches in a scratch worktree and forced the guard to print its own number rather than trusting an estimate: Set to 177. 176 would pass by 188 bytes, which is precisely the near-zero-margin failure mode this guard's own comment criticises — before the 160→170 raise it sat 21–28 bytes from the ceiling and correct changes failed on byte count alone. #1963's comment predicted this merge would land at ~180 002 B; the real figure is 180 036 B, within 34 bytes, and I've recorded both numbers in the resolved comment so the next raise has a real audit trail. The guard's stated precondition for a legitimate raise holds: Verification: 286 passed across the six affected suites (
Copilot re-review requested on this head. Not merged. |
…ied labels Copilot review flagged that Step 2e's reconciliation claimed issues "received their labels". #1963 (now merged to dev) settled the vocabulary: an accepted safe output is queued this turn and applied by the post-agent job, so the agent has evidence only that a call was accepted for a specific target -- never that a label reached GitHub. Step 2e was the remaining place still asserting application, which conflicts with the accepted-vs-applied rule the same skill now states. - `labeled` is defined as issues whose add_labels call was accepted. - The report_incomplete reason reads "had a label operation accepted". - "proof that every label landed" -> "every label operation was accepted". - Added an explicit statement that the counts track label operations, not labels present on GitHub. Tests: new assertion guards the positive and both retired over-claim phrasings; verified by reverting the prose (1 failed, 28 passed) rather than assuming. Tightened the sibling assertion to match the exact count definitions -- the previous `.*`-joined form matched a distant "was accepted" elsewhere in the flattened skill and so failed to detect a reverted definition. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0515ad5d-4c5b-48bb-92a6-4e1296594cab
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A new test assertion uses a regex alternation that makes the check effectively non-assertive, so the suite may pass even if the intended invariant is removed.
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 (1)
| Severity | Finding |
|---|---|
test/gh-aw-activation-capacity.test.ts — The regex here is effectively vacuous because the |activated.*artifact alternative will match any… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
workflows/squad.md — Step 2e’s reconciliation message currently says issues “received their labels”, but at agent time… View resolved comment |
Copilot review caught that the regex ended in an `|activated.*artifact` alternative. Alternation binds loosest, so the whole pattern collapsed to that branch and matched any mention of the artifact -- the test would have passed with the `labeled < activated` condition deleted from the prose, which is the entire invariant it names. Anchored to the full sentence and verified by mutation: removing the condition from Step 2e now fails (1 failed / 28 passed) where it previously passed. Second instance of this defect class in this suite; the sibling count assertions were tightened in 4ec89dd for the same reason. Audited the remaining alternations here -- `/finish green|still succeeds/` is legitimate (two accepted phrasings, both specific). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0515ad5d-4c5b-48bb-92a6-4e1296594cab
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are narrowly scoped to capacity/rationale + reconciliation safeguards and are backed by compiled-runtime contract tests, with only minor documentation nits noted.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
test/gh-aw-activation-capacity.test.ts — The regex here is effectively vacuous because the |activated.*artifact alternative will match any… View resolved comment |
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
test/gh-aw-quality.test.ts:908
- The source-growth guard below counts the entire workflow file (
squadContentincludes frontmatter), so the comment claiming all #1961 growth is inside thesquad-plan-activateinline skill is inaccurate: this PR also changes the safe-outputs frontmatter (e.g., add-labels max and capacity comments). Please reword to reflect that frontmatter changes exist (and are still stripped from the ambient prompt by the assembler) so future readers don't draw the wrong conclusion about what was measured.
// All of #1961's growth is inside the `squad-plan-activate` inline skill, which the
// extractor strips from the ambient prompt and loads on demand; ambient re-measured at
// 32 KB against 40 KB on the merged tree.
workflows/squad.md:1874
- In the capacity table, the column header is "Safe output" and the
create-issuerow uses the frontmatter key style, but the next row uses the tool name style (add_labels). This inconsistency can be confusing when someone tries to reconcile the table with thesafe-outputs:frontmatter (which isadd-labels:). Consider usingadd-labelsin the table and reserveadd_labelsfor places where you're explicitly talking about the tool call/runtime type name.
| `create-issue` | 50 — one per epic/task | 75 |
| `add_labels` | 50 — one per created issue | 110 |
| Labels in one call | 2 — `squad` + `squad:{agent}` | not capped |


Closes #1961
Parent #1957
Working as FIDO (Quality Owner)
Problem
An activation could label fewer issues than it created and still report a clean success. Two things made that possible:
add-labels: max: 80sat below the worst-case label-name count for a full activation, and nothing compared "issues activated" against "issues labeled" before the summary was written.Capacity derivation
The largest activation supported in one run is 50 issues — the
enterpriseprofile'smax_issues: 50(the highest documented profile limit), which is also the threshold the workflow's own phased-activation rule enforces.maxcreate-issueadd_labelsadd_labelscallsquad+squad:{agent}maxWhy 110 and not 80.
maxcounts safe-output items (tool calls), so 50 calls never actually exhausted a cap of 80 — there was no runtime overflow. The real hazard is what gh-aw injects into the prompt: it renders the cap as"Maximum 80 label(s) can be added", which reads as a budget of label names, against a worst case of 100 names. An agent that believes it has 80 names for 100 names' worth of work rations them and self-truncates. 110 satisfies the worst case under both readings — 50 calls and 100 names — so neither interpretation can justify skipping a label operation. The margin stays bounded (a test caps it at 2× worst case) so this doesn't trade a silent-truncation bug for an unbounded-write one.Safeguards
Frontmatter —
add-labels: max80 → 110, with a comment recording the derivation and pointing at the full analysis.squad-plan-activateprose — the capacity table above, plus two corrected statements of runtime behaviour:maxlimits items, not label names; a two-label call costs one unit of budget.New Step 2e — Label-Operation Reconciliation. The agent keeps two counts while Steps 2b/2c run:
activatedandlabeled. A call that was never made, was rejected, or errored counts as unlabeled.labeled == activated→ complete, proceed.labeled < activated→ callreport_incompletenaming the shortfall and listing every affected work item.For items created this run, the report names the stable temporary ID plus title and intended labels — not a GitHub issue number. Issue creation is deferred to the post-agent job, so no real number exists during the agent turn; a real number is quoted only for an issue that already existed and was recognized.
Never predict, infer, or invent a number.(Added per the PR #1965 review, to avoid reintroducing #1962's invalid-number assumption.)Reconciliation keys on absence of success, never on presence of an error — a call can simply never be made, and a surplus item is dropped at collection time with only a warning.
Step 2d — an existing sentence that let the agent guess a cap as the cause of a partial run now requires the cap to have been observed in Step 2e.
Corrections after review
All three verified directly against
githubnext/gh-aw@v0.87.10, the CI pin.1.
report_incompletedoes not make the run conclude non-successfully.report_incomplete_handler.cjsemitscore.warningonly.handle_agent_failure.cjscontains zerosetFailed/process.exitcalls across 4453 lines. What it actually does is open or update a durable[aw] {workflow} reported incomplete resulttracking issue or comment. gh-aw's own tool description — "treated as a failure signal even when the agent exits successfully" — is misleading, and the first commit trusted it. Trusting injected prose over the runtime contract is precisely the failure mode #1961 exists to prevent.2. Cap enforcement is dual. Per Safe Outputs spec MCE4,
enforcePerTypeMaxinsafe_outputs_handlers.cjsthrows JSON-RPCE002: {type} limit reached — {n} of {max} already used this runat invocation time (live for us, sincegetExplicitMaxonly applies whenmaxis explicitly configured). The collector then drops surplus items with a warning. The claim that an over-limit item "never appears as an error to the agent" was wrong and is removed. Absence-of-success reconciliation is retained regardless, and the prose now warns that the absence of anE002is not proof every label landed.3. The 110 rationale is the injected wording, not a proven overflow. The first commit implied 50 calls could overflow a cap of 80. They cannot. The docblock now carries an explicit "Note what is not claimed" paragraph recording this.
Tests
New
test/gh-aw-activation-capacity.test.ts— 28 tests. Prefers compiled/runtime evidence over prose: the boundary tests read the compiled.lock.ymlsafe-outputs config, so a frontmatter edit that fails to compile is caught.labeled < activatedbranch, absence-of-success semantics, the temporary-ID rule, and the false-success prohibition.Mutation-checked, not just green:
max: 110→80Validation
test/gh-aw-activation-capacity.test.ts,test/gh-aw-quality.test.ts,test/gh-aw-activation-label-provisioning.test.ts→ 221/221 passingsquad,squad-review,squad-implement-worker,squad-deps-worker); only the two pre-existingsquad.mdwarnings remainnpm run buildpassesScope
No permission changes —
add-labelskeeps its existing least-privileged grant; onlymaxmoved.Deliberately untouched: temporary-ID linkage (#1962 / PR #1965), fast-path parity (#1959), label-result reporting beyond the explicit overflow path (#1963), broad contract coverage (#1960), E4 (#1958), checker distribution.
Integration note — this PR is stacked on #1965
Base is
bradygaster-plan-activate-temporary-ids(PR #1965), notdev. #1965 head528ba9b0is merged into this branch (merge commit8cbf9907, no force-push), so this PR reviews as a clean delta on top of the temporary-ID contract. Merge #1965 first, then this one; the base retargets todevautomatically when #1965 lands.git merge-base 528ba9b0 HEAD==528ba9b0, and the diff against the new base is 3 files — this PR contains no #1965 content, and nothing from #1966/#1963.The one conflict (Step 2d) was resolved deliberately. #1965's Self-Validation sentence is taken wholesale — it is strictly stronger than this branch's version ("requested/recognized", "the last task's temporary ID", "never substitute a guessed issue number"). Only this PR's minimal trailing clause is reapplied, which is #1961's concern and unaddressed by #1965: never surface a safe-output cap as a guessed reason for a partial run — name a cap only when Step 2e observed one being reached.
Step 2e now targets #1965's real mechanism rather than forward-referencing it. The report identifier for an item created this run is the
temporary_idminted under the Temporary-ID Contract (#aw_epic{K}/#aw_task{N}) — not an issue number, because creation is deferred to the safe-output job. A real number may be quoted only where independently verified (dedup-by-title match, or Step 1's idempotent-rerun path). This mirrors #1965's own "Existing and reused issues" rule instead of restating a parallel contract, so #1962's invalid-number assumption is not reintroduced.Disclosed third-file change
test/gh-aw-quality.test.tsraises the source-growth guard 170 → 172 KB. This is a stacking consequence, not #1961 scope creep, and it follows that guard's own documented criterion rather than silencing a red test. The value was measured on the resolved branch, not guessed:workflows/squad.md(118 258 B) plus its four imports.## skill:blocks that gh-aw strips from the always-loaded prompt. The guard's own failure message names this as exactly when a raise is legitimate.dev's 170? It passes by only 839 bytes. That is the same stale-threshold failure this guard already hit once: before the 160 → 170 raise it sat 21–28 bytes from its ceiling, which is less than a single paragraph, so two independently-compliant PRs could not coexist and correct changes failed on byte count alone.dev's Provision labels correctly in the recommended Squad activation fast path #1959/Use gh-aw temporary IDs for plan activation label targeting #1962 rationale comment is preserved verbatim; Prevent silent truncation of Squad activation label operations #1961's is appended beneath it.Rebased onto
dev#1965 (
fa5cc26c) and #1966 (9e3f6b34) both landed ondev, so this PR is no longer stacked — its base isdevand the diff is #1961-only (3 files).workflows/squad.mdauto-merged with no conflict this round: #1966's edits land in the/squad activatefast path and Step 4, while #1961's are insquad-plan-activateStep 2d/2e. Anchor-grep verified all three contracts survive unduplicated — #1966's Fast-Path Label Provisioning and non-roster reporting, #1965's Temporary-ID Contract and created-count wording, and #1961's capacity budget /max: 110/ Step 2e.Validation
gh-aw-activation-capacity,gh-aw-quality,gh-aw-activation-label-provisioning,gh-aw-activation-temporary-ids(fix(workflows): target activation labels via gh-aw temporary IDs #1965's),gh-aw-activate-fast-path-label-provisioning(fix(workflows): provision labels in the /squad activate fast path #1966's, passes untouched), plus the four other suites importingtest/helpers/gh-aw-lock.ts.max: 110→80fails 4 tests including the compiled-runtime assertion; deleting Step 2e's temporary-ID rule fails its test; reintroducing the retracted "never appears as an error" claim fails its negative guard.gh aw compile --strict; only the pre-existing warnings appear.npm run buildpasses.squad.lock.ymlconfirms both contracts coexist at runtime:add_labels max 110(Prevent silent truncation of Squad activation label operations #1961) alongsidecreate_issue max 75+require_temporary_id: true(fix(workflows): target activation labels via gh-aw temporary IDs #1965).