diff --git a/docs/memory/pipeline/change-lifecycle.md b/docs/memory/pipeline/change-lifecycle.md index 9c6bd46d..4048e2e7 100644 --- a/docs/memory/pipeline/change-lifecycle.md +++ b/docs/memory/pipeline/change-lifecycle.md @@ -1,6 +1,6 @@ --- type: memory -description: "Change naming, folder structure, `.status.yaml` (6-stage pipeline + `plan:` block, flock-serialized writes + fsync durability + sparse-key insertion), `.fab-status.yaml` symlink (atomic temp+rename swap, dangling-target validation), `stage_hooks` pre/post commands around start/finish (documented in c5tr), git integration (bookkeeping decoupled; ship-time branch↔change hard guard since w7dp), `/fab-status` (six-glyph progress legend incl. `⏭` skipped), `/fab-switch` (six-state `{state}` qualifier), backlog scanning, optional `summary` log field (FKF C-lite source, 5943) + the generated FKF memory-artifact surface (`fab memory-index` emits per-folder `log.md` + `fkf_version`/`type: memory` frontmatter, bmzo)" +description: "Change naming, folder structure, `.status.yaml` (6-stage pipeline + `plan:` block, flock-serialized writes + fsync durability + sparse-key insertion), `.fab-status.yaml` symlink (atomic temp+rename swap, dangling-target validation), `stage_hooks` pre/post commands around start/finish (documented in c5tr), git integration (bookkeeping decoupled; ship-time branch↔change hard guard since w7dp), `/fab-status` (six-glyph progress legend incl. `⏭` skipped), `/fab-switch` (six-state `{state}` qualifier), backlog scanning, optional `summary` log field (FKF C-lite source, 5943) + the generated FKF memory-artifact surface (`fab memory-index` emits per-folder `log.md` + `fkf_version`/`type: memory` frontmatter, bmzo); adoption (`/fab-adopt`) as an alternate pipeline entry-point with the honest `apply: skipped` state (composed from existing skip/reset transitions, t54n)" --- # Change Lifecycle @@ -151,6 +151,12 @@ The stages split into four phases: - **Hydration** (4): hydrate (hydrates into memory files) - **Integration** (5-6): ship (PR creation via `/git-pr`), review-pr (PR review feedback via `/git-pr-review`) +### Adoption — an Alternate Entry-Point with `apply` Skipped (t54n) + +The pipeline normally starts at intake. **`/fab-adopt`** (see [execution-skills.md](/pipeline/execution-skills.md) § `/fab-adopt`) is an **alternate entry-point** for a *completed* change authored **off-pipeline** — a feature branch with an OPEN (or not-yet-created) PR whose code was written without fab (scenario B; a MERGED PR is the out-of-scope scenario A and STOPs). Of the six stages, exactly one — **apply** — cannot meaningfully re-run on an adopted change (the code already exists; there is nothing to generate). Every other stage *can* run for real, just *late*: intake reconstructed from the diff, review run on the diff before merge, hydrate writing memory before merge, ship retrofitting the PR `## Meta`, review-pr resuming normally. Adoption is therefore **the real pipeline entered late, with apply skipped** — not a parallel "fake pipeline." + +**`apply: skipped` honest-state pattern.** The defining stance is *honest state*: only the stage that genuinely cannot run is marked `skipped`; the rest are marked truthfully and actually execute. `/fab-adopt` reaches the end-state `apply: skipped`, `review: active`, `hydrate/ship/review-pr: pending` by composing the **existing** `skip` and `reset` transitions — `fab status skip {name} apply` (forward-cascades all downstream to `skipped`) then `fab status reset {name} review fab-adopt` (re-activates review `skipped→active` and cascades its downstream back to `pending`); no new Go transition is added (see [schemas.md](/pipeline/schemas.md) § Adoption's `apply=skipped, review=active` is a transition COMPOSITION). The off-pipeline fact is recorded once via `fab status set-summary {name} "adopted off-pipeline change; apply skipped"`. This honors Constitution II — `docs/memory/` finally reflects the off-pipeline change at hydrate (the permanent-loss recovery the bypass skipped), and the change becomes visible to the generated `log.md` history via its `summary:`. A `skipped` apply also *displays* honestly via the `⏭` glyph (`/fab-status`, `/fab-switch`, `fab change list`). Adoption adds **no new state** to the vocabulary — `skipped` already existed; it is simply the first pipeline path that marks `apply` skipped as a deliberate, supported entry rather than an edge case. + After hydrate completes, the change folder remains in `fab/changes/`. To move it to the archive, run `/fab-archive` — a standalone housekeeping command (not a pipeline stage). `/fab-archive` moves the folder to `fab/changes/archive/yyyy/mm/` (date-bucketed by the folder's `YYMMDD` prefix), updates the archive index, marks backlog items done (exact-ID check always; keyword scan with interactive confirmation), and conditionally removes `.fab-status.yaml` if the archived change was active. Whether it was active is captured by reading the symlink directly (exact folder match) **before** the folder rename (mz4q — a prerequisite of resolve's dangling-target validation, which would otherwise never see the post-rename pointer as the archived change); `pointer: cleared` is reported only when the symlink actually pointed at the archived change, `pointer: skipped` otherwise — including when no symlink exists, where the old post-rename resolution path could have single-change-guessed the answer. To restore an archived change, run `/fab-archive restore ` — this moves the folder back to `fab/changes/`, removes the index entry, and optionally activates via `--switch`. Existing flat archive entries are migrated to the date-bucketed structure via `/fab-setup migrations` (see `$(fab kit-path)/migrations/0.24.0-to-0.32.0.md`). **Archive CLI exit semantics (k4ge)**: `fab change archive` requires exactly one argument (`cobra.ExactArgs(1)` — zero args is a non-zero usage error, no longer exit-0 help text). Re-archiving an already-archived change is an idempotent soft skip — `already archived: {change}` on stdout, exit 0 — including the genuinely-archived case where the source folder is gone from `fab/changes/` (an archive-scan fallback in `Archive()`; see [execution-skills.md](/pipeline/execution-skills.md)). An argument matching no change anywhere (active or archived) still propagates the resolution error non-zero, and ambiguous archive matches do not soft-skip. Partial failure (the move succeeded but the backlog mark failed) prints the YAML report and exits non-zero — re-running soft-skips. **`fab batch archive` confirmation/preview model (753q)**: archive is the one bulk-mutating member of the batch family whose moves are effectively irreversible within the loop, so it diverges from `batch new`/`batch switch` (which stay list-by-default behind `--all`) to a list-then-confirm model with a `--yes` escape hatch (the apt/npm/gh pattern; this replaced the 260612-ye8r explicit-`--all` model). A bare `fab batch archive` on an interactive stdin lists the archivable set then prompts `Archive these N? [y/N]` with **default No** — a bare Enter or any non-`y`/`yes` answer aborts (exit 0, nothing archived). `--yes`/`-y` archives all archivable changes with no prompt (the non-interactive escape hatch; resolved behavior of the former `--all`). `--dry-run` lists what would be archived with no prompt and no action (the former `--list`). A non-TTY stdin without `--yes` refuses rather than hangs — the handler returns a single multi-line error (it does not print its own `ERROR:` lines) so `main()`'s centralized printer emits it once as `ERROR: refusing to prompt for confirmation on a non-interactive stdin.` followed by `Re-run with --yes to archive non-interactively` on stderr, exit non-zero (the tmux/operator runtime is non-interactive, so those call sites pass `--yes`). Explicit args (`fab batch archive foo bar`) archive the named changes with no prompt and no TTY guard (naming them is the opt-in). `--dry-run --yes` is mutually exclusive → exit non-zero. The empty archivable set stays a benign no-op (`No archivable changes found.` + `Archived 0, skipped 0, failed 0.` footer, exit 0), checked **before** any prompt or non-TTY guard so finding F49 is preserved; explicitly named targets that are genuinely archived route to the loop's `already archived, skipping` path (counted skipped, exit 0); the explicit-args path where nothing resolves anywhere keeps exit 1 (`No valid changes to archive.`). diff --git a/docs/memory/pipeline/execution-skills.md b/docs/memory/pipeline/execution-skills.md index 5e59deb3..19042ac0 100644 --- a/docs/memory/pipeline/execution-skills.md +++ b/docs/memory/pipeline/execution-skills.md @@ -1,6 +1,6 @@ --- type: memory -description: "Apply (unified `plan.md` `## Requirements`+`## Tasks`+`## Acceptance` generated at entry), review, hydrate, archive, and orchestrator behavior — `/fab-continue` for pipeline stages, `/fab-archive` for housekeeping, `/fab-proceed` for context-aware pipeline orchestration; `/git-pr` ship behavior with the mechanically-rendered `## Meta` block via `fab pr-meta`, a unified Step 0 resolution (szxd), and git-state hardening — detached-HEAD STOP, expected-area staging guard, PR-state branching, resolved default-branch guard (g8st); `/git-pr-review` split commit/push failure semantics + unpushed-commit re-run gate (g8st); fab-continue loads `_generation`/`_review` stage-conditionally at point of use (zc9m); the shared ff/fff bracket lives in `_pipeline.md` with once-stated rework choreography (cycle cap = the `{max_cycles}` knob from code-review.md § Rework Budget, default 3 — c5tr), exhaustion terminal state review:failed, and fab-continue's review-failed rework-menu dispatch row (szxd); re-archiving a genuinely archived change soft-skips (exit 0) and restore `--switch` surfaces `pointer: failed` (k4ge); archive/restore index writes are atomic with honest `index: failed` reporting on both paths (hv7t); `/git-pr`/`/git-pr-review` accept an optional explicit `` argument and guard branch↔change correspondence before any mutation, fab-continue gained the review-pr/failed dispatch row + idempotent reset, and recovery guidance is override-aware (w7dp); pipeline hydrate gained a refuse-before-regen guard consulting `fab memory-index --check` exit 2 (destructive loss), defense-in-depth + born-compatible no-op (glwc); the Auto-Rework Loop states the cycle-count invariant + baseline convention (iterations == initial entry + each re-entry, N cycles → N+1; a prose gap, not a Go bug) and `/git-pr-review` Phase 2 carries the synchronous-poll discipline + corrected two-login query semantics (landed-review predicate `author.login == copilot-pull-request-reviewer`; `Copilot` under requested_reviewers; REST requested_reviewers confirms requests since GraphQL omits bot reviewers) (qg64); pipeline hydrate now authors FKF frontmatter (`type: memory` + `description:`), stops writing per-file `## Changelog` (records the what-changed via `fab status set-summary`), and uses bundle-relative memory↔memory links — 8fr5; `/git-pr` Step 3 gained sub-step 3a-bis — a post-commit/pre-push `fab memory-index` regen with a separate `docs: refresh memory indexes` follow-up commit (no `--amend`), gated on `{has_fab}` AND 3a-just-committed and diff-guarded; it is retained for **`log.md` only** (freeze-on-write needs the post-commit projection to capture the change's own entry while its commits are still reachable, pre-squash — the index carries no dates since ugde, so its regen half is a reliable no-op) (o203; rationale narrowed by ugde). Operator coordination moved to runtime/operator.md." +description: "Apply (unified `plan.md` `## Requirements`+`## Tasks`+`## Acceptance` generated at entry), review, hydrate, archive, and orchestrator behavior — `/fab-continue` for pipeline stages, `/fab-archive` for housekeeping, `/fab-proceed` for context-aware pipeline orchestration; `/git-pr` ship behavior with the mechanically-rendered `## Meta` block via `fab pr-meta`, a unified Step 0 resolution (szxd), and git-state hardening — detached-HEAD STOP, expected-area staging guard, PR-state branching, resolved default-branch guard (g8st); `/git-pr-review` split commit/push failure semantics + unpushed-commit re-run gate (g8st); fab-continue loads `_generation`/`_review` stage-conditionally at point of use (zc9m); the shared ff/fff bracket lives in `_pipeline.md` with once-stated rework choreography (cycle cap = the `{max_cycles}` knob from code-review.md § Rework Budget, default 3 — c5tr), exhaustion terminal state review:failed, and fab-continue's review-failed rework-menu dispatch row (szxd); re-archiving a genuinely archived change soft-skips (exit 0) and restore `--switch` surfaces `pointer: failed` (k4ge); archive/restore index writes are atomic with honest `index: failed` reporting on both paths (hv7t); `/git-pr`/`/git-pr-review` accept an optional explicit `` argument and guard branch↔change correspondence before any mutation, fab-continue gained the review-pr/failed dispatch row + idempotent reset, and recovery guidance is override-aware (w7dp); pipeline hydrate gained a refuse-before-regen guard consulting `fab memory-index --check` exit 2 (destructive loss), defense-in-depth + born-compatible no-op (glwc); the Auto-Rework Loop states the cycle-count invariant + baseline convention (iterations == initial entry + each re-entry, N cycles → N+1; a prose gap, not a Go bug) and `/git-pr-review` Phase 2 carries the synchronous-poll discipline + corrected two-login query semantics (landed-review predicate `author.login == copilot-pull-request-reviewer`; `Copilot` under requested_reviewers; REST requested_reviewers confirms requests since GraphQL omits bot reviewers) (qg64); pipeline hydrate now authors FKF frontmatter (`type: memory` + `description:`), stops writing per-file `## Changelog` (records the what-changed via `fab status set-summary`), and uses bundle-relative memory↔memory links — 8fr5; `/git-pr` Step 3 gained sub-step 3a-bis — a post-commit/pre-push `fab memory-index` regen with a separate `docs: refresh memory indexes` follow-up commit (no `--amend`), gated on `{has_fab}` AND 3a-just-committed and diff-guarded; it is retained for **`log.md` only** (freeze-on-write needs the post-commit projection to capture the change's own entry while its commits are still reachable, pre-squash — the index carries no dates since ugde, so its regen half is a reliable no-op) (o203; rationale narrowed by ugde); `/fab-adopt` adopts a completed off-pipeline change — guards → one-pass intake+plan from the diff → `skip apply`+`reset review` → outward-only review → hydrate → `/git-pr` Meta-retrofit → review-pr; `_review.md` gained a `mode` (full | outward-only) param and `/git-pr` gained sub-step 3d (`## Meta` body-retrofit onto an existing OPEN PR via `fab pr-meta` + `gh pr edit --body-file -`, gated/idempotent) (t54n). Operator coordination moved to runtime/operator.md." --- # Execution Skills @@ -62,6 +62,8 @@ Each prefix step (the `_intake` Create-Intake Procedure, `/fab-switch`, `/git-br **Post-commit memory-index refresh — sub-step 3a-bis (o203; rationale narrowed by ugde)**: `/git-pr` Step 3 gained a `#### 3a-bis. Refresh Memory Indexes` sub-step positioned **between 3a (Commit) and 3b (Push)** — the only pipeline position where `git log` reports the change's own content commit. Its job is **`log.md`**, a freeze-on-write projection of *committed* git history: it must capture this change's own entry while the change's commit is still reachable (pre-squash). The hydrate-stage regen (Step 5) runs entirely **pre-commit**, so it cannot see the change's own commit; 3a-bis closes that gap at the only pipeline position where `git log` already knows the change's commit — immediately after 3a commits, before 3b pushes. The **index** no longer participates: since ugde it carries no dates (a pure function of content), so its regen half is a reliable no-op and `log.md` is the sole reason 3a-bis remains. (Originally, o203 added 3a-bis to fix the index `Last Updated` date drift — the index was born "one regen behind" because hydrate's regen was pre-commit, and `fab memory-index --check` then flagged benign tier-1 date drift at review-pr; dropping the index date column in ugde removed that index-side justification entirely, leaving only `log.md`'s freeze-on-write need.) It runs `fab memory-index` (byte-stable; writes only `docs/memory/` index + log files) and, when `docs/memory/` changed (`git diff --quiet -- docs/memory` exits non-zero), stages `git add docs/memory` and makes a **SEPARATE** `docs: refresh memory indexes` follow-up commit — **never `--amend`** (keeps 3a's authored content commit intact and reviewable; squash collapses the pair on merge anyway). It is gated on **BOTH** `{has_fab}` (the Step 0 variable) AND 3a-having-just-committed-this-invocation (the `has_uncommitted` path ran) — skipped entirely otherwise, so it is a **silent no-op** on the "already shipped" / no-change re-run paths and when `/git-pr` runs standalone outside a fab project (`{has_fab}` false → general-purpose standalone use unaffected). The `git diff --quiet -- docs/memory` guard suppresses an empty follow-up commit when nothing drifted (Constitution III idempotency); a regen-or-commit failure → **report + STOP**, leaving the 3a content commit intact (a failed refresh degrades to a benign stale `log.md` recoverable by re-running `fab memory-index` — never a torn state). There is **no push inside 3a-bis** — 3b ("if has_unpushed or just committed") pushes both the content commit and the index-refresh commit together. The step lives in **ship, not hydrate**, precisely because hydrate is entirely pre-commit (no in-hydrate regen can see the change's own commit); this is the in-skill `{has_fab}`-gated route, chosen over `stage_hooks.ship.post`. The original o203 change was **skill-prose + spec-mirror only** — no Go change, no new CLI command, no migration; it adds a new *caller* of the unchanged `fab memory-index`, mirrored into `docs/specs/skills/SPEC-git-pr.md`'s Flow tree. The Key Properties Idempotent? row records that 3a-bis is gated on 3a-having-just-committed (a re-run on the no-commit path skips it) and is byte-stable + diff-guarded even if reached. This belongs in the same `/git-pr`-hardening lineage as g8st / w7dp / glwc below. +**`## Meta` body-retrofit onto an existing OPEN PR — sub-step 3d (t54n)**: `/git-pr` injects the `## Meta` block only on PR **create** (Step 3c), so an OPEN PR authored off-pipeline (or created before fab adopted the branch) carries a body with **no `## Meta`**. Step 3d closes that gap on the **existing-OPEN-PR path** — the path that previously only *recorded* the URL. It is the ship-stage Meta retrofit `/fab-adopt` relies on (Step 5 of that orchestrator), but it is **general**: any OPEN-PR ship benefits. It fires only when **`{has_fab}`** AND the PR was **already OPEN at Step 1** (`pr_state == OPEN` — i.e. Step 3c did NOT just create it; a freshly created PR already carries Meta from 3c, so retrofitting would be redundant). **Idempotency guard**: if the PR body already contains a `## Meta` heading, it makes **no** edit and continues (a second run is a no-op — Constitution III), keeping the "already shipped" re-run a clean no-op. Otherwise it renders the block by **reusing Step 3c's mechanism** — `META=$(fab pr-meta "{name}" --type {type} --issues "{issues}")` — and applies it by **prepending** Meta above the existing body via `printf '%s\n\n%s\n' "$META" "$existing_body" | gh pr edit --body-file -` (stdin, avoiding shell-escaping the body). On empty/failed `fab pr-meta` (no fab context) it omits the block, exactly as the create path's `{has_fab} = false` fallback does. **No Go change** — `fab pr-meta` / `prmeta.Render` and `gh pr edit --body-file -` already exist and are reused. This belongs in the same `/git-pr`-hardening lineage as 3a-bis / g8st / w7dp / glwc. + **Ship-pipeline git-state hardening (g8st)**: `/git-pr` verifies git state before mutating, instead of assuming an attached, main-defaulted, clean world: - **Detached-HEAD STOP**: Step 1 captures the current branch; an empty `git branch --show-current` means a detached HEAD (`git symbolic-ref -q HEAD` exits 1). Step 2's detached-HEAD guard — checked **before** the default-branch guard — STOPs with `Cannot ship from a detached HEAD — check out a branch first (run /git-branch).` before any commit or push. (Previously the empty branch name passed the literal-`main`/`master` guard, Step 3a committed, and Step 3b emitted a refspec-less `git push -u origin`.) Since w7dp the detached-HEAD STOP also fires in Step 0, before Step 0a's `fab status start`, on the `{has_fab}` path (verify-before-mutate; Step 2's guard still covers the no-fab path), and the Step 1b mismatch nudge this guard once coexisted with is gone — removed as superseded by the hard branch-matches-change guard. @@ -139,7 +141,9 @@ Loads: config, constitution, `specs/index.md`, `plan.md` (when it exists), `inta `/fab-continue` dispatches to review behavior after apply completes. Review behavior is defined in `_review.md` (a shared internal skill file following the same pattern as `_generation.md`). `fab-continue.md` delegates to `_review.md` rather than inlining review dispatch logic — the same delegation pattern used for planning stages — and reads `.claude/skills/_review/SKILL.md` at this point of use (stage-conditional since zc9m; not a frontmatter helper). For `/fab-ff`/`/fab-fff`, the shared bracket (`_pipeline.md` Step 2, szxd) notes `_review.md` as the authoritative source for review dispatch instructions (both wrappers keep it as an unconditional frontmatter helper — see § Stage-Conditional Helper Loading above). `_review.md`'s trailing note points the rework loop at `fab-continue.md`'s Verdict section (manual) and `_pipeline.md` § Auto-Rework Loop (ff/fff) — its former "fab-ff.md Step 3, fab-fff.md Step 3" pointer was corrected in szxd. -**Dual sub-agent dispatch**: The review stage dispatches **two sub-agents in parallel**: +**Review Mode (`full` | `outward-only`, t54n)**: `_review.md` is the single authority for review dispatch, so the orchestrator MAY pass a **`mode`** parameter that selects which sub-agents run — inherited uniformly by `/fab-continue`, `/fab-ff`, `/fab-fff`, and `/fab-adopt`. `full` (the default when the param is omitted — so every existing caller is unaffected, the concept is purely additive) dispatches **inward + outward** in parallel; `outward-only` dispatches the **outward sub-agent alone** (used by `/fab-adopt` — see § `/fab-adopt`). `mode` gates two steps: the inward **Preconditions** (`plan.md` MUST exist with `## Requirements`/`## Tasks`/`## Acceptance`, all tasks `[x]`) are checked **only** in `full` — in `outward-only` they are **skipped** entirely (there is no inward sub-agent and nothing for it to validate; the outward sub-agent reads the diff directly, not `plan.md`); and **Parallel Dispatch** dispatches only the selected sub-agent(s). **Findings Merge and the pass/fail rule are identical for both modes** — "any must-fix → fail" works with a single source, deduplication is a no-op when there is one source, and **zero findings passes**, so an empty `outward-only` result (e.g. all `review_tools` disabled/unavailable) **passes** best-effort (adoption must not hard-block when no external reviewer is available). No speculative `inward-only` value is added — no caller exists (parsimony). + +**Dual sub-agent dispatch**: In `full` mode the review stage dispatches **two sub-agents in parallel**: - **Inward sub-agent**: validates implementation against `plan.md`'s `## Requirements`/`## Tasks`/`## Acceptance` with fresh perspective — no shared context with the applying agent beyond explicitly provided artifacts. Performs eight validation checks: tasks complete, quality checklist, run affected tests, spot-check requirements, memory drift check, code quality check, parsimony pass, and deletion-candidate prompt. - **Outward sub-agent** (new in `_review.md`): performs a holistic diff review with full repository access. Receives the git diff of all changed files, the list of changed file paths, and standard subagent context. Has full tool access (Read, Bash, Agent) and MAY read any file in the repo. Uses a **Codex → Claude cascade**: attempts Codex first; if unavailable or fails, falls back to Claude; if neither tool is available, returns empty findings (graceful no-op). Always on — no config flag. Focus areas: interface contract violations, inconsistencies with documented patterns in memory files, missing cross-references, behavioral regressions requiring full-repo context, and structural issues (duplication, abstraction violations). @@ -231,6 +235,18 @@ Hydration modifies memory files in-place. If the merge goes wrong, the only reco Loads: config, constitution, `specs/index.md`, `plan.md` (incl. its `## Requirements`), `intake.md`, target memory file(s) from `docs/memory/`, `docs/memory/index.md` and relevant domain indexes. +### `/fab-adopt []` (Adoption Orchestrator) + +`/fab-adopt` brings a **completed-but-off-pipeline** change into the Fab pipeline (t54n). The trigger is *mid-flight adoption* (scenario B): a feature branch authored **without** fab with an **OPEN PR or no PR yet** — typically after a reviewer notes "you skipped fab-kit and might have missed a few checks." It is a **thin orchestrator** in the `/fab-proceed` / `/fab-ff` lineage: it reuses existing skills/procedures as sub-agents and introduces only what is genuinely new (the diff→intake and thin diff→plan procedures in `_generation.md`, see [planning-skills.md](/pipeline/planning-skills.md) § Shared Generation Partial; and `_review.md`'s `outward-only` mode, § Review Behavior above). It declares `helpers: [_srad, _generation, _review, _pipeline]` (`_srad` because the reconstructed intake is graded) and takes an optional `` (derived from the PR title or branch name when omitted). The framing — **the real pipeline entered late, with apply skipped** — is recorded in [change-lifecycle.md](/pipeline/change-lifecycle.md) § Adoption. + +**Step 0 — Guards & diff base (no mutation before all pass)**: reusing `/git-pr`'s guard idioms verbatim, STOP (no `fab change new`, no status mutation) on: **detached HEAD** or the **default branch** (`/git-pr`'s messages); PR `state == MERGED` (scenario A — retroactive backfill of already-merged work, out of scope — `OPEN` and no-PR both proceed); a **branch already mapped to a fab change** (`fab change resolve "$(git branch --show-current)"` succeeds → already in the pipeline, point at `/fab-continue`); or an **empty diff** against the resolved merge-base (`base=$(git merge-base HEAD origin/{default})`; nothing to adopt). The diff (`git diff {base}...HEAD`) and changed-file list are captured here for the generation pass and the outward review. + +**Steps 1+2 — ONE main-session generation pass**: the same agent (NOT a dispatched apply) reads the diff + PR body once and: (1) `fab change new --slug {slug}` against the current branch + activate it (the branch already exists, so `/fab-new` Step 11 row 1/2 "already active"/"checked out" applies — no recreate/rename); (2) reconstruct `intake.md` via the **Intake-from-Diff Procedure** (SRAD + `fab score`); (3) a **human-confirmation checkpoint** presents the reconstructed intent + assumptions for confirm/correct — the late deliberation the bypass skipped — then on confirm `fab status advance {name} intake` → `finish {name} intake` (auto-activates apply); (4) write the **MINIMAL `plan.md`** via the **Plan-from-Diff Procedure** from the same understanding (no re-read). Both -from-Diff procedures live in `_generation.md`. + +**Step 2 (state) — `apply → skipped`, `review → active`** via existing transitions only (no Go change): `fab status skip {name} apply` (cascades all downstream → skipped) then `fab status reset {name} review fab-adopt` (`skipped → active`, cascades its downstream → pending), then `fab status set-summary {name} "adopted off-pipeline change; apply skipped"`. The `skip`-then-`reset` ordering is load-bearing — see [schemas.md](/pipeline/schemas.md) § Adoption's `apply=skipped, review=active` is a transition COMPOSITION for the mechanism. + +**Steps 3–6 — review → hydrate → ship → review-pr (the real, late pipeline)**: the orchestrator owns every verdict/`finish` transition (the dispatched blocks run no `fab status` — the universal block contract). **Step 3 — Review, dispatched `mode: outward-only`** (resolve `fab resolve-agent review --alias`, apply model+effort seams): outward reads `git diff {base}...HEAD` natively, inward preconditions are skipped. Pass (incl. zero findings → best-effort pass) → `finish review`; fail → auto-rework per `_pipeline.md` § Auto-Rework Loop budget **when run autonomously**, but the **interactive default hands findings back** to the user rather than auto-editing a hand-authored branch (resolved Open Question — respecting hand-authored code). **Step 4 — Hydrate, dispatched verbatim** (reuses `_pipeline.md` Step 3 unchanged) → `finish hydrate`: this is the permanent-loss recovery — `docs/memory/` finally reflects what shipped. **Step 5 — Ship**: dispatch `/git-pr {name}` (folder name, not bare id); the OPEN-PR path's **Step 3d body-retrofit** injects `## Meta` when the PR body lacks one (§ `## Meta` body-retrofit above), or `/git-pr` creates the PR fresh when `pr_state == none` (no separate `--no-pr` flag — the create path already handles it; resolved Open Question). **Step 6** — `finish ship` auto-activates review-pr; output `Next: /git-pr-review`. The skill prints an **honest-state summary**: intake/review/hydrate/ship/review-pr all genuinely ran (just late, after the code was written) — **only apply is `skipped`**. `/fab-adopt` makes no commit itself (Step 5's `/git-pr` commits the reconstructed `fab/` artifacts and retrofits the PR body); it is partially idempotent (Step 0 guards STOP cleanly pre-mutation; the collision guard routes a post-creation re-run to `/fab-continue`; the dispatched stages and the Meta retrofit are themselves resumable/idempotent). Discoverable under the **Planning** `/fab-help` group via a one-line `"fab-adopt": "Planning"` entry in `skillToGroupMap` (`fabhelp.go`) — a deliberate, graded one-line Go deviation from the intake's "no Go change" note (the help auto-scan would otherwise bucket it under "Other"; `fabhelp_test.go` asserts a subset, so no test change was required). + ### `/fab-archive` (Standalone Skill) `/fab-archive` is a standalone housekeeping command — not a pipeline stage. It supports two modes: **archive** (default) moves completed changes to the archive; **restore** moves archived changes back to active. Since szxd, `fab-archive.md` is a **single document**: mode detection and both argument lists are stated once at the top, and the former second `#`-level document is demoted to a `## Restore Mode` section holding only restore-unique content — including the mode-specific pre-flight waiver (no standard preflight, hydrate guard waived — the opposite of archive mode), which is preserved verbatim. Behavior in both modes is unchanged. diff --git a/docs/memory/pipeline/index.md b/docs/memory/pipeline/index.md index 727c02c2..1d1618de 100644 --- a/docs/memory/pipeline/index.md +++ b/docs/memory/pipeline/index.md @@ -7,9 +7,9 @@ description: "The change pipeline — stage lifecycle & state machine, planning/ | File | Description | |------|-------------| -| [change-lifecycle](change-lifecycle.md) | Change naming, folder structure, `.status.yaml` (6-stage pipeline + `plan:` block, flock-serialized writes + fsync durability + sparse-key insertion), `.fab-status.yaml` symlink (atomic temp+rename swap, dangling-target validation), `stage_hooks` pre/post commands around start/finish (documented in c5tr), git integration (bookkeeping decoupled; ship-time branch↔change hard guard since w7dp), `/fab-status` (six-glyph progress legend incl. `⏭` skipped), `/fab-switch` (six-state `{state}` qualifier), backlog scanning, optional `summary` log field (FKF C-lite source, 5943) + the generated FKF memory-artifact surface (`fab memory-index` emits per-folder `log.md` + `fkf_version`/`type: memory` frontmatter, bmzo) | +| [change-lifecycle](change-lifecycle.md) | Change naming, folder structure, `.status.yaml` (6-stage pipeline + `plan:` block, flock-serialized writes + fsync durability + sparse-key insertion), `.fab-status.yaml` symlink (atomic temp+rename swap, dangling-target validation), `stage_hooks` pre/post commands around start/finish (documented in c5tr), git integration (bookkeeping decoupled; ship-time branch↔change hard guard since w7dp), `/fab-status` (six-glyph progress legend incl. `⏭` skipped), `/fab-switch` (six-state `{state}` qualifier), backlog scanning, optional `summary` log field (FKF C-lite source, 5943) + the generated FKF memory-artifact surface (`fab memory-index` emits per-folder `log.md` + `fkf_version`/`type: memory` frontmatter, bmzo); adoption (`/fab-adopt`) as an alternate pipeline entry-point with the honest `apply: skipped` state (composed from existing skip/reset transitions, t54n) | | [clarify](clarify.md) | `/fab-clarify` skill — intake-only (j6cs), dual modes (suggest/auto), intake taxonomy scan, structured questions, coverage reports, audit trail, grade reclassification, always-recompute intake score; hosts the [AUTO-MODE] Skill Invocation Protocol and is sole bulk-confirm authority (zc9m); bulk confirm evaluated before the zero-gaps exit, confirmations graded by recomputed composite (no fiat Certain), unified audit-trail placement rules (c5tr) | -| [execution-skills](execution-skills.md) | Apply (unified `plan.md` `## Requirements`+`## Tasks`+`## Acceptance` generated at entry), review, hydrate, archive, and orchestrator behavior — `/fab-continue` for pipeline stages, `/fab-archive` for housekeeping, `/fab-proceed` for context-aware pipeline orchestration; `/git-pr` ship behavior with the mechanically-rendered `## Meta` block via `fab pr-meta`, a unified Step 0 resolution (szxd), and git-state hardening — detached-HEAD STOP, expected-area staging guard, PR-state branching, resolved default-branch guard (g8st); `/git-pr-review` split commit/push failure semantics + unpushed-commit re-run gate (g8st); fab-continue loads `_generation`/`_review` stage-conditionally at point of use (zc9m); the shared ff/fff bracket lives in `_pipeline.md` with once-stated rework choreography (cycle cap = the `{max_cycles}` knob from code-review.md § Rework Budget, default 3 — c5tr), exhaustion terminal state review:failed, and fab-continue's review-failed rework-menu dispatch row (szxd); re-archiving a genuinely archived change soft-skips (exit 0) and restore `--switch` surfaces `pointer: failed` (k4ge); archive/restore index writes are atomic with honest `index: failed` reporting on both paths (hv7t); `/git-pr`/`/git-pr-review` accept an optional explicit `` argument and guard branch↔change correspondence before any mutation, fab-continue gained the review-pr/failed dispatch row + idempotent reset, and recovery guidance is override-aware (w7dp); pipeline hydrate gained a refuse-before-regen guard consulting `fab memory-index --check` exit 2 (destructive loss), defense-in-depth + born-compatible no-op (glwc); the Auto-Rework Loop states the cycle-count invariant + baseline convention (iterations == initial entry + each re-entry, N cycles → N+1; a prose gap, not a Go bug) and `/git-pr-review` Phase 2 carries the synchronous-poll discipline + corrected two-login query semantics (landed-review predicate `author.login == copilot-pull-request-reviewer`; `Copilot` under requested_reviewers; REST requested_reviewers confirms requests since GraphQL omits bot reviewers) (qg64); pipeline hydrate now authors FKF frontmatter (`type: memory` + `description:`), stops writing per-file `## Changelog` (records the what-changed via `fab status set-summary`), and uses bundle-relative memory↔memory links — 8fr5; `/git-pr` Step 3 gained sub-step 3a-bis — a post-commit/pre-push `fab memory-index` regen with a separate `docs: refresh memory indexes` follow-up commit (no `--amend`), gated on `{has_fab}` AND 3a-just-committed and diff-guarded; it is retained for **`log.md` only** (freeze-on-write needs the post-commit projection to capture the change's own entry while its commits are still reachable, pre-squash — the index carries no dates since ugde, so its regen half is a reliable no-op) (o203; rationale narrowed by ugde). Operator coordination moved to runtime/operator.md. | -| [planning-skills](planning-skills.md) | `/fab-new`, `/fab-continue`, `/fab-ff`, `/fab-clarify` — the planning stage (intake only) and the shared `_generation.md` partial (Intake + unified Plan procedures); requirement capture + plan generation live at apply entry (spec stage removed in j6cs); fab-new/fab-draft re-run/resume semantics (ID collisions route to resume, 9u91); change_type is hook-owned — skills verify via .status.yaml and override only if wrong (uliv); SRAD framework lives in the `_srad` helper, declared by the 6 planning skills (zc9m); pre-boundary intake creation (fab-new Steps 0–9) lives in the shared `_intake` helper parameterized by `{questioning-mode}` (interactive | promptless-defer) — fab-new/fab-draft/fab-proceed are thin call-sites, completing the one-shared-helper-per-phase symmetry with `_pipeline`; fab-new keeps the activate/branch tail, fab-draft stops at ready (momentum warning retired), fab-proceed dispatches promptless-defer + chains /fab-switch→/git-branch (3xaj); fab-ff/fab-fff are thin wrappers over the shared `_pipeline` bracket, fab-new Step 11 is a single first-match-wins branch table (szxd; 6 rows since g8st — remote-only `--track` checkout, dirty-tree carried-over note excluding the change's own artifacts, same-change rename); SRAD grades by half-open bands, the plan walk emits `## Assumptions` explicitly, artifacts always carry the section (omit-when-zero is display-only), fab-new output puts the Assumptions block last (c5tr); SRAD v2 demerit confidence scoring — composite weights `0.20/0.30/0.30/0.20`, grade bands 80/50/20 (indicative only), both hard-fail short-circuits (Unresolved→0.0 and `R<25∧A<25`) removed, blocking emergent from the penalty curve (4yi8); fab-new's backlog-ID collision pre-check is exact-ID anchored (`fab resolve --id` equality) and fab-proceed's promptless `_intake` dispatch carries the defer-and-surface contract with the matching `_srad` Critical-Rule carve-out (w7dp); fab-new/fab-draft `Next:` lines derive at runtime per the _preamble Lookup Procedure and `_generation` names fab-continue in both consumer groups (d9rs); change_type explicit set is sticky — set-change-type marks `change_type_source: explicit` and the hook re-infers only when source is absent/inferred, retiring the re-verify-after-later-writes caveat (jznd) | +| [execution-skills](execution-skills.md) | Apply (unified `plan.md` `## Requirements`+`## Tasks`+`## Acceptance` generated at entry), review, hydrate, archive, and orchestrator behavior — `/fab-continue` for pipeline stages, `/fab-archive` for housekeeping, `/fab-proceed` for context-aware pipeline orchestration; `/git-pr` ship behavior with the mechanically-rendered `## Meta` block via `fab pr-meta`, a unified Step 0 resolution (szxd), and git-state hardening — detached-HEAD STOP, expected-area staging guard, PR-state branching, resolved default-branch guard (g8st); `/git-pr-review` split commit/push failure semantics + unpushed-commit re-run gate (g8st); fab-continue loads `_generation`/`_review` stage-conditionally at point of use (zc9m); the shared ff/fff bracket lives in `_pipeline.md` with once-stated rework choreography (cycle cap = the `{max_cycles}` knob from code-review.md § Rework Budget, default 3 — c5tr), exhaustion terminal state review:failed, and fab-continue's review-failed rework-menu dispatch row (szxd); re-archiving a genuinely archived change soft-skips (exit 0) and restore `--switch` surfaces `pointer: failed` (k4ge); archive/restore index writes are atomic with honest `index: failed` reporting on both paths (hv7t); `/git-pr`/`/git-pr-review` accept an optional explicit `` argument and guard branch↔change correspondence before any mutation, fab-continue gained the review-pr/failed dispatch row + idempotent reset, and recovery guidance is override-aware (w7dp); pipeline hydrate gained a refuse-before-regen guard consulting `fab memory-index --check` exit 2 (destructive loss), defense-in-depth + born-compatible no-op (glwc); the Auto-Rework Loop states the cycle-count invariant + baseline convention (iterations == initial entry + each re-entry, N cycles → N+1; a prose gap, not a Go bug) and `/git-pr-review` Phase 2 carries the synchronous-poll discipline + corrected two-login query semantics (landed-review predicate `author.login == copilot-pull-request-reviewer`; `Copilot` under requested_reviewers; REST requested_reviewers confirms requests since GraphQL omits bot reviewers) (qg64); pipeline hydrate now authors FKF frontmatter (`type: memory` + `description:`), stops writing per-file `## Changelog` (records the what-changed via `fab status set-summary`), and uses bundle-relative memory↔memory links — 8fr5; `/git-pr` Step 3 gained sub-step 3a-bis — a post-commit/pre-push `fab memory-index` regen with a separate `docs: refresh memory indexes` follow-up commit (no `--amend`), gated on `{has_fab}` AND 3a-just-committed and diff-guarded; it is retained for **`log.md` only** (freeze-on-write needs the post-commit projection to capture the change's own entry while its commits are still reachable, pre-squash — the index carries no dates since ugde, so its regen half is a reliable no-op) (o203; rationale narrowed by ugde); `/fab-adopt` adopts a completed off-pipeline change — guards → one-pass intake+plan from the diff → `skip apply`+`reset review` → outward-only review → hydrate → `/git-pr` Meta-retrofit → review-pr; `_review.md` gained a `mode` (full | outward-only) param and `/git-pr` gained sub-step 3d (`## Meta` body-retrofit onto an existing OPEN PR via `fab pr-meta` + `gh pr edit --body-file -`, gated/idempotent) (t54n). Operator coordination moved to runtime/operator.md. | +| [planning-skills](planning-skills.md) | `/fab-new`, `/fab-continue`, `/fab-ff`, `/fab-clarify` — the planning stage (intake only) and the shared `_generation.md` partial (Intake + unified Plan procedures); requirement capture + plan generation live at apply entry (spec stage removed in j6cs); fab-new/fab-draft re-run/resume semantics (ID collisions route to resume, 9u91); change_type is hook-owned — skills verify via .status.yaml and override only if wrong (uliv); SRAD framework lives in the `_srad` helper, declared by the 6 planning skills (zc9m); pre-boundary intake creation (fab-new Steps 0–9) lives in the shared `_intake` helper parameterized by `{questioning-mode}` (interactive | promptless-defer) — fab-new/fab-draft/fab-proceed are thin call-sites, completing the one-shared-helper-per-phase symmetry with `_pipeline`; fab-new keeps the activate/branch tail, fab-draft stops at ready (momentum warning retired), fab-proceed dispatches promptless-defer + chains /fab-switch→/git-branch (3xaj); fab-ff/fab-fff are thin wrappers over the shared `_pipeline` bracket, fab-new Step 11 is a single first-match-wins branch table (szxd; 6 rows since g8st — remote-only `--track` checkout, dirty-tree carried-over note excluding the change's own artifacts, same-change rename); SRAD grades by half-open bands, the plan walk emits `## Assumptions` explicitly, artifacts always carry the section (omit-when-zero is display-only), fab-new output puts the Assumptions block last (c5tr); SRAD v2 demerit confidence scoring — composite weights `0.20/0.30/0.30/0.20`, grade bands 80/50/20 (indicative only), both hard-fail short-circuits (Unresolved→0.0 and `R<25∧A<25`) removed, blocking emergent from the penalty curve (4yi8); fab-new's backlog-ID collision pre-check is exact-ID anchored (`fab resolve --id` equality) and fab-proceed's promptless `_intake` dispatch carries the defer-and-surface contract with the matching `_srad` Critical-Rule carve-out (w7dp); fab-new/fab-draft `Next:` lines derive at runtime per the _preamble Lookup Procedure and `_generation` names fab-continue in both consumer groups (d9rs); change_type explicit set is sticky — set-change-type marks `change_type_source: explicit` and the hook re-infers only when source is absent/inferred, retiring the re-verify-after-later-writes caveat (jznd); `_generation.md` gained Intake-from-Diff + Plan-from-Diff procedures for `/fab-adopt`, which reconstructs both artifacts in ONE main-session pass from a branch diff — the thin plan is plain-language `## Requirements` (hydrate's sole input) + all-`[x]` Tasks/Acceptance stubs with no R#/T#/A# scaffolding (t54n) | | [preflight](preflight.md) | `lib/preflight.sh` script — validation, accessor-based architecture, structured YAML output, skill integration | -| [schemas](schemas.md) | Workflow schema authority — the Go state machine (`internal/status` transitions + `internal/statusfile` stage order/progress; declarative `workflow.yaml` retired in c5tr): 6-stage pipeline, states, transitions, validation rules; `.status.yaml` `plan:` (`## Requirements`-aware), `confidence:` (indicative retired), and lazy `true_impact:` block schemas (incl. the `tests` sub-block + render-time `impl` residual, 7t5a); `fab impact` and `fab pr-meta` helper subcommands (rj31); allowed-states-enforced transition targets, `fab score --check-gate` non-zero gate-fail exit, iterations-preserving reset cascade (k4ge; cycle-count accuracy is a choreography property not a state-machine one, the fix lives in skill prose — qg64); `fab score` normal-mode hard-fail on load/persist/read errors (hv7t); per-stage allowed-states + transition-event tables enumerated, pinned by the exhaustive 216-cell matrix test (tb6f); `change_type_source: inferred|explicit` guard (set-change-type marks explicit, hook re-infers only when not explicit), read-time `acceptance_completed` via `status.LiveAcceptance` (counter demoted to cache), and `internal/resolve` `ErrNotFound`/`ErrAmbiguous` sentinels (jznd); optional `summary` field + `set-summary`/`get-summary` verbs (FKF C-lite log source, 5943); the `log.md` C-lite schema + registry-gated change-id join + extended (benign-only, no-new-category) `--check` loss tiers (bmzo); the `log.seed.md` seed-merge — `parseSeedLog`/`mergeSeedEntries` curated-sidecar input merged beneath git-projected entries, idempotent, seed-only folders still emit a log.md, loss tier stays benign (oovf); freeze-on-write `log.md` generation — existing log is authoritative/write-once, `parseLog` reads it back, append-only on `(file-base, change-id)`, unattributable-freeze, `--rebuild` destructive escape hatch, `--check` superset-PASS / missing-or-hand-edit-FAIL via merge-as-rendered (tayp) | +| [schemas](schemas.md) | Workflow schema authority — the Go state machine (`internal/status` transitions + `internal/statusfile` stage order/progress; declarative `workflow.yaml` retired in c5tr): 6-stage pipeline, states, transitions, validation rules; `.status.yaml` `plan:` (`## Requirements`-aware), `confidence:` (indicative retired), and lazy `true_impact:` block schemas (incl. the `tests` sub-block + render-time `impl` residual, 7t5a); `fab impact` and `fab pr-meta` helper subcommands (rj31); allowed-states-enforced transition targets, `fab score --check-gate` non-zero gate-fail exit, iterations-preserving reset cascade (k4ge; cycle-count accuracy is a choreography property not a state-machine one, the fix lives in skill prose — qg64); `fab score` normal-mode hard-fail on load/persist/read errors (hv7t); per-stage allowed-states + transition-event tables enumerated, pinned by the exhaustive 216-cell matrix test (tb6f); `change_type_source: inferred|explicit` guard (set-change-type marks explicit, hook re-infers only when not explicit), read-time `acceptance_completed` via `status.LiveAcceptance` (counter demoted to cache), and `internal/resolve` `ErrNotFound`/`ErrAmbiguous` sentinels (jznd); optional `summary` field + `set-summary`/`get-summary` verbs (FKF C-lite log source, 5943); the `log.md` C-lite schema + registry-gated change-id join + extended (benign-only, no-new-category) `--check` loss tiers (bmzo); the `log.seed.md` seed-merge — `parseSeedLog`/`mergeSeedEntries` curated-sidecar input merged beneath git-projected entries, idempotent, seed-only folders still emit a log.md, loss tier stays benign (oovf); freeze-on-write `log.md` generation — existing log is authoritative/write-once, `parseLog` reads it back, append-only on `(file-base, change-id)`, unattributable-freeze, `--rebuild` destructive escape hatch, `--check` superset-PASS / missing-or-hand-edit-FAIL via merge-as-rendered (tayp); adoption's `apply=skipped, review=active` is a `skip apply`+`reset review` transition COMPOSITION (no new Go transition — t54n) | diff --git a/docs/memory/pipeline/planning-skills.md b/docs/memory/pipeline/planning-skills.md index a7265605..7ba742e1 100644 --- a/docs/memory/pipeline/planning-skills.md +++ b/docs/memory/pipeline/planning-skills.md @@ -1,6 +1,6 @@ --- type: memory -description: "`/fab-new`, `/fab-continue`, `/fab-ff`, `/fab-clarify` — the planning stage (intake only) and the shared `_generation.md` partial (Intake + unified Plan procedures); requirement capture + plan generation live at apply entry (spec stage removed in j6cs); fab-new/fab-draft re-run/resume semantics (ID collisions route to resume, 9u91); change_type is hook-owned — skills verify via .status.yaml and override only if wrong (uliv); SRAD framework lives in the `_srad` helper, declared by the 6 planning skills (zc9m); pre-boundary intake creation (fab-new Steps 0–9) lives in the shared `_intake` helper parameterized by `{questioning-mode}` (interactive | promptless-defer) — fab-new/fab-draft/fab-proceed are thin call-sites, completing the one-shared-helper-per-phase symmetry with `_pipeline`; fab-new keeps the activate/branch tail, fab-draft stops at ready (momentum warning retired), fab-proceed dispatches promptless-defer + chains /fab-switch→/git-branch (3xaj); fab-ff/fab-fff are thin wrappers over the shared `_pipeline` bracket, fab-new Step 11 is a single first-match-wins branch table (szxd; 6 rows since g8st — remote-only `--track` checkout, dirty-tree carried-over note excluding the change's own artifacts, same-change rename); SRAD grades by half-open bands, the plan walk emits `## Assumptions` explicitly, artifacts always carry the section (omit-when-zero is display-only), fab-new output puts the Assumptions block last (c5tr); SRAD v2 demerit confidence scoring — composite weights `0.20/0.30/0.30/0.20`, grade bands 80/50/20 (indicative only), both hard-fail short-circuits (Unresolved→0.0 and `R<25∧A<25`) removed, blocking emergent from the penalty curve (4yi8); fab-new's backlog-ID collision pre-check is exact-ID anchored (`fab resolve --id` equality) and fab-proceed's promptless `_intake` dispatch carries the defer-and-surface contract with the matching `_srad` Critical-Rule carve-out (w7dp); fab-new/fab-draft `Next:` lines derive at runtime per the _preamble Lookup Procedure and `_generation` names fab-continue in both consumer groups (d9rs); change_type explicit set is sticky — set-change-type marks `change_type_source: explicit` and the hook re-infers only when source is absent/inferred, retiring the re-verify-after-later-writes caveat (jznd)" +description: "`/fab-new`, `/fab-continue`, `/fab-ff`, `/fab-clarify` — the planning stage (intake only) and the shared `_generation.md` partial (Intake + unified Plan procedures); requirement capture + plan generation live at apply entry (spec stage removed in j6cs); fab-new/fab-draft re-run/resume semantics (ID collisions route to resume, 9u91); change_type is hook-owned — skills verify via .status.yaml and override only if wrong (uliv); SRAD framework lives in the `_srad` helper, declared by the 6 planning skills (zc9m); pre-boundary intake creation (fab-new Steps 0–9) lives in the shared `_intake` helper parameterized by `{questioning-mode}` (interactive | promptless-defer) — fab-new/fab-draft/fab-proceed are thin call-sites, completing the one-shared-helper-per-phase symmetry with `_pipeline`; fab-new keeps the activate/branch tail, fab-draft stops at ready (momentum warning retired), fab-proceed dispatches promptless-defer + chains /fab-switch→/git-branch (3xaj); fab-ff/fab-fff are thin wrappers over the shared `_pipeline` bracket, fab-new Step 11 is a single first-match-wins branch table (szxd; 6 rows since g8st — remote-only `--track` checkout, dirty-tree carried-over note excluding the change's own artifacts, same-change rename); SRAD grades by half-open bands, the plan walk emits `## Assumptions` explicitly, artifacts always carry the section (omit-when-zero is display-only), fab-new output puts the Assumptions block last (c5tr); SRAD v2 demerit confidence scoring — composite weights `0.20/0.30/0.30/0.20`, grade bands 80/50/20 (indicative only), both hard-fail short-circuits (Unresolved→0.0 and `R<25∧A<25`) removed, blocking emergent from the penalty curve (4yi8); fab-new's backlog-ID collision pre-check is exact-ID anchored (`fab resolve --id` equality) and fab-proceed's promptless `_intake` dispatch carries the defer-and-surface contract with the matching `_srad` Critical-Rule carve-out (w7dp); fab-new/fab-draft `Next:` lines derive at runtime per the _preamble Lookup Procedure and `_generation` names fab-continue in both consumer groups (d9rs); change_type explicit set is sticky — set-change-type marks `change_type_source: explicit` and the hook re-infers only when source is absent/inferred, retiring the re-verify-after-later-writes caveat (jznd); `_generation.md` gained Intake-from-Diff + Plan-from-Diff procedures for `/fab-adopt`, which reconstructs both artifacts in ONE main-session pass from a branch diff — the thin plan is plain-language `## Requirements` (hydrate's sole input) + all-`[x]` Tasks/Acceptance stubs with no R#/T#/A# scaffolding (t54n)" --- # Planning Skills @@ -16,8 +16,9 @@ The planning skills (`/fab-new`, `/fab-clarify`) handle the single planning stag Artifact generation is defined in a single shared partial: `$(fab kit-path)/skills/_generation.md`. As of j6cs the standalone Spec Generation Procedure is **deleted** — requirement generation is folded into the Plan Generation Procedure invoked by the apply skill (within `/fab-continue`) at apply entry. -The partial contains two procedures — the **Intake Generation Procedure** (consumers: `/fab-new`, `/fab-draft`, and `/fab-continue`'s intake-`active` regeneration row) and the **Plan Generation Procedure** below (consumers: `/fab-continue`, `/fab-ff`, `/fab-fff`, at apply entry). `/fab-continue` belongs to **both** consumer groups — d9rs corrected the partial's header/intro, which had placed it only in the plan group, and dropped the stale "auto-clarify" residue from the partial's orchestration-concerns note (auto-clarify was removed in j6cs): +The partial contains four procedures — the two **forward** procedures (Intake Generation, Plan Generation) and, as of t54n, two **diff-based** procedures (Intake-from-Diff, Plan-from-Diff) for `/fab-adopt`. The **Intake Generation Procedure** (consumers: `/fab-new`, `/fab-draft`, and `/fab-continue`'s intake-`active` regeneration row) and the **Plan Generation Procedure** below (consumers: `/fab-continue`, `/fab-ff`, `/fab-fff`, at apply entry). `/fab-continue` belongs to **both** forward consumer groups — d9rs corrected the partial's header/intro, which had placed it only in the plan group, and dropped the stale "auto-clarify" residue from the partial's orchestration-concerns note (auto-clarify was removed in j6cs): - **Plan Generation Procedure** — template loading (`plan.md`), one walk that emits `## Requirements` (RFC-2119 statements with stable `R#` IDs + GIVEN/WHEN/THEN scenarios, generated FIRST from the intake-derived design), then paired Task + Acceptance entries per requirement with sequential `T-NNN`/`A-NNN` IDs. It reads `intake.md` as input (not a separate `spec.md`), and includes a one-release legacy `spec.md` ingestion path (fold a leftover `spec.md` into `## Requirements` if present and `plan.md` lacks them). Trace annotations are **REQUIRED** (j6cs): each `## Tasks` item carries a `` annotation; each `## Acceptance` item names its `R#`. No `[NEEDS CLARIFICATION]` markers are emitted into `plan.md` — under-specified points become graded SRAD assumptions in `## Assumptions` instead, and as of c5tr the walk carries an explicit numbered **`## Assumptions` emission step** (step 7, immediately before the renumbered write step — previously the walk consumed the section without any step emitting it): persist every inline-graded assumption per `_srad.md` § Assumptions Summary Block — three grades only (Certain/Confident/Tentative; Unresolved is intake-only), the `Scores` column required on every row, footer `{N} assumptions ({Ce} certain, {Co} confident, {T} tentative).` The section is ALWAYS present in the artifact — `0 assumptions.` footer with no table rows when empty (`_srad`'s omit-when-zero rule is scoped to the displayed output summary only, never to artifacts, keeping `fab score` parsing uniform); the Intake Generation step 4 notes the same always-present rule. The procedure runs at apply entry, not as a separate planning stage. Counts (`task_count`, `acceptance_count`, `acceptance_completed`) flow into `.status.yaml` `plan:` block via the PostToolUse `artifactBookkeeping` hook on every `plan.md` write; skills MAY also call `fab status set-acceptance` for explicit updates (e.g., review marking acceptance items complete). +- **Diff-based procedures — Intake-from-Diff + Plan-from-Diff (t54n; sole consumer: `/fab-adopt`, see [execution-skills.md](/pipeline/execution-skills.md) § `/fab-adopt`)**. The forward procedures generate an artifact from *intent* (the description, then the intake-derived design); the diff-based procedures **reverse-engineer** the artifacts from a *fixed existing branch diff* — the artifact *describes* what shipped rather than *plans* what to build. **Intake-from-Diff**: reconstruct `intake.md` from `git diff {base}...HEAD` + the changed-file list + the PR body/title (or branch name) — Origin = `adopted from {PR url or branch}`, Why/What-Changes synthesised from the diff, **Affected Memory** inferred from which `docs/memory/` domains the changed paths map to, Impact from the changed paths; SRAD + `fab score` still apply (the reconstructed design is graded like any intake). **Plan-from-Diff**: write a deliberately **MINIMAL** `plan.md` from the *same* understanding (no re-read of the diff) — plain-language `## Requirements` (a restatement of the intake's What-Changes, grouped by change area; **the only part hydrate reads, so effort concentrates here**), a single all-`[x]` `## Tasks` stub, a single all-`[x]` `## Acceptance` stub, and a header note that apply was skipped and the plan is reverse-engineered to feed hydrate. It deliberately emits **NO** `R#`/`T-NNN`/`A-NNN` traceability IDs, GIVEN/WHEN/THEN scenarios, phases, or `[P]` markers — the apply↔review traceability loop never runs for an adopted change, so that scaffolding is dead weight. The only stable parser contract preserved is the three heading literals `## Requirements` / `## Tasks` / `## Acceptance` (confirmed against `templates/plan.md`). **Both artifacts are generated by the same main-session agent in ONE pass** (NOT via a dispatched apply) — the agent reads the diff + PR body once and writes intake then plan from that single understanding, because both merely describe one fixed existing diff and a context boundary between them would only invite drift and waste; a human-confirmation checkpoint sits between them (the late deliberation the bypass skipped, mirroring `/fab-new`'s interactive intake moment). Command invocations are auto-logged via `preflight.sh --driver ` — skills no longer call `log-command` manually. All event commands (`start`, `advance`, `finish`, `reset`, `fail`) accept an optional `driver` parameter; skills always pass it to identify the invoking skill (e.g., `fab-continue`, `fab-ff`). diff --git a/docs/memory/pipeline/schemas.md b/docs/memory/pipeline/schemas.md index d7ea534e..07671fd2 100644 --- a/docs/memory/pipeline/schemas.md +++ b/docs/memory/pipeline/schemas.md @@ -1,6 +1,6 @@ --- type: memory -description: "Workflow schema authority — the Go state machine (`internal/status` transitions + `internal/statusfile` stage order/progress; declarative `workflow.yaml` retired in c5tr): 6-stage pipeline, states, transitions, validation rules; `.status.yaml` `plan:` (`## Requirements`-aware), `confidence:` (indicative retired), and lazy `true_impact:` block schemas (incl. the `tests` sub-block + render-time `impl` residual, 7t5a); `fab impact` and `fab pr-meta` helper subcommands (rj31); allowed-states-enforced transition targets, `fab score --check-gate` non-zero gate-fail exit, iterations-preserving reset cascade (k4ge; cycle-count accuracy is a choreography property not a state-machine one, the fix lives in skill prose — qg64); `fab score` normal-mode hard-fail on load/persist/read errors (hv7t); per-stage allowed-states + transition-event tables enumerated, pinned by the exhaustive 216-cell matrix test (tb6f); `change_type_source: inferred|explicit` guard (set-change-type marks explicit, hook re-infers only when not explicit), read-time `acceptance_completed` via `status.LiveAcceptance` (counter demoted to cache), and `internal/resolve` `ErrNotFound`/`ErrAmbiguous` sentinels (jznd); optional `summary` field + `set-summary`/`get-summary` verbs (FKF C-lite log source, 5943); the `log.md` C-lite schema + registry-gated change-id join + extended (benign-only, no-new-category) `--check` loss tiers (bmzo); the `log.seed.md` seed-merge — `parseSeedLog`/`mergeSeedEntries` curated-sidecar input merged beneath git-projected entries, idempotent, seed-only folders still emit a log.md, loss tier stays benign (oovf); freeze-on-write `log.md` generation — existing log is authoritative/write-once, `parseLog` reads it back, append-only on `(file-base, change-id)`, unattributable-freeze, `--rebuild` destructive escape hatch, `--check` superset-PASS / missing-or-hand-edit-FAIL via merge-as-rendered (tayp)" +description: "Workflow schema authority — the Go state machine (`internal/status` transitions + `internal/statusfile` stage order/progress; declarative `workflow.yaml` retired in c5tr): 6-stage pipeline, states, transitions, validation rules; `.status.yaml` `plan:` (`## Requirements`-aware), `confidence:` (indicative retired), and lazy `true_impact:` block schemas (incl. the `tests` sub-block + render-time `impl` residual, 7t5a); `fab impact` and `fab pr-meta` helper subcommands (rj31); allowed-states-enforced transition targets, `fab score --check-gate` non-zero gate-fail exit, iterations-preserving reset cascade (k4ge; cycle-count accuracy is a choreography property not a state-machine one, the fix lives in skill prose — qg64); `fab score` normal-mode hard-fail on load/persist/read errors (hv7t); per-stage allowed-states + transition-event tables enumerated, pinned by the exhaustive 216-cell matrix test (tb6f); `change_type_source: inferred|explicit` guard (set-change-type marks explicit, hook re-infers only when not explicit), read-time `acceptance_completed` via `status.LiveAcceptance` (counter demoted to cache), and `internal/resolve` `ErrNotFound`/`ErrAmbiguous` sentinels (jznd); optional `summary` field + `set-summary`/`get-summary` verbs (FKF C-lite log source, 5943); the `log.md` C-lite schema + registry-gated change-id join + extended (benign-only, no-new-category) `--check` loss tiers (bmzo); the `log.seed.md` seed-merge — `parseSeedLog`/`mergeSeedEntries` curated-sidecar input merged beneath git-projected entries, idempotent, seed-only folders still emit a log.md, loss tier stays benign (oovf); freeze-on-write `log.md` generation — existing log is authoritative/write-once, `parseLog` reads it back, append-only on `(file-base, change-id)`, unattributable-freeze, `--rebuild` destructive escape hatch, `--check` superset-PASS / missing-or-hand-edit-FAIL via merge-as-rendered (tayp); adoption's `apply=skipped, review=active` is a `skip apply`+`reset review` transition COMPOSITION (no new Go transition — t54n)" --- # Schemas @@ -55,6 +55,7 @@ The former declarative schema artifact `src/kit/schemas/workflow.yaml` was **ret - **Cascade preserves `iterations` (k4ge)**: when the `reset`/`skip` cascade sets a stage to `pending`/`skipped`, a `stage_metrics` entry with `iterations > 0` is kept with only its `iterations` counter (timing fields `started_at`/`driver`/`completed_at` cleared; the next activation rewrites them); zero-iteration entries are still deleted, so no empty `{}` entries linger. This keeps `stage_metrics.review.iterations` truthful across the rework choreography's `fail review` + `reset apply`, making the cycle count `fab pr-meta` reports real. Preservation is uniform across all stages, not review-only — since tb6f it is regression-tested end-to-end through the public `Fail`/`Reset`/`Finish` functions (3-cycle rework choreography, iterations 1→2→3) and across the `skip` cascade; the test passes against the shipped implementation, confirming the PR #402 "1 cycle" anomaly is not a state-machine bug. **Cycle-count accuracy is an orchestrator-CHOREOGRAPHY property, not a state-machine one (qg64)**: the iterations-preserving cascade here is correct **as-is** and MUST NOT be touched — `iterations` is advanced by exactly one event, a review `→ active` transition (`status.go:627`), and the `reset` cascade deliberately neither increments nor zeroes it. So whether `fab pr-meta` renders the true cycle count depends entirely on the orchestrator skills driving exactly one counted review `→ active` re-entry per rework cycle (via the per-cycle `finish apply` auto-activation). The PR #402 "1 cycle" under-report was therefore fixed in **skill prose** (`_pipeline.md`'s Auto-Rework Loop now states the counting invariant + the baseline convention — iterations == initial entry + each re-entry, so initial fail + N cycles → N+1), **not** in `internal/status`. See [execution-skills.md](/pipeline/execution-skills.md) § Shared Pipeline Bracket (Per-cycle rework choreography → Cycle-count invariant) for the choreography half. See [change-lifecycle.md](/pipeline/change-lifecycle.md) for full `stage_metrics` semantics - **Exhaustively tested (tb6f)**: `internal/status/transitions_test.go` walks all 216 cells (6 stages × 6 events × 6 from-states) of the two tables above and asserts every cell's outcome — target state, forbidden-target rejection, or no-valid-transition rejection — against hand-written copies of the tables, deliberately NOT the implementation's own vars, so a table regression cannot silently rewrite the test's expectations. The enumerations in this doc and that test pin the same spec - **History shape is caller-identity-blind (fgxx)**: the `.status.yaml` / `.history.jsonl` transition history left by a conforming rework run depends only on the **call sequence**, never on caller identity. `driver` flows solely into `applyMetricsSideEffect` (it annotates `stage_metrics.driver` and the transition-log entry) — no state transition reads it — so the manual/foreground path (`driver=""`) and the dispatched-orchestrator path (`driver="fab-fff"`) issue the identical `Finish(intake) → Finish(apply) → [Fail(review) → Reset(apply) → Finish(apply)]×N` sequence and leave history of **identical shape** — agreeing on every caller-blind field (stage/action/from/reason), equal modulo the per-run `ts` timestamp and the optional `driver` field. `internal/status/mutators_test.go`'s `TestHistoryShape_IdenticalRegardlessOfDriver` pins this: it drives the rework choreography twice (`driver=""` and `driver="fab-fff"`) and asserts the `.history.jsonl` stage-transition entries match in count/stage/action/from/reason, differing only where `driver` is recorded. This is the structural invariant that made collapsing the post-intake dual execution mode safe (fgxx — both modes already issued the same CLI calls); the test guards against a future skills-layer regression that diverges the two sequences + - **Adoption's `apply=skipped, review=active` is a transition COMPOSITION, not a new transition (t54n)**: `/fab-adopt` (see [execution-skills.md](/pipeline/execution-skills.md) § `/fab-adopt`) needs the honest end-state `apply: skipped`, `review: active`, `hydrate/ship/review-pr: pending` — apply cannot re-run for already-written code, but every later stage genuinely runs (late). **No new Go transition was added.** The state is composed from the two existing rows above, run in sequence: `fab status skip {name} apply` (`skip`: `{pending,active}→skipped` with forward cascade, so review/hydrate/ship/review-pr all cascade to `skipped`) then `fab status reset {name} review {driver}` (`reset`: accepts `skipped→active` per the row above, re-activating review and cascading *its* downstream — hydrate/ship/review-pr — back to `pending`). The net is exactly the target state, reached entirely through `lookupTransition`-validated cells (both `skip apply` and `reset review` are in-table). The `skip`-then-`reset` ordering matters: `skip apply` must run first so the forward cascade marks review `skipped`, giving `reset review` a valid `skipped` source. `iterations` is untouched (this is a fresh change with no prior review entry; the cascade-preserves-`iterations` rule above applies if a re-run ever re-enters). This reuse-over-invention is the schema half of t54n's "adopt is the real pipeline entered late, with apply skipped" framing 4. **Progression** — How to navigate the workflow - Current stage detection: first `active` or `ready` stage, or first `pending` after last `done`/`skipped`, or `review-pr` if all done/skipped (`CurrentStage`'s all-done fallback — this doc previously mis-stated it as `hydrate`; corrected in k4ge) diff --git a/docs/specs/glossary.md b/docs/specs/glossary.md index b94c5574..b709e324 100644 --- a/docs/specs/glossary.md +++ b/docs/specs/glossary.md @@ -49,6 +49,7 @@ | `/fab-ff` | Fast-forward — confidence-gated pipeline from intake through hydrate. Single intake gate, no clarify steps inside the bracket (`/fab-clarify` is the user-invoked escape valve before the gate); sub-agent review with autonomous rework and bounded retry. Resumable. | | `/fab-fff` | Fast-forward-further — confidence-gated pipeline from intake through review-pr (extends ff through ship and PR review). Same gates and behavior as ff, wider scope. Resumable. | | `/fab-clarify` | Deepens and refines the **intake** artifact (intake-only, 1.10.0) without advancing. Resolves `[NEEDS CLARIFICATION]` markers and `` comments, then recomputes the intake score. Idempotent and non-advancing. | +| `/fab-adopt` | Brings a completed off-pipeline change (scenario B — a branch authored without fab, with an OPEN or not-yet-created PR) into the pipeline. The real pipeline entered late with **apply** marked `skipped`; intake is reconstructed from the diff, review runs outward-only, hydrate/ship/review-pr run for real. A MERGED PR (scenario A) is out of scope. See **Adopt**. | | `/fab-continue` (apply) | Co-generates `plan.md` (`## Requirements` from `intake.md` + `## Tasks` + `## Acceptance`) at the entry sub-step and executes the unchecked tasks under `## Tasks` (main sub-step). Resumable — re-invoking skips plan generation when `plan.md` already exists and picks up from the first unchecked task. | | `/fab-continue` (review) | Validates implementation against `plan.md` `## Acceptance` items, the plan's `## Requirements`, and constitution. On failure, offers rework options (fix code, revise plan, revise requirements). | | `/fab-continue` (hydrate) | Completes the pipeline — hydrates change artifacts into memory files. | @@ -110,6 +111,7 @@ | Term | Definition | |------|-----------| +| **Adopt** | Bringing a completed change that bypassed the pipeline back in (`/fab-adopt`). Scoped to **scenario B** — mid-flight adoption of a branch whose code was authored without fab and whose PR is **OPEN or not yet created**. The real pipeline entered late: only **apply** is `skipped` (the code already exists), while intake (reconstructed from the diff), review (outward-only), hydrate (writes `docs/memory/` — the permanent-loss recovery), ship (retrofits the PR `## Meta`), and review-pr all run for real. **Scenario A** — retroactive backfill of an already-MERGED PR — is explicitly out of scope. | | **[AUTO-MODE]** | Prefix signaling autonomous mode when one skill invokes another internally — no user interaction, machine-readable result. Protocol defined in `fab-clarify.md` § Skill Invocation Protocol (moved from `_preamble.md` in 260611-zc9m); no skill currently invokes another with it. | | **Context loading** | The convention that every skill loads relevant project files (config, constitution, memory index, change artifacts) before generating output. Defined in `_preamble.md`. | | **Fast-forward** | Confidence-gated pipeline from intake through hydrate (`/fab-ff`). Proceeds autonomously once the single intake gate passes — no clarify steps inside the bracket; ambiguity is resolved inline as graded SRAD assumptions, with a sub-agent review + auto-rework loop. | diff --git a/docs/specs/overview.md b/docs/specs/overview.md index 2dbe424d..97bdc0be 100644 --- a/docs/specs/overview.md +++ b/docs/specs/overview.md @@ -94,6 +94,7 @@ For detailed visual maps of how commands connect — including shortcuts, rework | `/fab-continue` → review | Validate (sub-agent) | Prioritized findings report | | `/fab-continue` → hydrate | Complete & hydrate | Updated memory | | `/fab-proceed` | Context-aware orchestrator — runs needed prefix steps (fab-new, fab-switch, git-branch), then delegates to `/fab-fff` | Full pipeline from conversation context | +| `/fab-adopt` | Adopt a completed off-pipeline change (OPEN/not-yet-created PR) — reconstruct intake + plan from the diff, run review (outward-only) → hydrate → ship → review-pr with apply `skipped` | Reconstructed intake + thin plan; pipeline entered late | | `/git-pr` | Ship — commit, push, create draft PR | Draft PR, `ship` done | | `/git-pr-review` | Process PR review comments (fix/defer/skip + replies) | Fix commits + replies, `review-pr` done | | `/fab-archive` | Archive completed change | Folder moved to archive/ | diff --git a/docs/specs/skills.md b/docs/specs/skills.md index 2124578f..2b5cac09 100644 --- a/docs/specs/skills.md +++ b/docs/specs/skills.md @@ -47,6 +47,7 @@ helpers: [_generation, _review, _srad, _pipeline] |-------|------------| | `fab-new`, `fab-draft` | `[_generation, _srad, _intake]` | | `fab-ff`, `fab-fff` | `[_generation, _review, _srad, _pipeline]` (the shared bracket lives in `_pipeline.md`) | +| `fab-adopt` | `[_srad, _generation, _review, _pipeline]` (orchestrator — reuses the diff-generation procedures, outward-only review, and the auto-rework budget) | | `fab-continue` | `[_srad]` (+ `_generation`/`_review` stage-conditionally, in-body) | | `fab-clarify` | `[_srad]` | | `fab-operator` | `[_cli-fab, _cli-external]` | @@ -430,6 +431,31 @@ When called without arguments, `/fab-setup` runs the full bootstrap: invokes `fa --- +## `/fab-adopt` + +**Purpose**: Bring a **completed-but-off-pipeline** change into the Fab pipeline (scenario B — a feature branch authored without fab, with an **OPEN** or **not-yet-created** PR). It is the *real* pipeline entered late, with **apply** marked `skipped` (the only stage that cannot meaningfully re-run when the code already exists); intake/review/hydrate/ship/review-pr all genuinely run. A **MERGED** PR (scenario A — retroactive backfill) is out of scope and STOPs at Step 0. A thin orchestrator on the `/fab-proceed`/`/fab-ff` pattern — `helpers: [_srad, _generation, _review, _pipeline]`. + +**Prerequisite**: An active branch (not detached HEAD, not the default branch) with a non-empty diff against the default-branch merge-base, and no fab change already mapping to that branch. + +**Context**: config, constitution; the branch diff (`git diff {base}...HEAD`) and PR body — read once in the one main-session generation pass. + +**Behavior**: +1. **Step 0 — Guards & diff base**: reuse `/git-pr`'s guard idioms — STOP on detached HEAD / default branch / MERGED PR (scenario A) / branch-already-maps-to-a-change (point at `/fab-continue`) / empty diff. `OPEN` and `none` PR states proceed. Resolve `base=$(git merge-base HEAD origin/{default})` and capture the diff. +2. **Steps 1+2 — one main-session generation pass** (same agent, not dispatched): `fab change new --slug {slug}` + activate (branch exists — `/fab-new` Step 11 row 1/2); reconstruct `intake.md` via the **Intake-from-Diff Procedure** (`_generation.md`); **human-confirmation checkpoint** (confirm/correct the reconstructed intent — the late deliberation the bypass skipped) → `fab status advance/finish {name} intake`; write a deliberately MINIMAL `plan.md` via the **Plan-from-Diff Procedure**. +3. **Step 2 (state)**: `fab status skip {name} apply` (cascades downstream → skipped) then `fab status reset {name} review fab-adopt` (skipped → active, downstream → pending) — yields `apply=skipped, review=active`, **no Go change**; record the fact via `fab status set-summary`. +4. **Step 3 — Review** (dispatched, `mode: outward-only` — the new `_review.md` parameter): the orchestrator owns the verdict (pass incl. zero-findings best-effort → `finish review`; fail → auto-rework per `_pipeline.md` budget when autonomous, hand findings back when interactive). +5. **Step 4 — Hydrate** (dispatched, verbatim per `_pipeline.md` Step 3): the permanent-loss recovery — `docs/memory/` finally reflects what shipped → `finish hydrate`. +6. **Step 5 — Ship**: `/git-pr {name}` retrofits `## Meta` onto the OPEN PR (its Step 3d, gated on body-lacks-`## Meta`) or creates the PR fresh when `none`; `finish ship` auto-activates review-pr. +7. **Step 6**: land in review-pr; print the honest-state summary and `Next: /git-pr-review`. + +**Key properties**: +- Only **apply** is `skipped`; every other stage runs for real (just late) +- Outward-only review via the general `mode` parameter on `_review.md` — not an adopt-specific branch +- State composed from existing `skip`/`reset` transitions; PR Meta retrofit reuses `fab pr-meta` + `gh pr edit` — **no Go change** +- Idempotent guards: re-run after the change is created routes to `/fab-continue` via the collision guard; the Meta retrofit is body-gated + +--- + ## `/fab-clarify` **Purpose**: Deepen and refine the **intake** artifact (`intake.md`) without advancing. Clarification is intake-only (1.10.0) — it is where human judgment lives, gated by the single intake confidence gate. There is no post-intake clarify; inside apply the agent resolves ambiguity inline as graded SRAD assumptions in `plan.md`. diff --git a/docs/specs/skills/SPEC-_generation.md b/docs/specs/skills/SPEC-_generation.md index a77ef11b..f1a8ac46 100644 --- a/docs/specs/skills/SPEC-_generation.md +++ b/docs/specs/skills/SPEC-_generation.md @@ -2,7 +2,7 @@ ## Summary -Shared artifact generation procedures used by five skills across two consumer groups: `/fab-new`, `/fab-draft`, and `/fab-continue` (its intake-`active` regeneration row) follow the **Intake Generation Procedure**; `/fab-continue`, `/fab-ff`, and `/fab-fff` follow the **Plan Generation Procedure** (invoked at apply entry, before any task executes) — `/fab-continue` belongs to both groups. Each skill references these procedures instead of inlining them, so generation behavior is authoritative in one location. Orchestration (stage guards, question handling, design decisions, resumability) stays in each consuming skill's own file. +Shared artifact generation procedures. The two **forward** procedures are used by five skills across two consumer groups: `/fab-new`, `/fab-draft`, and `/fab-continue` (its intake-`active` regeneration row) follow the **Intake Generation Procedure**; `/fab-continue`, `/fab-ff`, and `/fab-fff` follow the **Plan Generation Procedure** (invoked at apply entry, before any task executes) — `/fab-continue` belongs to both groups. Two **-from-Diff** procedures are the adoption variants used by `/fab-adopt` only: **Intake-from-Diff** reconstructs `intake.md` from a fixed existing branch diff + PR body (Origin = `adopted from {PR/branch}`, Affected Memory inferred from touched `docs/memory/` domains), and **Plan-from-Diff** writes a deliberately thin `plan.md` — plain-language `## Requirements` (the only part hydrate reads), all-`[x]` `## Tasks`/`## Acceptance` stubs, and **no** R#/T#/A# scaffolding or GIVEN/WHEN/THEN (the apply↔review traceability loop never runs for an adopted change). `/fab-adopt` runs both diff procedures in **one main-session pass** (same agent, reads the diff once). Each skill references these procedures instead of inlining them, so generation behavior is authoritative in one location. Orchestration (stage guards, the human-confirmation checkpoint, question handling, design decisions, resumability) stays in each consuming skill's own file. This is an internal partial (`user-invocable: false`) — never invoked directly. Skills load it via `helpers: [_generation]` frontmatter. @@ -64,6 +64,30 @@ Consumer skill reads _generation.md (via helpers: declaration) └─ Write: fab/changes/{name}/plan.md (PostToolUse hook updates .status.yaml plan.* counters — no manual fab status set-acceptance needed at generation time) + +── Adoption variants (fab-adopt only — one main-session pass, diff read once) ── + +├─ Intake-from-Diff Procedure +│ Inputs: git diff {base}...HEAD, --name-only, PR body/title (or branch) +│ ├─ Origin = "adopted from {PR url or branch}" +│ ├─ Why / What Changes synthesised from the diff + PR body (descriptive) +│ ├─ Affected Memory inferred from which docs/memory/ domains the paths touch +│ ├─ Impact from changed paths; apply SRAD + fab score +│ └─ Write: fab/changes/{name}/intake.md +│ (human-confirmation checkpoint is fab-adopt orchestration, not this proc) +│ +└─ Plan-from-Diff Procedure (deliberately MINIMAL — sole consumer is hydrate) + ├─ Header note: "Adopted change — code authored off-pipeline. Apply was + │ skipped; this plan is reverse-engineered from the branch diff to feed hydrate." + ├─ ## Requirements — plain-language restatement of intake's What-Changes, + │ grouped by area (NO R# IDs, no RFC-2119 ceremony, no GIVEN/WHEN/THEN) + ├─ ## Tasks — single all-[x] stub (no T# / phases / [P] / ) + ├─ ## Acceptance — single all-[x] stub (no A# / R#) + ├─ ## Assumptions — present ("0 assumptions." when none; diff-reading + │ assumptions live on the intake, not duplicated here) + └─ Write: fab/changes/{name}/plan.md + (parser contract = only the three heading literals — confirmed against + templates/plan.md; omitting R#/T#/A# is correct, the loop never runs) ``` ### Tools used diff --git a/docs/specs/skills/SPEC-_pipeline.md b/docs/specs/skills/SPEC-_pipeline.md index 43141892..5b275401 100644 --- a/docs/specs/skills/SPEC-_pipeline.md +++ b/docs/specs/skills/SPEC-_pipeline.md @@ -2,7 +2,7 @@ ## Summary -Shared pipeline bracket executed by `/fab-ff` and `/fab-fff` (added in 260611-szxd — f007/f071). The bracket is a **pure sequencer** (affirmed 260613-fgxx): dispatch block → read returned status/findings → decide proceed / loop / stop; it owns the `fab status` transitions and `fab resolve-agent ` resolution and never reaches into block internals. The subagent dispatch note's "do NOT run `fab status`; return results only" instruction is the **universal block contract** for post-intake `/fab-continue` blocks (Apply/Review/Hydrate) — not an override this orchestrator imposes — because plain `/fab-continue` is itself a one-stage sequencer that dispatches those blocks identically and runs their transitions after they return (see `SPEC-fab-continue.md`). Single authoritative source for everything the two drivers share: pre-flight (intake prerequisite + the single intake confidence gate, flat 3.0), context loading, the subagent dispatch note ("do NOT run `fab status`; return results only"), resumability (skip `done` stages; review-`failed` recovery via `fab status start review`), Steps 1–3 (apply with internal plan generation → review → hydrate — each preceded by `fab resolve-agent --alias` per-stage model resolution since 260613-l3ja (`--alias` since 260613-yky7 — emits the Agent-tool-valid short alias on the `model=` line), and since 260613-m3d4 each site **surfaces** the resolved `model=/effort=` (visibility — a skipped resolution is then detectable, not silent) and dispatches via two seams: model on the Agent `model` param, effort as an imperative instruction in the dispatch prompt (no Agent effort param; omitted when empty); review resolves once and applies the same model + same effort-prompt instruction to both reviewers + merge; rework items 3/4 re-resolve, re-surface, and re-inject before re-dispatch), the auto-rework loop with its **explicit per-cycle choreography**, the exhaustion stop, and the shared error rows. All user-facing stop/re-run guidance in the bracket (gate-fail, task-fail, exhaustion stop) names the change in the suggested commands — the run may be driving a non-active override, and argless re-runs would resolve the active change (260612-w7dp). +Shared pipeline bracket executed by `/fab-ff` and `/fab-fff` (added in 260611-szxd — f007/f071). `/fab-adopt` (260630-t54n) is a **partial consumer** — it declares `_pipeline` as a helper but runs its own Steps 0–6, reusing only the auto-rework loop (for its outward-only review verdict) and the hydrate dispatch, not the full apply → review → hydrate bracket; it is not a `{driver}` of the full sequence. The bracket is a **pure sequencer** (affirmed 260613-fgxx): dispatch block → read returned status/findings → decide proceed / loop / stop; it owns the `fab status` transitions and `fab resolve-agent ` resolution and never reaches into block internals. The subagent dispatch note's "do NOT run `fab status`; return results only" instruction is the **universal block contract** for post-intake `/fab-continue` blocks (Apply/Review/Hydrate) — not an override this orchestrator imposes — because plain `/fab-continue` is itself a one-stage sequencer that dispatches those blocks identically and runs their transitions after they return (see `SPEC-fab-continue.md`). Single authoritative source for everything the two drivers share: pre-flight (intake prerequisite + the single intake confidence gate, flat 3.0), context loading, the subagent dispatch note ("do NOT run `fab status`; return results only"), resumability (skip `done` stages; review-`failed` recovery via `fab status start review`), Steps 1–3 (apply with internal plan generation → review → hydrate — each preceded by `fab resolve-agent --alias` per-stage model resolution since 260613-l3ja (`--alias` since 260613-yky7 — emits the Agent-tool-valid short alias on the `model=` line), and since 260613-m3d4 each site **surfaces** the resolved `model=/effort=` (visibility — a skipped resolution is then detectable, not silent) and dispatches via two seams: model on the Agent `model` param, effort as an imperative instruction in the dispatch prompt (no Agent effort param; omitted when empty); review resolves once and applies the same model + same effort-prompt instruction to both reviewers + merge; rework items 3/4 re-resolve, re-surface, and re-inject before re-dispatch), the auto-rework loop with its **explicit per-cycle choreography**, the exhaustion stop, and the shared error rows. All user-facing stop/re-run guidance in the bracket (gate-fail, task-fail, exhaustion stop) names the change in the suggested commands — the run may be driving a non-active override, and argless re-runs would resolve the active change (260612-w7dp). **Parameters** (declared by each driver's own file): diff --git a/docs/specs/skills/SPEC-_preamble.md b/docs/specs/skills/SPEC-_preamble.md index c78feeaf..1b61b6ee 100644 --- a/docs/specs/skills/SPEC-_preamble.md +++ b/docs/specs/skills/SPEC-_preamble.md @@ -85,6 +85,11 @@ Skill reads _preamble.md │ (state table lookup → "Next:" line — skills whose │ Output/Key Properties declare a different ending │ are exempt; the skill file wins) +│ (adoption note 260630-t54n: /fab-adopt needs no new +│ row — it enters with apply skipped + review active and +│ drives review→hydrate→ship→review-pr, states the table +│ already covers; a skipped stage is passed over by the +│ lookup exactly like a done stage) │ ├─ Skill Invocation Protocol (pointer) │ (protocol defined in fab-clarify.md) diff --git a/docs/specs/skills/SPEC-_review.md b/docs/specs/skills/SPEC-_review.md index a9502d2f..286f6b5d 100644 --- a/docs/specs/skills/SPEC-_review.md +++ b/docs/specs/skills/SPEC-_review.md @@ -7,9 +7,11 @@ ## Summary -Shared review dispatch logic invoked by `/fab-continue`, `/fab-ff`, and `/fab-fff` at the review stage. Defines the inward sub-agent (validates implementation against `plan.md`'s `## Requirements`, `## Tasks`, and `## Acceptance` with eight validation checks, including the parsimony pass and deletion-candidate prompt) and the outward sub-agent (Codex→Claude cascade with full repo access for holistic diff review). Both sub-agents are dispatched in parallel; their findings are merged into a single prioritized set with three severity tiers. +Shared review dispatch logic invoked by `/fab-continue`, `/fab-ff`, `/fab-fff`, and `/fab-adopt` at the review stage. Defines the inward sub-agent (validates implementation against `plan.md`'s `## Requirements`, `## Tasks`, and `## Acceptance` with eight validation checks, including the parsimony pass and deletion-candidate prompt) and the outward sub-agent (Codex→Claude cascade with full repo access for holistic diff review). The dispatched sub-agents are run in parallel; their findings are merged into a single prioritized set with three severity tiers. -This is an internal partial (`user-invocable: false`) — it is never invoked directly. Skills reference it via `helpers:` frontmatter (`/fab-ff`, `/fab-fff`) or a stage-conditional in-body read at review entry (`/fab-continue` — deliberately absent from its frontmatter list, per `_preamble.md` § Skill Helper Declaration). +**Review Mode** (general `mode` parameter, added for `/fab-adopt`): the orchestrator MAY pass `mode` to select which sub-agents run — `full` (default — inward + outward) or `outward-only` (outward sub-agent only, for adopted changes that have no forward requirements for inward to validate). There is deliberately **no `inward-only`** value (no caller needs it — parsimony). `mode` gates exactly two steps: **Preconditions** (the inward `plan.md` checks — `## Tasks`/`## Acceptance` present, all tasks `[x]`) are checked **only** in `full` mode and skipped in `outward-only`; **Parallel Dispatch** dispatches only the sub-agent(s) the mode selects. **Findings Merge** and the deterministic pass/fail rule are identical for both modes — "any must-fix → fail", and zero findings passes (so an empty `outward-only` result, e.g. all `review_tools` disabled/unavailable, **passes** best-effort). Default is `full` when the param is omitted, so every existing caller is unaffected — the mode concept is purely additive. + +This is an internal partial (`user-invocable: false`) — it is never invoked directly. Skills reference it via `helpers:` frontmatter (`/fab-ff`, `/fab-fff`, `/fab-adopt`) or a stage-conditional in-body read at review entry (`/fab-continue` — deliberately absent from its frontmatter list, per `_preamble.md` § Skill Helper Declaration). The rework loop is NOT defined here — the file's trailing note points at the orchestrators: `fab-continue.md`'s Verdict section for manual rework, and `_pipeline.md` § Auto-Rework Loop for `/fab-ff`/`/fab-fff` (pointer corrected in 260611-szxd; it previously cited "fab-ff.md/fab-fff.md Step 3"). @@ -18,14 +20,17 @@ The rework loop is NOT defined here — the file's trailing note points at the o ## Flow ``` -Orchestrator (fab-continue / fab-ff / fab-fff) reads _review.md +Orchestrator (fab-continue / fab-ff / fab-fff / fab-adopt) reads _review.md +│ +├─ Review Mode: full (default — inward + outward) | outward-only (outward only) +│ (no inward-only value; default full → existing callers unaffected) │ -├─ Preconditions +├─ Preconditions [checked only in mode=full; skipped in outward-only] │ Read: plan.md (## Tasks all [x], ## Acceptance present) │ -├─ Parallel Dispatch (Agent tool) +├─ Parallel Dispatch (Agent tool — dispatches only the sub-agent(s) the mode selects) │ │ -│ ├─ Inward Sub-Agent +│ ├─ Inward Sub-Agent [full only] │ │ Context: plan.md (## Requirements + ## Tasks + ## Acceptance), │ │ touched source files, target memory files, │ │ change_type (260612-w7dp — read from .status.yaml by @@ -54,7 +59,7 @@ Orchestrator (fab-continue / fab-ff / fab-fff) reads _review.md │ │ Output: structured findings (must-fix / │ │ should-fix / nice-to-have) │ │ -│ └─ Outward Sub-Agent +│ └─ Outward Sub-Agent [both modes — the sole sub-agent in outward-only] │ Context: full diff (git diff ...HEAD), │ changed file paths, full repo access │ Cascade: Codex → Claude (controlled by @@ -67,13 +72,14 @@ Orchestrator (fab-continue / fab-ff / fab-fff) reads _review.md │ Output: structured findings (must-fix / │ should-fix / nice-to-have) │ -└─ Findings Merge - 1. Collect all findings - 2. Deduplicate by file:line (keep higher severity) +└─ Findings Merge [identical for both modes] + 1. Collect all findings (both sub-agents in full; outward only in outward-only) + 2. Deduplicate by file:line (keep higher severity) — no-op in outward-only (single source) 3. Merge by severity into unified set 4. Pass/fail (deterministic): any must-fix → review fails; no must-fix findings (including zero findings) → review passes - (should-fix / nice-to-have are reported but never block) + (should-fix / nice-to-have are reported but never block; + an empty outward-only result — no available external reviewer — passes best-effort) ``` ### Validation Steps Inventory (Inward Sub-Agent) diff --git a/docs/specs/skills/SPEC-fab-adopt.md b/docs/specs/skills/SPEC-fab-adopt.md new file mode 100644 index 00000000..bcdec753 --- /dev/null +++ b/docs/specs/skills/SPEC-fab-adopt.md @@ -0,0 +1,107 @@ +# fab-adopt + +## Contents + +- Summary +- Flow + +## Summary + +Adopts a **completed-but-off-pipeline** change (scenario B: a feature branch authored without fab, with an **OPEN** or **not-yet-created** PR) into the Fab pipeline. A **MERGED** PR (scenario A — retroactive backfill) is out of scope and STOPs at Step 0. The framing is honest: of the six stages, only **apply** cannot meaningfully re-run on an adopted change (the code already exists), so `/fab-adopt` enters the *real* pipeline late with `apply` marked **skipped** — intake/review/hydrate/ship/review-pr all genuinely run. + +A thin orchestrator (the `/fab-proceed` / `/fab-ff` pattern): declares `helpers: [_srad, _generation, _review, _pipeline]` and reuses existing skills/procedures as sub-agents. It introduces only what is genuinely new — the **Intake-from-Diff** and **Plan-from-Diff** procedures in `_generation.md`, and `_review.md`'s **`outward-only`** mode. + +**Key design decisions**: + +- **Real pipeline entered late, not a fake parallel pipeline** — only apply is `skipped`; every other stage runs (just late, after the code was written). Honest state, not "mark all done" (Constitution II — memory must reflect what shipped). +- **Outward-only review** — the inward sub-agent validates conformance against `plan.md` requirements, but an adopted change has no forward requirements to conform to, so inward is skipped via the general `mode: outward-only` parameter on `_review.md` (not an adopt-specific branch). An empty outward result (no available external reviewer) passes best-effort. +- **State via existing transitions** — `fab status skip {name} apply` (cascades downstream → skipped) then `fab status reset {name} review fab-adopt` (skipped → active, cascades downstream → pending) yields `apply=skipped, review=active`. **No Go transition added.** +- **Intake + thin plan in one main-session pass** — both artifacts describe one fixed existing diff, so the same agent reads the diff once and writes both (no dispatched-apply split that would invite drift). A human-confirmation checkpoint between them is the late deliberation the bypass skipped. +- **PR Meta retrofit** — Step 5's `/git-pr` injects `## Meta` onto the existing OPEN PR via its Step 3d body-retrofit path (gated on body-lacks-`## Meta`; reuses `fab pr-meta` + `gh pr edit --body-file -`). **No Go change.** + +## Flow + +``` +User invokes /fab-adopt [] +│ +├─ Step 0: Guards & diff base (reuse /git-pr guard idioms — STOP before any mutation) +│ ├─ Bash: git branch --show-current → detached HEAD or default branch → STOP +│ ├─ Bash: gh pr view --json number,state,url +│ │ → MERGED → STOP (scenario A, out of scope) +│ │ → OPEN / none → proceed (capture {pr_state}, {pr_url}, {pr_number}) +│ ├─ Collision guard: fab change resolve "$(git branch --show-current)" +│ │ succeeds → STOP (already in the pipeline → /fab-continue) +│ └─ Bash: base=$(git merge-base HEAD origin/{default}); +│ git diff {base}...HEAD + --name-only +│ empty diff → STOP (nothing to adopt) +│ +├─ Steps 1+2: ONE main-session generation pass (same agent, NOT dispatched — +│ │ reads the diff + PR body once) +│ ├─ Bash: fab change new --slug {slug}; activate (branch already exists — +│ │ fab-new Step 11 row 1/2 "already active"/"checked out") +│ ├─ Reconstruct intake.md via Intake-from-Diff Procedure (_generation.md): +│ │ Origin = adopted from {PR or branch}; Why/What-Changes from diff + PR body; +│ │ Affected Memory inferred from touched docs/memory/ domains; +│ │ Impact from changed paths; apply SRAD + fab score +│ ├─ Human-confirmation checkpoint (confirm/correct reconstructed intent + SRAD) +│ │ → on confirm: fab status advance {name} intake; finish {name} intake +│ │ (auto-activates apply) +│ └─ Write MINIMAL plan.md via Plan-from-Diff Procedure (_generation.md): +│ plain-language ## Requirements (the only part hydrate reads), +│ all-[x] ## Tasks + ## Acceptance stubs, NO R#/T#/A# scaffolding, +│ "Adopted change…" header note +│ +├─ Step 2 (state): apply → skipped, review → active (existing transitions, no Go change) +│ ├─ Bash: fab status skip {name} apply (cascades downstream → skipped) +│ ├─ Bash: fab status reset {name} review fab-adopt (skipped → active, downstream → pending) +│ └─ Bash: fab status set-summary {name} "adopted off-pipeline change; apply skipped" +│ +├─ Step 3: Review — dispatched, mode: outward-only (Agent tool, general-purpose) +│ ├─ Bash: fab resolve-agent review --alias (surface model=/effort=) +│ ├─ Dispatch /fab-continue Review Behavior, mode: outward-only +│ │ (prompt: do NOT run fab status; return results only; +│ │ inward preconditions skipped in outward-only; outward reads +│ │ git diff {base}...HEAD natively) +│ └─ Verdict (owned here): +│ pass (incl. zero findings, best-effort) → fab status finish {name} review fab-adopt +│ fail → auto-rework per _pipeline.md budget (autonomous) / +│ hand findings back (interactive default for hand-authored code) +│ +├─ Step 4: Hydrate — dispatched, verbatim (_pipeline.md Step 3) +│ └─ Dispatch /fab-continue Hydrate Behavior → on success fab status finish {name} hydrate fab-adopt +│ (the permanent-loss recovery — docs/memory/ finally reflects what shipped) +│ +├─ Step 5: Ship — dispatch /git-pr {name} (folder name, not bare id) +│ └─ OPEN PR → existing-PR path + Step 3d Meta retrofit (body-lacks-## Meta); +│ none → /git-pr creates the PR fresh; /git-pr runs finish ship itself +│ (auto-activates review-pr) +│ +└─ Step 6: Land in review-pr → print honest-state summary + Next: /git-pr-review +``` + +### Tools used + +| Tool | Purpose | +|------|---------| +| Bash | `git` (branch/merge-base/diff), `gh pr view`, `fab change new`, `fab status skip/reset/advance/finish/set-summary`, `fab score`, `fab resolve-agent` | +| Read | The diff + PR body (Step 0/1), templates via `$(fab kit-path)` (through the `_generation.md` procedures) | +| Write | `intake.md` + `plan.md` in the reconstructed change folder (the one main-session pass) | +| Agent | Review (outward-only) + Hydrate sub-agents; Step 5 `/git-pr` (folder-name target) | + +### Sub-agents + +| Agent | When | Purpose | +|-------|------|---------| +| `/fab-continue` Review Behavior (`mode: outward-only`) | Step 3 | Outward-only diff review; verdict transition owned by `/fab-adopt` | +| `/fab-continue` Hydrate Behavior | Step 4 | Write `docs/memory/` from the thin plan's `## Requirements` — the permanent-loss recovery | +| `/git-pr {name}` | Step 5 | Commit/push the reconstructed `fab/` artifacts, retrofit the PR `## Meta` block (Step 3d), finish ship | + +### Bookkeeping commands (hook candidates) + +| Step | Command | Trigger | +|------|---------|---------| +| Intake finish | `fab status advance/finish {name} intake` | `/fab-adopt` (after the human-confirmation checkpoint) | +| State composition | `fab status skip {name} apply`; `fab status reset {name} review fab-adopt`; `fab status set-summary {name} …` | `/fab-adopt` Step 2 | +| Review verdict | `fab status finish/fail {name} review fab-adopt` | `/fab-adopt` (orchestrator owns it; sub-agent does not) | +| Hydrate finish | `fab status finish {name} hydrate fab-adopt` | `/fab-adopt` Step 4 | +| Ship finish | `fab status finish {name} ship git-pr` | `/git-pr` (best-effort, auto-activates review-pr) | diff --git a/docs/specs/skills/SPEC-git-pr.md b/docs/specs/skills/SPEC-git-pr.md index 596d9feb..58ca21c9 100644 --- a/docs/specs/skills/SPEC-git-pr.md +++ b/docs/specs/skills/SPEC-git-pr.md @@ -19,6 +19,8 @@ Autonomously commits, pushes, and creates a draft GitHub PR. No prompts, no ques **Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (the `fab change resolve` ID/substring/name forms, type-vs-change arg classification, the default-branch probe safety-net, and the 3a-bis "why here" rationale already in the Summary), and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). +**OPEN-PR `## Meta` retrofit** (260630-t54n): a sub-step **3d. Retrofit `## Meta` onto an existing OPEN PR** closes the gap that `## Meta` was injected only on PR **create** (Step 3c) — an OPEN PR authored off-pipeline (or created before fab adopted the branch) has a body with no `## Meta`. Step 3d fetches the current body (`gh pr view --json body -q '.body'`), and **only when the body lacks a `## Meta` heading** renders the block via `fab pr-meta {name} --type {type} --issues "{issues}"` (the same self-contained mechanism Step 3c uses) and prepends it to the existing body via `gh pr edit --body-file -` (stdin — avoids multi-line quoting issues). It is gated on **`{has_fab}` AND the PR being already OPEN at Step 1** (a freshly created PR already carries `## Meta` from 3c, so it is not re-retrofitted), runs on both the normal existing-OPEN-PR path and the "already shipped" short-circuit, and is **idempotent** (body-already-has-`## Meta` → no-op; non-zero `fab pr-meta` / empty render → no edit, same graceful degradation as 3c). **No Go change** — `fab pr-meta` / `prmeta.Render` are reused. This is the ship-stage retrofit `/fab-adopt` relies on, but it is general — any OPEN-PR ship benefits. + ## Flow ``` @@ -127,6 +129,16 @@ Autonomously commits, pushes, and creates a draft GitHub PR. No prompts, no ques │ fallback, evaluated BEFORE the creation-failure STOP — a body │ failure never reaches it; PR-creation failure → report + STOP) │ +├─ Step 3d: Retrofit ## Meta onto existing OPEN PR (260630-t54n — +│ │ if {has_fab} AND PR was already OPEN at Step 1; runs on the +│ │ normal existing-OPEN-PR path AND the "already shipped" short-circuit) +│ ├─ Bash: existing_body=$(gh pr view --json body -q '.body') (fetch current body) +│ ├─ Idempotency guard: body already has a ## Meta heading → no edit, continue +│ ├─ Bash: META=$(fab pr-meta {name} --type {type} --issues "{issues}") +│ │ (non-zero / empty → no edit, continue — same graceful degradation as 3c) +│ └─ Bash: printf '%s\n\n%s\n' "$META" "$existing_body" | gh pr edit --body-file - +│ (prepend Meta to the existing body; gh edit failure → report + STOP) +│ ├─ Step 4a: Record PR URL (if {has_fab}) │ └─ Bash: fab status add-pr {name} │ diff --git a/docs/specs/stage-models.md b/docs/specs/stage-models.md index 05bc47d0..d26b4663 100644 --- a/docs/specs/stage-models.md +++ b/docs/specs/stage-models.md @@ -202,7 +202,7 @@ override a tier to Haiku (pass-through doesn't forbid it); fab just doesn't ship ## Skill wiring — orchestrator/dispatch consume `fab resolve-agent` -The orchestrators (`/fab-ff`, `/fab-fff`, `/fab-proceed`) and `/fab-continue`'s sub-agent dispatch call +The orchestrators (`/fab-ff`, `/fab-fff`, `/fab-proceed`, `/fab-adopt`) and `/fab-continue`'s sub-agent dispatch call `fab resolve-agent ` immediately before dispatching each stage's sub-agent, **surface** the resolved `model=/effort=` lines (so a skipped or mis-resolved tier is visible in output rather than silent — the available stand-in for an enforcement guard, since dispatch is harness-internal), and diff --git a/docs/specs/user-flow.md b/docs/specs/user-flow.md index 63da2976..f969b00b 100644 --- a/docs/specs/user-flow.md +++ b/docs/specs/user-flow.md @@ -35,7 +35,7 @@ flowchart TD ## 2. The Same Flow, With Fab -Each transition is now a `/fab-*` command. `/fab-ff` fast-forwards from intake through hydrate; `/fab-fff` fast-forwards further through ship and PR review. `/fab-archive` is a separate housekeeping step after the pipeline completes. +Each transition is now a `/fab-*` command. `/fab-ff` fast-forwards from intake through hydrate; `/fab-fff` fast-forwards further through ship and PR review. `/fab-archive` is a separate housekeeping step after the pipeline completes. `/fab-adopt` is the **alternate entry point** for work that bypassed the pipeline: a branch authored without fab (with an OPEN or not-yet-created PR) enters *late* — intake is reconstructed from the diff, **apply is `skipped`**, and review (outward-only) → hydrate → ship → review-pr run for real. ```mermaid flowchart TD @@ -57,6 +57,13 @@ flowchart TD (fast-forward-further, confidence-gated)"| RP IDEA -->|"/fab-proceed"| RP + %% Adoption — alternate entry for off-pipeline work (apply skipped) + OFF[off-pipeline branch + + OPEN/no PR] -->|"/fab-adopt + (reconstruct intake from diff, + apply skipped, outward-only review)"| R + style OFF fill:#f3e5f5,stroke:#9C27B0 + %% Apply-review loop (sub-agent review with auto-rework) R -.->|"sub-agent review auto-rework (fab-ff, fab-fff)"| A @@ -88,6 +95,7 @@ stateDiagram-v2 direction TB [*] --> intake: /fab-new + [*] --> review: /fab-adopt (adopt off-pipeline change; intake reconstructed, apply skipped) intake --> apply: /fab-continue (co-generates plan.md, runs tasks) intake --> hydrate: /fab-ff (fast-forward, intake-gated) diff --git a/fab/changes/260630-t54n-adopt-off-pipeline-change/.history.jsonl b/fab/changes/260630-t54n-adopt-off-pipeline-change/.history.jsonl new file mode 100644 index 00000000..ffb1fb81 --- /dev/null +++ b/fab/changes/260630-t54n-adopt-off-pipeline-change/.history.jsonl @@ -0,0 +1,11 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-06-30T01:13:11Z"} +{"args":"fab-adopt — a new skill to bring an off-pipeline change into the fab pipeline mid-flight (scenario B: open-PR branch authored without fab). Reconstruct intake+minimal-plan from the diff, run outward-only review, real hydrate, retrofit PR Meta, land in review-pr. Only apply is skipped.","cmd":"fab-new","event":"command","ts":"2026-06-30T01:13:11Z"} +{"delta":"+4.9","event":"confidence","score":4.9,"trigger":"calc-score","ts":"2026-06-30T01:14:42Z"} +{"delta":"+0.0","event":"confidence","score":4.9,"trigger":"calc-score","ts":"2026-06-30T01:14:47Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-30T02:10:23Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-30T02:23:04Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-30T02:32:57Z"} +{"event":"review","result":"passed","ts":"2026-06-30T02:32:57Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-30T02:38:33Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-06-30T02:45:17Z"} +{"event":"review","result":"passed","ts":"2026-06-30T02:52:27Z"} diff --git a/fab/changes/260630-t54n-adopt-off-pipeline-change/.status.yaml b/fab/changes/260630-t54n-adopt-off-pipeline-change/.status.yaml new file mode 100644 index 00000000..67153b42 --- /dev/null +++ b/fab/changes/260630-t54n-adopt-off-pipeline-change/.status.yaml @@ -0,0 +1,57 @@ +id: t54n +name: 260630-t54n-adopt-off-pipeline-change +created: 2026-06-30T01:13:11Z +created_by: sahil-noon +change_type: feat +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: done + review-pr: done +plan: + generated: true + task_count: 15 + acceptance_count: 18 + acceptance_completed: 18 +confidence: + certain: 9 + confident: 1 + tentative: 0 + unresolved: 0 + score: 4.9 + fuzzy: true + dimensions: + signal: 89.5 + reversibility: 84.0 + competence: 86.5 + disambiguation: 87.0 +stage_metrics: + intake: {started_at: "2026-06-30T01:13:11Z", driver: fab-new, iterations: 1, completed_at: "2026-06-30T02:10:23Z"} + apply: {started_at: "2026-06-30T02:10:23Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-30T02:23:04Z"} + review: {started_at: "2026-06-30T02:23:04Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-30T02:32:57Z"} + hydrate: {started_at: "2026-06-30T02:32:57Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-30T02:38:33Z"} + ship: {started_at: "2026-06-30T02:38:33Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-30T02:45:17Z"} + review-pr: {started_at: "2026-06-30T02:45:17Z", driver: git-pr, iterations: 1, completed_at: "2026-06-30T02:52:27Z"} +prs: + - https://github.com/sahil87/fab-kit/pull/451 +change_type_source: explicit +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + tests: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-06-30T02:38:33Z" + computed_at_stage: hydrate +summary: documented /fab-adopt adoption orchestrator, _review.md outward-only mode, /git-pr Meta body-retrofit, and the skip-apply+reset-review state composition +# true_impact: lazily created on first apply-finish (no placeholder here). +last_updated: 2026-06-30T02:52:27Z diff --git a/fab/changes/260630-t54n-adopt-off-pipeline-change/intake.md b/fab/changes/260630-t54n-adopt-off-pipeline-change/intake.md new file mode 100644 index 00000000..991cec8e --- /dev/null +++ b/fab/changes/260630-t54n-adopt-off-pipeline-change/intake.md @@ -0,0 +1,127 @@ +# Intake: Adopt an off-pipeline change into the Fab pipeline + +**Change**: 260630-t54n-adopt-off-pipeline-change +**Created**: 2026-06-30 + +## Origin + +> Many times, users discuss with AI agents to create changes, and also create a git PR. The fab pipeline gets skipped, either by mistake or deliberately. Now after the change is completed, there should a way to "bring it" into the fab pipeline. What key parts would have gotten missed by bypassing the fab pipeline? How to ensure those get followed? + +Conversational design session (`/fab-discuss` → exploratory). The user steered the scope across several turns; the resulting design diverges from the normal pipeline in deliberate, reasoned ways: + +- **Scenario narrowed to (B), not (A).** The user explicitly de-prioritised retroactive backfill of an *already-merged* PR (scenario A) and chose **mid-flight adoption** (scenario B): a feature branch with an **OPEN PR (or no PR yet)** whose code was authored without fab. The motivating trigger is *social* — "a PR review team points out: hey, you forgot to use fab-kit and might have missed a few checks." +- **Generic-user lens, not fab-kit-repo lens.** An early proposal leaned on fab-kit's own SPEC-mirror sweep discipline; the user corrected this — the skill must serve *any* project using fab-kit and lean on *their* `code-quality.md` / `code-review.md` / `review_tools` config, not fab-kit-specific mirror classes. +- **Review = outward-only.** After weighing the loss (inward review checks conformance against requirements — the most valuable check for a bypassed change), the user chose **outward-only** review for adopted changes, implemented as a **general `mode` parameter** in `_review.md` (not an adopt-specific branch). +- **PR Meta decoration is in scope.** The user wants the git PR's `## Meta` decoration added retrospectively to the existing open PR. +- **Honest state.** Stages that can't truly re-run are marked truthfully; the user chose "mark skipped + note" over "mark done." +- **Intake + plan written by the same main-session agent.** The user confirmed both artifacts are generated in one pass by the same agent reading the diff once (no dispatched-apply split), because both merely *describe one fixed existing diff* — a context boundary between them would only invite drift and waste. + +Two open verification items raised during the discussion were resolved against the Go source *before* this intake was written (see Assumptions #8, #9) — both land as **skill-layer-only** changes; no Go binary change is required. + +## Why + +**Problem.** The Fab pipeline's value is not the PR ceremony — `/git-pr` is a thin shipping step. The value lives in four artifacts an ad-hoc PR never produces: the **intake + confidence decision record** (the deliberated "why"), the **plan's requirements** (the conformance baseline), the **review pass** (correctness/quality/cross-reference checks against the project's own config), and — with permanent consequences — **hydrate's update to `docs/memory/`**. Constitution II makes memory the source of truth; a change shipped outside the pipeline never updates memory, so the docs silently drift from what actually shipped, and the change is invisible to the generated `log.md` history. + +**Consequence if unfixed.** Teams that adopt fab-kit but occasionally bypass it (deliberately or by mistake) accumulate undocumented changes. There is today **no supported path to bring a completed-but-off-pipeline change back in**. The only options are to re-do the work through the pipeline (wasteful — the code already exists and may be reviewed) or to leave memory permanently stale. A reviewer's "you skipped fab-kit" comment has no actionable remedy. + +**Why this approach.** Of the six stages, exactly one — **apply** — cannot meaningfully re-run on an adopted change (the code already exists; there is nothing to generate). Every other stage *can* run for real, just *late*: intake reconstructed from the diff, review run on the diff before merge, hydrate writing memory before merge, ship retrofitting the PR decoration, review-pr resuming normally. So adoption is **not** a parallel "fake pipeline" — it is the *real* pipeline entered late, with apply skipped. This is the honest, minimal framing and it dictates the whole design: build a thin orchestrator that reuses existing skills as sub-agents (the `/fab-proceed`/`/fab-ff` pattern), introduces only what is genuinely new (diff→intake, thin diff→plan, an outward-only review mode), and closes two small skill-layer gaps. + +## What Changes + +### 1. New skill `/fab-adopt` — `src/kit/skills/fab-adopt.md` (+ `docs/specs/skills/SPEC-fab-adopt.md` mirror) + +A thin orchestrator skill. Arguments: optional `` (derived from branch name / PR title if omitted). It reads the `_preamble` always-load layer and declares helpers `[_srad, _generation, _review, _pipeline]`. + +**Step 0 — Guards & diff base** (reuse `/git-pr`'s guard idioms verbatim): +- `git branch --show-current` — STOP on **detached HEAD** or the **default branch** (with `/git-pr`'s messages). +- `gh pr view --json number,state,url` — if `state == MERGED`, STOP: *"PR is already merged — that's retroactive backfill (scenario A), out of scope for /fab-adopt. Adopt operates on in-flight (open or not-yet-created) PRs."* `OPEN` and `none` both proceed. +- **Collision guard**: if a fab change already maps to this branch (`fab change resolve "$(git branch --show-current)"` succeeds), STOP and point at `/fab-continue` — it is already in the pipeline. +- Resolve the diff base: `base=$(git merge-base HEAD origin/{default})`; capture `git diff {base}...HEAD` and `git diff --name-only {base}...HEAD`. STOP if the diff is empty (nothing to adopt). + +**Steps 1+2 — ONE main-session generation pass** (same agent, not dispatched — reads the diff + PR body once): +1. `fab change new --slug {slug}` against the current branch; activate it (the change branch already exists — `/fab-new`'s Step 11 row 1/2 "already active"/"checked out" applies). +2. **Reconstruct `intake.md`** via the new **Intake-from-Diff Procedure** (added to `_generation.md`): Origin = `adopted from {PR or branch}`; Why/What-Changes synthesised from the diff + PR body; **Affected Memory** inferred from which `docs/memory/` domains the diff touches; Impact from the changed paths. Apply SRAD and `fab score`. +3. **Human confirmation checkpoint** — present the reconstructed intent + SRAD assumptions for the user to confirm/correct. This *is* the late deliberation the bypass skipped (mirrors `/fab-new`'s interactive intake moment). On confirm: `fab status advance {name} intake` → `finish {name} intake` (auto-activates apply). +4. **Write a deliberately MINIMAL `plan.md`** via the new **Plan-from-Diff Procedure** (added to `_generation.md`), from the *same* understanding (no re-read of the diff): + - `## Requirements` — **plain-language** restatement of the intake's What-Changes, grouped by change area. **This is the only part hydrate reads to write accurate memory, so effort concentrates here.** Largely lifts the intake's What-Changes into plan form. + - `## Tasks` — a single all-`[x]` stub (e.g. `- [x] Adopted: implementation authored outside the pipeline (see {PR}).`). + - `## Acceptance` — a single all-`[x]` stub. + - **NO** `R#`/`T{NNN}`/`A-{NNN}` traceability IDs, GIVEN/WHEN/THEN scenarios, phases, or `[P]` markers — the apply↔review traceability loop never runs for adopted changes, so that scaffolding is dead weight. (The stable parser contract is only the three heading literals `## Requirements`/`## Tasks`/`## Acceptance`, confirmed against `templates/plan.md`.) + - Carry a header note: *"Adopted change — code authored off-pipeline. Apply was skipped; this plan is reverse-engineered from the branch diff to feed hydrate."* + +**Step 2 (state) — apply → skipped, review → active** (the resolved transition path, Assumption #8): +```bash +fab status skip {name} apply # apply → skipped; cascades review/hydrate/ship/review-pr → skipped +fab status reset {name} review {driver} # review skipped → active; cascades hydrate/ship/review-pr back to pending +``` +Net state: `apply: skipped`, `review: active`, `hydrate/ship/review-pr: pending`. Honest, uses only existing transitions, **no Go change**. Record the off-pipeline fact via `fab status set-summary {name} "adopted off-pipeline change; apply skipped"`. + +**Step 3 — Review, dispatched, `mode: outward-only`** (the new `_review.md` parameter, §2 below): dispatch `/fab-continue` Review Behavior as a sub-agent (per the standard dispatch contract + `fab resolve-agent review --alias`), passing `mode: outward-only`. Outward review already uses `git diff {base}...HEAD` natively, so no file-set prompt hack is needed. The orchestrator owns the verdict transition (pass → `finish review`; fail → auto-rework per `_pipeline.md` budget, or hand back). + +**Step 4 — Hydrate, dispatched, verbatim**: reuse `_pipeline.md` Step 3 unchanged. This is the permanent-loss recovery — `docs/memory/` finally reflects what shipped. On success `finish {name} hydrate`. + +**Step 5 — Ship (retrofit Meta onto the existing OPEN PR)**: dispatch `/git-pr {name}`. Because the PR is OPEN, `/git-pr` takes its existing-PR path — which today only *records* the URL. The new `/git-pr` body-retrofit path (§3 below) injects the `## Meta` block when the open PR's body lacks one. + +**Step 6 — Land in review-pr**: `finish ship` auto-activates `review-pr`; output `Next: /git-pr-review`. Normal tail resumes. + +**Honest-state summary the skill prints**: *intake, review, hydrate, ship, review-pr all genuinely ran (just late, after the code was written). Only apply is `skipped`.* + +### 2. New `mode` parameter in `src/kit/skills/_review.md` (+ `SPEC-_review.md` mirror) + +Add a **Review Mode** concept to `_review.md` — the single authority for review dispatch, so `/fab-continue`, `/fab-ff`, `/fab-fff`, and `/fab-adopt` all inherit it. + +- A `mode` parameter passed in the review dispatch: `full` (default — inward + outward) and `outward-only` (outward sub-agent only). **Do not** add a speculative `inward-only` value — no caller exists today (parsimony; the repo's own review enforces this). +- **Precondition gating**: the inward Preconditions (`plan.md` MUST exist with `## Tasks`/`## Acceptance`; all tasks `[x]`) are checked **only** in `full` mode. In `outward-only` they are skipped — there is nothing for inward to validate. +- **Parallel Dispatch**: dispatch only the sub-agent(s) selected by `mode`. +- **Findings Merge / pass-fail**: unchanged — "any must-fix → fail" works with a single source. An empty outward result (all `review_tools` disabled/unavailable) → zero findings → **passes** (best-effort; adoption must not hard-block when no external reviewer is available). +- Default is `full` (param omitted) so all existing callers are unaffected. + +### 3. `/git-pr` body-retrofit path — `src/kit/skills/git-pr.md` (+ `SPEC-git-pr.md` mirror) + +Close the gap that `/git-pr` injects `## Meta` only on PR **create** (Step 3c), never when an open PR already exists. Add to the existing-OPEN-PR path: if the PR body lacks a `## Meta` block, render it via `fab pr-meta {name} --type {type} --issues "{issues}"` and apply with `gh pr edit --body-file -` (stdin). Gated on body-lacks-`## Meta` for idempotency (a second run is a no-op). **No Go change** — `prmeta.Render`/`fab pr-meta` already exist and are reused. + +### 4. Aggregate spec & doc sweeps + +Per the project's Sibling & Mirror Sweep discipline, update every place that enumerates skills or stages: +- `docs/specs/skills.md` — add `/fab-adopt` behavior. +- `docs/specs/glossary.md` — add "adopt" terminology. +- `docs/specs/overview.md` and `docs/specs/user-flow.md` — note the adoption entry-point into the pipeline. +- `_preamble.md` Next-Steps **State Table** — adoption is an alternate entry; assess whether a row/note is warranted. +- `fab-help.md` — list the new command. + +## Affected Memory + +- `pipeline/execution-skills.md`: (modify) document `/fab-adopt` — the adoption orchestrator, its skip-apply/reset-review state path, and the `/git-pr` body-retrofit path +- `pipeline/planning-skills.md`: (modify) note the Intake-from-Diff and Plan-from-Diff generation procedures added to `_generation.md`, and that adopt generates both artifacts in one main-session pass +- `pipeline/change-lifecycle.md`: (modify) record adoption as an alternate pipeline entry-point and the `apply: skipped` honest-state pattern +- `pipeline/schemas.md`: (modify) document the `skip apply` + `reset review` transition composition that yields `apply=skipped, review=active` (no new transition added) + +## Impact + +- **Skill files**: `src/kit/skills/fab-adopt.md` (new), `_review.md` (mode param), `_generation.md` (two new procedures), `git-pr.md` (body-retrofit path), `fab-help.md`. +- **Spec mirrors** (constitution-required): `SPEC-fab-adopt.md` (new), `SPEC-_review.md`, `SPEC-_generation.md`, `SPEC-git-pr.md`; aggregate specs `skills.md`, `glossary.md`, `overview.md`, `user-flow.md`. +- **Go binary**: **none required.** Both open items resolved to skill-layer changes (state composition uses existing `skip`/`reset` transitions; Meta-retrofit reuses existing `fab pr-meta`/`prmeta.Render` + `gh pr edit`). +- **Tests**: no Go test changes expected (no Go change). Skill changes carry SPEC-mirror updates per the constitution. +- **External tools used**: `git`, `gh` (incl. `gh pr edit --body-file -`), `fab pr-meta`, `fab status skip/reset/finish/set-summary`, `fab resolve-agent`. + +## Open Questions + +- Should `/fab-adopt` offer a `--no-pr` mode for branches with no PR yet (reconstruct + review + hydrate, then create the PR fresh via `/git-pr`'s normal create path)? The current design handles `pr_state == none` by letting Step 5's `/git-pr` create the PR — likely sufficient, but worth confirming during apply. +- Should the auto-rework loop apply to adopted-change review at all, given the code is already written and the author may not want autonomous edits to a hand-authored branch? (Leaning: yes for `/fab-adopt` when run autonomously, but the interactive default should hand findings back rather than auto-edit. Resolve at apply.) + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | Scenario is (B) mid-flight adoption of an OPEN/not-yet-created PR; MERGED PRs (scenario A) are explicitly out of scope and STOP | User stated this directly and twice in the discussion | S:95 R:90 A:90 D:95 | +| 2 | Certain | Review for adopted changes is **outward-only**, via a general `mode` param on `_review.md` (not an adopt-specific branch) | User chose both the outward-only behavior and the general-mode placement explicitly | S:95 R:85 A:90 D:95 | +| 3 | Certain | Only **apply** is marked `skipped`; intake/review/hydrate/ship/review-pr genuinely run; honest "skipped + note" state (not "mark done") | User chose "mark skipped + note" explicitly | S:95 R:90 A:90 D:95 | +| 4 | Certain | Intake + plan are generated by the **same main-session agent in one pass** reading the diff once — not via the dispatched-apply path | User confirmed this and the drift/waste rationale explicitly | S:90 R:85 A:90 D:90 | +| 5 | Confident | The thin `plan.md` carries plain-language `## Requirements` (for hydrate) + all-`[x]` `## Tasks`/`## Acceptance` stubs; no R#/T#/A# scaffolding | Derived from verified facts: hydrate is the only consumer needing requirements; the parser contract is just the three headings (confirmed against templates/plan.md); apply↔review loop never runs | S:85 R:80 A:85 D:80 | +| 6 | Confident | `/fab-adopt` is a thin orchestrator reusing `_pipeline.md`/`_generation.md`/`_review.md`/existing skills as sub-agents (the `/fab-proceed`/`/fab-ff` pattern) | Matches the established orchestrator pattern and the constitution's single-authority discipline | S:85 R:80 A:85 D:80 | +| 7 | Confident | Outward-only with zero findings (no external reviewer available) **passes** (best-effort), not blocks | Matches `_review.md`'s existing graceful-no-op contract for an empty outward cascade; adoption shouldn't hard-block on tool availability | S:85 R:80 A:80 D:80 | +| 8 | Certain | apply→skipped + review→active is achieved by `skip apply` (cascades all downstream → skipped) then `reset review` (skipped→active, cascades its downstream → pending) — **no Go change** | Verified against `src/go/fab/internal/status/status.go`: `skip` From={pending,active}→skipped with forward cascade (L246–278); `reset` From includes `skipped`→active (L41); `start` does NOT accept skipped→active | S:95 R:90 A:90 D:95 | +| 9 | Certain | Meta-retrofit onto an open PR is skill-layer only: `fab pr-meta` + `gh pr edit --body-file -`, gated on body lacking `## Meta` — **no Go change** | Verified: `prmeta.Render` (L93) + `fab pr-meta` CLI already exist; `git-pr.md` injects Meta only on create (L279); `gh pr edit --body`/`--body-file` confirmed available | S:95 R:90 A:90 D:95 | +| 10 | Tentative | Aggregate sweep targets are skills.md, glossary.md, overview.md, user-flow.md, _preamble State Table, fab-help.md | Standard sweep class for a new skill; exact set confirmed during apply by grepping the enumerations | S:75 R:70 A:75 D:65 | + +10 assumptions (6 certain, 3 confident, 1 tentative, 0 unresolved). diff --git a/fab/changes/260630-t54n-adopt-off-pipeline-change/plan.md b/fab/changes/260630-t54n-adopt-off-pipeline-change/plan.md new file mode 100644 index 00000000..1b88c04a --- /dev/null +++ b/fab/changes/260630-t54n-adopt-off-pipeline-change/plan.md @@ -0,0 +1,187 @@ +# Plan: Adopt an off-pipeline change into the Fab pipeline + +**Change**: 260630-t54n-adopt-off-pipeline-change +**Intake**: `intake.md` + +## Requirements + +### Adopt Skill: Orchestrator + +#### R1: A new `/fab-adopt` skill brings a completed off-pipeline change into the Fab pipeline +A new skill file `src/kit/skills/fab-adopt.md` SHALL be a thin orchestrator that reuses existing skills as sub-agents (the `/fab-proceed` / `/fab-ff` pattern). It SHALL accept an optional `` argument (derived from branch name / PR title if omitted), read the `_preamble` always-load layer, and declare `helpers: [_srad, _generation, _review, _pipeline]`. It SHALL enter the real pipeline late with **apply skipped** — every other stage runs for real. + +- **GIVEN** a feature branch with code authored without fab and an OPEN (or not-yet-created) PR +- **WHEN** the user runs `/fab-adopt` +- **THEN** the skill reconstructs intake + a thin plan from the diff, marks apply `skipped`, runs review (outward-only) → hydrate → ship (Meta retrofit) → review-pr, and lands the change in the normal pipeline tail + +#### R2: Adopt guards reject out-of-scope states before any mutation +`/fab-adopt` Step 0 SHALL STOP (no mutation) when: the branch is detached HEAD or the default branch (reusing `/git-pr`'s guard messages); the PR `state == MERGED` (scenario A — retroactive backfill, out of scope); a fab change already maps to the current branch (`fab change resolve "$(git branch --show-current)"` succeeds — already in the pipeline, point at `/fab-continue`); or the diff against the merge-base is empty (nothing to adopt). `OPEN` and `none` PR states both proceed. + +- **GIVEN** a branch whose PR is already MERGED +- **WHEN** `/fab-adopt` runs Step 0 +- **THEN** it STOPs with the scenario-A out-of-scope message and makes no `.status.yaml` or git mutation + +#### R3: Intake + thin plan are generated in one main-session pass from the diff +`/fab-adopt` SHALL generate `intake.md` and `plan.md` in a single main-session pass (the same agent, not a dispatched apply), reading the diff and PR body once. It SHALL run `fab change new --slug {slug}` against the current branch and activate it, reconstruct `intake.md` via the **Intake-from-Diff Procedure**, present a human-confirmation checkpoint, then on confirm advance+finish intake and write a **minimal** `plan.md` via the **Plan-from-Diff Procedure**. + +- **GIVEN** an adopted diff and PR body +- **WHEN** `/fab-adopt` reaches Steps 1–2 +- **THEN** the same agent writes both `intake.md` (full) and `plan.md` (thin) without re-reading the diff, with a human-confirmation checkpoint between them + +#### R4: Adopt sets honest state via existing transitions — apply skipped, review active +`/fab-adopt` SHALL achieve `apply: skipped, review: active, hydrate/ship/review-pr: pending` using only existing `fab status` transitions: `fab status skip {name} apply` (cascades all downstream → skipped) then `fab status reset {name} review {driver}` (skipped → active, cascades its downstream → pending). It SHALL record the off-pipeline fact via `fab status set-summary {name} "..."`. **No Go change.** + +- **GIVEN** an activated adopted change at apply +- **WHEN** `/fab-adopt` runs the Step 2 state transition +- **THEN** `.status.yaml` shows `apply: skipped`, `review: active`, downstream `pending` — and no new Go transition was added + +#### R5: Adopt dispatches review outward-only, then hydrate, ship, review-pr +`/fab-adopt` SHALL dispatch review as a sub-agent with `mode: outward-only` (per R6), own the verdict transition (pass → `finish review`; fail → auto-rework per `_pipeline.md` budget or hand back), then dispatch hydrate verbatim (`finish hydrate`), then ship via `/git-pr {name}` (Meta retrofit per R8), then land in review-pr via `finish ship`. It SHALL print an honest-state summary noting only apply is `skipped`. + +- **GIVEN** an adopted change at review (active) +- **WHEN** `/fab-adopt` runs Steps 3–6 +- **THEN** review runs outward-only, hydrate writes memory, ship retrofits the PR Meta, and the change lands in review-pr with `Next: /git-pr-review` + +### Review: Mode Parameter + +#### R6: `_review.md` gains a general `mode` parameter (`full` | `outward-only`) +`src/kit/skills/_review.md` SHALL add a **Review Mode** concept: a `mode` parameter with values `full` (default — inward + outward) and `outward-only` (outward sub-agent only). It SHALL NOT add a speculative `inward-only`. The inward Preconditions (`plan.md` MUST exist with `## Tasks`/`## Acceptance`; all tasks `[x]`) SHALL be checked **only** in `full` mode. Parallel Dispatch SHALL dispatch only the sub-agent(s) selected by `mode`. Findings Merge / pass-fail SHALL be unchanged; an empty outward result (all `review_tools` disabled/unavailable → zero findings) SHALL **pass**. Default is `full` when the param is omitted, so all existing callers are unaffected. + +- **GIVEN** a review dispatch with `mode: outward-only` +- **WHEN** `_review.md` runs +- **THEN** inward Preconditions are skipped, only the outward sub-agent is dispatched, and zero findings → pass +- **AND** **GIVEN** no `mode` param **WHEN** any existing caller dispatches review **THEN** behavior is identical to today (`full`) + +### Generation: Diff-Based Procedures + +#### R7: `_generation.md` gains Intake-from-Diff and Plan-from-Diff procedures +`src/kit/skills/_generation.md` SHALL add two procedures. **Intake-from-Diff Procedure**: reconstruct `intake.md` from the branch diff + PR body — Origin = `adopted from {PR or branch}`; Why/What-Changes synthesised from the diff; Affected Memory inferred from which `docs/memory/` domains the diff touches; Impact from changed paths; apply SRAD + `fab score`. **Plan-from-Diff Procedure**: write a deliberately thin `plan.md` — plain-language `## Requirements` (the only part hydrate reads; effort concentrates here), an all-`[x]` `## Tasks` stub, an all-`[x]` `## Acceptance` stub, **no** R#/T#/A# IDs, GIVEN/WHEN/THEN, phases, or `[P]` markers (the apply↔review traceability loop never runs for adopted changes); carry a header note that apply was skipped and the plan is reverse-engineered to feed hydrate. + +- **GIVEN** an adopted change +- **WHEN** `/fab-adopt` invokes the two new `_generation.md` procedures +- **THEN** `intake.md` is reconstructed from the diff and `plan.md` carries plain-language requirements with all-`[x]` task/acceptance stubs and no traceability scaffolding +- **AND** the three heading literals `## Requirements` / `## Tasks` / `## Acceptance` (the stable parser contract) are present + +### Ship: PR Body Meta-Retrofit + +#### R8: `/git-pr` retrofits the `## Meta` block onto an existing OPEN PR +`src/kit/skills/git-pr.md` SHALL close the gap that `## Meta` is injected only on PR **create**. On the existing-OPEN-PR path: if the PR body lacks a `## Meta` block, render it via `fab pr-meta {name} --type {type} --issues "{issues}"` and apply with `gh pr edit --body-file -` (stdin). The retrofit SHALL be gated on body-lacks-`## Meta` for idempotency (a second run is a no-op). **No Go change** — `fab pr-meta` / `prmeta.Render` already exist and are reused. + +- **GIVEN** an OPEN PR whose body lacks `## Meta` +- **WHEN** `/git-pr` runs its existing-OPEN-PR path +- **THEN** it renders Meta via `fab pr-meta` and applies it with `gh pr edit --body-file -` +- **AND** **GIVEN** an OPEN PR whose body already has `## Meta` **WHEN** `/git-pr` re-runs **THEN** it makes no edit (idempotent) + +### Discoverability & Documentation + +#### R9: `/fab-adopt` is discoverable in `/fab-help` +The new skill SHALL be listed by `/fab-help`. `fab fab-help` auto-scans skill frontmatter, so a valid `name`/`description` frontmatter makes it appear; it SHALL be grouped correctly (Planning) via `skillToGroupMap` in `src/go/fab/cmd/fab/fabhelp.go` so it does not fall into the "Other" bucket. + +- **GIVEN** the new `fab-adopt.md` skill exists with valid frontmatter +- **WHEN** the user runs `/fab-help` +- **THEN** `/fab-adopt` is listed under the Planning group + +#### R10: All SPEC mirrors and aggregate specs enumerating skills/stages are swept +Per Constitution (skill change MUST update its SPEC mirror) and the project Sibling & Mirror Sweep discipline, this change SHALL: create `docs/specs/skills/SPEC-fab-adopt.md`; update `SPEC-_review.md`, `SPEC-_generation.md`, `SPEC-git-pr.md`; and update the aggregate specs that enumerate skills/stages — `docs/specs/skills.md` (new section + helpers row + checklist note), `docs/specs/glossary.md` (Skills table + "adopt" term), `docs/specs/overview.md` (Quick Reference), `docs/specs/user-flow.md` (alternate entry-point). `_preamble.md` § Next Steps State Table SHALL be assessed for an adoption row/note. + +- **GIVEN** the skill-file edits in R1–R8 +- **WHEN** the sweep is performed +- **THEN** every SPEC mirror and every aggregate spec that enumerates skills/stages references `/fab-adopt` / "adopt", confirmed by grep + +### Non-Goals + +- Scenario A (retroactive backfill of an already-MERGED PR) — explicitly out of scope; `/fab-adopt` STOPs on MERGED. +- An `inward-only` review mode — no caller exists (parsimony); only `full` and `outward-only` are added. +- Any Go binary change to status transitions or Meta rendering — both reuse existing CLI surface. + +### Design Decisions + +1. **Adopt is the real pipeline entered late, not a parallel fake pipeline** — only apply is skipped; intake/review/hydrate/ship/review-pr genuinely run. *Why*: honest state (Constitution II — memory must reflect what shipped). *Rejected*: marking all stages "done" (dishonest, loses the hydrate value). +2. **Outward-only review via a general `mode` param on `_review.md`** — not an adopt-specific branch. *Why*: single review authority, inherited by all callers. *Rejected*: an adopt-only review fork (duplicates dispatch logic). +3. **State via `skip apply` + `reset review` composition** — *Why*: yields `apply=skipped, review=active` with existing transitions, no Go change. *Rejected*: a new Go transition (unnecessary). +4. **fab-help grouping via the Go `skillToGroupMap`** — a one-line map entry under "Planning". *Why*: the New Skill Checklist item 8 requires correct grouping, and the auto-scan would otherwise bucket it under "Other" — a visible drift review would flag. *Rejected*: relying on the "Other" bucket (the intake's "no Go change" was about state/Meta mechanics, not the cosmetic help map). This is a graded deviation from the intake's Impact note — see Assumptions. + +## Tasks + +### Phase 1: Core Skill-Layer Mechanics + +- [x] T001 Add the `mode` parameter (Review Mode concept — `full` default | `outward-only`) to `src/kit/skills/_review.md`: gate inward Preconditions on `full`, dispatch only selected sub-agent(s), keep Findings Merge unchanged, document zero-findings-passes for outward-only +- [x] T002 Add the **Intake-from-Diff Procedure** and **Plan-from-Diff Procedure** to `src/kit/skills/_generation.md` (Contents list + two new procedure sections) +- [x] T003 Add the body-retrofit path to `src/kit/skills/git-pr.md` existing-OPEN-PR branch (render via `fab pr-meta`, apply with `gh pr edit --body-file -`, gated on body-lacks-`## Meta`); update Flow + Key Properties idempotency note + +### Phase 2: Orchestrator Skill + +- [x] T004 Create `src/kit/skills/fab-adopt.md` — the orchestrator (frontmatter with `helpers: [_srad, _generation, _review, _pipeline]`, preamble-read line, Steps 0–6 per intake design, Output, Error Handling + Key Properties tables, `Next:` line) + +### Phase 3: Discoverability (Go help grouping) + +- [x] T005 Add `"fab-adopt": "Planning"` to `skillToGroupMap` in `src/go/fab/cmd/fab/fabhelp.go` so `/fab-help` groups it correctly + +### Phase 4: SPEC Mirrors & Aggregate Sweeps + +- [x] T006 [P] Create `docs/specs/skills/SPEC-fab-adopt.md` (Summary + Flow + Tools/Sub-agents tables, mirroring the new skill) +- [x] T007 [P] Update `docs/specs/skills/SPEC-_review.md` (mode parameter, mode-gated preconditions, mode-selected dispatch) +- [x] T008 [P] Update `docs/specs/skills/SPEC-_generation.md` (two new diff-based procedures) +- [x] T009 [P] Update `docs/specs/skills/SPEC-git-pr.md` (body-retrofit path) +- [x] T010 [P] Update `docs/specs/skills.md` (new `/fab-adopt` section, helpers row, New Skill Checklist coverage) +- [x] T011 [P] Update `docs/specs/glossary.md` (Skills table row + "adopt" terminology) +- [x] T012 [P] Update `docs/specs/overview.md` (Quick Reference row) +- [x] T013 [P] Update `docs/specs/user-flow.md` (adoption as alternate pipeline entry-point) +- [x] T014 Assess `src/kit/skills/_preamble.md` § Next Steps State Table for an adoption row/note and update if warranted; sweep `SPEC-_preamble.md` if changed + +### Phase 5: Verify + +- [x] T015 Run `go test ./...` in `src/go/fab/` for the `fabhelp` package after the T005 map edit; grep the enumeration set to confirm full sweep coverage + +## Acceptance + +### Functional Completeness + +- [x] A-001 R1: `src/kit/skills/fab-adopt.md` exists as a thin orchestrator with the correct frontmatter (`helpers: [_srad, _generation, _review, _pipeline]`) and Steps 0–6 implementing the intake design +- [x] A-002 R2: `/fab-adopt` Step 0 STOPs (no mutation) on detached HEAD, default branch, MERGED PR, branch-already-mapped-to-a-change, and empty diff +- [x] A-003 R3: intake + thin plan are generated in one main-session pass with a human-confirmation checkpoint; the change is created via `fab change new --slug` and activated +- [x] A-004 R4: the state transition uses `skip apply` + `reset review {driver}` only (no new Go transition) and yields `apply=skipped, review=active, downstream pending` +- [x] A-005 R5: review dispatches outward-only, hydrate runs verbatim, ship retrofits Meta, the change lands in review-pr; an honest-state summary prints +- [x] A-006 R6: `_review.md` documents `mode` (`full` default | `outward-only`), no `inward-only`; preconditions gated on `full`; dispatch selects sub-agent(s) by mode; outward-only zero findings → pass +- [x] A-007 R7: `_generation.md` documents the Intake-from-Diff and Plan-from-Diff procedures; the thin plan has plain-language requirements, all-`[x]` task/acceptance stubs, no R#/T#/A# scaffolding, and the three heading literals +- [x] A-008 R8: `git-pr.md` documents the body-retrofit path (render via `fab pr-meta`, apply with `gh pr edit --body-file -`, gated on body-lacks-`## Meta`, idempotent) +- [x] A-009 R9: `/fab-adopt` appears in `/fab-help` under the Planning group (frontmatter + `skillToGroupMap` entry) +- [x] A-010 R10: every SPEC mirror (`SPEC-fab-adopt.md` new, `SPEC-_review.md`, `SPEC-_generation.md`, `SPEC-git-pr.md`) and every aggregate spec (`skills.md`, `glossary.md`, `overview.md`, `user-flow.md`) references `/fab-adopt` / "adopt" + +### Behavioral Correctness + +- [x] A-011 R6: with `mode` omitted, all existing review callers (`/fab-continue`, `/fab-ff`, `/fab-fff`) behave identically to before (default `full`) + +### Edge Cases & Error Handling + +- [x] A-012 R8: a second `/git-pr` run against a PR whose body already has `## Meta` makes no edit (idempotent) +- [x] A-013 R6: outward-only with all `review_tools` disabled/unavailable returns zero findings and **passes** (best-effort, no hard block) + +### Code Quality + +- [x] A-014 Pattern consistency: new skill + SPEC follow the structure of sibling files (frontmatter, preamble-read line, Contents/Behavior, Key Properties; SPEC Summary + Flow) +- [x] A-015 No unnecessary duplication: `/fab-adopt` reuses `_pipeline.md` / `_generation.md` / `_review.md` / `/git-pr` as sub-agents rather than inlining their logic; no `inward-only` mode added speculatively +- [x] A-016 Canonical source only: all skill edits are under `src/kit/skills/` — no `.claude/skills/` edits (Constitution V; code-quality.md Anti-Patterns) +- [x] A-017 SPEC-mirror sync (documentation_accuracy): every edited `src/kit/skills/*.md` carries its `docs/specs/skills/SPEC-*.md` update in this change +- [x] A-018 Cross-references (cross_references): the enumeration sweep is complete — grep for `fab-clarify|fab-proceed` over `docs/specs/ src/kit/skills/` confirms `/fab-adopt` is added wherever siblings are listed (or a documented reason it is not) + +## Notes + +- Check items as you review: `- [x]` +- The Go change (T005) is a single-line `skillToGroupMap` entry — a deliberate, graded deviation from the intake's "no Go change" Impact note (see Assumptions). It requires no test or command-signature change; `fabhelp_test.go` asserts a subset (`expectedMapped`), not a fixed total. + +## Deletion Candidates + +None — this change adds new functionality (a new `/fab-adopt` orchestrator skill, an additive `mode: outward-only` review path, and an additive `/git-pr` Step 3d body-retrofit branch) without making existing code redundant. The `mode` parameter defaults to `full`, so no existing review path is superseded; Step 3d is a new branch on the existing-OPEN-PR path, not a replacement; and the two `_generation.md` -from-Diff procedures sit alongside (do not replace) the forward Intake/Plan Generation Procedures. + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Confident | Add a one-line `"fab-adopt": "Planning"` entry to the Go `skillToGroupMap` despite the intake's "no Go change" Impact note | The New Skill Checklist (skills.md item 8) requires correct help grouping; without it the skill silently falls into the "Other" bucket — a visible drift review flags. The intake's "no Go change" was scoped to the state-composition and Meta-retrofit mechanics (Assumptions #8/#9), not the cosmetic help map. No test or command-signature change is needed (`fabhelp_test.go` checks a subset, not a total). | S:80 R:85 A:85 D:75 | +| 2 | Confident | Place `/fab-adopt` in the "Planning" help group (alongside fab-continue/ff/fff/proceed/clarify) | Adopt is a pipeline orchestrator like the other Planning entries; it is not a Start/Navigate or Completion skill | S:85 R:90 A:85 D:80 | +| 3 | Confident | Add an adoption row to `_preamble.md` § Next Steps State Table only if it cleanly fits; otherwise add a derivation note | The State Table keys on pipeline *state*, not entry skill; adoption reaches the same states (review/hydrate/ship/review-pr) so the existing rows already cover the `Next:` lookups — a note clarifying adoption as an alternate entry is the minimal correct change | S:75 R:80 A:80 D:70 | +| 4 | Confident | `/fab-adopt` `helpers:` = `[_srad, _generation, _review, _pipeline]` per the intake; `_srad` is included because the diff-reconstructed intake applies SRAD scoring | Matches the intake's explicit declaration and the planning-skill helper pattern (fab-ff/fff declare the same set) | S:85 R:85 A:85 D:80 | +| 5 | Tentative | Resolve the two Open Questions toward the intake's leanings: handle `pr_state == none` by letting Step 5's `/git-pr` create the PR fresh (no separate `--no-pr` flag); auto-rework applies when run autonomously but the interactive default hands findings back | The intake marks both as "resolve at apply" and states the leaning; a separate `--no-pr` flag is unneeded (the create path already handles it), and hand-back-by-default respects a hand-authored branch | S:70 R:70 A:70 D:65 | + +5 assumptions (0 certain, 4 confident, 1 tentative). diff --git a/src/go/fab/cmd/fab/fabhelp.go b/src/go/fab/cmd/fab/fabhelp.go index 0ea5ae7b..5e2a4d22 100644 --- a/src/go/fab/cmd/fab/fabhelp.go +++ b/src/go/fab/cmd/fab/fabhelp.go @@ -33,6 +33,7 @@ var skillToGroupMap = map[string]string{ "fab-fff": "Planning", "fab-proceed": "Planning", "fab-clarify": "Planning", + "fab-adopt": "Planning", "fab-archive": "Completion", "git-pr": "Completion", "git-pr-review": "Completion", diff --git a/src/kit/skills/_generation.md b/src/kit/skills/_generation.md index 83b57c25..9d3b99dc 100644 --- a/src/kit/skills/_generation.md +++ b/src/kit/skills/_generation.md @@ -1,6 +1,6 @@ --- name: _generation -description: "Artifact generation procedures — Intake Generation (used by fab-new, fab-draft, and fab-continue's intake regeneration) and Plan Generation (used by fab-continue, fab-ff, fab-fff at apply entry)." +description: "Artifact generation procedures — Intake Generation (used by fab-new, fab-draft, and fab-continue's intake regeneration), Plan Generation (used by fab-continue, fab-ff, fab-fff at apply entry), and the diff-based Intake-from-Diff + Plan-from-Diff procedures (used by fab-adopt to reconstruct artifacts from an existing branch diff)." user-invocable: false disable-model-invocation: true metadata: @@ -12,8 +12,10 @@ metadata: > `/fab-draft`, and `/fab-continue` (its intake-`active` regeneration row) follow the > **Intake Generation Procedure**; `/fab-continue`, `/fab-ff`, and `/fab-fff` follow the > **Plan Generation Procedure** (at apply entry) — `/fab-continue` belongs to both consumer -> groups. Each skill references these procedures instead of inlining them, ensuring -> generation behavior is authoritative in one location. +> groups. The two **-from-Diff** procedures (Intake-from-Diff, Plan-from-Diff) are the +> adoption variants used by `/fab-adopt` only — they reconstruct both artifacts from a fixed +> existing branch diff rather than from a forward design. Each skill references these procedures +> instead of inlining them, ensuring generation behavior is authoritative in one location. > > **Orchestration** (stage guards, question handling, design decisions, resumability) > remains in each skill's own file. This partial covers only the mechanics of producing each artifact. @@ -22,6 +24,8 @@ metadata: - Intake Generation Procedure - Plan Generation Procedure +- Intake-from-Diff Procedure +- Plan-from-Diff Procedure --- @@ -151,3 +155,79 @@ metadata: are required at generation time. Skills that wish to assert the counts explicitly MAY call `fab status set-acceptance ` (valid fields: `generated`, `task_count`, `acceptance_count`, `acceptance_completed`). + +--- + +## Intake-from-Diff Procedure + +> **Adoption variant** (used by `/fab-adopt` only). Where the Intake Generation Procedure +> reconstructs a *forward* design from a conversation, this procedure reverse-engineers the +> intake from a **fixed existing branch diff** plus the PR body — the code already exists, so +> the artifact *describes* it rather than *plans* it. Both this procedure and the Plan-from-Diff +> Procedure run in **one main-session pass** by the same agent reading the diff once (no dispatched +> sub-agent, no re-read between the two artifacts — a context boundary would only invite drift). + +**Inputs** (resolved by `/fab-adopt` Step 0, passed into this procedure): the diff against the +default-branch merge-base (`git diff {base}...HEAD`), the changed file list +(`git diff --name-only {base}...HEAD`), and the PR body / title when a PR exists (else the branch +name). + +1. Read the template from `$(fab kit-path)/templates/intake.md` and fill the metadata fields + (`{CHANGE_NAME}`, `{YYMMDD-XXXX-slug}`, `{DATE}`) as in the Intake Generation Procedure. +2. Fill each section **from the diff + PR body**, not from a design conversation: + - **Origin**: `adopted from {PR url or, if no PR, the branch name}` — state plainly that the + code was authored off-pipeline and is being brought in via `/fab-adopt`. + - **Why**: synthesise the problem/rationale from the PR body (its description, linked issues) + and what the diff actually does. When the PR body is thin, infer conservatively from the diff + and record the inference as a graded SRAD assumption. + - **What Changes**: synthesise per-area subsections from the changed files — group the diff by + directory/domain and describe what each area changed. This is descriptive (what the code + does), not prescriptive. + - **Affected Memory**: infer from which `docs/memory/` domains the changed paths map to (the + same domain mapping the Memory File Lookup uses). This list feeds hydrate — be accurate. + - **Impact**: derive from the changed paths (files touched, rough scale). Do not fabricate test + coverage the diff does not contain. +3. Apply SRAD per `_srad.md` and append the `## Assumptions` section (every inference made while + reading the diff is a graded row). Run `fab score` on the result. +4. Write the completed intake to `fab/changes/{name}/intake.md`. + +> **Human-confirmation checkpoint** (owned by `/fab-adopt`, noted here for context): the +> reconstructed intent + SRAD assumptions are presented for the user to confirm/correct before the +> pipeline advances. This is the late deliberation the bypass skipped. The checkpoint itself is +> orchestration and lives in `fab-adopt.md`, not in this procedure. + +--- + +## Plan-from-Diff Procedure + +> **Adoption variant** (used by `/fab-adopt` only). Apply was **skipped** for an adopted change — +> the code already exists and the apply↔review traceability loop never runs — so this plan is +> deliberately **minimal**. It exists for **one consumer**: hydrate, which reads `## Requirements` +> to write accurate memory. Effort concentrates entirely on plain-language requirements; the +> `## Tasks` / `## Acceptance` sections are all-`[x]` stubs that carry no traceability scaffolding. +> Runs in the same main-session pass as the Intake-from-Diff Procedure (no re-read of the diff). + +1. Read the template from `$(fab kit-path)/templates/plan.md` and fill the metadata fields + (`{CHANGE_NAME}`, `{YYMMDD-XXXX-slug}`, keep the `Intake` link). +2. Carry a header note immediately below the metadata, verbatim intent: + `> Adopted change — code authored off-pipeline. Apply was skipped; this plan is reverse-engineered from the branch diff to feed hydrate.` +3. **`## Requirements`** — a **plain-language** restatement of the intake's What-Changes, grouped + by change area. This is the only part hydrate reads, so it is the only part that needs care. + Largely lifts the intake's What-Changes into plan form. **Do NOT** emit `R#` IDs, RFC-2119 + ceremony, or GIVEN/WHEN/THEN scenarios — plain prose per area is correct and sufficient. +4. **`## Tasks`** — a single all-`[x]` stub, e.g.: + `- [x] Adopted: implementation authored outside the pipeline (see {PR or branch}).` + No `T{NNN}` IDs, no phases, no `[P]` markers, no `` traces. +5. **`## Acceptance`** — a single all-`[x]` stub, e.g.: + `- [x] Adopted: code already authored and (where applicable) PR-reviewed; outward review runs in this pipeline.` + No `A-{NNN}` IDs, no `R#` references. +6. **`## Assumptions`** — present per the artifact rule (footer `0 assumptions.` when none; the + diff-reading assumptions are recorded on the **intake**, not duplicated here). +7. Write the completed plan to `fab/changes/{name}/plan.md`. + +> **Parser-contract guarantee**: the only stable contract the pipeline parses is the three heading +> literals `## Requirements`, `## Tasks`, and `## Acceptance` (confirmed against +> `templates/plan.md`). The thin plan keeps all three headings, so every downstream reader +> (hydrate, the PostToolUse counters) parses it without special-casing. The R#/T#/A# IDs and +> GIVEN/WHEN/THEN scenarios are presentational scaffolding the apply↔review loop needs — and that +> loop never runs for an adopted change, so omitting them is correct, not a gap. diff --git a/src/kit/skills/_pipeline.md b/src/kit/skills/_pipeline.md index e10fa28e..b6c8c52d 100644 --- a/src/kit/skills/_pipeline.md +++ b/src/kit/skills/_pipeline.md @@ -1,6 +1,6 @@ --- name: _pipeline -description: "Shared ff/fff pipeline bracket — intake gate, apply → review → hydrate steps, auto-rework loop with explicit per-cycle choreography (cycle cap from code-review.md Rework Budget, default 3), and the exhaustion stop. Parameterized by driver name and terminal stage." +description: "Shared ff/fff pipeline bracket — intake gate, apply → review → hydrate steps, auto-rework loop with explicit per-cycle choreography (cycle cap from code-review.md Rework Budget, default 3), and the exhaustion stop. Parameterized by driver name and terminal stage. Full bracket used by /fab-ff and /fab-fff; /fab-adopt is a partial consumer (reuses the auto-rework loop + hydrate dispatch, not the full bracket)." user-invocable: false disable-model-invocation: true metadata: @@ -12,6 +12,11 @@ metadata: > skill (the **driver**) declares two parameters before executing this bracket — read them from > the driver's own file: > +> **Partial consumer**: `/fab-adopt` declares `_pipeline` as a helper but does NOT execute the +> full bracket — it runs its own Steps 0–6 and reuses only the § Auto-Rework Loop (for its +> outward-only review verdict) and Step 3's hydrate dispatch. It is not a `{driver}` of the full +> apply → review → hydrate sequence. +> > - **`{driver}`** — the driver name passed to the `fab status` event commands this bracket > shows it on (the fail/recovery commands are deliberately driver-less — see the Behavior > note below) and used in re-run guidance: `fab-ff` or `fab-fff` diff --git a/src/kit/skills/_preamble.md b/src/kit/skills/_preamble.md index 1aeeb724..aacf8bf9 100644 --- a/src/kit/skills/_preamble.md +++ b/src/kit/skills/_preamble.md @@ -242,6 +242,8 @@ Skills MUST end their output with a `Next:` line derived from the State Table be - **review (fail)**: `progress.review == failed` - **hydrate**: `progress.hydrate == done` +> **Adoption note**: `/fab-adopt` (bring an off-pipeline change in) needs no new State Table row. It enters the pipeline late with `apply: skipped` and an `active` review, then drives review → hydrate → ship → review-pr — all states the table already covers. `apply: skipped` is not itself a `Next:` lookup target: the lookup keys on the stage that is `active`/`ready`, which after adoption is review (then the normal tail). The skipped apply stage is simply passed over by the lookup, exactly as a `done` stage is. + ### Lookup Procedure 1. Determine the state reached after the skill's action diff --git a/src/kit/skills/_review.md b/src/kit/skills/_review.md index 14a9660f..8d26991f 100644 --- a/src/kit/skills/_review.md +++ b/src/kit/skills/_review.md @@ -1,6 +1,6 @@ --- name: _review -description: "Review behavior — inward sub-agent (plan requirements/acceptance) and outward sub-agent (Codex→Claude cascade with full repo access), dispatched in parallel during the review stage." +description: "Review behavior — inward sub-agent (plan requirements/acceptance) and outward sub-agent (Codex→Claude cascade with full repo access), dispatched in parallel during the review stage; a `mode` parameter (full | outward-only) selects which sub-agents run (full dispatches both in parallel; outward-only runs the outward sub-agent alone, used by fab-adopt)." user-invocable: false disable-model-invocation: true metadata: @@ -19,6 +19,7 @@ metadata: ## Contents +- Review Mode - Preconditions - Inward Sub-Agent Dispatch - Outward Sub-Agent Dispatch @@ -27,8 +28,25 @@ metadata: --- +## Review Mode + +The orchestrator MAY pass a **`mode`** parameter in the review dispatch. It selects which sub-agents run: + +| `mode` | Sub-agents dispatched | Preconditions checked | Used by | +|--------|-----------------------|-----------------------|---------| +| `full` (default — param omitted) | inward + outward | yes (see Preconditions) | `/fab-continue`, `/fab-ff`, `/fab-fff` | +| `outward-only` | outward only | no (skipped — nothing for inward to validate) | `/fab-adopt` | + +- **Default is `full`** when the param is omitted, so every existing caller is unaffected — the mode concept is purely additive. +- There is **no `inward-only` value** — no caller needs it today (parsimony). Do not add one speculatively. +- `mode` gates two steps below: **Preconditions** (checked only in `full`) and **Parallel Dispatch** (dispatches only the selected sub-agent(s)). **Findings Merge** and the pass/fail rule are identical for both modes — "any must-fix → fail" works with a single source, and an empty result set (zero findings) passes. + +--- + ## Preconditions +> **Gated on `mode` (see Review Mode).** These preconditions are checked **only** in `full` mode — they validate the inward sub-agent's inputs. In `outward-only` mode they are **skipped** entirely (there is no inward sub-agent and nothing for it to validate; the outward sub-agent reads the diff directly, not `plan.md`). + - `plan.md` MUST exist with both `## Tasks` and `## Acceptance` sections populated. If `## Acceptance` is missing, STOP with "plan.md missing Acceptance section." - All tasks under `## Tasks` MUST be `[x]`. If not: STOP with "{N} of {total} tasks are incomplete." @@ -147,16 +165,19 @@ Each finding includes: severity tier, description, and file:line reference where ## Parallel Dispatch -Both sub-agents (inward and outward) are dispatched **in parallel**. The orchestrator waits for both to return before proceeding to the Findings Merge step. +Dispatch the sub-agent(s) selected by `mode` (see Review Mode): + +- **`full`** (default): both sub-agents (inward and outward) are dispatched **in parallel**. The orchestrator waits for both to return before proceeding to the Findings Merge step. +- **`outward-only`**: dispatch the outward sub-agent only. There is no inward sub-agent to wait for; proceed to Findings Merge once the outward sub-agent returns. --- ## Findings Merge -After both sub-agents return, their findings are merged into a single prioritized set: +After the dispatched sub-agent(s) return, their findings are merged into a single prioritized set. In `outward-only` mode there is a single source (the outward sub-agent); the merge steps below operate on that one source, and the deduplication step (2) is a no-op. The pass/fail rule (4) is identical in both modes — including that **zero findings passes**, so an empty `outward-only` result (e.g. all `review_tools` disabled or unavailable) **passes** (best-effort; adoption must not hard-block when no external reviewer is available). -1. **Collect**: Gather all findings from both sub-agents -2. **Deduplicate**: If both sub-agents flag the same file:line issue, consolidate into a single finding (use the higher severity if they differ) +1. **Collect**: Gather all findings from the dispatched sub-agent(s) — both in `full` mode, the outward sub-agent only in `outward-only` +2. **Deduplicate**: If both sub-agents flag the same file:line issue, consolidate into a single finding (use the higher severity if they differ). (No-op in `outward-only` — a single source cannot collide with itself.) 3. **Merge by severity**: Combine into a unified must-fix / should-fix / nice-to-have list 4. **Pass/fail determination** (deterministic — no agent discretion): - If **any must-fix** finding exists (from either sub-agent) → review **fails** diff --git a/src/kit/skills/fab-adopt.md b/src/kit/skills/fab-adopt.md new file mode 100644 index 00000000..b00c74ab --- /dev/null +++ b/src/kit/skills/fab-adopt.md @@ -0,0 +1,191 @@ +--- +name: fab-adopt +description: "Adopt a completed off-pipeline change (a feature branch with an OPEN or not-yet-created PR, authored without fab) into the Fab pipeline — reconstruct intake + plan from the diff, run review/hydrate/ship/review-pr for real, with apply marked skipped." +helpers: [_srad, _generation, _review, _pipeline] +--- + +# /fab-adopt [] + +> Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding. + +--- + +## Contents + +- Purpose +- Arguments +- Behavior +- Output +- Error Handling +- Key Properties + +--- + +## Purpose + +Bring a **completed-but-off-pipeline** change into the Fab pipeline. The trigger is mid-flight adoption (scenario B): a feature branch whose code was authored **without** fab, with an **OPEN PR or no PR yet** — typically after a reviewer points out "you skipped fab-kit and might have missed a few checks." + +Of the six stages, exactly one — **apply** — cannot meaningfully re-run on an adopted change (the code already exists; there is nothing to generate). Every other stage *can* run for real, just *late*: intake reconstructed from the diff, review run on the diff before merge, hydrate writing memory before merge, ship retrofitting the PR decoration, review-pr resuming normally. So `/fab-adopt` is **not** a parallel "fake pipeline" — it is the *real* pipeline entered late, with apply skipped. + +`/fab-adopt` is a thin orchestrator: it reuses existing skills/procedures as sub-agents (the `/fab-proceed` / `/fab-ff` pattern) and introduces only what is genuinely new (the diff→intake and thin diff→plan procedures in `_generation.md`, and `_review.md`'s `outward-only` mode). + +**Scope** — `/fab-adopt` covers scenario B only. A **MERGED** PR (scenario A — retroactive backfill of already-merged work) is explicitly out of scope and STOPs at Step 0. + +--- + +## Arguments + +- **``** *(optional)* — the folder-name suffix for the reconstructed change. If omitted, derive it from the PR title (when a PR exists) or the branch name. Same slug semantics as `/fab-new`. + +--- + +## Behavior + +### Step 0 — Guards & diff base + +Reuse `/git-pr`'s guard idioms verbatim. Run these checks **before any mutation**; STOP (no `fab change new`, no status mutation) on any failure. + +```bash +branch=$(git branch --show-current) +default_branch=$(git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||') +[ -n "$default_branch" ] || default_branch=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null) +[ -n "$default_branch" ] || default_branch=$(git rev-parse --verify -q refs/remotes/origin/main >/dev/null && echo main || echo master) +gh pr view --json number,state,url 2>/dev/null || echo "NO_PR" +``` + +1. **Detached HEAD / default-branch guard** (reuse `/git-pr`'s guard idioms; the detached-HEAD message is verbatim, the default-branch message is adapted to the adopt context): + - `branch` empty (detached HEAD) → STOP: `Cannot ship from a detached HEAD — check out a branch first (run /git-branch).` + - `branch` is the default branch (or literal `main`/`master`) → STOP: `Cannot adopt from the default branch ({default_branch}).` +2. **PR-state guard**: from `gh pr view`: + - `state == MERGED` → STOP: `PR is already merged — that's retroactive backfill (scenario A), out of scope for /fab-adopt. Adopt operates on in-flight (open or not-yet-created) PRs.` + - `OPEN` and `none` (no PR) both **proceed**. Capture `{pr_state}`, `{pr_url}`, `{pr_number}` for later steps. +3. **Collision guard**: if a fab change already maps to this branch (`fab change resolve "$(git branch --show-current)" 2>/dev/null` succeeds), STOP: `Branch '{branch}' already maps to fab change '{name}' — it is already in the pipeline. Run /fab-continue (or /fab-fff) to advance it.` +4. **Resolve the diff base and capture the diff**: + ```bash + base=$(git merge-base HEAD "origin/$default_branch") + git diff "$base"...HEAD # the adopted diff + git diff --name-only "$base"...HEAD # the changed file list + ``` + If the diff is **empty** (no changed files) → STOP: `No diff against {default_branch} — nothing to adopt.` + +### Steps 1+2 — ONE main-session generation pass + +These run in the **main session** (the same agent, NOT a dispatched sub-agent) reading the diff + PR body once — both artifacts merely *describe one fixed existing diff*, so a context boundary between them would only invite drift and waste. + +1. **Create + activate the change**: `fab change new --slug {slug}` against the current branch, then activate it. The change branch already exists, so `/fab-new`'s Step 11 row 1 ("already active") or row 2 ("checked out") applies — do NOT recreate or rename the branch. +2. **Reconstruct `intake.md`** via the **Intake-from-Diff Procedure** (`_generation.md`), passing the diff, the changed-file list, and the PR body/title (or branch name). Apply SRAD and run `fab score`. +3. **Human-confirmation checkpoint**: present the reconstructed intent (Origin / Why / What Changes / Affected Memory) + the SRAD assumptions for the user to confirm or correct. This *is* the late deliberation the bypass skipped (it mirrors `/fab-new`'s interactive intake moment). On confirm: + ```bash + fab status advance {name} intake + fab status finish {name} intake # auto-activates apply + ``` +4. **Write a deliberately MINIMAL `plan.md`** via the **Plan-from-Diff Procedure** (`_generation.md`), from the *same* understanding (no re-read of the diff): plain-language `## Requirements` (the only part hydrate reads — concentrate effort here), an all-`[x]` `## Tasks` stub, an all-`[x]` `## Acceptance` stub, no R#/T#/A# scaffolding, plus the "Adopted change …" header note. + +### Step 2 (state) — apply → skipped, review → active + +Compose the honest state from **existing** transitions only — no Go change: + +```bash +fab status skip {name} apply # apply → skipped; cascades review/hydrate/ship/review-pr → skipped +fab status reset {name} review {driver} # review skipped → active; cascades hydrate/ship/review-pr back to pending +fab status set-summary {name} "adopted off-pipeline change; apply skipped" +``` + +`{driver}` is `fab-adopt`. Net state: `apply: skipped`, `review: active`, `hydrate/ship/review-pr: pending`. (Verified transition composition: `skip` is `{pending,active}→skipped` with forward cascade; `reset` accepts `skipped→active` and cascades its downstream back to `pending`.) + +### Step 3 — Review (dispatched, `mode: outward-only`) + +Resolve the review model: run `fab resolve-agent review --alias`, surface the resolved `model=/effort=`, and apply both halves (model via the Agent `model` param; effort via the imperative prompt instruction) per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. + +Dispatch `/fab-continue` Review Behavior as a sub-agent (Agent tool, `general-purpose`), passing **`mode: outward-only`** (the `_review.md` parameter). The prompt MUST include the standard subagent context files and **"do NOT run `fab status` commands; return results only"** — this orchestrator owns the verdict transition. Outward review reads `git diff {base}...HEAD` natively, so no file-set prompt hack is needed; inward preconditions (`plan.md` tasks all `[x]`) are skipped in `outward-only` mode. + +**Verdict** (owned here): +- **Pass** (no must-fix, including zero findings — outward-only with no available external reviewer passes best-effort): `fab status finish {name} review {driver}`. +- **Fail**: auto-rework per `_pipeline.md` § Auto-Rework Loop budget when run autonomously; when run interactively, **hand the findings back** to the user rather than auto-editing a hand-authored branch (the default for adopted code is hand-back — see Key Properties). On exhaustion, stop per the bracket's exhaustion rule. + +### Step 4 — Hydrate (dispatched, verbatim) + +Reuse `_pipeline.md` Step 3 unchanged: resolve the hydrate model, dispatch `/fab-continue` Hydrate Behavior as a sub-agent (same prompt contract — do NOT run `fab status`; return results only). This is the permanent-loss recovery — `docs/memory/` finally reflects what shipped. On success: `fab status finish {name} hydrate {driver}`. + +### Step 5 — Ship (retrofit Meta onto the existing PR) + +Dispatch `/git-pr {name}` (pass the **folder name**, not a bare id). Because the PR is OPEN (or `none`), `/git-pr` takes its existing-PR path (or creates the PR fresh when `pr_state == none`). Its **Step 3d body-retrofit** injects the `## Meta` block when the open PR's body lacks one (idempotent — gated on body-lacks-`## Meta`). `/git-pr` runs `finish ship` itself (best-effort), which auto-activates review-pr. + +### Step 6 — Land in review-pr + +After ship, `review-pr` is active. Print `Next: /git-pr-review`. The normal pipeline tail resumes from here. + +**Honest-state summary** the skill prints at the end: + +``` +Adopted {name}. + + intake ✓ ran (reconstructed from the diff) + apply — skipped (code authored off-pipeline) + review ✓ ran (outward-only) + hydrate ✓ ran (docs/memory/ updated) + ship ✓ ran (## Meta retrofitted onto the PR) + review-pr → active + +Only apply is skipped; every other stage genuinely ran (just late, after the code was written). +``` + +--- + +## Output + +``` +/fab-adopt — adopting {branch} into the pipeline + +--- Reconstruct --- +{intake reconstructed from diff; human-confirmation checkpoint} + +--- State --- +apply → skipped, review → active + +--- Review (outward-only) --- +{review output} + +--- Hydrate --- +{hydrate output} + +--- Ship --- +{git-pr output, incl. Meta retrofit} + +{honest-state summary} + +Next: /git-pr-review +``` + +--- + +## Error Handling + +| Condition | Action | +|-----------|--------| +| Detached HEAD | STOP: "Cannot ship from a detached HEAD — check out a branch first (run /git-branch)." | +| On default branch | STOP: "Cannot adopt from the default branch ({default_branch})." | +| PR already MERGED | STOP: scenario-A out-of-scope message (Step 0.2) | +| Branch already maps to a fab change | STOP: point at /fab-continue (already in the pipeline) | +| Empty diff against default branch | STOP: "No diff against {default_branch} — nothing to adopt." | +| `fab change new` failure | Surface stderr and STOP | +| Review fails (autonomous) | Auto-rework per `_pipeline.md` budget; on exhaustion stop with the per-cycle summary | +| Review fails (interactive) | Hand findings back to the user (default for hand-authored code); do not auto-edit | +| Hydrate fails | Surface the sub-agent's failure and STOP (review remains done; re-runnable) | +| `/git-pr` fails | Surface its error and STOP (review/hydrate already done; re-runnable) | + +--- + +## Key Properties + +| Property | Value | +|----------|-------| +| Idempotent? | Partially — Step 0 guards STOP cleanly before any mutation; the collision guard makes a re-run after the change is created route to `/fab-continue` rather than re-create. The dispatched stages (review/hydrate/ship) are themselves resumable/idempotent, and the Step 5 Meta retrofit is gated on body-lacks-`## Meta` | +| Advances stage? | Yes — reconstructs intake, marks apply `skipped`, then runs review → hydrate → ship → review-pr via existing transitions | +| Modifies `.fab-status.yaml`? | Yes — activates the reconstructed change (Step 1) | +| Modifies git state? | Indirectly — Step 5's `/git-pr` commits/pushes the reconstructed `fab/` artifacts and retrofits the PR body; `/fab-adopt` makes no commit itself | +| Go change? | None — state composed from existing `skip`/`reset` transitions; Meta retrofit reuses `fab pr-meta` + `gh pr edit` | + +--- + +Next: {derive at runtime per `_preamble.md` § Lookup Procedure — after ship the state is review-pr (active): `/git-pr-review`} diff --git a/src/kit/skills/git-pr.md b/src/kit/skills/git-pr.md index de89b9a2..f57bfdda 100644 --- a/src/kit/skills/git-pr.md +++ b/src/kit/skills/git-pr.md @@ -157,7 +157,7 @@ If the MERGED STOP did not fire, run each step in order, skipping steps that are Nothing to do. ``` -Before stopping, attempt to record the existing PR URL per Steps 4a–4c (silently, no errors). Then STOP. +Before stopping, run **Step 3d** (Meta retrofit — the body may still lack `## Meta` even when the code is already shipped), then attempt to record the existing PR URL per Steps 4a–4c (silently, no errors). Then STOP. (Step 3d is itself idempotent — a body that already has `## Meta` is a no-op — so the "already shipped" path stays a clean re-run.) **Otherwise**, print the header and execute: @@ -283,7 +283,33 @@ Print: ` ✓ push — origin/` Print: ` ✓ pr — ` -**If an OPEN PR already exists** (from Step 1), just print: ` ✓ pr — (existing)` +**If an OPEN PR already exists** (from Step 1), just print: ` ✓ pr — (existing)`, then run **Step 3d** (Meta retrofit). + +#### 3d. Retrofit `## Meta` onto an existing OPEN PR (if `{has_fab}` AND an OPEN PR already existed) + +`/git-pr` injects `## Meta` only on PR **create** (Step 3c). An OPEN PR authored off-pipeline (or created before fab adopted this branch) therefore has a body with **no `## Meta` block**. This sub-step closes that gap — it is the ship-stage Meta retrofit `/fab-adopt` relies on, but it is general: any OPEN-PR ship benefits. + +Gated on BOTH — skip the entire sub-step otherwise: + +- `{has_fab}` (Step 0) is true, AND +- the PR was **already OPEN** at Step 1 (`pr_state` = `OPEN`) — i.e. Step 3c did NOT just create it (a freshly created PR already carries `## Meta` from 3c, so retrofitting would be redundant). + +When both hold: + +1. Fetch the current PR body into `existing_body`: `existing_body=$(gh pr view --json body -q '.body')`. +2. **Idempotency guard**: if the body already contains a `## Meta` heading, make **no** edit — print nothing and continue (a second run is a no-op; Constitution III). +3. Otherwise render the Meta block via `fab pr-meta` (reusing Step 3c's mechanism — same `{name}`, resolved `{type}`, space-joined `{issues}`): + ```bash + META=$(fab pr-meta "{name}" --type {type} --issues "{issues}" 2>/dev/null) || META="" + ``` + - If exit non-zero or `META` is empty (no fab context / change unresolved / `.status.yaml` absent): make **no** edit and continue silently — same graceful degradation as Step 3c's Meta omission. +4. Prepend the rendered Meta block to the existing body (Meta first, then a blank line, then the original body verbatim) and apply it via stdin (avoids shell-quoting issues with multi-line bodies): + ```bash + printf '%s\n\n%s\n' "$META" "$existing_body" | gh pr edit --body-file - + ``` +5. If the `gh pr edit` fails → report the error and STOP. + +Print (ONLY when an edit was actually made): ` ✓ meta — retrofitted ## Meta onto existing PR` ### Step 4a: Record PR URL @@ -357,7 +383,7 @@ Derived from [Conventional Commits](https://www.conventionalcommits.org/en/v1.0. | Property | Value | |----------|-------| -| Idempotent? | Yes — re-run after ship is a no-op. The "already shipped" path (no uncommitted changes, no unpushed commits, an OPEN PR exists) re-records the existing PR URL silently and stops; `fab status add-pr` is idempotent and the Step 4c status commit is guarded by `git diff --cached --quiet`. Sub-step 3a-bis is gated on 3a having just committed this invocation, so a re-run skips it; even if reached it is byte-stable with the `git diff --quiet -- docs/memory` guard suppressing an empty commit (see 3a-bis, 4c guards) | +| Idempotent? | Yes — re-run after ship is a no-op. The "already shipped" path (no uncommitted changes, no unpushed commits, an OPEN PR exists) re-records the existing PR URL silently and stops; `fab status add-pr` is idempotent and the Step 4c status commit is guarded by `git diff --cached --quiet`. Sub-step 3a-bis is gated on 3a having just committed this invocation, so a re-run skips it; even if reached it is byte-stable with the `git diff --quiet -- docs/memory` guard suppressing an empty commit (see 3a-bis, 4c guards). Sub-step 3d (Meta retrofit onto an existing OPEN PR) is guarded on the body lacking a `## Meta` heading, so a second run is a no-op | | Advances stage? | Yes — ship (start/finish, best-effort) | | Modifies `.fab-status.yaml`? | No | | Modifies git state? | Yes — commit, push, PR creation |