diff --git a/test/gh-aw-activate-fast-path-label-provisioning.test.ts b/test/gh-aw-activate-fast-path-label-provisioning.test.ts new file mode 100644 index 000000000..58b1b04fb --- /dev/null +++ b/test/gh-aw-activate-fast-path-label-provisioning.test.ts @@ -0,0 +1,416 @@ +/** + * Fresh-repo label provisioning for the recommended `/squad activate` fast path (#1959). + * + * Parent: #1957. Depends on #1962 (`gh-aw-activation-temporary-ids.test.ts`), which + * established the temporary-ID targeting pattern for the full `/squad plan activate` + * path and enabled workflow-global `require-temporary-id: true`. + * + * `/squad activate` routes to the `squad-plan-accept` skill. When only a flat `plan` + * artifact exists (no `program`/`implementation`), it takes its own fast path with its + * own `create-issue` calls. #1955 fixed provisioning in `squad-plan-activate` only, and + * #1962 added just enough temporary-ID compliance here to survive the new global flag — + * so on a fresh repository the fast path still leaned on `create-issue`'s `labels:` + * field, which GitHub silently drops for label names that do not already exist. + * + * This suite locks in the fast path's own copy of the verified pattern: + * - every fast-path `create-issue` still carries a unique temporary ID (#1962's + * compliance edit is preserved, not regressed) + * - `add_labels` is called in the same turn, targeting that call's temporary ID + * - `item_number` is mandatory, with the silent triggering-issue fallback named + * - reused/pre-existing issues are targeted by verified real numbers instead + * - base `squad`, single certified owner, `@copilot`, multi-owner, and non-roster + * behavior stays at parity with `squad-plan-activate` — including the `@copilot` + * mapping stated in the *primary* Step 2 label computation, not only in the + * provisioning prose, and omit-and-record (not a hard stop) for an uncertified owner + * - the origin intent issue is never an `add_labels` target + * - reruns stay idempotent + * + * Out of scope: activation-result reporting redesign (#1963), operation-capacity policy + * (#1961), the broad behavioral suite (#1960), checker distribution, and E4 (#1958). + */ + +import { afterAll, describe, it, expect } from 'vitest'; +import { cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { join } from 'node:path'; + +const WORKFLOWS_DIR = join(process.cwd(), 'workflows'); +const SQUAD_WORKFLOW = join(WORKFLOWS_DIR, 'squad.md'); +const TEST_WORKSPACES_DIR = join(process.cwd(), '.test-workspaces-fast-path-labels'); + +afterAll(() => { + rmSync(TEST_WORKSPACES_DIR, { recursive: true, force: true }); +}); + +/** Read a text file with line endings normalized to LF (Windows checkouts materialize CRLF). */ +function readText(filePath: string): string { + return readFileSync(filePath, 'utf8').replace(/\r\n/g, '\n'); +} + +const workflow = readText(SQUAD_WORKFLOW); + +/** + * Isolate the `squad-plan-accept` skill body — the fast path `/squad activate` routes to. + * It has no `## end skill:` marker of its own, so its body runs to the next `## skill:` + * heading (`squad-plan-revise`). + */ +const ACCEPT_START = workflow.indexOf('## skill: `squad-plan-accept`'); +const ACCEPT_END = workflow.indexOf('## skill: `squad-plan-revise`'); +expect(ACCEPT_START, '"## skill: `squad-plan-accept`" is missing from workflows/squad.md').toBeGreaterThan(-1); +expect(ACCEPT_END, '"## skill: `squad-plan-revise`" is missing from workflows/squad.md').toBeGreaterThan(ACCEPT_START); +const acceptSkill = workflow.slice(ACCEPT_START, ACCEPT_END); + +/** Whitespace-collapsed view for prose assertions that may span wrapped lines. */ +const acceptProse = acceptSkill.replace(/\s+/g, ' '); + +/** The `squad-plan-activate` body, used only for path-parity comparisons. */ +const ACTIVATE_START = workflow.indexOf('## skill: `squad-plan-activate`'); +const ACTIVATE_END = workflow.indexOf('## end skill: `squad-plan-activate`'); +expect(ACTIVATE_START, '"## skill: `squad-plan-activate`" is missing from workflows/squad.md').toBeGreaterThan(-1); +expect(ACTIVATE_END, '"## end skill: `squad-plan-activate`" is missing').toBeGreaterThan(ACTIVATE_START); +const activateProse = workflow.slice(ACTIVATE_START, ACTIVATE_END).replace(/\s+/g, ' '); + +describe('gh-aw: /squad activate fast path provisions labels on a fresh repo (#1959)', () => { + it('routes `/squad activate` to squad-plan-accept, so this skill is the fast path', () => { + // If routing ever moves, every assertion below is inspecting the wrong skill. + expect(workflow).toContain('| `activate` | `squad-plan-accept` |'); + expect(workflow).toContain('| `/squad activate` | Activate (recommended fast-path) |'); + }); + + it('names the root cause: create-issue labels: cannot create a missing label', () => { + expect( + /GitHub silently drops label names that do not already exist/i.test(acceptProse), + "The fast path must state why create-issue's own labels: field is insufficient — " + + 'GitHub drops unknown label names on POST /issues rather than creating them. ' + + 'Without this the model has no reason to make the extra add_labels call.', + ).toBe(true); + expect(acceptProse).toMatch(/`create-issue`'s (own )?`labels:` field (alone )?cannot/i); + }); + + it('declares add_labels with create-if-missing as the provisioning mechanism', () => { + expect(acceptSkill).toContain('add_labels'); + expect(acceptSkill).toContain('create-if-missing'); + expect(acceptProse).toMatch(/`add-labels` safe output \(`allowed: \[squad, "squad:\*"\]`, `create-if-missing: true`\)/); + expect( + /fresh repository/i.test(acceptProse), + 'The fresh-repository case is the entire point of #1959 and must be named explicitly.', + ).toBe(true); + }); + + it('calls add_labels in the same turn as each create-issue call', () => { + expect(acceptProse).toMatch(/In the same turn as each `create-issue` call in Step 2, call `add_labels`/i); + // The per-item bullet must carry the instruction too, not just the section prose, + // so a model reading only the create-issue recipe still emits the label call. + const labelApplication = acceptSkill.match(/^- Label application:.*$/m)?.[0] ?? ''; + expect(labelApplication, 'Step 2 needs a per-item "Label application:" bullet').not.toBe(''); + expect(labelApplication).toContain('add_labels'); + expect(labelApplication).toContain('item_number'); + }); +}); + +describe('#1959: fast-path add_labels targets an explicit, verified item', () => { + it('targets the minting create-issue call\'s own temporary ID', () => { + expect(acceptProse).toMatch( + /`item_number` — that same `create-issue` call's `temporary_id` \(`#aw_ph\{N\}` for a phase issue, `#aw_wi\{N\}` for a work item\)/i, + ); + const labelApplication = acceptSkill.match(/^- Label application:.*$/m)?.[0] ?? ''; + expect(labelApplication).toMatch(/`item_number` set to this item's own temporary ID/i); + }); + + it('preserves #1962\'s minimal temp-ID compliance on every fast-path create-issue', () => { + // Regression guard: #1962 enabled workflow-global require-temporary-id: true, which + // rejects any create-issue call lacking one. #1959 must not weaken that. + const tempIdBullet = acceptSkill.match(/^- Temporary ID:.*$/m)?.[0] ?? ''; + expect(tempIdBullet, 'Step 2 must keep its "Temporary ID:" bullet').not.toBe(''); + expect(tempIdBullet).toContain('require-temporary-id: true'); + expect(tempIdBullet).toContain('#aw_ph{N}'); + expect(tempIdBullet).toContain('#aw_wi{N}'); + expect(tempIdBullet).toContain('^#?aw_[A-Za-z0-9_]{3,12}$'); + // Uniqueness invariant. #1962's follow-up (528ba9b0) byte-tightened the wording from + // "must not repeat within this run" to "be unique in this run" — the same contract, + // fewer bytes. Accept either phrasing, but keep requiring BOTH the uniqueness mandate + // and the silent-duplicate hazard that motivates it, so the invariant cannot be + // dropped by quietly rewording it away. + expect( + /(must not repeat within|be unique in) this run/i.test(tempIdBullet), + 'The "Temporary ID:" bullet must state the per-run uniqueness requirement.', + ).toBe(true); + expect( + /silently lets (the|a duplicate's) last writer own the mapping/i.test(tempIdBullet), + 'The bullet must keep the reason uniqueness matters: gh-aw does not reject a ' + + 'duplicate temporary_id, it silently lets the last writer own the mapping.', + ).toBe(true); + // The phase-issue parent linkage from #1962 must still resolve via temporary ID. + expect(acceptProse).toMatch(/pass its `#aw_ph\{N\}` temporary ID/i); + }); + + it('makes item_number mandatory and names the silent triggering-issue fallback', () => { + expect(acceptProse).toMatch(/Every `add_labels` call MUST pass `item_number`/); + expect( + /Omitting it does not fail — it silently applies the labels to the \*\*triggering intent issue\*\*/i.test( + acceptProse, + ), + 'The hazard must be stated as a silent mislabel of the user\'s own issue, not a ' + + 'generic "always pass item_number" — the model needs the consequence to weigh it.', + ).toBe(true); + // Parity: squad-plan-activate states the identical hazard. + expect(activateProse).toMatch(/silently labels the \*\*triggering intent issue\*\*/i); + }); + + it('forbids borrowing another item\'s temporary ID or labeling an uncreated item', () => { + expect(acceptProse).toMatch(/Never reuse another item's temporary ID/i); + expect(acceptProse).toMatch( + /never emit `add_labels` for an item whose `create-issue` call was not made in this run/i, + ); + }); + + it('uses verified real numbers for reused or pre-existing issues', () => { + expect(acceptProse).toMatch( + /For an issue this run did \*\*not\*\* create — one recognized by Step 1a's idempotency check or matched by title — pass its verified real number instead/i, + ); + expect(acceptProse).toMatch(/a temporary ID maps only issues this run created/i); + }); + + it('forbids predicting a returned issue number or waiting for one', () => { + expect(acceptProse).toMatch( + /`create-issue` returns no real issue number during this run, so never predict or infer one and never pause between the two calls waiting for one/i, + ); + expect(acceptProse).toMatch(/`create-issue` first and `add_labels` immediately after is the supported order/i); + }); +}); + +describe('#1959: fast-path label sets stay at parity with squad-plan-activate', () => { + it('applies the base squad label to every activated item', () => { + expect(acceptProse).toMatch(/Work item: `squad`, plus `squad:\{owner\}`/); + expect(acceptProse).toMatch(/Phase issue: `squad`, plus `squad:\{owner\}`/); + }); + + it('derives the member label from that row\'s own frozen certified Owner', () => { + expect(acceptProse).toMatch( + /derived from that row's own frozen certified `Owner`, lowercased/i, + ); + expect(acceptProse).toMatch( + /never inherit the phase issue's owner or carry the previous row's value forward/i, + ); + // The Step 2 computation bullet must still name its certified binding source. + const labelRule = acceptSkill.match(/^- Labels:.*$/m)?.[0] ?? ''; + expect(labelRule).toMatch(/frozen row `Owner` lowercased/i); + expect(labelRule).toMatch(/only from that task's certified binding/i); + }); + + it('maps @copilot to squad:copilot in the primary label computation itself', () => { + // The bug this guards: the Step 2 `- Labels:` bullet is the *primary* computation — + // it says "the frozen row `Owner` lowercased", which turns `@copilot` into the + // invalid `squad:@copilot`. Stating the mapping only in the provisioning section + // below leaves the primary rule wrong, and a model following the bullet literally + // still emits the bad label. Assert against the extracted bullet, not the whole + // skill, so a correct-elsewhere/wrong-here workflow cannot pass. + const labelRule = acceptSkill.match(/^- Labels:.*$/m)?.[0] ?? ''; + expect(labelRule, 'Step 2 must have a "- Labels:" computation bullet').not.toBe(''); + expect( + /Map `@copilot` to `squad:copilot`/.test(labelRule), + 'The `- Labels:` bullet must carry the @copilot mapping inline.', + ).toBe(true); + expect( + /never `squad:@copilot`/.test(labelRule), + 'The `- Labels:` bullet must forbid the lowercased-verbatim form explicitly.', + ).toBe(true); + + // Still stated in the provisioning section, and identical to the full path. + expect(acceptProse).toMatch(/`@copilot` maps to the existing `squad:copilot` routing label — never `squad:@copilot`/); + expect(activateProse).toMatch(/`@copilot` maps to the existing `squad:copilot` routing label — never `squad:@copilot`/); + }); + + it('gives a multi-owner phase issue only squad, and records the omission', () => { + expect(acceptProse).toMatch( + /only when every accepted row in that phase names one and the same owner/i, + ); + expect(acceptProse).toMatch( + /Two or more distinct owners is a multi-owner phase: apply only `squad`, choose none of them/i, + ); + expect(acceptProse).toMatch(/record it under a `Non-roster agent values` heading/i); + // Parity with the epic rule in squad-plan-activate. + expect(activateProse).toMatch(/multi-owner epic: apply only `squad`/i); + }); + + it('omits and records a non-roster owner instead of stopping, matching full activation', () => { + // #1959 requires parity with squad-plan-activate. The fast path previously stopped + // before any mutation on an uncertified Owner; that is now aligned to omit-and-record + // so both activation paths behave identically for the same plan. + const labelRule = acceptSkill.match(/^- Labels:.*$/m)?.[0] ?? ''; + expect( + /An `Owner` certified by neither route gets `squad` alone: omit the owner label, continue, and record the value under `Non-roster agent values`/.test( + labelRule, + ), + 'The primary label computation must carry the omit-and-record rule inline, not ' + + 'defer it to prose elsewhere in the skill.', + ).toBe(true); + + expect(acceptProse).toMatch( + /An individual `Owner` matching no certified active roster name and not `@copilot` does \*\*not\*\* stop the run/i, + ); + expect(acceptProse).toMatch( + /create that issue with the base `squad` label only, omit the owner label, continue, and record the value under `Non-roster agent values`/i, + ); + expect(acceptProse).toMatch( + /an uncertified value can reach `add_labels` only as the base `squad` label/i, + ); + + // The old stop must be gone — a lingering `/squad plan revise` hard stop would + // contradict the rule above and reintroduce the divergence. + expect( + acceptProse, + 'The pre-mutation stop on an uncertified Owner must be removed, not merely ' + + 'supplemented — otherwise the fast path still diverges from squad-plan-activate.', + ).not.toMatch(/stop before mutation and require `\/squad plan revise`/i); + + // Parity: squad-plan-activate's own omit-and-record rule. + expect(activateProse).toMatch( + /MUST NOT become a\s+`squad:\{agent\}` label: apply only `squad` for that issue/i, + ); + }); + + it('still stops before mutation when the roster itself is unreadable', () => { + // ROSTER_UNREADABLE is a different failure: no certified set exists at all, so + // omit-and-record has nothing to certify against. squad-plan-activate stops here + // too — this is parity, not a leftover of the removed per-owner stop. + expect(acceptProse).toMatch( + /If TG-2 emitted a `ROSTER_UNREADABLE:` line, stop before mutation and report that named reason/i, + ); + expect(activateProse).toMatch(/If TG-2 emitted a `ROSTER_UNREADABLE:` line, STOP:/); + }); + + it('requires the Non-roster agent values heading the omissions feed', () => { + // The omit-and-record rule is only coherent if the heading it records into is + // actually mandated. Without this, "record it" points at nothing. + expect(acceptProse).toMatch( + /Whenever an accepted `Owner` did not become a `squad:\{owner\}` label — a multi-owner phase issue, or a value certified by neither the roster nor `@copilot` — a `Non-roster agent values` heading is \*\*required\*\* in this summary, naming the value and the issue it applied to/i, + ); + expect(acceptProse).toMatch(/Omitting the label while omitting the heading reports a clean run that did not happen/i); + }); + + it('never targets the triggering intent issue with an owner label', () => { + expect(acceptProse).toMatch( + /The triggering intent issue is never an `add_labels` target/i, + ); + expect(acceptProse).toMatch( + /It is the flat-plan parent, not an activated item, and receives no owner label from this run/i, + ); + }); + + it('keeps reruns idempotent', () => { + expect(acceptProse).toMatch( + /Re-applying an already-present label on a rerun is a no-op under add-only merge semantics/i, + ); + expect(acceptProse).toMatch(/safe under Step 1a's idempotency path/i); + // The pre-existing whole-plan idempotency contract must survive untouched. + expect(acceptProse).toMatch(/\*\*Whole-plan idempotency:\*\* when `requested_phase` is null/i); + }); + + it('treats an auto-provisioned label\'s default color as expected, not a failure', () => { + expect(acceptProse).toMatch( + /auto-provisioned by `create-if-missing` on a fresh repository instead receives gh-aw's deterministic color and an empty description/i, + ); + expect(acceptProse).toMatch(/that is expected, not a failure, and must not be reported as one/i); + }); + + it('reports labels actually applied, never merely intended', () => { + expect(acceptProse).toMatch( + /Report only the labels a successful `add_labels` call actually applied; never a label that was skipped, deferred, or merely intended/i, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Compiled-artifact verification +// --------------------------------------------------------------------------- +// +// The assertions above lock the prose contract. This section proves the fast path's +// calls are actually executable: `gh aw compile --strict` must expose add_labels and +// create_issue as tools, wire create_if_missing into the handler config, and keep +// #1962's require_temporary_id enforcement on. Fails closed (never skips) per this +// repo's #1833/#1834 convention: an unmeasured contract is indistinguishable from a +// violated one. + +const GH_AW_INSTALL_HINT = + '`gh aw` is required to compile the workflow this gate inspects. Install it with ' + + '`gh extension install github/gh-aw` (matches .github/workflows/squad-ci.yml). This ' + + 'gate fails closed rather than skipping: an unmeasured contract is indistinguishable ' + + 'from a violated one (#1834).'; + +describe('gh-aw: the fast path\'s label calls compile and stay enforceable (#1959)', () => { + let compiledLock: string | null = null; + + function lockText(): string { + if (compiledLock !== null) return compiledLock; + + const versionProbe = spawnSync('gh', ['aw', '--version'], { encoding: 'utf8' }); + if (versionProbe.status !== 0) { + throw new Error(`gh aw --version failed. ${GH_AW_INSTALL_HINT}`); + } + + mkdirSync(TEST_WORKSPACES_DIR, { recursive: true }); + const workspace = mkdtempSync(join(TEST_WORKSPACES_DIR, 'fast-path-labels-')); + execFileSync('git', ['init', '--quiet'], { cwd: workspace }); + // gh-aw's dispatch-workflow validation resolves cross-workflow references against a + // `.github/workflows/` directory relative to the compiled file. This repo ships the + // gh-aw *source* from a top-level `workflows/` dir; consumers install it into + // `.github/workflows/` via `gh aw add`. Mirror that real deployment layout. + cpSync(WORKFLOWS_DIR, join(workspace, '.github', 'workflows'), { recursive: true }); + execFileSync( + 'gh', + ['aw', 'compile', '.github/workflows/squad.md', '--strict', '--approve', '--no-check-update'], + { cwd: workspace, encoding: 'utf8', stdio: 'pipe' }, + ); + + const lockPath = join(workspace, '.github', 'workflows', 'squad.lock.yml'); + if (!existsSync(lockPath)) { + throw new Error( + 'gh aw compile produced no squad.lock.yml — the compiled artifact this gate ' + + `inspects is absent, so the contract is unmeasured. ${GH_AW_INSTALL_HINT}`, + ); + } + compiledLock = readText(lockPath); + return compiledLock; + } + + it('strict-compiles the workflow carrying the fast-path label calls', () => { + expect(lockText().length).toBeGreaterThan(0); + }); + + it('keeps add_labels and create_issue callable together, so the fast path can pair them', () => { + const compiled = lockText(); + expect(compiled).toMatch(/"tools":\[[^\]]*"add_labels"[^\]]*\]/); + expect(compiled).toMatch(/"tools":\[[^\]]*"create_issue"[^\]]*\]/); + }); + + it('wires create_if_missing into the handler config the fast path relies on', () => { + // gh-aw embeds this JSON config inside a double-quoted YAML env value with every + // `"` backslash-escaped. De-escape before matching so this holds across versions. + const compiled = lockText().replace(/\\"/g, '"'); + expect(compiled).toMatch(/"add_labels":\{[^}]*"create_if_missing":true/); + expect(compiled).toMatch(/"add_labels":\{[^}]*"allowed":\["squad","squad:\*"\]/); + }); + + it('keeps #1962\'s require_temporary_id enforcement on for the fast path\'s create_issue calls', () => { + // require-temporary-id is workflow-global, enforced at the MCP tool layer from the + // single create_issue config block — it covers squad-plan-accept's calls too. + const compiled = lockText().replace(/\\"/g, '"'); + expect(compiled).toMatch(/"create_issue":\{[^}]*"require_temporary_id":true/); + expect(compiled).toMatch(/"required_field_additions":\s*\{\s*"create_issue":\s*\[\s*"temporary_id"\s*\]/); + }); + + it('leaves label writes in the safe-output job, not the agent job', () => { + const compiled = lockText(); + const agentJobAt = compiled.indexOf('\n agent:\n'); + const safeOutputsJobAt = compiled.indexOf('\n safe_outputs:\n'); + expect(agentJobAt, '"agent:" job is missing from the compiled lock file').toBeGreaterThan(-1); + expect(safeOutputsJobAt, '"safe_outputs:" job is missing from the compiled lock file').toBeGreaterThan(agentJobAt); + + const permsMatch = compiled.slice(agentJobAt, safeOutputsJobAt).match(/permissions:\n((?:\s{6}\S.*\n)+)/); + expect(permsMatch, 'agent job permissions block not found').not.toBeNull(); + const permsBlock = permsMatch ? permsMatch[1] : ''; + expect(permsBlock).toContain('issues: read'); + expect(permsBlock).not.toContain('issues: write'); + }); +}); diff --git a/test/gh-aw-activation-label-provisioning.test.ts b/test/gh-aw-activation-label-provisioning.test.ts index b2c234a89..553eaca11 100644 --- a/test/gh-aw-activation-label-provisioning.test.ts +++ b/test/gh-aw-activation-label-provisioning.test.ts @@ -23,7 +23,10 @@ * Out of scope (per #1955's delegated task boundaries): running the E4 live end-to-end * test, and the `squad-plan-accept` fast-path's own separate create-issue/label logic * (only reachable when a flat `plan` artifact exists with no `program`/`implementation` - * artifacts — see workflows/squad.md's `squad-plan-accept` skill). + * artifacts — see workflows/squad.md's `squad-plan-accept` skill). #1959 subsequently + * brought that fast path onto the same pattern; its own contract is locked in + * `gh-aw-activate-fast-path-label-provisioning.test.ts`, and the boundary test below now + * asserts the parity instead of the former gap. */ import { afterAll, describe, it, expect } from 'vitest'; @@ -195,7 +198,7 @@ describe('gh-aw: fresh-repo label provisioning in squad-plan-activate (#1955)', expect(activateProse).toMatch(/Re-runs are idempotent via title match/i); }); - it('does not touch the separate squad-plan-accept fast-path label logic (out of scope)', () => { + it('extends the same provisioning pattern to the squad-plan-accept fast path (#1959)', () => { const acceptStart = workflow.indexOf('## skill: `squad-plan-accept`'); expect(acceptStart, '"## skill: `squad-plan-accept`" is missing from workflows/squad.md').toBeGreaterThan(-1); // squad-plan-accept has no "## end skill" marker of its own; its body runs until the @@ -204,11 +207,11 @@ describe('gh-aw: fresh-repo label provisioning in squad-plan-activate (#1955)', expect(acceptEnd, '"## skill: `squad-plan-revise`" is missing from workflows/squad.md').toBeGreaterThan(acceptStart); const acceptSkill = workflow.slice(acceptStart, acceptEnd); - // #1955 is scoped to squad-plan-activate only; squad-plan-accept's own - // create-issue/label fast path (reachable only for a flat plan with no - // program/implementation artifacts) is intentionally unmodified here. - expect(acceptSkill).not.toContain('add_labels'); - expect(acceptSkill).not.toContain('create-if-missing'); + // #1955 landed provisioning in squad-plan-activate only. #1959 applied the same + // add_labels + create-if-missing pattern to the fast path, so the two activation + // paths now provision labels identically. Assert the parity rather than the old gap. + expect(acceptSkill).toContain('add_labels'); + expect(acceptSkill).toContain('create-if-missing'); }); }); diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index d0db6f557..5677e4507 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -876,7 +876,15 @@ describe('gh-aw: prompt budget & planning import regression', () => { // "keeps the ambient prompt under 40 KB". The check below is only a source-GROWTH // regression guard: it keeps unbounded authoring growth visible without pretending // authored bytes are delivered bytes. - const SOURCE_GROWTH_BUDGET_KB = 160; + // + // Raised 160 → 170 KB by #1959. The fast-path label-provisioning contract it adds + // lives entirely inside the `squad-plan-accept` inline `## skill:` block, so gh-aw + // strips it from the ambient prompt — "keeps the ambient prompt under 40 KB" is + // unaffected and still passing, which is exactly the condition the comment above + // names as making a raise legitimate. #1962's follow-up trims bought some room back + // but not enough: combined source measures 164.8 KB after both changes, still over + // 160, so the raise stays. 170 KB leaves a usable margin without removing the signal. + const SOURCE_GROWTH_BUDGET_KB = 170; const SOURCE_GROWTH_BUDGET_BYTES = SOURCE_GROWTH_BUDGET_KB * 1024; it('squad-planning-ontology.md is in the imports list', () => { diff --git a/workflows/squad.md b/workflows/squad.md index 8d932a931..2bf5c76bb 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -1270,23 +1270,83 @@ Do not create an additional epic, summary, root, or phase issue for a flat plan. Before any `create-issue` call, run Team Guard Step TG-2 and validate every accepted plan row. Freeze a binding for each task number containing that row's -original `Owner` and `Depends On` values. If any `Owner` does not match a certified -active roster name, stop before mutation and require `/squad plan revise`; never -substitute, re-route, or fall back to another identity during acceptance. +original `Owner` and `Depends On` values. If TG-2 emitted a `ROSTER_UNREADABLE:` +line, stop before mutation and report that named reason. An individual `Owner` +matching no certified active roster name and not `@copilot` does **not** stop the +run — matching `squad-plan-activate`, create that issue with the base `squad` label +only, omit the owner label, continue, and record the value under +`Non-roster agent values` (Step 4). Never substitute, re-route, or fall back to +another identity during acceptance. For each work item, `create-issue`: - Title: work item title - Temporary ID: `temporary_id` is required on every `create-issue` call (`require-temporary-id: true`). Mint one per item: `#aw_ph{N}` for a phase issue and `#aw_wi{N}` for a work item, where `{N}` is that row's plan number with non-alphanumeric characters replaced by `_`. Must match `^#?aw_[A-Za-z0-9_]{3,12}$` and be unique in this run — gh-aw silently lets a duplicate's last writer own the mapping. -- Labels: `squad` (color `9B8FCC`), plus `squad:{owner}` (color `9B8FCC`) where `{owner}` is the frozen row `Owner` lowercased. Mint the member label only from that task's certified binding; never re-read team.md, re-route the task, or carry another row's owner forward. On `ROSTER_UNREADABLE:`, stop and report that reason; never mint from a preset or remembered roster. +- Labels: `squad` (color `9B8FCC`), plus `squad:{owner}` (color `9B8FCC`) where `{owner}` is the frozen row `Owner` lowercased. Map `@copilot` to `squad:copilot`; never `squad:@copilot` — `@copilot` is the one permitted non-roster value and it is mapped, not lowercased verbatim. Mint the member label only from that task's certified binding; never re-read team.md, re-route the task, or carry another row's owner forward. An `Owner` certified by neither route gets `squad` alone: omit the owner label, continue, and record the value under `Non-roster agent values` (Step 4). On `ROSTER_UNREADABLE:`, stop and report that reason; never mint from a preset or remembered roster. This computes the label set; `add_labels` applies it (see Fast-Path Label Provisioning) — `create-issue`'s `labels:` field alone cannot land it on a fresh repository. - Body: scope, acceptance criteria, context (parent, phase, size, depends on, owner), notes, footer - Parent: phase issue (hierarchical) or root (flat). For a phase issue created in this run, pass its `#aw_ph{N}` temporary ID — `create-issue` resolves it. The flat-plan root is the triggering issue's own real number. Never guess a number for an issue this run created. - Size: set Project field if available, else body `**Size:**` line +- Label application: in the same turn as this `create-issue` call, call `add_labels` with `item_number` set to this item's own temporary ID and this item's computed label set (see Fast-Path Label Provisioning). `create-if-missing` provisions `squad`/`squad:{owner}` on a fresh repository automatically. Copy every frozen `Depends On` value into the created issue body. Cross-phase deps: look up real issue numbers from prior acceptance comments without changing the declared task dependencies. -Create in dependency order. Labels must have descriptions and colors. +Create in dependency order. + +##### Fast-Path Label Provisioning + +`create-issue`'s own `labels:` field cannot provision a label: GitHub silently drops +label names that do not already exist in the target repository instead of creating +them, so on a fresh repository every fast-path issue would come out unlabeled. The +`add-labels` safe output (`allowed: [squad, "squad:*"]`, `create-if-missing: true`) is +the only operation here that creates a missing label, and it is the same one +`squad-plan-activate` uses — both activation paths provision labels identically, so +`/squad activate` needs no manual label setup on a fresh repository. + +In the same turn as each `create-issue` call in Step 2, call `add_labels` with: + +- `item_number` — that same `create-issue` call's `temporary_id` (`#aw_ph{N}` for a + phase issue, `#aw_wi{N}` for a work item). For an issue this run did **not** create + — one recognized by Step 1a's idempotency check or matched by title — pass its + verified real number instead; a temporary ID maps only issues this run created. +- `labels` — exactly the set computed below for that one issue, and nothing else. + +**Explicit targeting is mandatory.** Every `add_labels` call MUST pass `item_number`. +Omitting it does not fail — it silently applies the labels to the **triggering intent +issue**, branding the user's own request with an activated item's owner label. Never +reuse another item's temporary ID, and never emit `add_labels` for an item whose +`create-issue` call was not made in this run. `create-issue` returns no real issue +number during this run, so never predict or infer one and never pause between the two +calls waiting for one; gh-aw resolves `add_labels` after the `create-issue` that +minted the ID, so `create-issue` first and `add_labels` immediately after is the +supported order. + +**Label set, per issue:** + +- Work item: `squad`, plus `squad:{owner}` derived from that row's own frozen + certified `Owner`, lowercased. `@copilot` maps to the existing `squad:copilot` + routing label — never `squad:@copilot`. Re-read each row's frozen `Owner`; never + inherit the phase issue's owner or carry the previous row's value forward. +- Phase issue: `squad`, plus `squad:{owner}` only when every accepted row in that + phase names one and the same owner. Two or more distinct owners is a multi-owner + phase: apply only `squad`, choose none of them, and record it under a + `Non-roster agent values` heading in the Step 4 summary. +- The triggering intent issue is never an `add_labels` target. It is the flat-plan + parent, not an activated item, and receives no owner label from this run. + +An `Owner` that is neither a certified roster name nor `@copilot` never becomes a +`squad:{owner}` label: send `squad` alone for that issue and record the value under +`Non-roster agent values` in the Step 4 summary — the same omit-and-record contract +`squad-plan-activate` uses, so an uncertified value can reach `add_labels` only as +the base `squad` label. + +Re-applying an already-present label on a rerun is a no-op under add-only merge +semantics, so this is safe under Step 1a's idempotency path. Labels must have +descriptions and intentional colors when they already exist; a label auto-provisioned +by `create-if-missing` on a fresh repository instead receives gh-aw's deterministic +color and an empty description — that is expected, not a failure, and must not be +reported as one. Report only the labels a successful `add_labels` call actually +applied; never a label that was skipped, deferred, or merely intended. ##### Step 3: Preserve Dependencies @@ -1311,6 +1371,12 @@ and whether dependencies use native edges or the body-reference fallback. Never claim an epic, phase issue, sub-issue relationship, or native dependency edge that was not created. +Whenever an accepted `Owner` did not become a `squad:{owner}` label — a multi-owner +phase issue, or a value certified by neither the roster nor `@copilot` — a +`Non-roster agent values` heading is **required** in this summary, naming the value +and the issue it applied to. Omitting the label while omitting the heading reports a +clean run that did not happen. + ##### Step 5: Update Fast-Path Lifecycle Call `upsert_lifecycle_state` once with the complete lifecycle body.