From f6fa1ea42d35da7ac904f57dfc6a4bb2892af369 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Wed, 5 Aug 2026 14:40:57 +0700 Subject: [PATCH] =?UTF-8?q?fix(lanes):=20SKILL.md=20=E2=80=94=20unblock=20?= =?UTF-8?q?Stage=206/7,=20add=20PR-base=20guard=20at=20Stage=2012=20(E1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps found via audit against Shipyard's source: 1. Stage 6/7 still said "this agent does not exist yet" and told a driving session to treat any lane reaching those stages as blocked — stale since E3 shipped qc-local + senior-gate-reviewer. Replaced with an unconditional launch plus a one-time "ccam lanes agents install" preflight note in Setup. 2. Stage 12's conflict-resolution path called `ccam lanes sync-base` directly on any CONFLICTING PR, with no check that the PR's base is actually `development` first. Shipyard's original has this guard (its own "legacy main-based PR" case) — ported here in general form: never auto-merge into a PR whose base drifted from `development`. --- .claude/skills/ship-feature-lane/SKILL.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.claude/skills/ship-feature-lane/SKILL.md b/.claude/skills/ship-feature-lane/SKILL.md index 67865ec..f20f081 100644 --- a/.claude/skills/ship-feature-lane/SKILL.md +++ b/.claude/skills/ship-feature-lane/SKILL.md @@ -35,6 +35,7 @@ Then check your current position: Resume from the stage shown. If state says `stage=X status=running`, you were mid-stage X when context compacted — re-run that stage from the top (all `ccam` commands are idempotent). - **MCP preflight (fail fast):** confirm this session actually loaded the lane's required Playwright MCPs (their `browser_*` tools must be available) — `playwright` and a local-QC MCP are always required for Stage 3/6. `ccam lanes mcp sync` (F, not built yet) would normally do this for you; until then, tell the human to configure `.mcp.json` manually and restart the session if a required MCP is missing. Catching this at Stage 0 costs a minute; catching it at Stage 13 strands a merged feature unverified. +- **Agents preflight:** if this is the lane's first run through this pipeline, run `ccam lanes agents install` once — it writes the `qc-local` and `senior-gate-reviewer` agent definitions into the lane's own `.claude/agents/` (idempotent; re-running just overwrites with the current templates). Stage 6/7 launch these by `subagent_type`; skipping this leaves those stages unable to find the agent. ## Hard rules @@ -110,12 +111,12 @@ The e2e hook doesn't run migrations itself — it tests the already-running stac ### 6 — Browser QC via the qc-local agent - **Test-only fast-path:** on a fix-loop re-entry whose change is ENTIRELY test files (see the fix-loop rule), SKIP this stage — the app's runtime UI is unchanged — and record `--evidence "QC skipped: test-only change"`. Otherwise run it: -- `ccam stage qc --status running`, then launch the **qc-local** agent (Agent tool, `subagent_type: qc-local` — FOREGROUND; it gates the pipeline. **This agent does not exist yet as of this skill's authoring — a separate, later task ports it.** Until then, this stage cannot run; treat a lane that reaches here as `--status blocked --note "qc-local agent not yet available"` and report to the human). When it exists, give it: the lane's working directory, the feature slug, the feature title, the acceptance points (lane spec), and the **QC Plan** (lane spec, Stage 5) as the authoritative scope to execute against. It owns the whole local browser QC and proof capture (`ccam lanes proof-link` first, then screenshots land under the proof gallery automatically). It runs against the lane's feature-branch stack from Stage 3. Do NOT drive the browser yourself at this stage. +- `ccam stage qc --status running`, then launch the **qc-local** agent (Agent tool, `subagent_type: qc-local` — FOREGROUND; it gates the pipeline; the Setup section's agents preflight already installed it). Give it: the lane's working directory, the feature slug, the feature title, the acceptance points (lane spec), and the **QC Plan** (lane spec, Stage 5) as the authoritative scope to execute against. It owns the whole local browser QC and proof capture (`ccam lanes proof-link` first, then screenshots land under the proof gallery automatically). It runs against the lane's feature-branch stack from Stage 3. Do NOT drive the browser yourself at this stage. - Parse its last line: `LOCAL-QC: PASS` → continue. `LOCAL-QC: FAIL — ` → fix on the feature branch → re-run from Stage 2. Keep its report — it is the feature user-flow review for the senior gate. - **Fold back discoveries:** if its report lists scenarios it found that weren't in the plan (its "Scenarios discovered during QC" section), add them to the `## QC Plan` in-scope list in the lane spec. ### 7 — Senior GO/NO-GO gate *(authorizes the publish)* -- Launch the **senior-gate-reviewer** agent (Agent tool, `subagent_type: senior-gate-reviewer`. **This agent does not exist yet as of this skill's authoring — same later task as Stage 6.** Until then, treat a lane reaching here as `--status blocked --note "senior-gate-reviewer agent not yet available"`). When it exists, give it: the lane's working directory, the requirement + Stage-0 answers (the lane spec file), the feature branch, the Stage-4 code-review findings + resolutions, the Stage-6 `qc-local` report (the user-flow review), and confirmation that gates/e2e/review/QC passed. The agent inspects the local diff with `git diff origin/development...feat/` — no open PR is required (and none exists yet). +- Launch the **senior-gate-reviewer** agent (Agent tool, `subagent_type: senior-gate-reviewer` — installed the same way as `qc-local`, see Stage 6). Give it: the lane's working directory, the requirement + Stage-0 answers (the lane spec file), the feature branch, the Stage-4 code-review findings + resolutions, the Stage-6 `qc-local` report (the user-flow review), and confirmation that gates/e2e/review/QC passed. The agent inspects the local diff with `git diff origin/development...feat/` — no open PR is required (and none exists yet). - Parse its final line: - `VERDICT: GO` → proceed to Stage 8. - `VERDICT: NO-GO — ` → fix on the feature branch, re-run **from Stage 2**. No attempt cap — the loop re-enters; the dashboard's time-on-stage surfaces a lane stuck cycling so the human can step in. Set `--status blocked` only for a genuine blocker you can't resolve. @@ -150,7 +151,7 @@ The e2e hook doesn't run migrations itself — it tests the already-running stac - Check PR state: `gh pr view --json state,mergeable -q '.state + " " + (.mergeable|tostring)'`, and bump the heartbeat (`ccam stage watching-pr`). - `MERGED` → a human merged it: go to **Stage 13** (post-merge verification). - `CLOSED` (unmerged) → the human rejected/abandoned it: `ccam stage done --status passed --note "PR closed unmerged by human"` → STOP. - - `CONFLICTING` → the feature branch conflicts with `development`. Resolve it as real work: + - `CONFLICTING` → the feature branch conflicts with `development`. **Base guard first:** check the PR's actual base — `gh pr view --json baseRefName -q .baseRefName`. If it is NOT `development` (the base drifted — a human retargeted the PR, or it predates this pipeline), do NOT auto-merge anything: `--status blocked --note "PR base is not development — human decision"` and STOP. If the base IS `development`, resolve it as real work: - `ccam lanes sync-base feat/` (merges the latest `origin/development` INTO the feature branch — the only sanctioned merge). A migration-number collision (exit 5) means nothing was merged — rename the printed file on the feature branch, re-run Stage 2, then retry this step. - **Exit 4 — merge conflict, left in place on purpose.** Resolve every conflict thoughtfully on the feature branch — keep `development`'s behavior for code unrelated to this feature, preserve the feature's intent where they overlap; when genuinely ambiguous, STOP and escalate (`--status blocked`, note the files) rather than guess. Never hand-merge a generated contract/client file listed in the profile's `GENERATED_MERGE_PATHS` — the keep-ours driver + regen own them. `git add` ONLY the conflicted files, `git commit --no-edit`, then `ccam lanes sync-base --continue feat/` (folds any regenerated artifacts into a follow-up commit). - Re-enter the pipeline **from Stage 2 through Stage 8** (the push updates the PR), then return here and keep watching.