From 0ee8b86fa593b6cd769a6af298ca32dd3229d321 Mon Sep 17 00:00:00 2001 From: Sahil Ahuja Date: Sat, 20 Jun 2026 18:44:55 +0530 Subject: [PATCH] =?UTF-8?q?docs:=20optimize=20skill=20files=20=E2=80=94=20?= =?UTF-8?q?dedup=20prose,=20add=20TOCs,=20sync=20SPEC=20mirrors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch /internal-skill-optimize pass over src/kit/skills/: - Content trims on 16 skills (>=80 lines): replace re-explanation of partial-owned concepts with references, collapse duplicated restatements, dedup error/arg rows. Behavior-preserving. - Add ## Contents TOC to all 23 skill files + 8 partials over 100 lines (incl. git-branch and single-section _intake per the mechanical rule). - SPEC mirrors (verify-then-touch): dated (260620-skop) prose-optimization annotation on each edited skill's SPEC; TOCs added to the 5 SPEC files over 100 lines. No behavioral SPEC sections changed. --- docs/specs/skills/SPEC-_generation.md | 2 + docs/specs/skills/SPEC-_intake.md | 2 + docs/specs/skills/SPEC-_pipeline.md | 2 + docs/specs/skills/SPEC-_preamble.md | 8 ++ docs/specs/skills/SPEC-_review.md | 7 + docs/specs/skills/SPEC-_srad.md | 2 + docs/specs/skills/SPEC-docs-hydrate-memory.md | 2 + docs/specs/skills/SPEC-docs-hydrate-specs.md | 2 + docs/specs/skills/SPEC-docs-reorg-memory.md | 2 + docs/specs/skills/SPEC-docs-reorg-specs.md | 2 + docs/specs/skills/SPEC-fab-archive.md | 2 + docs/specs/skills/SPEC-fab-clarify.md | 2 + docs/specs/skills/SPEC-fab-continue.md | 7 + docs/specs/skills/SPEC-fab-fff.md | 2 + docs/specs/skills/SPEC-fab-new.md | 2 + docs/specs/skills/SPEC-fab-operator.md | 14 ++ docs/specs/skills/SPEC-fab-proceed.md | 7 + docs/specs/skills/SPEC-fab-setup.md | 7 + docs/specs/skills/SPEC-fab-switch.md | 2 + docs/specs/skills/SPEC-git-branch.md | 2 + docs/specs/skills/SPEC-git-pr-review.md | 9 ++ docs/specs/skills/SPEC-git-pr.md | 7 + src/kit/skills/_cli-external.md | 10 ++ src/kit/skills/_cli-fab.md | 26 ++++ src/kit/skills/_generation.md | 5 + src/kit/skills/_intake.md | 4 + src/kit/skills/_pipeline.md | 7 + src/kit/skills/_preamble.md | 14 ++ src/kit/skills/_review.md | 8 ++ src/kit/skills/_srad.md | 10 ++ src/kit/skills/docs-hydrate-memory.md | 29 ++-- src/kit/skills/docs-hydrate-specs.md | 15 +- src/kit/skills/docs-reorg-memory.md | 22 ++- src/kit/skills/docs-reorg-specs.md | 27 ++-- src/kit/skills/fab-archive.md | 19 ++- src/kit/skills/fab-clarify.md | 29 ++-- src/kit/skills/fab-continue.md | 48 ++++--- src/kit/skills/fab-fff.md | 20 ++- src/kit/skills/fab-new.md | 9 ++ src/kit/skills/fab-operator.md | 37 ++--- src/kit/skills/fab-proceed.md | 27 ++-- src/kit/skills/fab-setup.md | 132 +++++++----------- src/kit/skills/fab-switch.md | 27 ++-- src/kit/skills/git-branch.md | 8 ++ src/kit/skills/git-pr-review.md | 22 +-- src/kit/skills/git-pr.md | 35 +++-- 46 files changed, 486 insertions(+), 198 deletions(-) diff --git a/docs/specs/skills/SPEC-_generation.md b/docs/specs/skills/SPEC-_generation.md index f49e34c7..a77ef11b 100644 --- a/docs/specs/skills/SPEC-_generation.md +++ b/docs/specs/skills/SPEC-_generation.md @@ -6,6 +6,8 @@ Shared artifact generation procedures used by five skills across two consumer gr This is an internal partial (`user-invocable: false`) — never invoked directly. Skills load it via `helpers: [_generation]` frontmatter. +**Prose optimization** (260620-skop): a `## Contents` TOC added to `_generation.md` (structural check, file >100 lines); no prose trimmed and no behavioral change (Flow unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-_intake.md b/docs/specs/skills/SPEC-_intake.md index 6f138203..c0b2ae64 100644 --- a/docs/specs/skills/SPEC-_intake.md +++ b/docs/specs/skills/SPEC-_intake.md @@ -25,6 +25,8 @@ This is the **only** behavioral fork in intake creation, and it is legitimately This is an internal partial (`user-invocable: false`, `disable-model-invocation: true`, `metadata: internal: true`) — never invoked directly. Canonical source is the flat `src/kit/skills/_intake.md`; `fab sync` deploys it to `.claude/skills/_intake/SKILL.md`. +**Prose optimization** (260620-skop): a single-entry `## Contents` TOC added to `_intake.md` (structural check, file >100 lines, applied mechanically though the file has one `##` section); no prose trimmed and no behavioral change (Flow unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-_pipeline.md b/docs/specs/skills/SPEC-_pipeline.md index 983e3999..43141892 100644 --- a/docs/specs/skills/SPEC-_pipeline.md +++ b/docs/specs/skills/SPEC-_pipeline.md @@ -15,6 +15,8 @@ A third value, **`{max_cycles}`**, is defined by the bracket itself (260612-c5tr This is an internal partial (`user-invocable: false`) — never invoked directly. Drivers declare it via `helpers: [_pipeline]` frontmatter. +**Prose optimization** (260620-skop): a `## Contents` TOC added to `_pipeline.md` (structural check, file >100 lines); no prose trimmed and no behavioral change (Per-Cycle Rework Choreography / Flow unchanged). + ## Per-Cycle Rework Choreography (f071) Stated exactly once, in the Auto-Rework Loop. Every cycle (the initial Step 2 failure and each later re-review failure alike): diff --git a/docs/specs/skills/SPEC-_preamble.md b/docs/specs/skills/SPEC-_preamble.md index bd14187d..c78feeaf 100644 --- a/docs/specs/skills/SPEC-_preamble.md +++ b/docs/specs/skills/SPEC-_preamble.md @@ -1,11 +1,19 @@ # _preamble +## Contents + +- Summary +- Subsection Inventory +- Flow + ## Summary Shared context preamble loaded by every Fab skill. Defines path conventions, context loading layers (always-load — descriptive, with a skill-file-wins override and a derived, never-enumerated exception set; change context; memory lookup; source code), the **Skill Helper Declaration** frontmatter convention (including stage-conditional in-body loading), inlined **Naming Conventions**, inlined **Run-Kit (rk) Reference**, the **Common fab Commands** headline table, the next-steps convention (with a skill-file-declared ending opt-out) with state table, a pointer to the skill invocation protocol (defined in `fab-clarify.md` since 260611-zc9m), subagent dispatch pattern with standard subagent context and **Per-Stage Model Resolution** (260613-l3ja — `fab resolve-agent ` before each pipeline-stage dispatch; resolved model+effort passed to the Agent dispatch with empty ⇒ omit/inherit; review resolves once for both reviewers + merge; per-stage selection applies on every post-intake stage — every post-intake stage now dispatches a sub-agent (including plain `/fab-continue` as a one-stage sequencer), so `fab resolve-agent` applies uniformly across apply/review/hydrate, with the residual advisory case narrowed to a stage skill genuinely run with no dispatch at all (260613-fgxx); **the two halves dispatch through two seams (260613-m3d4)** — model via the Agent tool `model` param (a hard enum of short aliases `opus`/`sonnet`/`haiku`/`fable`, resolved with `fab resolve-agent --alias` so the alias is emitted directly — the deterministic Agent-tool adapter that replaced the earlier prompt-side id→alias hand-mapping; 260613-yky7) and effort via an explicit imperative instruction in the subagent prompt (the Agent tool has no effort param; omitted when empty), plus a **compliance-visibility** expectation that each site surface the resolved `model=/effort=` so a skipped/mis-resolved tier is visible rather than silent; resolution itself stays provider-neutral; the lone residual is a first-class per-sub-agent effort param on the Agent tool — a harness ask outside fab's control), a pointer to the SRAD autonomy framework (extracted to `_srad.md` in 260611-zc9m), and slimmed confidence scoring (gate threshold + invocation; schema/formula/template moved to `_cli-fab.md` § fab score). This is an internal partial (`user-invocable: false`) — it is never invoked directly. Skills reference it via the opening instruction: "Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding." +**Prose optimization** (260620-skop): a `## Contents` TOC added to `_preamble.md` (structural check, file >100 lines); no prose trimmed and no behavioral change (Flow / Subsection Inventory unchanged). This SPEC also received a `## Contents` block under the same structural check. + ## Subsection Inventory Post-260418-or0o, `_preamble.md` contains four additional subsections inlined from previously-separate helpers or lifted out of `_cli-fab.md`. Each is a canonical source within `_preamble`: diff --git a/docs/specs/skills/SPEC-_review.md b/docs/specs/skills/SPEC-_review.md index 5e96b2b1..a9502d2f 100644 --- a/docs/specs/skills/SPEC-_review.md +++ b/docs/specs/skills/SPEC-_review.md @@ -1,5 +1,10 @@ # _review +## Contents + +- Summary +- Flow + ## 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. @@ -8,6 +13,8 @@ This is an internal partial (`user-invocable: false`) — it is never invoked di 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"). +**Prose optimization** (260620-skop): a `## Contents` TOC added to `_review.md` (structural check, file >100 lines); no prose trimmed and no behavioral change (Flow unchanged). This SPEC also received a `## Contents` block under the same structural check. + ## Flow ``` diff --git a/docs/specs/skills/SPEC-_srad.md b/docs/specs/skills/SPEC-_srad.md index 70e7eac2..a2d41333 100644 --- a/docs/specs/skills/SPEC-_srad.md +++ b/docs/specs/skills/SPEC-_srad.md @@ -6,6 +6,8 @@ SRAD autonomy framework helper — the decision framework planning skills apply Extracted from `_preamble.md` § SRAD Autonomy Framework in 260611-zc9m (the preamble keeps a 3-line pointer). This is an internal partial (`user-invocable: false`) — never invoked directly. It is loaded via the frontmatter `helpers:` list of the six planning skills: `fab-new`, `fab-draft`, `fab-continue`, `fab-ff`, `fab-fff`, `fab-clarify`. Non-planning skills do not load it. +**Prose optimization** (260620-skop): a `## Contents` TOC added to `_srad.md` (structural check, file >100 lines); no prose trimmed and no behavioral change (Flow unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-docs-hydrate-memory.md b/docs/specs/skills/SPEC-docs-hydrate-memory.md index bf23196c..f218ac03 100644 --- a/docs/specs/skills/SPEC-docs-hydrate-memory.md +++ b/docs/specs/skills/SPEC-docs-hydrate-memory.md @@ -16,6 +16,8 @@ Distinct from generate mode (which *creates* files from source-code gaps), backf - **Caller-aware regen**: when dispatched by reorg (defer-regen signal in the prompt), it does NOT run `fab memory-index` (reorg runs it once at the end); when invoked directly, it runs `fab memory-index` as the final step like the other modes. - **Scope**: pure frontmatter operation — does NOT detect/relocate tombstone rows, flatten groupings, move files, or strip existing `## Changelog` bodies; those structural concerns belong to `/docs-reorg-memory` (and the changelog strip to FKF Change 4). +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (the `templates/memory.md` read + FKF frontmatter + no-`## Changelog` rule + shape bounds now stated once in ingest Step 3 and referenced from generate/backfill; refuse-before-regen guard and arg-classification reject strings compressed to pointers) and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-docs-hydrate-specs.md b/docs/specs/skills/SPEC-docs-hydrate-specs.md index 45172f01..59608d41 100644 --- a/docs/specs/skills/SPEC-docs-hydrate-specs.md +++ b/docs/specs/skills/SPEC-docs-hydrate-specs.md @@ -4,6 +4,8 @@ Reverse hydration: identifies gaps where memory covers topics that specs don't. Proposes concise additions to specs with per-gap user confirmation. Top 3 gaps ranked by impact. When no existing spec is a suitable home for a gap, it proposes a new `docs/specs/{kebab-topic}.md` under the same per-gap confirmation; on yes it creates the file and adds its `docs/specs/index.md` row — the one index edit the skill makes. +**Prose optimization** (260620-skop): skill content trimmed (the Pre-flight index-existence checks compressed to one line while keeping both literal STOP strings, which the Error Handling rows reference) and a `## Contents` TOC added; the Step 6 token handler, the no-target/new-file branch, and the one-index-edit note are unchanged. No behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-docs-reorg-memory.md b/docs/specs/skills/SPEC-docs-reorg-memory.md index 903dec9b..30f89b65 100644 --- a/docs/specs/skills/SPEC-docs-reorg-memory.md +++ b/docs/specs/skills/SPEC-docs-reorg-memory.md @@ -4,6 +4,8 @@ Analyzes memory files for themes and suggests reorganization. Read-only unless user approves changes. Identifies up to 10 themes, produces a **Shape Report** (folders over the ~12 width bound / over depth — folder depth >2, i.e. topic files past 3 path segments / under the ~5 floor; reserved domains `_shared`/`_unsorted` exempt), and proposes a reorg plan whose Migration Map `Kind` is one of `move-section` / `split-domain` / `merge-domain` / `flatten` / `move`. On approval it is the **memory rebalancer**: it performs the approved migrations, rewrites the **bundle-relative** links the moves break, preserves each moved file's FKF frontmatter, authors `description:` frontmatter (+ `type: memory`) on new files and creates each new sub-domain's `description:`-only index stub before `fab memory-index` runs, and regenerates indexes (and per-folder `log.md`) via `fab memory-index`. Index/log files (root, domain, and sub-domain tiers) are regenerated by `fab memory-index`, never hand-edited. It also **orchestrates a one-time compatibility migration** for pre-fab-kit memory trees (see below). +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (the §7 bundle-relative payoff, the "reorg authors ONE mechanical file / synthesis lives in backfill" delegation, the older-binary fallback, and the "indexes are generated, not hand-authored" rule now stated once and referenced elsewhere) and a `## Contents` TOC added; no behavioral change — the 5-step apply, strict-order compatibility orchestration, no-dangling-link hard block, and decline=no-mutation logic are unchanged (Flow / Tools / Sub-agents unchanged). + ## FKF-Aware Moves (`$(fab kit-path)/reference/fkf.md` §7) Memory↔memory links are **bundle-relative** (`](/{domain}[/{sub}]/{file}.md)`, resolved from `docs/memory/`), so a move's link blast radius is small: only links whose **bundle path changes** (links *to* a file that changes domain/sub-domain) need rewriting; a same-domain reshuffle that keeps files in the domain rewrites nothing — the §7 payoff ("reorg rewrites *far* fewer links"). Two invariants the move path enforces: diff --git a/docs/specs/skills/SPEC-docs-reorg-specs.md b/docs/specs/skills/SPEC-docs-reorg-specs.md index 40144a5e..0b6d3812 100644 --- a/docs/specs/skills/SPEC-docs-reorg-specs.md +++ b/docs/specs/skills/SPEC-docs-reorg-specs.md @@ -8,6 +8,8 @@ Analyzes spec files for themes and suggests reorganization. Read-only unless use **No FKF frontmatter on spec moves (specs are human-curated — a fab-kit design principle).** FKF (`type: memory` + `description:`) governs `docs/memory/` only; specs are out of FKF scope and stay frontmatter-free, human-curated per that **fab-kit design principle**. When the skill moves a spec file it MUST NOT stamp, add, or synthesize `type:` / `description:` frontmatter — a moved spec carries exactly its prior bytes (only its path and the `index.md` row change). This is the deliberate mirror of `docs-reorg-memory`'s frontmatter-*preserving* moves: memory moves keep FKF frontmatter, spec moves add none. A generated-index model for `docs/specs/index.md` is **not adopted** (specs are human-curated — a fab-kit design principle) — no `fab specs-index` generator; the specs index stays hand-rewritten and spec links stay ordinary repo-relative. +**Prose optimization** (260620-skop): skill content trimmed — the three reserved-path / no-backfill / no-FKF blockquotes collapsed to one consolidated `### Reserved Paths` blockquote stating the human-curated principle plus its three rules, with Step 5 and the Key-Properties rows reduced to back-refs — and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-archive.md b/docs/specs/skills/SPEC-fab-archive.md index 3df578ab..7c7f363d 100644 --- a/docs/specs/skills/SPEC-fab-archive.md +++ b/docs/specs/skills/SPEC-fab-archive.md @@ -8,6 +8,8 @@ Archives a completed change (post-hydrate) or restores an archived change. Deleg Since 260611-szxd (f087) the skill file is a **single document**: mode detection and both argument lists are stated once at the top; archive mode is the default body; restore-specific content lives in a `## Restore Mode` section holding only its unique Behavior/Output/Error-Handling/Key-Properties. The restore pre-flight is preserved as mode-specific content — it **waives** the standard preflight and the hydrate guard (opposite of archive mode; restore applies to any archived change regardless of state). +**Prose optimization** (260620-skop): skill content trimmed to dedupe the dirty-tree disclosure (Key-Properties "Leaves uncommitted changes?" rows now reference the § Purpose blockquote) and to point the restore Step-1 error narration at § Error Handling, and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-clarify.md b/docs/specs/skills/SPEC-fab-clarify.md index bc0c9902..4418e7e5 100644 --- a/docs/specs/skills/SPEC-fab-clarify.md +++ b/docs/specs/skills/SPEC-fab-clarify.md @@ -6,6 +6,8 @@ Refines the intake artifact without advancing. Two modes: Suggest (interactive, **Helpers**: Declares `helpers: [_srad]` in frontmatter per `docs/specs/skills.md § Skill Helpers`. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts — the post-intake/missing-intake STOP messages now point to the canonical Error Handling table, the re-grade-by-composite rule is stated once (Step 2's Artifact Update) and referenced from Step 4, the shared audit-trail placement/append rule is stated once and referenced, and the dormant "retained for future use" statement is consolidated to § Skill Invocation Protocol → Currently Applicable; a `## Contents` TOC added to the skill. No behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-continue.md b/docs/specs/skills/SPEC-fab-continue.md index 171d4979..e7060096 100644 --- a/docs/specs/skills/SPEC-fab-continue.md +++ b/docs/specs/skills/SPEC-fab-continue.md @@ -1,5 +1,10 @@ # fab-continue +## Contents + +- [Summary](#summary) +- [Flow](#flow) + ## Summary Advances through the 6-stage pipeline one step at a time. Each invocation handles the current stage's work and transitions to the next. Supports reset to a given stage (legacy `tasks`/`spec` targets error with a pointer to the `apply` and `intake` reset routes). Handles all six stages: intake (the only planning stage), apply (co-generates `plan.md` `## Requirements` + `## Tasks` + `## Acceptance` at entry then runs tasks), review (sub-agent), hydrate, ship (delegates to `/git-pr` behavior), and review-pr (delegates to `/git-pr-review` behavior). @@ -14,6 +19,8 @@ Advances through the 6-stage pipeline one step at a time. Each invocation handle **FKF hydrate prose** (260615-8fr5, 260616-2fm8): Hydrate Behavior authors memory files to the FKF contract — the shipped normative extract at `$(fab kit-path)/reference/fkf.md` (260616-frlo; mirror of the dev-repo design doc `docs/specs/fkf.md`). New memory files are created from the canonical memory-file template shipped at `$(fab kit-path)/templates/memory.md` (260616-2fm8) — read on demand the same way `_generation.md`/`_intake.md` read `$(fab kit-path)/templates/intake.md` — the single source of truth for the FKF frontmatter pair — `type: memory` (constant, §3.1) plus a curated `description:` one-liner (§3.2) — and the body skeleton; not `description:` alone. Hydrate no longer writes a per-file `## Changelog` section (§3.3): it records what changed once via `fab status set-summary {change} ""` (the C-lite `summary:` source line, §6.3, authored once at hydrate), which `fab memory-index` joins with git history to generate the per-folder `log.md` (§6). Memory↔memory cross-links use the bundle-relative `/...` form (§7); links out of the bundle stay repo-relative/absolute-URL. The "update existing" section list drops `Changelog` (now Requirements/Design Decisions only); the merge-without-duplication contract is unchanged. When hydrate edits an existing/legacy memory file missing `type: memory`, it stamps the constant in so the touched file becomes FKF-conforming (§2/§3.1 require `type: memory` on every memory file, stamped by every memory writer — not just on creation). This is FKF migration Change 3/4 — it stops *new* changelog writes; the strip of the 20 existing `## Changelog` sections is Change 4/4. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts — the ~5 near-identical "dispatched block / sequencer owns transitions" blockquotes collapsed to one canonical statement (Normal Flow Step 1's dispatch contract) plus per-section references, the per-stage model paragraphs reduced to references, the Step 3 procedure table folded into prose, and Hydrate Step 4's long paragraph reformatted as a bullet list (same content); a `## Contents` TOC added to both the skill and this SPEC. No behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-fff.md b/docs/specs/skills/SPEC-fab-fff.md index 5c63f7fe..f4d8996a 100644 --- a/docs/specs/skills/SPEC-fab-fff.md +++ b/docs/specs/skills/SPEC-fab-fff.md @@ -6,6 +6,8 @@ Full pipeline gated on the single intake gate (identical to fab-ff). Since 26061 **Helpers**: Declares `helpers: [_generation, _review, _srad, _pipeline]` in frontmatter per `docs/specs/skills.md § Skill Helpers`. +**Prose optimization** (260620-skop): skill content trimmed — the per-stage model recipe duplicated in the Step 4 and Step 5 preambles collapsed to one reference to the top-of-Behavior "Per-stage model" blockquote (matching `fab-ff.md`'s single-reference pattern), the Step 5 timeout outcome stated 3× reduced to one canonical literal in the Error Handling row (Step 5 and Output now reference it), and the synchronous-poll blockquote's rationale recap trimmed to a one-line pointer to `git-pr-review.md` while keeping the dispatch-seam MUST instruction itself; a `## Contents` TOC added. No behavioral change (Flow / Sub-agents / synchronous-poll directive / `{name}`-vs-`{id}` rule unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-new.md b/docs/specs/skills/SPEC-fab-new.md index d5a86688..8c4dfacd 100644 --- a/docs/specs/skills/SPEC-fab-new.md +++ b/docs/specs/skills/SPEC-fab-new.md @@ -12,6 +12,8 @@ Creates a new change from a natural language description, Linear ticket, or back **Helpers**: Declares `helpers: [_generation, _srad, _intake]` in frontmatter per `docs/specs/skills.md § Skill Helpers` (`_intake` added in 260613-3xaj; `_generation`/`_srad` kept declared directly, mirroring the `_pipeline` precedent where consumers declare underlying helpers alongside the orchestration helper). +**Prose optimization** (260620-skop): a `## Contents` TOC added to the skill file (>100 lines); no content trimmed (the skill is already thin post-`_intake` extraction) and no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-operator.md b/docs/specs/skills/SPEC-fab-operator.md index 202ecc8a..f9019967 100644 --- a/docs/specs/skills/SPEC-fab-operator.md +++ b/docs/specs/skills/SPEC-fab-operator.md @@ -1,5 +1,17 @@ # fab-operator +## Contents + +- Summary +- Section Structure +- Primitives +- Monitoring Tick +- Watches (§7) +- Auto-Nudge +- Autopilot +- Key Properties +- Resolved Design Decisions + ## Summary Standalone multi-agent coordination layer with proactive monitoring and auto-nudge. Runs in a dedicated tmux pane, observes all running fab agents across every session on its tmux server via `fab pane map --all-sessions`, routes commands via `tmux send-keys`, monitors progress via `/loop`, auto-answers routine agent questions, and drives autopilot queues through the full pipeline. @@ -12,6 +24,8 @@ Not a lifecycle enforcer — the operator coordinates across agents and proxies **Helpers**: Declares `helpers: [_cli-fab, _cli-external]` in frontmatter per `docs/specs/skills.md § Skill Helpers`. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (launcher degraded behavior → `_cli-fab.md` § fab operator + §9; state-file path/migration → §2/§9; tick-step field semantics → `_cli-fab.md` § fab pane map; `rk notify` contract → `_cli-external.md` § rk + `_preamble.md` § Run-Kit; implicit `--base` chaining defined once in Queue ordering) and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). A `## Contents` TOC was also added to this SPEC (>100 lines, structural rule). + --- ## Section Structure diff --git a/docs/specs/skills/SPEC-fab-proceed.md b/docs/specs/skills/SPEC-fab-proceed.md index 47d11219..536bc86e 100644 --- a/docs/specs/skills/SPEC-fab-proceed.md +++ b/docs/specs/skills/SPEC-fab-proceed.md @@ -1,5 +1,10 @@ # fab-proceed +## Contents + +- Summary +- Flow + ## Summary Context-aware orchestrator — detects pipeline state via a 5-step detection pipeline, runs prefix steps (create-intake via `_intake`, fab-switch, git-branch) as subagents, then delegates to `/fab-fff` via the Skill tool. No arguments, no flags — infers everything from context. Idempotent — re-running detects completed steps and skips them. Reads `_preamble.md` (per skill convention) but skips running preflight and defers project-context loading to `/fab-fff`. **Per-stage model** (260613-l3ja): the prefix steps are NOT pipeline stages and take no `fab resolve-agent` resolution (they dispatch at the inherited model); per-stage model selection belongs to the delegated `/fab-fff`, which resolves each of its own stages per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. @@ -8,6 +13,8 @@ Context-aware orchestrator — detects pipeline state via a 5-step detection pip Conversation context is the interpretive lens for any unactivated intakes: an unactivated intake is only resumed when it is clearly relevant to the current conversation or there is no competing conversation signal. An unrelated draft never hijacks the pipeline when the current conversation is about a different topic. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (Subagent Dispatch, the promptless-defer carve-out per `_srad.md`, gate-blocking mechanics per `_preamble.md` § Confidence Scoring, the `_intake`-stops-at-ready chaining) and a `## Contents` TOC added; a `## Contents` TOC was also added to this SPEC (>100 lines); no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-setup.md b/docs/specs/skills/SPEC-fab-setup.md index 5c164c74..1e294c15 100644 --- a/docs/specs/skills/SPEC-fab-setup.md +++ b/docs/specs/skills/SPEC-fab-setup.md @@ -1,9 +1,16 @@ # fab-setup +## Contents + +- [Summary](#summary) +- [Flow](#flow) + ## Summary Bootstraps a new project or manages config/constitution/migrations. Creates `fab/project/` files and — via `fab sync` — `docs/memory/`, `docs/specs/`, deployed skill copies, and gitignore entries. Safe to re-run. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts and consolidate the seven Migrations Output Format blocks to one canonical block plus exact-string variant notes, and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-fab-switch.md b/docs/specs/skills/SPEC-fab-switch.md index 1c2662b7..91577d8b 100644 --- a/docs/specs/skills/SPEC-fab-switch.md +++ b/docs/specs/skills/SPEC-fab-switch.md @@ -6,6 +6,8 @@ Switches the active change by creating the `.fab-status.yaml` symlink. Lists ava The status summary printed by `fab change switch` ends with `Next: {routing_stage} (via {default_command})`, where the command is the one that drives the routing stage (`intake`/`apply`/`review`/`hydrate` → `/fab-continue`, `ship` → `/git-pr`, `review-pr` → `/git-pr-review`), aligned with `/fab-status` and the `_preamble.md` state table; only when all stages are done/skipped does it collapse to `Next: /fab-archive` (post-review off-by-one fixed in 260612-k4ge). The `Stage:` line's `{state}` qualifier enumerates all six states the `display_state` derivation can emit — `active`, `failed`, `ready`, `done`, `skipped`, `pending` (260612-w7dp; the skill formerly documented only done/active/pending). +**Prose optimization** (260620-skop): skill content trimmed (Output prose compressed, the redundant `### Switch Flow` sub-section folded into Argument Flow, the "config.yaml not found → No impact" error row dropped as it duplicates Key Properties) and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-git-branch.md b/docs/specs/skills/SPEC-git-branch.md index 518e3350..1441eb31 100644 --- a/docs/specs/skills/SPEC-git-branch.md +++ b/docs/specs/skills/SPEC-git-branch.md @@ -4,6 +4,8 @@ Creates or switches to the git branch matching the active or specified change. Falls back to creating a standalone branch if the argument doesn't match any change — but an *ambiguous* multi-match STOPs with the candidate list instead of creating a junk branch (260612-g8st). Remote-only target branches are checked out with `--track` rather than recreated as divergent locals; a dirty tree adds a non-blocking carried-over note to create/rename reports. Does not modify fab state. +**Prose optimization** (260620-skop): a `## Contents` TOC added to the skill file (>100 lines, per the mechanical structural rule); no content trimmed (the skill is already lean) and no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Flow ``` diff --git a/docs/specs/skills/SPEC-git-pr-review.md b/docs/specs/skills/SPEC-git-pr-review.md index 7b4ea528..e73f29cd 100644 --- a/docs/specs/skills/SPEC-git-pr-review.md +++ b/docs/specs/skills/SPEC-git-pr-review.md @@ -1,9 +1,18 @@ # git-pr-review +## Contents + +- Summary +- Arguments +- Configuration +- Flow + ## Summary Processes PR review comments from any reviewer (human or bot). Fully autonomous — detects reviews, requests an automated Copilot review and polls up to 10 minutes for it to appear when no existing reviews are found, triages comments with disposition intent (fix/defer/skip), applies fixes, commits, pushes, and posts reply comments confirming outcomes. +**Prose optimization** (260620-skop): skill content trimmed to remove re-explanation of partial-owned concepts (the `fab change resolve` ID/substring/name forms) and to collapse the triplicated two-login and synchronous-poll restatements to a single statement each (the `copilot-pull-request-reviewer`-on-`reviews` predicate, the REST-not-GraphQL confirmation rule, the MUST-NOT-yield directive, and the n30u/u1m1 memory citation are all preserved), and a `## Contents` TOC added; no behavioral change (Flow / Tools / Sub-agents unchanged). + ## Arguments - **``** *(optional, 260612-w7dp)* — explicit change to target instead of the active one (any non-flag argument). Resolved transiently in Step 0 (`.fab-status.yaml` untouched); an explicit argument that fails to resolve STOPs (caller error), while argless failure proceeds with no change context. `/fab-fff` Step 5 passes the change folder name through (`/git-pr-review {name}` — folder names never collide with git-pr's type tokens, so both dispatches use the same form). diff --git a/docs/specs/skills/SPEC-git-pr.md b/docs/specs/skills/SPEC-git-pr.md index cd8b57fa..e1f3f76a 100644 --- a/docs/specs/skills/SPEC-git-pr.md +++ b/docs/specs/skills/SPEC-git-pr.md @@ -1,5 +1,10 @@ # git-pr +## Contents + +- Summary +- Flow + ## Summary Autonomously commits, pushes, and creates a draft GitHub PR. No prompts, no questions. Resolves PR type from status/intake/diff. Generates PR body from fab artifacts when available. Records PR URL in `.status.yaml`. @@ -12,6 +17,8 @@ Autonomously commits, pushes, and creates a draft GitHub PR. No prompts, no ques **Memory-index date-drift fix** (260620-o203): a new sub-step **3a-bis. Refresh Memory Indexes** sits between 3a (Commit) and 3b (Push). `fab memory-index` stamps each `index.md` row's "Last Updated" cell from `git log` (committed dates only); the hydrate-stage regen runs entirely pre-commit, so every file the change touched is stamped one regen behind until the content commit lands — a benign tier-1 `fab memory-index --check` drift at review-pr. 3a-bis closes the gap at the only pipeline position where `git log` already knows the change's real commit date: immediately after 3a commits, before 3b pushes. It regenerates byte-stably and, when `docs/memory/` actually drifted (`git diff --quiet -- docs/memory` non-zero), makes a **separate** `docs: refresh memory indexes` follow-up commit (never `--amend` — squash collapses the pair on merge); a no-drift regen produces no diff and no commit (Constitution III). It performs no push of its own — 3b's "if has_unpushed or just committed" trigger pushes both commits together. It is **gated on `{has_fab}` AND 3a-having-just-committed**, so it is a silent no-op for standalone `/git-pr` (`{has_fab}` false) and for the no-change re-run paths. On regen/commit failure it reports + STOPs with the 3a content commit intact (a benign stale-date index recoverable by re-running `fab memory-index` — never a torn state). +**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). + ## Flow ``` diff --git a/src/kit/skills/_cli-external.md b/src/kit/skills/_cli-external.md index 7a23d31b..368f496d 100644 --- a/src/kit/skills/_cli-external.md +++ b/src/kit/skills/_cli-external.md @@ -10,6 +10,16 @@ metadata: > Loaded by operator skills only (not part of the always-load layer). Documents non-fab CLI tools used for multi-agent coordination. +## Contents + +- Reference Model +- wt (Worktree Manager) +- idea (Backlog Manager) +- hop (Multi-Repo Navigator) +- tmux +- rk (run-kit) +- /loop + --- ## Reference Model diff --git a/src/kit/skills/_cli-fab.md b/src/kit/skills/_cli-fab.md index 0a1123e3..3ea2da55 100644 --- a/src/kit/skills/_cli-fab.md +++ b/src/kit/skills/_cli-fab.md @@ -10,6 +10,32 @@ metadata: > Loaded selectively via a skill's `helpers: [_cli-fab]` frontmatter. See `_preamble.md` § Common fab Commands for the 6 most-used commands (`preflight`, `score`, `log command`, `change`, `resolve`, `status`). This file documents the remaining commands and exhaustive flag details. +## Contents + +- Calling Convention +- fab change (extended subcommand details) +- fab status (extended subcommand details) +- fab score (extended) +- fab preflight (extended) +- fab log (extended) +- fab resolve (extended) +- fab resolve-agent +- fab hook +- fab pane +- fab doctor +- fab migrations-status +- fab kit-path +- fab shell-init +- fab impact +- fab pr-meta +- fab memory-index +- fab fab-help +- fab help-dump +- fab operator +- fab spawn-command +- fab batch +- Common Error Messages + --- ## Calling Convention diff --git a/src/kit/skills/_generation.md b/src/kit/skills/_generation.md index 3b25a1a1..83b57c25 100644 --- a/src/kit/skills/_generation.md +++ b/src/kit/skills/_generation.md @@ -18,6 +18,11 @@ metadata: > **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. +## Contents + +- Intake Generation Procedure +- Plan Generation Procedure + --- ## Intake Generation Procedure diff --git a/src/kit/skills/_intake.md b/src/kit/skills/_intake.md index 37223bad..a42a847b 100644 --- a/src/kit/skills/_intake.md +++ b/src/kit/skills/_intake.md @@ -36,6 +36,10 @@ metadata: > them. Mirror the proven `_pipeline.md` shape: shared body parameterized by one knob, call-site > tails stay in the call-site files. +## Contents + +- Create-Intake Procedure (Steps 0–9) + --- ## Create-Intake Procedure (Steps 0–9) diff --git a/src/kit/skills/_pipeline.md b/src/kit/skills/_pipeline.md index 5e48ff56..e10fa28e 100644 --- a/src/kit/skills/_pipeline.md +++ b/src/kit/skills/_pipeline.md @@ -23,6 +23,13 @@ metadata: > Output blocks and error rows) stays in each driver's own file. This partial is the single > authoritative source for everything the two drivers share. +## Contents + +- Pre-flight +- Context Loading +- Behavior +- Shared Error Handling + --- ## Pre-flight diff --git a/src/kit/skills/_preamble.md b/src/kit/skills/_preamble.md index ff0c10eb..1aeeb724 100644 --- a/src/kit/skills/_preamble.md +++ b/src/kit/skills/_preamble.md @@ -11,6 +11,20 @@ metadata: > This file defines shared conventions for all Fab skills. Each skill file should begin with: > ``Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding.`` +## Contents + +- Path Convention +- Context Loading +- Skill Helper Declaration +- Naming Conventions +- Run-Kit (rk) Reference +- Common fab Commands +- Next Steps Convention +- Skill Invocation Protocol (pointer) +- Subagent Dispatch (Orchestrator Skills) +- SRAD Autonomy Framework (pointer) +- Confidence Scoring + --- ## Path Convention diff --git a/src/kit/skills/_review.md b/src/kit/skills/_review.md index f7658150..14a9660f 100644 --- a/src/kit/skills/_review.md +++ b/src/kit/skills/_review.md @@ -17,6 +17,14 @@ metadata: > remains in each orchestrator's own file. This partial covers only the mechanics of dispatching > the review sub-agents and merging their findings. +## Contents + +- Preconditions +- Inward Sub-Agent Dispatch +- Outward Sub-Agent Dispatch +- Parallel Dispatch +- Findings Merge + --- ## Preconditions diff --git a/src/kit/skills/_srad.md b/src/kit/skills/_srad.md index 4361d54a..2072cc22 100644 --- a/src/kit/skills/_srad.md +++ b/src/kit/skills/_srad.md @@ -14,6 +14,16 @@ metadata: When generating artifacts, planning skills encounter decision points not explicitly addressed by user input. The SRAD framework provides a principled method for deciding when to ask, when to assume, and when to surface assumptions. +## Contents + +- SRAD Scoring +- Confidence Grades +- Critical Rule +- Skill-Specific Autonomy Levels +- Worked Examples +- Artifact Markers +- Assumptions Summary Block + --- ## SRAD Scoring diff --git a/src/kit/skills/docs-hydrate-memory.md b/src/kit/skills/docs-hydrate-memory.md index 390a3b06..f54f3d51 100644 --- a/src/kit/skills/docs-hydrate-memory.md +++ b/src/kit/skills/docs-hydrate-memory.md @@ -7,6 +7,19 @@ description: "Hydrate memory from external sources or generate from codebase ana > Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding. +## Contents + +- Purpose +- Pre-flight Check +- Context Loading +- Arguments +- Ingest Mode Behavior +- Generate Mode Behavior +- Backfill Mode Behavior +- Output +- Idempotency +- Error Handling + --- ## Purpose @@ -23,7 +36,7 @@ Mode is determined automatically by argument type (ingest/generate) or by the ex Index files (`index.md` at the root, domain, and sub-domain tiers) are **generated artifacts** — `fab memory-index` is their single writer. The one hand-curated field is the `description:` frontmatter (on topic files and on domain/sub-domain indexes). When a new domain or sub-domain is created, its `index.md` **stub** — only the `description:` frontmatter one-liner, nothing else — is created **before** `fab memory-index` runs; the command fills in the generated body and round-trips the description. Never hand-edit generated index rows or "Last Updated" cells. Both modes below follow this model. -> **Refuse-before-regen guard (destructive-loss).** Before any `fab memory-index` regeneration step below, consult `fab memory-index --check`: on **exit 2** (destructive loss — a curated description would regenerate to `—`, a tombstone row would drop, or a custom grouping would flatten), **refuse to regenerate** and surface the pointer `→ run /docs-reorg-memory to remediate (it relocates removal-history rows to _shared/removed-domains.md and backfills description: frontmatter via /docs-hydrate-memory) before regenerating.` (`/docs-reorg-memory` is the orchestrator for all three tier-2 categories — it relocates tombstone rows itself and dispatches *this* skill's backfill mode for the descriptions; backfill alone does NOT relocate tombstones.) This is the primary pre-fab-kit-tree entry point, so the guard protects the *first* regen of a legacy tree. **No-op on born-compatible fab-kit trees** — they are always exit 0/1, never 2, so the guard never fires (do not mistake it for dead code). It only ever fires on a pre-fab-kit tree reached via ingest/generate before the tree has been backfilled. (Backfill mode itself never destroys content — it only adds frontmatter — so when *it* runs the regen, the guard has by then become a no-op.) +> **Refuse-before-regen guard (destructive-loss).** Before any `fab memory-index` regeneration step below, consult `fab memory-index --check`: on **exit 2** (destructive loss — a curated description would regenerate to `—`, a tombstone row would drop, or a custom grouping would flatten), **refuse to regenerate** and surface the pointer `→ run /docs-reorg-memory to remediate (it relocates removal-history rows to _shared/removed-domains.md and backfills description: frontmatter via /docs-hydrate-memory) before regenerating.` (`/docs-reorg-memory` is the orchestrator for all three tier-2 categories — it relocates tombstone rows itself and dispatches *this* skill's backfill mode; backfill alone does NOT relocate tombstones.) **No-op on born-compatible fab-kit trees** (always exit 0/1, never 2 — not dead code); it fires only on a pre-fab-kit tree reached via ingest/generate before backfill. Backfill mode itself only adds frontmatter, never destroys, so by the time *it* regenerates the guard is already a no-op. --- @@ -57,9 +70,9 @@ Skips the always-load layer entirely (this section is the skill-file override th | Markdown file | Path ends `.md` | **Ingest** | | Folder | Resolves to existing directory | **Generate** | -**Mode disambiguation** — backfill is checked first: it is reached only by the explicit `backfill` keyword or a reorg dispatch, so it never collides with bare ingest/generate routing. The two are otherwise distinct by intent: **generate** *creates* memory files from source-code gaps; **backfill** *adds `description:` frontmatter to existing* memory files (no new content). All non-backfill arguments must classify to the same mode. **Mixed-mode → reject**: `Cannot mix ingest sources (URLs, .md files) with generate targets (folders). Run separately.` +**Mode disambiguation** — backfill is checked first: it is reached only by the explicit `backfill` keyword or a reorg dispatch, so it never collides with bare ingest/generate routing. The two are otherwise distinct by intent: **generate** *creates* memory files from source-code gaps; **backfill** *adds `description:` frontmatter to existing* memory files (no new content). All non-backfill arguments must classify to the same mode — **mixed-mode → reject** (Error Handling). -**Backfill takes no extra arguments** — backfill is an independent re-scan of `docs/memory/` with no caller manifest (see Backfill Mode Step 1), so any positional argument after the `backfill` keyword is meaningless. If `backfill` is the first argument **and** any further argument follows, **reject**: `backfill takes no arguments — it re-scans docs/memory/ itself. Run /docs-hydrate-memory backfill with no further arguments.` (The reorg-dispatch form never supplies extra args — it names only the operation.) +**Backfill takes no extra arguments** — it is an independent re-scan of `docs/memory/` with no caller manifest (see Backfill Mode Step 1), so any positional argument after the `backfill` keyword is meaningless; `backfill` first **and** any further argument → **reject** (Error Handling). The reorg-dispatch form never supplies extra args — it names only the operation. Folder paths must exist — abort with `Folder not found: {path}` if not. @@ -97,7 +110,7 @@ For each topic: ### Step 4: Regenerate Indexes (`fab memory-index`) -Run `fab memory-index` once. It deterministically regenerates the root (domains-only), every domain index, and every sub-domain index from folder contents + `description:` frontmatter + git dates — byte-stable and idempotent. Never hand-edit index rows or "Last Updated" cells; the command is the single writer. Any non-fatal shape warnings it prints to stderr are advisory (over-wide / over-deep folders). +Run `fab memory-index` once to regenerate the root (domains-only), every domain index, and every sub-domain index from folder contents + `description:` frontmatter + git dates (the single writer — see Index Ownership; never hand-edit rows or "Last Updated" cells). Any non-fatal shape warnings it prints to stderr are advisory (over-wide / over-deep folders). --- @@ -140,13 +153,11 @@ Cross-reference against existing memory — exclude already-covered areas. ### Step 3: Memory File Generation -For each selected gap: read **all source files** in scope, then synthesize into **one memory file per gap** using the canonical memory-file shape. **Read the shape from `$(fab kit-path)/templates/memory.md`** (the same on-demand template read `_generation.md`/`_intake.md` use for `$(fab kit-path)/templates/intake.md`) — it is the single source of truth for the FKF frontmatter pair (`type: memory` constant + curated `description:`, `$(fab kit-path)/reference/fkf.md` §3.1–§3.2) and the conventional body skeleton (Overview / Requirements + Scenario / Design Decisions, §3.3). Fill the scaffold from the analyzed code (Overview, Requirements as RFC 2119 statements derived from code not invented, Design Decisions where inferable) and strip the template's guidance comments. - -The template carries **no `## Changelog` section** — memory files no longer carry one (§3.3); change history lives in the per-folder generated `log.md` (§6, populated from git history + the `.status.yaml` `summary:` field). Any memory↔memory cross-link uses the bundle-relative `/...` form (§7). +For each selected gap: read **all source files** in scope, then synthesize into **one memory file per gap** from the canonical memory-file shape — **read `$(fab kit-path)/templates/memory.md`** and fill its full skeleton, per ingest Step 3 (FKF frontmatter pair `type: memory` + `description:`; **no `## Changelog`**; bundle-relative `/...` cross-links). Fill the scaffold from the analyzed code: Overview, Requirements as RFC 2119 statements derived from code not invented, Design Decisions where inferable; strip the template's guidance comments. Mark ambiguous inferences with `[INFERRED]` inline near the relevant requirement. -**Placement** follows the same rules as ingest-mode Step 3: target path is `docs/memory/{domain}/{topic}.md` (or `docs/memory/{domain}/{sub-domain}/{topic}.md`); create the domain folder and its `description:`-only index stub if needed — sub-domain stub likewise — before Step 4 runs (see Index Ownership); and the same shape bounds apply (~5–12 topic files per folder, max depth 3, a sub-domain only for a cohesive ≥8-file cluster, `_shared/`/`_unsorted/` width-exempt). +**Placement** follows ingest-mode Step 3 exactly: target path `docs/memory/{domain}/{topic}.md` (or `.../{sub-domain}/{topic}.md`); create the domain folder and its `description:`-only index stub (sub-domain stub likewise) before Step 4 runs (see Index Ownership); and the same **Shape bounds** apply (see ingest Step 3). ### Step 4: Regenerate Indexes @@ -170,7 +181,7 @@ For each discovered topic file missing `description:`: 1. Read the file's **own content** — Overview, first section, or `# H1` — and synthesize a concise one-line summary. 2. **Prefer a curated index row** where one maps to this file. If an existing hand-curated index file (e.g., a pre-fab-kit `index.md` whose rows line up file-by-file with the topic files) has a row whose description text describes this file, use that curated text as the source — it is higher quality than re-synthesis. -3. Write the FKF frontmatter as the **leading frontmatter block** of the file — the `type: memory` constant (`$(fab kit-path)/reference/fkf.md` §3.1) plus the synthesized `description:` (§3.2). Use the **frontmatter shape only** from `$(fab kit-path)/templates/memory.md` (the canonical `---\ntype: memory\ndescription: "..."\n---` block ingest/generate also draw from), so the backfilled file is FKF-conforming (§2 item 2). **Take only the frontmatter block from the template — never its body skeleton:** backfill is a pure-frontmatter operation and MUST **preserve the existing body byte-for-byte** — it only prepends or edits the leading frontmatter, never the content below it, and never imposes the template's Overview/Requirements/Design Decisions skeleton on an existing file. In particular, it does **NOT** strip any existing `## Changelog` section from the body (removing the 20 existing per-file changelogs is a separate change, the FKF migration trajectory documented in the dev-repo design doc `docs/specs/fkf.md` §10 item 2); backfill stays a pure frontmatter operation. +3. Write the FKF frontmatter (`type: memory` + synthesized `description:`, per ingest Step 3) as the **leading frontmatter block** — but take the **frontmatter shape only** from `$(fab kit-path)/templates/memory.md`, **never its body skeleton.** Backfill is a pure-frontmatter operation: **preserve the existing body byte-for-byte** — only prepend/edit the leading frontmatter, never the content below, never impose the template's Overview/Requirements/Design Decisions skeleton. In particular it does **NOT** strip an existing `## Changelog` section (removing the 20 existing per-file changelogs is a separate change — the FKF migration trajectory in the dev-repo design doc `docs/specs/fkf.md` §10 item 2). 4. **Skip files that already have a `description:`** — backfill never overwrites an existing one (and stamps `type: memory` only when it is adding the frontmatter for the first time). This makes a second pass a no-op (idempotency — a fab-kit design principle). ### Step 3: Create missing index stubs (stub-before-index) diff --git a/src/kit/skills/docs-hydrate-specs.md b/src/kit/skills/docs-hydrate-specs.md index 3eb8814f..631f1051 100644 --- a/src/kit/skills/docs-hydrate-specs.md +++ b/src/kit/skills/docs-hydrate-specs.md @@ -9,6 +9,18 @@ description: "Identify structural gaps between memory and specs, propose concise --- +## Contents + +- Purpose +- Arguments +- Pre-flight +- Context Loading +- Behavior +- Error Handling +- Key Properties + +--- + ## Purpose Detect structural gaps between `docs/memory/` and `docs/specs/` — topics memory covers but specs don't — and propose concise additions back to specs. Top 3 gaps ranked by impact, with exact markdown previews and per-gap user confirmation. @@ -25,8 +37,7 @@ This is the reverse of hydrate (specs → memory): hydrate-specs flows memory ## Pre-flight -1. `docs/memory/index.md` must exist. If not: STOP with `docs/memory/index.md not found. Run /fab-setup first.` -2. `docs/specs/index.md` must exist. If not: STOP with `docs/specs/index.md not found. Run /fab-setup first.` +Both index files must exist (see Error Handling); if one is missing, STOP with `{path} not found. Run /fab-setup first.` — i.e. `docs/memory/index.md not found. Run /fab-setup first.` or `docs/specs/index.md not found. Run /fab-setup first.` --- diff --git a/src/kit/skills/docs-reorg-memory.md b/src/kit/skills/docs-reorg-memory.md index b5c1c5a5..08279ca1 100644 --- a/src/kit/skills/docs-reorg-memory.md +++ b/src/kit/skills/docs-reorg-memory.md @@ -5,6 +5,16 @@ description: "Analyze memory files for themes and suggest reorganization. Read-o # /docs-reorg-memory +## Contents + +- Purpose +- Pre-flight +- Context Loading +- Behavior +- Output +- Error Handling +- Key Properties + --- ## Purpose @@ -133,7 +143,7 @@ List the tombstone candidates explicitly (with their source index and link targe `Kind` is one of: `move-section` (relocate a `##`/`###` block between files), `split-domain` (fan out an over-width folder into sub-domains), `merge-domain` (fold an under-floor folder into a sibling), `flatten` (reduce depth > 3), `move` (relocate a single file between domains/sub-domains without a split/merge/flatten). -For any `split-domain` / `merge-domain` / `flatten` / `move` row (any move-bearing migration), the proposal MUST also list, in a **Link Impact** note, **every bundle-relative link that would break** when files move. Under FKF the dominant case is links *to* a moved file (their path-after-`/` changes when the file changes domain/sub-domain); links *from* a moved file to its siblings only break when the *sibling's* bundle path also changes. Each entry pairs the current link with its rewrite, so the user sees the full blast radius before approving: +For any `split-domain` / `merge-domain` / `flatten` / `move` row (any move-bearing migration), the proposal MUST also list, in a **Link Impact** note, **every bundle-relative link that would break** when files move (dominant case: links *to* a moved file, whose path-after-`/` changes — per the §7 blockquote above). Each entry pairs the current link with its rewrite, so the user sees the full blast radius before approving: ``` ## Link Impact @@ -142,7 +152,7 @@ For any `split-domain` / `merge-domain` / `flatten` / `move` row (any move-beari - in `clarify.md`: `](/pipeline/runtime-agents.md#gc)` → `](/pipeline/runtime/runtime-agents.md#gc)` (anchor preserved) ``` -A bundle-relative link from a moved file to a sibling that did NOT move needs no rewrite — the sibling's bundle path is unchanged. This is the §7 payoff: a same-domain reshuffle that keeps files in the domain rewrites nothing; only files that actually change domain/sub-domain touch the link graph. If a move-bearing migration breaks zero links, state "Link Impact: none" explicitly so approval is informed. +A link from a moved file to a sibling that did NOT move needs no rewrite (the §7 payoff — see the FKF-aware-moves blockquote above). If a move-bearing migration breaks zero links, state "Link Impact: none" explicitly so approval is informed. Constraints: prefer fewer files per domain; preserve existing domain names where possible; keep files under ~300 lines; respect the Ideal Shape Bounds (split over-width, merge under-floor, flatten over-depth) but only when a genuine cluster justifies it; never touch `_shared`/`_unsorted`; say so if the current structure is fine. @@ -173,7 +183,7 @@ On approval, for each approved migration: 1. **Move files / sections, preserving FKF frontmatter.** Execute section moves (`move-section`) and file moves (`split-domain` / `merge-domain` / `flatten` / `move`) to their new paths. Use `git mv` semantics where possible to preserve history; a plain move is acceptable when `git mv` is unavailable. **A moved file keeps its FKF frontmatter (`type: memory` + `description:`) byte-for-byte** — moving never strips, regenerates, or re-stamps it (FKF §3; only `fab memory-index` round-trips index/log frontmatter, never topic-file `type:`). 2. **Rewrite bundle-relative links** broken by the move — every link in the proposal's **Link Impact** note. Edit each link's path-after-`/` to the moved file's new bundle path (`](/{new-domain}[/{sub}]/{file}.md)`), preserving any `#anchor`. A link to a sibling whose bundle path did NOT change needs no edit (§7). The guard below confirms no `](/…)` memory link dangles. 3. **Author `description:` frontmatter** — the single hand-curated index field. For any new topic file a split creates, add a `description:` frontmatter line (copy or synthesize from the source file's existing description) **alongside `type: memory`** (the FKF constant — stamp it on any genuinely new topic file so it is FKF-conforming; a *moved* file already carries it from step 1). For each **new sub-domain**, create a **stub `index.md` BEFORE `fab memory-index` runs (step 4)**: the stub is only the `description:` frontmatter block (a one-liner summarizing the cluster), nothing else — the same stub-before-index pattern as `/docs-hydrate-memory` (FKF §5 stub-before-index). Step 4's regeneration fills in the generated body and round-trips the description. -4. **Regenerate indexes (and logs)**: run `fab memory-index`. It rewrites the root, every domain `index.md`, AND every sub-domain `index.md` from folder contents (including the new sub-domain reference rows in the parent), and regenerates each folder's `log.md` (merging any `log.seed.md` seed input beneath the git-projected entries — FKF §6). Generated index/log content is never hand-edited — the `description:` frontmatter authored in step 3 (including the sub-domain stubs) is the only hand-curated part of any index file. +4. **Regenerate indexes (and logs)**: run `fab memory-index`. It rewrites the root, every domain `index.md`, AND every sub-domain `index.md` from folder contents (including the new sub-domain reference rows in the parent), and regenerates each folder's `log.md` (merging any `log.seed.md` seed input beneath the git-projected entries — FKF §6). Generated content is never hand-edited (see the Index-previews blockquote above) — the `description:` frontmatter from step 3 (including the sub-domain stubs) is the only hand-curated part. 5. **Verify (no-dangling-link guard).** Confirm no headings were lost AND no broken bundle-relative link remains. **A remaining dangling `](/…)` memory link is a hard block** — do NOT finalize that migration until every broken link is rewritten. Report any dangling link found and the file it is in. **Abort escape**: if a dangling link cannot be rewritten (its target is genuinely gone, or the correct target is ambiguous), abort that migration instead of blocking indefinitely — roll back its moves and link rewrites (restore original paths), re-run `fab memory-index`, report the rollback, and continue with the remaining approved migrations. Present a change summary after all approved migrations are finalized. @@ -211,7 +221,7 @@ If no changes needed: `Current structure is well-organized — no reorganization | File write/move fails during apply | Report error, roll back that migration, continue | | Content verification fails | Warn, show missing heading, ask to proceed | | `fab memory-index` unavailable (older binary) | Warn; fall back to hand-updating affected `index.md` files (legacy path) and tell the user to upgrade `fab` | -| `fab memory-index --check --json` loss tiers / `--json` unavailable (older binary) | Fall back to the **legacy prose compatibility detection** (Step 1 older-binary fallback) and warn the user to upgrade `fab` so detection becomes mechanical | +| `fab memory-index --check --json` loss tiers / `--json` unavailable (older binary) | See Step 1 older-binary fallback (legacy prose detection + upgrade warning) | | Broken bundle-relative link remains after a move | **Hard block** — report the dangling `](/…)` link; do not finalize that migration until it is rewritten. If it cannot be rewritten, take the abort escape defined in Step 5's Verify item (apply item 5): roll back that migration, regenerate indexes, continue with the rest | --- @@ -223,11 +233,11 @@ If no changes needed: `Current structure is well-organized — no reorganization | Advances stage? | No | | Requires active change? | No | | Idempotent? | Yes — a balanced tree proposes nothing; re-running `fab memory-index` is byte-stable; an already-converted tree re-runs as a no-op (no duplicate tombstone rows in `removed-domains.md`, backfill skips frontmatter-present files) | -| Modifies memory files? | Yes — moves + bundle-relative link rewrites, only with explicit confirmation. Moved files keep their FKF frontmatter (`type: memory` + `description:`) verbatim. On compatibility approval also authors the ONE mechanical file `docs/memory/_shared/removed-domains.md` (tombstone relocation); per-file `description:` synthesis is delegated to the `/docs-hydrate-memory` backfill sub-agent, not authored here | +| Modifies memory files? | Yes — moves + bundle-relative link rewrites, only with explicit confirmation. Moved files keep their FKF frontmatter (`type: memory` + `description:`) verbatim. On compatibility approval also authors the ONE mechanical file `docs/memory/_shared/removed-domains.md` (tombstone relocation); per-file `description:` synthesis is delegated to the backfill sub-agent, not authored here (Step 5 item 1) | | FKF-aware moves? | Yes — links are **bundle-relative** (`](/{domain}[/{sub}]/{file}.md)`, FKF §7); a move rewrites only links whose bundle path changes (far fewer than relative links would), preserves the moved file's `type: memory` + `description:` frontmatter, and stamps `type: memory` on any genuinely new topic file | | Requires config/constitution? | No | | Is the memory rebalancer? | Yes — supersedes any separate `/fab-rebalance-memory`; shape diagnosis + split/merge/flatten + the file-moving apply path live here | -| Orchestrates a pre-fab-kit compatibility migration? | Yes — detects divergence **mechanically** via `fab memory-index --check --json` (exit 2 + `losses[]`: `description`/`tombstone`/`grouping`; older-binary fallback = legacy prose detection); on approval relocates tombstones (`_shared/removed-domains.md`), dispatches `/docs-hydrate-memory` backfill mode as a sub-agent, then rebalances + regenerates once. Decline = report and stop, no mutation | +| Orchestrates a pre-fab-kit compatibility migration? | Yes — mechanical detection via `fab memory-index --check --json` (exit 2 + `losses[]`: `description`/`tombstone`/`grouping`; older-binary ⇒ legacy prose); on approval runs the strict-order remediation (relocate tombstones → backfill sub-agent → regenerate once) per Step 5's Compatibility orchestration. Decline = report and stop, no mutation | | Dispatches sub-agents? | Yes — `/docs-hydrate-memory` backfill mode (general-purpose sub-agent, standard subagent context) for per-file description synthesis during compatibility orchestration | | Link rewriting | Skill-driven (the agent edits links per the Link Impact list) — NOT a `fab` subcommand | | Indexes hand-edited? | No — regenerated by `fab memory-index` (domain + sub-domain tiers) | diff --git a/src/kit/skills/docs-reorg-specs.md b/src/kit/skills/docs-reorg-specs.md index 96f4d620..fe675735 100644 --- a/src/kit/skills/docs-reorg-specs.md +++ b/src/kit/skills/docs-reorg-specs.md @@ -7,17 +7,28 @@ description: "Analyze spec files for themes and suggest reorganization. Read-onl --- +## Contents + +- Purpose +- Pre-flight +- Context Loading +- Behavior +- Output +- Error Handling +- Key Properties + +--- + ## Purpose Read all spec files in `docs/specs/`, identify themes (up to 10), and propose a reorganization plan. Read-only by default — files only moved/rewritten with explicit user approval. ### Reserved Paths (exempt from reorganization) -`docs/specs/skills/SPEC-*.md` mirrors are constitution-pinned: their names derive mechanically from their `src/kit/skills/` sources (`SPEC-{source-filename}.md`), and the constitution requires every skill edit to update its mirror. Never propose renaming, moving, merging, or splitting them — they may be *read* for theme analysis, but a Migration Map row targeting a reserved path is invalid. - -> **No compatibility/backfill step for specs.** Unlike `/docs-reorg-memory` (which detects pre-fab-kit memory trees missing `description:` frontmatter and orchestrates a frontmatter backfill), `/docs-reorg-specs` has **no** compatibility or frontmatter-backfill step — and intentionally so. There is no specs-index generator (no counterpart to `fab memory-index`); the specs index is hand-rewritten (Step 5), so a spec missing frontmatter breaks nothing downstream — there is no compatibility contract to violate. Keeping specs human-curated is a fab-kit design principle. Do not "fix the asymmetry" by adding a specs backfill — it would invent a non-problem and push specs toward the generated-index model that human-curated principle rejects. - -> **No FKF frontmatter on specs — moves are frontmatter-neutral (specs are human-curated — a fab-kit design principle).** FKF (`type: memory` + `description:`) governs `docs/memory/` **only**; specs are out of FKF scope and stay frontmatter-free, human-curated per that **fab-kit design principle** (specs MUST NOT be auto-generated or tool-planted). When this skill moves a spec file, it MUST NOT stamp, add, or synthesize `type:` / `description:` frontmatter on it — a moved spec carries exactly the bytes it had before the move (only its path and the `index.md` row change). This is the mirror of `/docs-reorg-memory`'s frontmatter-*preserving* moves: memory moves keep FKF frontmatter, spec moves add none. There is no `fab specs-index` generator and a generated-index model for specs is **not adopted** (specs are human-curated — a fab-kit design principle) — specs links stay ordinary repo-relative, the index stays hand-rewritten. +> **Specs are human-curated (a fab-kit design principle), which drives three rules:** +> 1. **Reserved paths.** `docs/specs/skills/SPEC-*.md` mirrors are constitution-pinned: their names derive mechanically from their `src/kit/skills/` sources (`SPEC-{source-filename}.md`), and the constitution requires every skill edit to update its mirror. Never propose renaming, moving, merging, or splitting them — they may be *read* for theme analysis, but a Migration Map row targeting a reserved path is invalid. +> 2. **No compatibility/backfill step.** Unlike `/docs-reorg-memory` (which detects pre-fab-kit memory trees missing `description:` frontmatter and orchestrates a frontmatter backfill), `/docs-reorg-specs` has **no** compatibility or frontmatter-backfill step. There is no specs-index generator (no counterpart to `fab memory-index`); the specs index is hand-rewritten (Step 5), so a spec missing frontmatter breaks nothing downstream — there is no compatibility contract to violate, and no generated-index model for specs. Do not "fix the asymmetry" by adding a specs backfill — it would invent a non-problem and push specs toward the generated-index model the human-curated principle rejects. +> 3. **Frontmatter-neutral moves — no FKF on specs.** FKF (`type: memory` + `description:`) governs `docs/memory/` **only**; specs are out of FKF scope and stay frontmatter-free. When this skill moves a spec it MUST NOT stamp, add, or synthesize `type:` / `description:` frontmatter — a moved spec carries exactly the bytes it had before (only its path and the `index.md` row change). This mirrors `/docs-reorg-memory`'s frontmatter-*preserving* moves: memory moves keep FKF frontmatter, spec moves add none. Spec links stay ordinary repo-relative; the index stays hand-rewritten. --- @@ -80,7 +91,7 @@ Constraints: prefer fewer files, preserve existing names, keep files under ~300 Options: **Apply all**, **Cherry-pick** (select specific migrations), **Skip** (keep analysis only). -On approval: execute migrations (a moved spec keeps its exact bytes — **no FKF frontmatter is stamped**, per the "No FKF frontmatter on specs" note above — specs are human-curated, a fab-kit design principle), rewrite `docs/specs/index.md`, verify no headings lost, present change summary. +On approval: execute migrations (a moved spec keeps its exact bytes — no FKF frontmatter stamped, per Reserved Paths rule 3), rewrite `docs/specs/index.md`, verify no headings lost, present change summary. --- @@ -120,6 +131,6 @@ If no changes needed: `Current structure is well-organized — no reorganization | Advances stage? | No | | Requires active change? | No | | Idempotent? | Yes | -| Modifies spec files? | Yes — only with explicit confirmation; a moved spec keeps its exact bytes (no FKF frontmatter stamped — specs are out of FKF scope, a fab-kit design principle) | -| Stamps FKF frontmatter? | No — never adds `type:`/`description:` to a spec; no `fab specs-index` generator and a generated-index model for specs is not adopted (specs stay human-curated, hand-indexed — a fab-kit design principle) | +| Modifies spec files? | Yes — only with explicit confirmation; a moved spec keeps its exact bytes (no FKF stamped — Reserved Paths rule 3) | +| Stamps FKF frontmatter? | No — never adds `type:`/`description:` to a spec; no `fab specs-index` generator and no generated-index model for specs (Reserved Paths rules 2–3) | | Requires config/constitution? | No | diff --git a/src/kit/skills/fab-archive.md b/src/kit/skills/fab-archive.md index 5371b96a..6f9a892a 100644 --- a/src/kit/skills/fab-archive.md +++ b/src/kit/skills/fab-archive.md @@ -7,6 +7,17 @@ description: "Archive a completed change or restore an archived change — move > Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding. +## Contents + +- Purpose +- Arguments +- Pre-flight +- Context Loading +- Behavior +- Output +- Key Properties +- Restore Mode + --- ## Purpose @@ -111,7 +122,7 @@ Next: {per state table — initialized} |----------|-------| | Advances stage? | No — post-pipeline housekeeping | | Idempotent? | Yes — re-archive is a soft skip that still re-attempts the backlog mark (recovering a previously-failed mark); `fab change archive` marks the backlog idempotently (`already`) | -| Leaves uncommitted changes? | Yes — moved files + backlog/index edits, for the caller to commit (no autonomous commit step) | +| Leaves uncommitted changes? | Yes — moved files + backlog/index edits (see Dirty-tree disclosure in § Purpose) | | Modifies `.status.yaml`? | No (may update `last_updated`) | | Modifies `.fab-status.yaml`? | Yes — conditionally removes symlink (via command) | | Modifies `docs/memory/`? | No | @@ -139,9 +150,7 @@ Call `fab change restore` in a single invocation: fab change restore [--switch] ``` -Parse the structured YAML output for the report. If the command exits non-zero: -- **"Multiple archives match"** → list the matches from stderr, ask user to pick, re-run with the selected name -- **"No archive matches"** → call `fab change archive-list`, display available archives, inform user +Parse the structured YAML output for the report. On non-zero exit, handle per § Error Handling (multiple-match, no-match, etc.). #### Step 2: Format Report @@ -189,7 +198,7 @@ Next: {per state table — if --switch: restored change's state; otherwise: acti |----------|-------| | Advances stage? | No — post-archive housekeeping | | Idempotent? | Yes — the command detects already-restored folders | -| Leaves uncommitted changes? | Yes — moved files + archive-index edits, for the caller to commit (no autonomous commit step) | +| Leaves uncommitted changes? | Yes — moved files + archive-index edits (see Dirty-tree disclosure in § Purpose) | | Modifies `.status.yaml`? | No | | Modifies `.fab-status.yaml`? | Only with `--switch` flag (via the command) | | Modifies `docs/memory/`? | No | diff --git a/src/kit/skills/fab-clarify.md b/src/kit/skills/fab-clarify.md index 62375fae..6369462f 100644 --- a/src/kit/skills/fab-clarify.md +++ b/src/kit/skills/fab-clarify.md @@ -10,12 +10,25 @@ helpers: [_srad] --- +## Contents + +- [Purpose](#purpose) +- [Arguments](#arguments) +- [Pre-flight & Stage Guard](#pre-flight--stage-guard) +- [Suggest Mode (User Invocation)](#suggest-mode-user-invocation) +- [Skill Invocation Protocol](#skill-invocation-protocol) +- [Auto Mode](#auto-mode) +- [Error Handling](#error-handling) +- [Key Properties](#key-properties) + +--- + ## Purpose Deepen and refine the **intake** artifact (`intake.md`) without advancing. Clarification is an intake-only, human-facing activity: it is where the developer's decisions and disambiguation happen, 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`, not via this skill. Two modes: - **Suggest mode** (user invocation) — interactive question flow with recommendations -- **Auto mode** — autonomous resolution, returns machine-readable result (the protocol is retained for future use; no orchestrator currently invokes clarify automatically) +- **Auto mode** — autonomous resolution, returns machine-readable result (retained for future use; see § Skill Invocation Protocol → Currently Applicable) Mode determined by `[AUTO-MODE]` prefix (see § Skill Invocation Protocol below). Safe to call multiple times. @@ -25,7 +38,7 @@ Mode determined by `[AUTO-MODE]` prefix (see § Skill Invocation Protocol below) - **``** *(optional)* — target a specific change (see `_preamble.md` > Change-name override). `.fab-status.yaml` unchanged. -`/fab-clarify` operates only on `intake.md`. The legacy `spec`, `plan`, and `tasks` targets were removed: `spec` and `plan` no longer exist as clarify targets (the spec stage is gone; under-specified requirements at apply become inline SRAD assumptions, not clarify sessions). Any positional argument is treated as a change name. +`/fab-clarify` operates only on `intake.md`; the legacy `spec`/`plan`/`tasks` targets were removed (the spec stage is gone; under-specified requirements at apply become inline SRAD assumptions). Any positional argument is treated as a change name. --- @@ -34,7 +47,7 @@ Mode determined by `[AUTO-MODE]` prefix (see § Skill Invocation Protocol below) Run preflight per `_preamble.md` §2. - **Intake** is the only stage `/fab-clarify` operates at. With `intake` state `active` or `ready`, scan `intake.md` (scope boundaries, affected areas, blocking questions, impact, memory coverage). -- **Post-intake stages** (`apply`, `review`, `hydrate`, `ship`, `review-pr`): `/fab-clarify` does not apply. STOP with: "Clarification is intake-only. At apply or later, run /fab-continue for rework, or edit plan.md `## Requirements` directly. To re-clarify the intake, reset with /fab-continue intake first." If `intake.md` is missing entirely: STOP with "No intake.md found. Run /fab-new to create the intake first." +- **Post-intake stages** (`apply`, `review`, `hydrate`, `ship`, `review-pr`): `/fab-clarify` does not apply — STOP (see Error Handling for the message). If `intake.md` is missing entirely, STOP with the missing-intake message (Error Handling). --- @@ -42,7 +55,7 @@ Run preflight per `_preamble.md` §2. ### Step 1: Read Target Artifact -Read `intake.md`. If missing: STOP with "No intake.md found. Run /fab-new to create the intake first." +Read `intake.md`. If missing: STOP (missing-intake message, see Error Handling). ### Step 1.5: Taxonomy Scan @@ -119,7 +132,7 @@ For changed items, also update the Decision column with the user's new value. On #### Audit Trail -Append a `### Session {YYYY-MM-DD} (bulk confirm)` block under `## Clarifications` — same placement/append rules as Step 5: append to the existing `## Clarifications` section if present; create it (immediately before `## Assumptions`) if not; skip if 0 items were resolved: +Append a `### Session {YYYY-MM-DD} (bulk confirm)` block under `## Clarifications`. **Placement/append rule** (shared with Step 5): append to the existing `## Clarifications` section if present; create it (immediately before `## Assumptions`) if not; skip if 0 items were resolved. ```markdown ### Session {YYYY-MM-DD} (bulk confirm) @@ -145,12 +158,12 @@ Allow the user to accept the recommendation, pick an alternative, provide a free ### Step 4: Process Answer and Update 1. Update artifact in place: replace markers with resolved content, add `` for significant changes -2. Re-grade the resolved entry's row in the `## Assumptions` table **by recomputed composite, not by fiat** — same rule as bulk confirm (Step 2): set S → 95 (R, A, D unchanged), recompute the composite per `_srad.md` § SRAD Scoring, and assign the grade by its half-open thresholds. A direct answer typically lands the row in Certain, but a row whose recomputed composite stays below the Certain band keeps its banded grade — the grade is derived from the dimensions, never forced. +2. Re-grade the resolved entry's row in the `## Assumptions` table **by recomputed composite, not by fiat** — the same rule as Step 2's Artifact Update (set S → 95, R/A/D unchanged; recompute per `_srad.md` § SRAD Scoring; grade by the half-open thresholds). A direct answer typically lands the row in Certain, but a row whose recomputed composite stays below the Certain band keeps its banded grade. 3. Present next question or proceed to Step 5 after queue exhaustion / 5th answer / early termination ### Step 5: Audit Trail -Append `## Clarifications > ### Session {YYYY-MM-DD}` with Q&A pairs — same placement/append rules as Step 2's bulk-confirm trail: append to the existing `## Clarifications` section if present; create it (immediately before `## Assumptions`) if not; skip if 0 answers. +Append `## Clarifications > ### Session {YYYY-MM-DD}` with Q&A pairs — same placement/append rule as Step 2's bulk-confirm trail (skip if 0 answers). ### Step 6: Coverage Summary @@ -206,7 +219,7 @@ To add new mode signals, define new bracketed prefixes (e.g., `[BATCH-MODE]`) he ## Auto Mode -> **Note**: Bulk confirm (Step 2) is Suggest Mode only. Auto Mode skips it — there is no user to confirm with. No orchestrator currently invokes clarify automatically (the former `/fab-ff` and `/fab-fff` auto-clarify steps were removed in 1.10.0); this section is retained for future use and operates on `intake.md` only. +> **Note**: Bulk confirm (Step 2) is Suggest Mode only. Auto Mode skips it — there is no user to confirm with. Retained for future use only (see § Skill Invocation Protocol → Currently Applicable); operates on `intake.md` only. 1. **Read `intake.md`** (same as Suggest Step 1) 2. **Autonomous gap resolution**: Same intake taxonomy scan. Resolvable from context → resolve + ``. Needs user input → ``. Minor → leave as-is. diff --git a/src/kit/skills/fab-continue.md b/src/kit/skills/fab-continue.md index 90cccb8f..3d5a5411 100644 --- a/src/kit/skills/fab-continue.md +++ b/src/kit/skills/fab-continue.md @@ -12,11 +12,26 @@ helpers: [_srad] --- +## Contents + +- [Purpose](#purpose) +- [Arguments](#arguments) +- [Pre-flight](#pre-flight) +- [Normal Flow](#normal-flow) +- [Apply Behavior](#apply-behavior) +- [Review Behavior](#review-behavior) +- [Hydrate Behavior](#hydrate-behavior) +- [Reset Flow (with stage argument)](#reset-flow-with-stage-argument) +- [Error Handling](#error-handling) +- [Key Properties](#key-properties) + +--- + ## Purpose Advance through the 6-stage Fab pipeline one step at a time. Each invocation handles the current stage's work and transitions to the next. When called with a stage argument, resets to that stage and re-runs from there. -> **Per-stage model (one-stage sequencer)**: post-intake `/fab-continue` is a **one-stage sequencer** — it dispatches its stage as a sub-agent (see Normal Flow Step 1), and per-stage model selection (`fab resolve-agent ` → `agent.tiers`, see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution) is resolved **once, immediately before that dispatch**, surfaced (so a skipped/mis-resolved tier is visible), and applied through both seams — model via the Agent tool's `model` param (resolved with `fab resolve-agent --alias` so the alias is Agent-tool-valid), effort via an imperative instruction in the sub-agent's prompt (empty effort ⇒ omit). There is no foreground execution path for apply/review/hydrate to leave the tier merely advisory: every post-intake stage runs dispatched, so `fab resolve-agent ` applies uniformly. (Intake is pre-boundary — it runs in the main session and is not tiered by `/fab-continue`.) +> **Per-stage model (one-stage sequencer)**: post-intake `/fab-continue` is a **one-stage sequencer** — it dispatches its stage as a sub-agent (see Normal Flow Step 1) and resolves that stage's model once immediately before the dispatch. Mechanics in Step 1's dispatch contract; selection rules in `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. Every post-intake stage runs dispatched, so the tier applies uniformly and is never merely advisory. (Intake is pre-boundary — it runs in the main session and is not tiered by `/fab-continue`.) --- @@ -43,7 +58,7 @@ Both may be provided in any order. Stage names are treated as reset targets; all Dispatch on preflight's derived `stage` and `display_state`. If progress is `pending`, run `fab status start fab-continue` before dispatching. **Review-failed dispatch**: if `progress.review` is `failed` (an exhausted `/fab-ff`/`/fab-fff` rework loop, or an interrupted fail→reset sequence), do NOT re-run review — use the `review`/`failed` row below: present the rework menu directly. (Orchestrator re-runs of `/fab-ff`/`/fab-fff` instead recover via `fab status start review` per `_pipeline.md` Resumability — that autonomous path is theirs, not this skill's.) **Review-pr-failed dispatch**: if `progress.review-pr` is `failed` (a failed PR-review run — `gh` missing, no PR found, or a processing error), use the `review-pr`/`failed` row below: re-execute `/git-pr-review` behavior — a FAILED PR review MUST NOT fall through to the "all `done`" row and read as complete. -**State-based dispatch**: Intake is the only planning stage, and the only stage `/fab-continue` runs in the main session. **Every post-intake stage (apply / review / hydrate) is dispatched as a sub-agent** — `/fab-continue` is a one-stage sequencer for those stages: it resolves the stage's model, dispatches the block, reads the returned status/findings, and owns the `fab status` transition itself. (Ship/review-pr delegate to `/git-pr` / `/git-pr-review`, which self-manage their transitions — see their rows.) The dispatch is the SAME one the orchestrators (`_pipeline.md`) perform; the sequencer/block split is identical whether the caller is manual `/fab-continue` or an orchestrator. +**State-based dispatch**: Intake is the only planning stage, and the only stage `/fab-continue` runs in the main session. **Every post-intake stage (apply / review / hydrate) is dispatched as a sub-agent** — `/fab-continue` is a one-stage sequencer for those stages (full contract below). (Ship/review-pr delegate to `/git-pr` / `/git-pr-review`, which self-manage their transitions — see their rows.) The dispatch is the SAME one the orchestrators (`_pipeline.md`) perform; the sequencer/block split is identical whether the caller is manual `/fab-continue` or an orchestrator. - **`ready`** (intake) → Finish intake (auto-activates apply), then run the apply sequencer (resolve + dispatch the apply sub-agent — its entry sub-step generates `plan.md`, then runs tasks — then finish apply) - **`active`** (intake) → Generate intake if missing and advance to `ready` (backward compat for interrupted generations) — main session, no dispatch @@ -72,19 +87,13 @@ Load per `_preamble.md` layers. Stage-specific additions: intake loads memory fi **Intake only** (main session — the only non-dispatched stage): Apply SRAD (`_srad.md`, loaded via `helpers:`) before generating. Budget: 1-2 unresolved questions. Tentative decisions get `` markers. (Inside apply, under-specified requirements are resolved inline as graded SRAD assumptions in `plan.md` `## Assumptions` — not as questions or markers.) -For post-intake stages the procedure below runs **inside the dispatched sub-agent** (per the dispatch contract in Step 1); the sub-agent reads the named Behavior section and any stage-conditional helper at its point of use. The sequencer does not run the procedure itself. - -| Stage | Procedure (runs in the dispatched sub-agent) | -|-------|-----------| -| apply | [Apply Behavior](#apply-behavior) — entry sub-step invokes **Plan Generation Procedure** (`_generation.md`, read at point of use), which co-generates `## Requirements` + `## Tasks` + `## Acceptance` | -| review | **Review Behavior** (`_review.md`, read at point of use) | -| hydrate | [Hydrate Behavior](#hydrate-behavior) | +**Post-intake stages** run their procedure **inside the dispatched sub-agent** (per the dispatch contract in Step 1), not in the sequencer: apply → [Apply Behavior](#apply-behavior) (entry reads `_generation.md` at point of use), review → [Review Behavior](#review-behavior) (reads `_review.md` at point of use), hydrate → [Hydrate Behavior](#hydrate-behavior). The sub-agent reads its Behavior section and any stage-conditional helper at its point of use. **No scoring at any stage `/fab-continue` runs.** Intake scoring is authoritative and is performed by `/fab-new` / `/fab-clarify`; `/fab-continue` operates only at apply and later, where there is no scoring. ### Step 4: Update `.status.yaml` -Use event commands via CLI to update `.status.yaml`. The `finish` command handles the two-write transition atomically: `fab status finish fab-continue`. This sets `{completed}` → `done`, auto-activates the next pending stage, refreshes `last_updated`, and updates `stage_metrics`. +Use event commands via CLI to update `.status.yaml`. The `finish` command handles the transition atomically: `fab status finish fab-continue` (sets `{completed}` → `done`, auto-activates the next pending stage, refreshes `last_updated` + `stage_metrics`). For other state changes, use the appropriate event command (driver is always optional): - `fab status start fab-continue` — pending → active (plus failed → active for review/review-pr only) @@ -100,7 +109,7 @@ Display summary. Include Assumptions summary for planning stages. End with `Next ## Apply Behavior -> **This section is the apply block — it always runs in a dispatched sub-agent** (the one-stage sequencer in Normal Flow Step 1 dispatches it; orchestrators dispatch it identically). The block does NOT run any `fab status` command — it returns results only; the orchestrator (the sequencer in the manual path, `_pipeline.md` in the auto path) owns the `finish`/`fail`/`reset` transitions. The `finish` steps below are the **sequencer's** responsibility after this block returns; they are shown here for the end-to-end picture, not as block actions. +> **This section is the apply block — it always runs in a dispatched sub-agent** per the sub-agent dispatch contract in Normal Flow Step 1: the block runs no `fab status` command and returns results only; the sequencer owns the `finish`/`fail`/`reset` transitions. The `finish` steps below are the sequencer's, shown for the end-to-end picture. Apply runs as **two sub-steps in a single dispatch**: a Plan Generation entry sub-step that produces `plan.md`, followed by the Task Execution main sub-step. @@ -154,15 +163,15 @@ Plan Generation sub-step is skipped when `plan.md` already exists (idempotent on ## Review Behavior -> **This section is the review block — it always runs in a dispatched sub-agent** (the sequencer in Normal Flow Step 1 dispatches it; orchestrators dispatch it identically). The block's job is identical regardless of caller: review the diff, **return** pass/fail + prioritized must-fix / should-fix / nice-to-have findings. **Findings are the block's return value, not conversation.** The block runs no `fab status` command and takes no §Verdict-style decision itself — it never branches on caller. Who acts on a fail verdict is the orchestrator's concern: the interactive § Verdict menu below (Path A, run by the manual `/fab-continue` sequencer) or `_pipeline.md`'s autonomous Auto-Rework Loop (Paths B/C/D). The § Verdict transitions below are the **sequencer's** actions on the returned verdict, shown here for the end-to-end picture. +> **This section is the review block — it always runs in a dispatched sub-agent** per the sub-agent dispatch contract in Normal Flow Step 1. Its job: review the diff and **return** pass/fail + prioritized must-fix / should-fix / nice-to-have findings. **Findings are the block's return value, not conversation.** It takes no §Verdict-style decision itself and never branches on caller. Who acts on a fail verdict is the orchestrator's concern: the interactive § Verdict menu below (Path A, run by the manual `/fab-continue` sequencer) or `_pipeline.md`'s autonomous Auto-Rework Loop (Paths B/C/D). The § Verdict transitions below are the sequencer's actions on the returned verdict. Read `.claude/skills/_review/SKILL.md` (if not already loaded), then execute its **Shared Review Dispatch** end-to-end (Preconditions → Inward + Outward Sub-Agent Dispatch → Parallel Dispatch → Findings Merge). The `_review.md` skill defines both sub-agent dispatches (inward + outward) run in parallel, their preconditions, validation steps, structured output format, and the findings merge procedure. When dispatching the inward sub-agent, read `change_type` from the change's `.status.yaml` and pass it in the prompt per `_review.md`'s context contract (its Steps 7–8 skip condition keys on it). -> **Per-stage model resolution (nested reviewers)**: before dispatching the inward + outward reviewer sub-agents, run `fab resolve-agent review --alias` **once** (the `--alias` flag emits the Agent-tool-valid short alias on the `model=` line), surface the resolved `model=/effort=`, and apply both halves to BOTH reviewers and the merge — the same model via the Agent `model` param (empty ⇒ omit/inherit) and the same imperative effort instruction (``Operate at `` reasoning effort for this task.``) in each reviewer's prompt (empty effort ⇒ omit) — per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. (This is the review block resolving the tier for its own nested sub-agents; it is independent of the sequencer's resolution of the `review` stage when it dispatched this block.) The Claude Code adapter is the Agent tool `model` parameter (effort rides the prompt); the resolution itself is provider-neutral. +> **Per-stage model resolution (nested reviewers)**: before dispatching the inward + outward reviewer sub-agents, run `fab resolve-agent review --alias` **once** and apply the resolved tier to BOTH reviewers and the merge via the same two seams as Step 1's dispatch contract (model on the Agent `model` param, effort via an imperative prompt instruction; per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution). This is the review block resolving the tier for its own nested sub-agents — independent of the sequencer's resolution of the `review` stage when it dispatched this block. ### Verdict -> The Verdict transitions are run by the **sequencer** (the manual `/fab-continue` invocation in Path A) on the verdict this block returns — not by the dispatched review block. In Paths B/C/D the orchestrator's Auto-Rework Loop (`_pipeline.md`) takes the equivalent actions autonomously. +> Verdict transitions are the **sequencer's** (manual `/fab-continue`, Path A), not the dispatched block's. In Paths B/C/D the orchestrator's Auto-Rework Loop (`_pipeline.md`) takes the equivalent actions autonomously. **Pass**: Run `fab status finish review fab-continue` (auto-activates hydrate). Update acceptance progress via `fab status set-acceptance acceptance_completed `. Output report + `Next: {per state table}`. @@ -180,7 +189,7 @@ The applying agent triages review comments by priority — not all comments need ## Hydrate Behavior -> **This section is the hydrate block — it always runs in a dispatched sub-agent** (the sequencer in Normal Flow Step 1 dispatches it; orchestrators dispatch it identically). The block does NOT run any `fab status` command — it returns completion status only; the sequencer (manual path) or `_pipeline.md` (auto path) runs the `finish` transition after the block returns. Step 5 below is the **sequencer's** action, shown for the end-to-end picture. +> **This section is the hydrate block — it always runs in a dispatched sub-agent** per the sub-agent dispatch contract in Normal Flow Step 1: the block runs no `fab status` command and returns completion status only; the sequencer runs the `finish` transition. Step 5 below is the sequencer's, shown for the end-to-end picture. ### Preconditions @@ -192,7 +201,14 @@ The applying agent triages review comments by priority — not all comments need 1. Final validation: all `## Tasks` and `## Acceptance` items in `plan.md` are `[x]` 2. Concurrent change check: warn on overlap with other changes referencing same memory paths 3. **Read `## Deletion Candidates`** from `plan.md` when present — informational only. Hydrate MAY reference candidates in memory updates (e.g., a Design Decision noting follow-up cleanup). Hydrate MUST NOT generate or modify the section (generation is review's responsibility) and MUST treat an absent section as "no findings" without error -4. Hydrate `docs/memory/`: create new files/domains from the canonical memory-file shape — **read `$(fab kit-path)/templates/memory.md`** (the single source of truth for the shape, the same on-demand template read used for `$(fab kit-path)/templates/intake.md`) and fill its FKF frontmatter (`type: memory` constant plus a curated `description:` one-liner, per `$(fab kit-path)/reference/fkf.md` §3.1–§3.2) and body skeleton — update existing (Requirements, Design Decisions, keep `description:` accurate, and **stamp the `type: memory` constant when an edited legacy file is missing it** so the file you touch becomes FKF-conforming — FKF §2/§3.1 require `type: memory` on every memory file, stamped by every memory writer). **No per-file `## Changelog`**: memory files no longer carry a `## Changelog` section (FKF §3.3) — instead, record what changed once via `fab status set-summary {change} ""` (the C-lite `summary:` source line, FKF §6.3, authored once at hydrate; `fab memory-index` joins it with git history to generate the per-folder `log.md`). **Bundle-relative cross-links**: any memory↔memory link you write MUST use the bundle-relative `/...` form (resolved from `docs/memory/`, FKF §7); links *out* of the bundle (to source, specs, URLs) stay repo-relative/absolute-URL. **Merge without duplication**: before appending to a target memory file, check it for an existing entry referencing this change (by change name) and update that entry in place instead of appending a duplicate — the same "replaced in place (not duplicated)" contract as `docs-hydrate-memory.md` and `_review.md`'s `## Deletion Candidates`. Then run `fab memory-index` to regenerate the root (domains-only), domain, and sub-domain indexes — never hand-edit index rows or "Last Updated" cells. **Refuse-before-regen guard (defense-in-depth)**: before that regen, consult `fab memory-index --check`; on **exit 2** (destructive loss) refuse to regenerate and surface the pointer `→ run /docs-reorg-memory to remediate ...` (the orchestrator that relocates tombstone rows and dispatches `/docs-hydrate-memory` backfill mode for descriptions; backfill alone does not relocate tombstones). This guard is a **no-op for born-compatible fab-kit trees** — a tree hydrated by the pipeline is always exit 0/1, never 2, so the guard never fires here (do not mistake it for dead code or remove it); it is defense-in-depth for the pathological case of a pre-fab-kit tree reaching the pipeline's hydrate stage. **Shape SHOULD guidance**: aim for ~5–12 files/folder, depth ≤3, introduce a sub-domain only for a cohesive ≥8-file cluster; `_shared/` and `_unsorted/` are width-exempt. Heed any non-fatal shape warnings `fab memory-index` prints (advisory only). +4. Hydrate `docs/memory/`: + - **FKF frontmatter (from template)**: create new files/domains from the canonical memory-file shape — **read `$(fab kit-path)/templates/memory.md`** (the single source of truth for the shape, the same on-demand template read used for `$(fab kit-path)/templates/intake.md`) and fill its FKF frontmatter (`type: memory` constant plus a curated `description:` one-liner, per `$(fab kit-path)/reference/fkf.md` §3.1–§3.2) and body skeleton. Update existing files (Requirements, Design Decisions, keep `description:` accurate, and **stamp the `type: memory` constant when an edited legacy file is missing it** so the file you touch becomes FKF-conforming — FKF §2/§3.1 require `type: memory` on every memory file, stamped by every memory writer). + - **No per-file `## Changelog`**: memory files no longer carry a `## Changelog` section (FKF §3.3) — instead, record what changed once via `fab status set-summary {change} ""` (the C-lite `summary:` source line, FKF §6.3, authored once at hydrate; `fab memory-index` joins it with git history to generate the per-folder `log.md`). + - **Bundle-relative cross-links**: any memory↔memory link you write MUST use the bundle-relative `/...` form (resolved from `docs/memory/`, FKF §7); links *out* of the bundle (to source, specs, URLs) stay repo-relative/absolute-URL. + - **Merge without duplication**: before appending to a target memory file, check it for an existing entry referencing this change (by change name) and update that entry in place instead of appending a duplicate — the same "replaced in place (not duplicated)" contract as `docs-hydrate-memory.md` and `_review.md`'s `## Deletion Candidates`. + - **Regenerate indexes**: run `fab memory-index` to regenerate the root (domains-only), domain, and sub-domain indexes — never hand-edit index rows or "Last Updated" cells. + - **Refuse-before-regen guard (defense-in-depth)**: before that regen, consult `fab memory-index --check`; on **exit 2** (destructive loss) refuse to regenerate and surface the pointer `→ run /docs-reorg-memory to remediate ...` (the orchestrator that relocates tombstone rows and dispatches `/docs-hydrate-memory` backfill mode for descriptions; backfill alone does not relocate tombstones). This guard is a **no-op for born-compatible fab-kit trees** — a tree hydrated by the pipeline is always exit 0/1, never 2, so the guard never fires here (do not mistake it for dead code or remove it); it is defense-in-depth for the pathological case of a pre-fab-kit tree reaching the pipeline's hydrate stage. + - **Shape SHOULD guidance**: aim for ~5–12 files/folder, depth ≤3, introduce a sub-domain only for a cohesive ≥8-file cluster; `_shared/` and `_unsorted/` are width-exempt. Heed any non-fatal shape warnings `fab memory-index` prints (advisory only). 5. Return completion status — the sequencer runs `fab status finish hydrate fab-continue` after the block returns (the block runs no `fab status` command) 6. **Pattern capture** *(optional)*: If the change introduced non-obvious implementation patterns that future changes should follow (e.g., a new error handling approach, a reusable abstraction), note them in the relevant memory file's Design Decisions section with the change name for traceability. Skip for implementations that follow existing patterns without introducing new ones diff --git a/src/kit/skills/fab-fff.md b/src/kit/skills/fab-fff.md index 9bd3e569..14ec1a9c 100644 --- a/src/kit/skills/fab-fff.md +++ b/src/kit/skills/fab-fff.md @@ -10,6 +10,16 @@ helpers: [_generation, _review, _srad, _pipeline] --- +## Contents + +- Purpose +- Arguments +- Behavior +- Output +- Error Handling + +--- + ## Purpose Run the entire automated Fab pipeline — apply → review → hydrate → ship → review-pr — in a single invocation (everything after intake). Gated on the single intake confidence gate (flat 3.0, all types), checked before the bracket; review failures get a bounded auto-rework loop (`{max_cycles}` cycles — the `Max cycles:` knob in `fab/project/code-review.md` § Rework Budget, default 3) and then stop. No `/fab-clarify` runs inside the bracket — clarification is intake-only. Resumable — re-running picks up from the first incomplete stage. The difference from `/fab-ff` is scope only: `/fab-fff` extends through ship and review-pr; `/fab-ff` stops at hydrate. Both have the identical single intake gate. @@ -44,7 +54,7 @@ The bracket defines pre-flight (intake prerequisite + intake gate), context load *(Skip if `progress.ship` is `done`.)* -Resolve the ship model: run `fab resolve-agent ship --alias` (the `--alias` flag emits the Agent-tool-valid short alias on the `model=` line), surface the resolved `model=/effort=`, and apply both halves to the dispatch below — model via the Agent `model` param (empty ⇒ omit/inherit), effort via the imperative prompt instruction ``Operate at `` reasoning effort for this task.`` (empty effort ⇒ omit) — per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. Dispatch `/git-pr` as subagent — the prompt instructs it to invoke `/git-pr {name}` (the **explicit change argument**, using the folder name per the `{name}` note above: git-pr resolves it as a transient override, so the subagent targets this pipeline's change rather than self-resolving the active one, and its branch-matches-change guard verifies the checked-out branch before mutating anything). The subagent commits, pushes, and creates a GitHub PR. Handles `fab status` integration internally (start/finish ship stage). Returns PR URL or error. +Resolve the ship model per the Per-stage model note above (`fab resolve-agent ship --alias`, surface `model=/effort=`, dispatch via both seams), then dispatch `/git-pr` as subagent — the prompt instructs it to invoke `/git-pr {name}` (the **explicit change argument**, using the folder name per the `{name}` note above: git-pr resolves it as a transient override, so the subagent targets this pipeline's change rather than self-resolving the active one, and its branch-matches-change guard verifies the checked-out branch before mutating anything). The subagent commits, pushes, and creates a GitHub PR. Handles `fab status` integration internally (start/finish ship stage). Returns PR URL or error. **If git-pr fails**: STOP with the error from git-pr. The ship stage remains `active` for user retry. @@ -54,15 +64,15 @@ On success: `progress.ship` becomes `done`, `progress.review-pr` auto-activates. *(Skip if `progress.review-pr` is `done`.)* -Resolve the review-pr model: run `fab resolve-agent review-pr --alias` (the `--alias` flag emits the Agent-tool-valid short alias on the `model=` line), surface the resolved `model=/effort=`, and apply both halves to the dispatch below — model via the Agent `model` param (empty ⇒ omit/inherit), effort via the imperative prompt instruction ``Operate at `` reasoning effort for this task.`` (empty effort ⇒ omit) — per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. Dispatch `/git-pr-review` as subagent — the prompt instructs it to invoke `/git-pr-review {name}` (the **explicit change argument**, same transient-override + branch-guard contract as Step 4). The subagent detects existing reviews, triages comments, applies fixes, and pushes. If no reviews exist, it requests a Copilot review and polls up to 10 minutes — see the timeout outcome below. Handles `fab status` integration internally (start/finish/fail review-pr stage). Returns completion status. +Resolve the review-pr model per the Per-stage model note above (`fab resolve-agent review-pr --alias`, surface `model=/effort=`, dispatch via both seams), then dispatch `/git-pr-review` as subagent — the prompt instructs it to invoke `/git-pr-review {name}` (the **explicit change argument**, same transient-override + branch-guard contract as Step 4). The subagent detects existing reviews, triages comments, applies fixes, and pushes. If no reviews exist, it requests a Copilot review and polls up to 10 minutes — see the timeout outcome below. Handles `fab status` integration internally (start/finish/fail review-pr stage). Returns completion status. -> **Synchronous-poll directive (bake into the dispatch prompt).** The review-pr dispatch prompt MUST instruct the `/git-pr-review` subagent to **complete the Copilot poll synchronously and not yield mid-poll**: if it requests a Copilot review and enters the 30s × 20 (10-minute) poll, it MUST stay in that poll loop within the single invocation — never yielding, returning, or handing back control while the poll is pending — until a review appears or all 20 attempts are exhausted. The poll **stays inside `/git-pr-review`** (the subagent owns request + poll + triage synchronously); the wait is NOT relocated to this orchestrator. Rationale to carry in the prompt: the subagent stalled or died mid-poll 4× in prior efforts, and Copilot lands ~4.5–6.5 min — comfortably inside the window — so patience-to-completion is correct, not an early return. (This mirrors `git-pr-review.md` Step 2 Phase 2's own synchronous-poll discipline note into the dispatch seam.) +> **Synchronous-poll directive (bake into the dispatch prompt).** The review-pr dispatch prompt MUST instruct the `/git-pr-review` subagent to **complete the Copilot poll synchronously and not yield mid-poll**: if it requests a Copilot review and enters the 30s × 20 (10-minute) poll, it MUST stay in that poll loop within the single invocation — never yielding, returning, or handing back control while the poll is pending — until a review appears or all 20 attempts are exhausted. The poll **stays inside `/git-pr-review`** (the subagent owns request + poll + triage synchronously); the wait is NOT relocated to this orchestrator. Carry the rationale in the prompt — see `git-pr-review.md` Step 2 Phase 2's synchronous-poll discipline note (it mirrors that note into the dispatch seam). **If review-pr fails** (no PR found, processing error): STOP with the error. **If no actionable reviews** (no automated reviewer available, or reviews with no inline comments to process): the stage completes as `done` — this is a successful no-op. -**If timeout** (Copilot review requested but not available within 10 minutes — git-pr-review's Step 6 timeout outcome): the subagent deliberately leaves `review-pr` `active` (no finish, no fail). Report `Review-PR pending (Copilot review requested, timed out waiting) — re-run /git-pr-review {name} when ready` **instead of** `Pipeline complete.` and stop. +**If timeout** (Copilot review requested but not available within 10 minutes — git-pr-review's Step 6 timeout outcome): the subagent deliberately leaves `review-pr` `active` (no finish, no fail); report the pending message **instead of** `Pipeline complete.` and stop — see the Review-PR timeout row in Error Handling for the exact string. On success: `progress.review-pr` becomes `done`. @@ -96,7 +106,7 @@ Pipeline complete. Next: {per state table} ``` -Resuming shows `(resuming)...` header and `Skipping {stage} — already done.` for completed stages. Bail/failure stops at the relevant stage with `Next:` derived from the state reached per state table in `_preamble.md`. On the Step 5 timeout outcome, the closing line is `Review-PR pending (Copilot review requested, timed out waiting) — re-run /git-pr-review {name} when ready` instead of `Pipeline complete.` +Resuming shows `(resuming)...` header and `Skipping {stage} — already done.` for completed stages. Bail/failure stops at the relevant stage with `Next:` derived from the state reached per state table in `_preamble.md`. On the Step 5 timeout outcome, the closing line replaces `Pipeline complete.` with the pending message (exact string in the Error Handling Review-PR timeout row). --- diff --git a/src/kit/skills/fab-new.md b/src/kit/skills/fab-new.md index f170f5a1..2cf9062f 100644 --- a/src/kit/skills/fab-new.md +++ b/src/kit/skills/fab-new.md @@ -8,6 +8,15 @@ helpers: [_generation, _srad, _intake] > Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding. +## Contents + +- Pre-flight +- Arguments +- Behavior +- Output +- Error Handling +- Key Properties + --- ## Pre-flight diff --git a/src/kit/skills/fab-operator.md b/src/kit/skills/fab-operator.md index 5566c3cb..2fc243c9 100644 --- a/src/kit/skills/fab-operator.md +++ b/src/kit/skills/fab-operator.md @@ -8,12 +8,21 @@ helpers: [_cli-fab, _cli-external] > Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). Then follow its instructions before proceeding. +## Contents + +- 1. Principles +- 2. Startup +- 3. Safety +- 4. The Loop +- 5. Auto-Nudge +- 6. Coordination Patterns +- 7. Watches +- 8. Configuration +- 9. Key Properties + Multi-agent coordination layer. Runs in a dedicated tmux pane, observes agents across all sessions on its tmux server via `fab pane map --all-sessions`, routes commands via `tmux send-keys`, monitors progress via `/loop`. Spans multiple repos and sessions on one server. The loop is the heart of the operator. -Start via `fab operator` (singleton tmux tab named `operator`). The launcher requires **neither a git repo nor a resolvable `fab/` project** — matching the per-server, cross-repo singleton model, whose natural launch point is a neutral parent directory (e.g. `~/code`) with no `fab/` project. The degraded behavior is exact: -- **Window cwd**: the repo root when launched inside a git repo, else `os.Getwd()` (the current directory). It errors only if both resolutions fail. -- **Spawn command**: `agent.spawn_command` from the project's `fab/project/config.yaml` when a `fab/` project is resolvable, else the built-in default `spawn.DefaultSpawnCommand` (`claude --dangerously-skip-permissions`). When launched `fab/`-less, **no project `agent.spawn_command`/`agent.tiers` is read** — there is no project to customize from. -- **Doing-tier model**: the launcher resolves the **doing-tier** `{model, effort}` (via `fab resolve-agent apply`) and appends `--model`/`--effort` to the spawn command, so the coordinating agent runs on a deliberately-chosen model rather than whatever the spawn command happened to specify. With no resolvable `fab/` project (or any other failure) the doing tier resolves to its built-in default `{claude-opus-4-8, high}` — so a `fab/`-less launch composes a fully-defaulted command (default spawn command + doing default). +Start via `fab operator` (singleton tmux tab named `operator`). The launcher requires **neither a git repo nor a resolvable `fab/` project** — matching the per-server, cross-repo singleton model, whose natural launch point is a neutral parent directory (e.g. `~/code`). Its exact degraded behavior (window cwd, spawn command, doing-tier model resolution and built-in defaults) is documented in `_cli-fab.md` § fab operator and is the canonical §9 Key Properties rows below. --- @@ -126,9 +135,7 @@ The loop is the operator's heartbeat — a `/loop "operator tick"` that runs as ### Operator State File -Persistent state, read on startup and every tick, written after every state change. The term **operator state file** used throughout this skill refers to this file. - -The operator state file is **server-keyed**, not repo-rooted: it lives at `$XDG_STATE_HOME/fab/operator/.yaml` (falling back to `~/.local/state/fab/operator/.yaml` when `XDG_STATE_HOME` is unset), keyed by the tmux socket path so the one operator per server owns one file spanning every repo it coordinates (see §8, §9). The binary derives this path (`fab operator tick-start` reads/writes it); the operator never needs to compute it. Old repo-rooted `.fab-operator.yaml` files from before the server-keyed model are **not migrated** — they are abandoned in place (the monitored set is re-derivable from live `»`-prefixed panes). +Persistent state, read on startup and every tick, written after every state change. The term **operator state file** used throughout this skill refers to this file. It is **server-keyed**, not repo-rooted — one file per tmux server spanning every repo it coordinates (path, XDG fallback, binary-derivation, and the no-migration of old repo-rooted `.fab-operator.yaml` files: see §2 Init step 1 and §9). ```yaml tick_count: 47 @@ -205,7 +212,7 @@ The top-level `branch_map` persists change ID → `{ branch, repo }` mappings. E On each tick: -1. **Snapshot** — run `fab operator tick-start` (increments `tick_count`, writes `last_tick_at`, outputs `tick: N` and `now: HH:MM`). Parse stdout for the tick number and current time. Then run `fab pane map --all-sessions --json` (the `--all-sessions` flag is required so the operator sees agents in **every** session on its server, not just its own; `--json` exposes the per-row `repo` field — the agent's absolute main-worktree root, or `null` when the pane is not in a git repo — and the nullable per-row `display_state` field — `active|ready|done|failed|pending|skipped`, `null` under the same conditions as `stage` — an honest attention-state axis alongside `stage` (`failed` is reachable since the DisplayStage failed tier shipped with 260612-dkn3)) and read the server-keyed state file. **Group the rows first by `repo`, then by `session`** within each repo. Compute status for all tracked items: stage advances, completions, review failures, pane deaths, and watch statuses from the last persisted check (`last_checked` / `last_error` / last counts). Output the status frame — see **Status Frame Format** below. +1. **Snapshot** — run `fab operator tick-start` (increments `tick_count`, writes `last_tick_at`, outputs `tick: N` and `now: HH:MM`). Parse stdout for the tick number and current time. Then run `fab pane map --all-sessions --json` (flag/field semantics — `--all-sessions`, the per-row `repo` and nullable `display_state` fields — in `_cli-fab.md` § fab pane map) and read the server-keyed state file. **Group the rows first by `repo`, then by `session`** within each repo. Compute status for all tracked items: stage advances, completions, review failures, pane deaths, and watch statuses from the last persisted check (`last_checked` / `last_error` / last counts). Output the status frame — see **Status Frame Format** below. 2. **Auto-nudge** — for each idle agent, run question detection (§5). (No post-intake `/git-branch` nudge — `/fab-new` Step 11 creates or renames the branch inline; only a detected branch/change mismatch warrants a `/git-branch` send, per §3 pre-send validation item 4.) 3. **Watches** — for each watch, query the source, compare against `known` + `completed` (§7 step 2's dedupe rule), spawn on new matches (§7). @@ -354,14 +361,12 @@ In both branches the operator **continues ticking**. The user answers asynchrono ### Notification Send -The notification is a single out-of-band shell send the operator runs when it auto-picks or leaves open a Strategic prompt. The **default channel is `rk notify`** — a run-kit external contract: `rk notify [--title string]` (run-kit Web Push, released in `rk v2.3.2`; full command reference in `_cli-external.md` § rk (run-kit)). The send is gated on `command -v rk` and runs fail-silent per `_preamble.md` § Run-Kit (rk) Reference (which documents the gate and the fail-silent discipline; the `notify` subcommand itself is documented in `_cli-external.md` § rk): +The notification is a single out-of-band shell send the operator runs when it auto-picks or leaves open a Strategic prompt. The **default channel is `rk notify`** (contract, gate, and fail-silent discipline per `_cli-external.md` § rk (run-kit) and `_preamble.md` § Run-Kit (rk) Reference). The operator-specific send — gated on `command -v rk`, with the operator's message/title template — is: ```sh command -v rk >/dev/null 2>&1 && rk notify "{change}: {summary} ({repo})" --title "Operator: strategic question" ``` -`rk notify` delivers a real background mobile/desktop Web Push to every subscribed device and is **fail-silent by contract** (exits 0 / prints nothing on any error — server unreachable, no subscriptions — so it can never stall the loop). - **When `rk` is absent** (operator running where run-kit isn't installed), fall back to the first available **documented alternative**, configurable via the §8 `Notify channel` setting: - **ntfy.sh** — `curl -d "{change}: {summary} ({repo})" ntfy.sh/`. No account, curl-from-shell, cross-repo aggregator, mobile push. **High-entropy topic REQUIRED** — public topics are world-readable to anyone who knows the name (the topic name is the only secret), so use a long random topic (e.g. `op-9f3a2c7e-strat`) and never put secrets in the body. The strongest no-run-kit fallback. @@ -369,7 +374,7 @@ command -v rk >/dev/null 2>&1 && rk notify "{change}: {summary} ({repo})" --titl - **`PushNotification`** (built-in Claude Code harness tool) — zero infra, no topic secret to leak, headless-safe; a *personal* push to the user's Claude apps, not a shared searchable feed. Good "just ping me" fallback. - **Slack MCP** (`mcp__claude_ai_Slack__slack_send_message`) — searchable channel feed, mobile push; caveat: an interactively-authed MCP may be **absent in headless/cron** runs, so it cannot be a headless default. -**All notify sends fail silently.** A notification that cannot be delivered (`rk`/run-kit server unreachable, channel down, no subscriptions, `curl`/tool missing) MUST NOT crash or stall the loop — the operator logs one line and keeps ticking. `rk notify` is already fail-silent by contract; the fallback path matches it. This mirrors the `_preamble.md` § Run-Kit (rk) Reference "fail silently" discipline. +**All notify sends fail silently** (the fallback path matches `rk notify`'s contract per `_preamble.md` § Run-Kit (rk) Reference). A notification that cannot be delivered (server unreachable, channel down, no subscriptions, `curl`/tool missing) MUST NOT crash or stall the loop — the operator logs one line and keeps ticking. ### Sending Auto-Answers @@ -518,12 +523,12 @@ Dependency resolution is **two-tier**, split by repo. Each entry in `depends_on` Dependencies are declared through three conversational paths, all of which coexist: 1. **Explicit**: "cd34 depends on ab12" — operator sets `depends_on: [ab12]` on the monitored entry -2. **Autopilot queue (implicit)**: user-provided ordering implies `--base` chaining by default — every change after the first automatically gets `depends_on: []`: the closest earlier queue entry in the **same repo** (cherry-picked), falling back to the immediately previous entry when no earlier entry shares the repo (cross-repo → ordering-only barrier, no code) +2. **Autopilot queue (implicit)**: user-provided ordering implies `--base` chaining by default — every change after the first gets `depends_on: []` (definition + cross-repo fallback in § Autopilot → Queue ordering, "User-provided") 3. **`--base` flag (explicit)**: autopilot `--base ` explicitly sets `depends_on: []` for the subsequent change (matches path 2's pick when the previous entry is same-repo; available for ad-hoc overrides) ### Working a Change -> **Pipeline-first routing (§1):** All three work paths below MUST go through the fab pipeline. For *new* work, this means `/fab-new` followed by a pipeline command; for already-intaked changes, start from the appropriate pipeline command stage instead of repeating `/fab-new`. The operator MUST NOT send raw implementation instructions directly to agent panes. See the "Pipeline-first routing" principle in §1. +> **Pipeline-first routing (§1):** all three work paths below MUST go through the fab pipeline (`/fab-new` then a pipeline command for new work; the appropriate stage for already-intaked changes) — never raw implementation instructions to agent panes. The operator accepts work in three forms. Each runs the §6 spawn sequence above (establish target repo → `wt create` in it → existence-guarded pointer activation → resolve dependencies → `fab spawn-command --repo ` → open agent tab → enroll with `repo` + `session`); only the entry-form specifics below differ: @@ -541,7 +546,7 @@ User provides a queue of changes. Confirmation prompt reflects the active mode: - **Default (stack-then-review):** "Confirm upfront (creates PRs — merge after review)." - **`--merge-on-complete`:** "Confirm upfront (merges PRs on completion)." -A queue **may span repos**. The dependency semantics are mixed: implicit `--base` chaining (and explicit `depends_on`) cherry-picks **within a repo** and **degrades to an ordering-only barrier across repo boundaries** (per Dependency Resolution above). Implicit chaining picks each change's **nearest same-repo predecessor** — not blindly the previous queue entry, which would silently break same-repo stacking whenever a cross-repo entry sits in between (a cross-repo dep contributes no code). So a chain `ab12 → cd34 → ef56` where `cd34` lives in a different repo means: `cd34` gets `depends_on: [ab12]` (cross-repo — waits for `ab12` to reach its stop/terminal stage, no code), and `ef56` (back in `ab12`'s repo) gets `depends_on: [ab12]` — its nearest same-repo predecessor — and cherry-picks from it; queue order still runs `ef56` after `cd34`. +A queue **may span repos**, with mixed dependency semantics: implicit `--base` chaining (and explicit `depends_on`) cherry-picks **within a repo** and **degrades to an ordering-only barrier across repo boundaries** (per Dependency Resolution above; the nearest-same-repo-predecessor rule is defined in Queue ordering below). Worked example — a chain `ab12 → cd34 → ef56` where `cd34` lives in a different repo: `cd34` gets `depends_on: [ab12]` (cross-repo — waits for `ab12` to reach its stop/terminal stage, no code), and `ef56` (back in `ab12`'s repo) gets `depends_on: [ab12]` — its nearest same-repo predecessor — and cherry-picks from it; queue order still runs `ef56` after `cd34`. Queue ordering: @@ -551,7 +556,7 @@ Queue ordering: | Confidence-based | Sort by confidence score descending. Highest-confidence first (independent changes) | | Hybrid | User provides constraints (partial order); operator sorts unconstrained by confidence | -**`--merge-on-complete`** — opt-in flag that reverts to the previous merge-as-you-go behavior: merge each PR on completion, then `git fetch origin` and rebase the next change onto `origin/{default_branch}` (the default branch resolved per Dependency Resolution step 0 — never a hardcoded `origin/main`). Implicit `--base` chaining is disabled under this flag — each change rebases onto `origin/{default_branch}` independently instead of stacking on the previous change's branch. Natural language equivalents: "merge as you go", "merge on complete", "merge each when done". Without this flag, the default is stack-then-review: PRs are created but not merged until the user explicitly requests merging, and implicit `--base` chaining is active (every change after the first gets `depends_on: []`, falling back to the immediately previous entry across repo boundaries). +**`--merge-on-complete`** — opt-in flag that reverts to the previous merge-as-you-go behavior: merge each PR on completion, then `git fetch origin` and rebase the next change onto `origin/{default_branch}` (the default branch resolved per Dependency Resolution step 0 — never a hardcoded `origin/main`). Implicit `--base` chaining is disabled under this flag — each change rebases onto `origin/{default_branch}` independently instead of stacking on the previous change's branch. Natural language equivalents: "merge as you go", "merge on complete", "merge each when done". Without this flag, the default is stack-then-review: PRs are created but not merged until the user explicitly requests merging, and implicit `--base` chaining is active (per Queue ordering, "User-provided"). The operator works each change through the pipeline. Pre-send validation (§3) applies to any command sent to an existing pane; the initial pipeline command itself is **embedded at spawn** (§6 step 6) — the single dispatch point: diff --git a/src/kit/skills/fab-proceed.md b/src/kit/skills/fab-proceed.md index 473d1f82..0ece8d95 100644 --- a/src/kit/skills/fab-proceed.md +++ b/src/kit/skills/fab-proceed.md @@ -9,6 +9,17 @@ Read the `_preamble` skill first (deployed to `.claude/skills/` via `fab sync`). > `/fab-proceed` follows `_preamble.md` conventions but skips preflight/context loading itself — it delegates all pipeline context loading to `/fab-fff`. +## Contents + +- Purpose +- Arguments +- State Detection +- Relevance Assessment +- Dispatch Behavior +- Error Handling +- Output +- Key Properties + --- ## Purpose @@ -74,7 +85,7 @@ Enumerate candidate intakes: ls -d fab/changes/*/intake.md 2>/dev/null | grep -v archive/ | sed 's|fab/changes/||;s|/intake.md||' | sort -r ``` -The pipeline lists change folders with intakes, excludes archived changes, extracts folder names, and sorts the full folder names in descending order — the `YYMMDD` prefix dominates, and the `XXXX-slug` tail makes the order among same-day changes deterministic. Retain the full list — the date-descending sort is used only for tiebreaks in Step 5, not to pre-pick a single candidate. +This lists non-archived change folders with intakes, full names sorted date-descending (deterministic tiebreak: `YYMMDD` prefix dominates, `XXXX-slug` tail breaks same-day ties). Retain the full list — the sort is used only for tiebreaks in Step 5, not to pre-pick a single candidate. ### Step 5: Dispatch Decision @@ -116,8 +127,8 @@ When a candidate's relevance is genuinely ambiguous (neither clearly relevant no The failure modes are asymmetric: -- **False positive** (activate unrelated draft): corrupts the draft's content, wastes its original intent, conflates two unrelated features in pipeline output. Recovery requires manual rollback. -- **False negative** (create new when draft was relevant): leaves the draft intact and recoverable. The user sees the bypass note, and can run `/fab-switch ` to recover. +- **False positive** (activate unrelated draft): corrupts the draft and conflates two unrelated features in pipeline output — recovery requires manual rollback. +- **False negative** (create new when draft was relevant): leaves the draft intact — the user sees the bypass note and can run `/fab-switch ` to recover. Biasing toward the recoverable failure is the design intent. @@ -129,18 +140,18 @@ Relevance judgment is performed by the invoking agent inline — no external cla ### Subagent Dispatch (Prefix Steps) -Each prefix step (the `_intake` Create-Intake Procedure, `/fab-switch`, `/git-branch`) SHALL be dispatched as a subagent using the Agent tool (`subagent_type: "general-purpose"`) per `_preamble.md` § Subagent Dispatch. Each subagent prompt MUST instruct the subagent to read the standard subagent context files per `_preamble.md` § Standard Subagent Context. +Each prefix step (the `_intake` Create-Intake Procedure, `/fab-switch`, `/git-branch`) SHALL be dispatched as a subagent per `_preamble.md` § Subagent Dispatch (Agent tool, `subagent_type: "general-purpose"`, reading the standard subagent context files). -> **Per-stage model**: the prefix steps are NOT pipeline stages, so they take **no** `fab resolve-agent` resolution — they dispatch at the inherited model. Per-stage model selection belongs to the pipeline `/fab-proceed` delegates to: the final `/fab-fff` invocation (run via the Skill tool in the current context) resolves `fab resolve-agent ` for each of its own stages per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. +> **Per-stage model**: the prefix steps are NOT pipeline stages, so they take **no** `fab resolve-agent` resolution — they dispatch at the inherited model. Per-stage model selection belongs to the pipeline `/fab-proceed` delegates to: the final `/fab-fff` invocation resolves `fab resolve-agent ` for each of its own stages per `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. #### Create-Intake Dispatch Runs when the dispatch table selects the create-new path (`_intake`): either substantive conversation + no intake, or substantive conversation + ≥1 intake but none clearly relevant. The create-an-intake sub-operation is routed through the shared `_intake` Create-Intake Procedure (the same Steps 0–9 `/fab-new` runs) in its `promptless-defer` mode — `/fab-proceed` decides *whether* to create an intake; `_intake` performs it. After it returns (intake at `ready`, not activated), the dispatch table chains `/fab-switch` → `/git-branch` to activate and branch. 1. Synthesize a description from the conversation (see Conversation Context Synthesis below). The synthesis MUST NOT pull from bypassed drafts — only the live conversation is the source. -2. Dispatch subagent: read `.claude/skills/_intake/SKILL.md`, execute the **Create-Intake Procedure** with `{questioning-mode} = promptless-defer` and the synthesized description. The dispatch is promptless — there is no interactive relay — and `promptless-defer` is exactly the **defer-and-surface contract**: the procedure asks NO questions; any decision SRAD would normally ask (Unresolved) is instead recorded in the intake's `## Assumptions` table as an Unresolved row with Rationale `Deferred — promptless dispatch`, and listed in the subagent result. (This is the `_srad.md` § Critical Rule **promptless-dispatch carve-out** — the MUST-ask is satisfied by deferring and surfacing, not by silently assuming.) The procedure stops at intake `ready`; it does NOT activate or branch (those are `/fab-new`'s tail, not part of the shared procedure) — the `/fab-switch`/`/git-branch` prefix steps are dispatched separately per the dispatch table when needed. +2. Dispatch subagent: read `.claude/skills/_intake/SKILL.md`, execute the **Create-Intake Procedure** with `{questioning-mode} = promptless-defer` and the synthesized description. `promptless-defer` is the defer-and-surface contract per `_srad.md` § Critical Rule (promptless-dispatch carve-out): the procedure asks NO questions; any would-be-asked decision lands in the intake's `## Assumptions` table as an Unresolved row with Rationale `Deferred — promptless dispatch`, and is listed in the subagent result. The procedure stops at intake `ready`; it does NOT activate or branch (those are `/fab-new`'s tail) — the `/fab-switch`/`/git-branch` prefix steps are dispatched separately per the dispatch table. 3. Capture the created change folder name **and any deferred Unresolved decisions** from the subagent result -4. **Surface deferred decisions**: before delegating to `/fab-fff`, emit one line per deferred decision (informational — `/fab-proceed` stays zero-prompt). The intake gate is the structural backstop: a deferred decision blocks the gate **by itself only when its composite is below 20** (a composite ≥ 20 row still adds penalty and can help fail the gate alongside other weak rows) — there is no special gate for deferrals; blocking is emergent from the demerit curve. Because a genuine unknown is scored with honestly-low dimensions (composite < 20, penalty ≥ 2.0), such a deferral fails the `/fab-fff` gate and the pipeline stops normally for the user to resolve via `/fab-clarify`. +4. **Surface deferred decisions**: before delegating to `/fab-fff`, emit one line per deferred decision (informational — `/fab-proceed` stays zero-prompt). The intake gate is the structural backstop: deferred Unresolved rows penalize the gate per `_preamble.md` § Confidence Scoring; a genuine unknown (scored with honestly-low dimensions) fails it and the pipeline stops normally for the user to resolve via `/fab-clarify`. #### fab-switch Dispatch @@ -151,7 +162,7 @@ Runs when the dispatch table selects `/fab-switch` (substantive + clearly releva #### git-branch Dispatch -Runs when the dispatch table selects `/git-branch`: the branch-mismatch row (active change, branch doesn't match), the `/fab-switch`-prefixed relevant-intake rows, and the `_intake`-prefixed create-new rows (since `_intake` stops at `ready` without branching, `/git-branch` creates the matching branch after `/fab-switch` activates). +Runs when the dispatch table selects `/git-branch`: the branch-mismatch row (active change, branch doesn't match), the `/fab-switch`-prefixed relevant-intake rows, and the `_intake`-prefixed create-new rows (which chain `/git-branch` after `/fab-switch` — see the dispatch-table note above). 1. Dispatch subagent: read `.claude/skills/git-branch/SKILL.md`, follow its behavior for the active change 2. Capture the branch creation/checkout result from the subagent result diff --git a/src/kit/skills/fab-setup.md b/src/kit/skills/fab-setup.md index 364be4f2..8f6a3614 100644 --- a/src/kit/skills/fab-setup.md +++ b/src/kit/skills/fab-setup.md @@ -10,14 +10,28 @@ description: "Set up a new project, manage config/constitution, or apply version > - **bare / config / constitution**: Skip the "Always Load" context layer if files don't exist (first-run). Load them only if they already exist (re-run scenario). > - **migrations**: Load `fab/project/config.yaml` (MUST exist). Skip Change Context loading — migrations operate on project-level files, not a specific change. +## Contents + +- [Arguments](#arguments) +- [Pre-flight Check](#pre-flight-check) +- [Bootstrap Behavior](#bootstrap-behavior) +- [Config Behavior](#config-behavior) +- [Constitution Behavior](#constitution-behavior) +- [Migrations Behavior](#migrations-behavior) +- [Applying a Migration](#applying-a-migration) +- [Migrations Output Format](#migrations-output-format) +- [Idempotency](#idempotency) +- [Key Properties](#key-properties) +- [Next Steps Reference](#next-steps-reference) + --- ## Arguments -- **No arguments** — full structural bootstrap (default behavior) -- **`config [section]`** — create or update `fab/project/config.yaml` interactively. Optional `[section]` skips the menu and edits that section directly. Valid sections: `project`, `source_paths`, `checklist`. -- **`constitution`** — create or amend `fab/project/constitution.md` with semantic versioning -- **`migrations [file]`** — apply version migrations to bring project files in sync with the installed kit version (absorbed from fab-update) +- **No arguments** — full structural bootstrap +- **`config [section]`** — create/update `fab/project/config.yaml` interactively; optional `[section]` edits one section directly (valid: `project`, `source_paths`, `checklist`) +- **`constitution`** — create/amend `fab/project/constitution.md` with semantic versioning +- **`migrations [file]`** — apply version migrations to sync project files with the installed kit (absorbed from fab-update) - **`validate`** — redirect message: "Validation is built into `/fab-setup config` and `/fab-setup constitution` — each validates after every edit." Any unrecognized argument triggers: "Unknown subcommand: {arg}. Valid: config, constitution, migrations. Run `/fab-setup` with no arguments for full setup." @@ -74,7 +88,7 @@ Each step is **idempotent** — skip if the artifact already exists and is valid #### 1a. `fab/project/config.yaml` -If missing, raw template (contains `{PROJECT_NAME}`), or missing the required fields `project.name`/`project.description` (the canonical `fab init` flow writes a `fab_version`-only config.yaml before sync's copy-if-absent runs): execute **Config Behavior** (below) in create mode. +If the create-mode trigger holds (see [Config Create Mode](#config-create-mode)): execute **Config Behavior** (below) in create mode. If exists with the required fields and not a raw template: report "config.yaml already exists — skipping". #### 1b. `fab/project/constitution.md` @@ -98,32 +112,26 @@ Report how many skills were created, repaired, or already valid, plus the scaffo #### 1d. `fab/.kit-migration-version` -Handled by `fab sync` (step 1c). The sync command creates `fab/.kit-migration-version` with version logic based on project state: - -- **New project** (no `fab/project/config.yaml`): copies `$(fab kit-path)/VERSION` value (engine version) -- **Existing project** (has `fab/project/config.yaml`, no `fab/.kit-migration-version`): writes `0.1.0` (base version, run `/fab-setup migrations` to migrate) -- **Already exists**: preserves existing `fab/.kit-migration-version` — no overwrite +Handled by `fab sync` (step 1c) — version logic by project state, with the matching bootstrap output line: -On bootstrap output: -- New project: `Created: fab/.kit-migration-version ({engine_version})` -- Existing project: `Created: fab/.kit-migration-version (0.1.0 — existing project, run "/fab-setup migrations" to migrate)` -- Re-run: `fab/.kit-migration-version` reported as part of scaffold output (no modification) +- **New project** (no `fab/project/config.yaml`): copies `$(fab kit-path)/VERSION` (engine version) → `Created: fab/.kit-migration-version ({engine_version})` +- **Existing project** (has `fab/project/config.yaml`, no `fab/.kit-migration-version`): writes `0.1.0` (base; run `/fab-setup migrations` to migrate) → `Created: fab/.kit-migration-version (0.1.0 — existing project, run "/fab-setup migrations" to migrate)` +- **Already exists**: preserves existing value, no overwrite → reported as part of scaffold output (no modification) ### Bootstrap Output ``` Found kit v{VERSION}. Initializing project... -{config.yaml prompts and creation} -{constitution.md generation} +{config.yaml + constitution.md interactive creation} Created: fab/project/config.yaml Created: fab/project/constitution.md -{fab sync report — scaffold files (context.md, code-quality.md, code-review.md, docs/memory/index.md, docs/specs/index.md), fab/changes/ (+ archive), fab/.kit-migration-version ({version}), skills deployed to .claude/skills/, .gitignore merge (.fab-*)} +{fab sync report — scaffold files, fab/changes/ (+ archive), fab/.kit-migration-version ({version}), skills to .claude/skills/, .gitignore merge (.fab-*)} fab/ initialized successfully. Next: {per state table — initialized} ``` -On re-run, report config/constitution as OK/repaired instead of Created and surface sync's idempotent report, ending with `fab/ structure verified.` +Re-run variant: report config/constitution as OK/repaired instead of `Created`, surface sync's idempotent report, and end with `fab/ structure verified.` --- @@ -140,11 +148,13 @@ Create a new `fab/project/config.yaml` interactively or update specific sections ### Config Pre-flight - **Update mode**: `fab/project/config.yaml` must exist. If missing (direct invocation): STOP with `fab/project/config.yaml not found. Run /fab-setup to create it.` -- **Create mode** (from bootstrap): `fab/project/config.yaml` does not exist, is a raw template, or is missing the required fields `project.name`/`project.description` (e.g., a `fab init`-created, `fab_version`-only config). +- **Create mode** (from bootstrap): the create-mode trigger holds (see [Config Create Mode](#config-create-mode)). ### Config Create Mode -When `fab/project/config.yaml` does not exist (or exists without the required `project.name`/`project.description` fields): +**Create-mode trigger** (canonical): `fab/project/config.yaml` is missing, is a raw template (contains `{PROJECT_NAME}`), OR is missing the required fields `project.name`/`project.description` — e.g. the canonical `fab init` flow writes a `fab_version`-only config.yaml before sync's copy-if-absent runs. + +When that trigger holds: 1. Read the project's README, package.json, or other root-level files for context 2. Ask the user: project name, description, source paths @@ -290,14 +300,9 @@ When `[file]` is provided, read and apply that specific migration file directly, ### Migrations Step 1: Discover Migrations -Discovery is owned by the binary — do NOT read, parse, or compare the version files, and do NOT scan, validate, or sort the migrations directory by hand. The binary exits non-zero with remediation hints on a missing `fab/.kit-migration-version` or engine `VERSION` file — surface its stderr and stop. +Discovery is binary-owned (per the Migrations Behavior intro and Context Loading) — do nothing by hand. The binary exits non-zero with remediation hints on a missing `fab/.kit-migration-version` or engine `VERSION` file — surface its stderr and stop. -1. Run `fab migrations-status --json` and parse the result. The shape is: - `{local, engine, applicable:[{from,to,file}], gap_skips, overlaps}`. - - `local` / `engine` — the project's `fab/.kit-migration-version` and the kit's `$(fab kit-path)/VERSION`, already read and parsed by the binary - - `applicable` — the ordered list of migration files to apply, FROM ascending (already discovered, gap-skipped, and chained by the binary) - - `gap_skips` — human-readable "no migration needed for X -> Y, skipping" lines to surface in output - - `overlaps` — pairs of conflicting filenames; non-empty means the migrations directory is malformed +1. Run `fab migrations-status --json` and parse the result. Shape: `{local, engine, applicable:[{from,to,file}], gap_skips, overlaps}` — `local`/`engine` are the parsed project + kit versions; `applicable` is the ordered (FROM-ascending, gap-skipped, chained) list of files to apply; `gap_skips` are human-readable "no migration needed for X -> Y, skipping" lines to surface; `overlaps` are conflicting-filename pairs (non-empty = malformed migrations directory). 2. **If `overlaps` is non-empty**: STOP and report the conflict (see [Overlapping Ranges](#overlapping-ranges)). Do NOT apply anything. 3. **If `applicable` is empty** (and no overlap): nothing to do — pick the output by comparing the returned `local`/`engine` fields: equal → [Versions Already Equal](#versions-already-equal); `local` ahead of `engine` → [Local Version Ahead](#local-version-ahead); otherwise → [No Migrations Apply](#no-migrations-apply). (Semver comparison: compare MAJOR, then MINOR, then PATCH as integers — `2.10.0` > `2.9.7`; never compare lexicographically.) `fab upgrade-repo` already stamps `fab/.kit-migration-version` silently in the no-op case, so this subcommand has no version to write. @@ -329,7 +334,7 @@ For each migration file: ## Migrations Output Format -### Successful Multi-Step Migration +Canonical happy path (successful multi-step migration). The header scaffolding (`Local version:` / `Engine version:` / `Migrations found:`) and per-step block below are reused by the variants noted after it: ``` Local version: {current} @@ -347,60 +352,23 @@ Migrations found: {N} All migrations complete. fab/.kit-migration-version: {original} -> {final} ``` -### Migration with Gap Skip - -``` -Local version: {current} -Engine version: {target} -Migrations found: {N} - -No migration needed for {current} -> {FROM}, skipping. - -[1/{N}] Applying {FROM} -> {TO}... -{migration output} --> fab/.kit-migration-version updated to {TO} - -All migrations complete. fab/.kit-migration-version: {original} -> {final} -``` - -### Versions Already Equal - -``` -Already up to date ({version}). -``` - -### Local Version Ahead - -``` -Local version (fab/.kit-migration-version) is ahead of engine version ($(fab kit-path)/VERSION): {local} > {engine}. -This is unexpected — check your kit cache installation. -``` - -### No Migrations Apply - -``` -Local version: {current} -Engine version: {target} -No migrations apply. -``` - -(`fab migrations-status` returned an empty `applicable` list. `fab upgrade-repo` silently stamps `fab/.kit-migration-version` to the engine version in this no-op case, so there is nothing for this subcommand to write.) - -### Overlapping Ranges - -``` -Overlapping migration ranges detected: {file1} and {file2}. Fix the migrations directory. -``` - -### Mid-Chain Failure - -``` -[{N}/{total}] Applying {FROM} -> {TO}... -{partial output} -FAIL: Migration failed at {Pre-check|Changes|Verification} step: {description} -fab/.kit-migration-version remains at {current_version}. -Fix the issue and re-run /fab-setup migrations to continue from {current_version}. -``` +**Variants** (same header scaffolding unless noted; every literal below is exact): + +- **Gap skip**: before the first `[1/{N}]` block (and after the header), insert: `No migration needed for {current} -> {FROM}, skipping.` +- **Versions already equal**: `Already up to date ({version}).` +- **Local version ahead**: + `Local version (fab/.kit-migration-version) is ahead of engine version ($(fab kit-path)/VERSION): {local} > {engine}.` + `This is unexpected — check your kit cache installation.` +- **No migrations apply**: header scaffolding (just `Local version:` / `Engine version:`, no `Migrations found:`) followed by `No migrations apply.` (`fab migrations-status` returned an empty `applicable` list. `fab upgrade-repo` silently stamps `fab/.kit-migration-version` to the engine version in this no-op case, so there is nothing for this subcommand to write.) +- **Overlapping ranges**: `Overlapping migration ranges detected: {file1} and {file2}. Fix the migrations directory.` +- **Mid-chain failure** (replaces the per-step block from the failing step onward): + ``` + [{N}/{total}] Applying {FROM} -> {TO}... + {partial output} + FAIL: Migration failed at {Pre-check|Changes|Verification} step: {description} + fab/.kit-migration-version remains at {current_version}. + Fix the issue and re-run /fab-setup migrations to continue from {current_version}. + ``` --- diff --git a/src/kit/skills/fab-switch.md b/src/kit/skills/fab-switch.md index 7454afbc..48c7f805 100644 --- a/src/kit/skills/fab-switch.md +++ b/src/kit/skills/fab-switch.md @@ -9,6 +9,17 @@ description: "Switch the active change to a different one. Lists available chang --- +## Contents + +- Arguments +- Context Loading +- Behavior +- Output +- Error Handling +- Key Properties + +--- + ## Arguments - **``** *(optional)* — full or partial name of the change to switch to. Supports full folder names, partial slug matches, or any substring. Case-insensitive. @@ -47,19 +58,12 @@ If the command exits 1 and stderr contains "Multiple changes match": parse the c If the command exits 1 and stderr contains "No change matches": list all available changes, inform user. +`fab change switch` handles the full flow internally (resolves the name, creates the `.fab-status.yaml` symlink, outputs a structured summary with stage and next command); the skill displays its stdout directly. + ### Deactivation Flow (`--none`) Run `fab change switch --none`. Display the command's stdout output. -### Switch Flow - -`fab change switch` handles the full flow internally: -1. Resolves the change name -2. Creates `.fab-status.yaml` symlink -3. Outputs structured summary with stage and next command - -The skill displays the command's stdout directly. - ### Command Logging After a successful switch (not `--none`), log the command invocation: @@ -93,9 +97,9 @@ Next: {routing_stage} (via {default_command}) Tip: run /git-branch to create or switch to the matching branch ``` -Where `{display_stage}` is "where you are" (last active or last done stage) and `{routing_stage}` is "what's next" — the stage whose work runs next (first active/ready stage). `{default_command}` is the command that drives that stage (`intake`/`apply`/`review`/`hydrate` → `/fab-continue`, `ship` → `/git-pr`, `review-pr` → `/git-pr-review`), matching `/fab-status` and the `_preamble.md` state table. The `{state}` qualifier is any state the `display_state` derivation can emit: `active`, `failed`, `ready`, `done`, `skipped`, or `pending` — `ready` is the standard state of a freshly switched draft, `failed` surfaces a parked review/review-pr failure, `skipped` a skipped trailing stage. When all stages are done (or trailing stages skipped), `Next:` shows only `/fab-archive`. The pipeline has six stages; the `confidence.indicative` suffix is retired (1.10.0) — intake scoring is authoritative, so confidence is shown without qualifier. When score is `0.0` and no assumptions exist, shows `not yet scored`. +`{display_stage}` is "where you are" (last active/done stage); `{routing_stage}` is "what's next" (first active/ready stage), driven by `{default_command}` (`intake`/`apply`/`review`/`hydrate` → `/fab-continue`, `ship` → `/git-pr`, `review-pr` → `/git-pr-review`), matching `/fab-status` and the `_preamble.md` state table. When all stages are done (or trailing stages skipped), `Next:` shows only `/fab-archive`. `{state}` is any state `display_state` can emit: `active`, `failed`, `ready`, `done`, `skipped`, `pending` (`ready` = freshly switched draft, `failed` = parked review/review-pr failure, `skipped` = skipped trailing stage). Confidence has no qualifier (six-stage pipeline; the `confidence.indicative` suffix was retired in 1.10.0 — intake scoring is authoritative); score `0.0` with no assumptions shows `not yet scored`. -For the no-argument flow (listing changes), the skill reads `fab change list` output (format `name:display_stage:display_state:score` — the `:indicative` 5th field was dropped in 1.10.0) and displays confidence info alongside stage info in the numbered list. +No-argument flow: the skill reads `fab change list` output (format `name:display_stage:display_state:score` — the `:indicative` 5th field dropped in 1.10.0) and shows confidence alongside stage in the numbered list. Tip line omitted for `--none`. Deactivation shows `No active change.`. Already-deactivated shows `No active change (already deactivated).` @@ -108,7 +112,6 @@ Tip line omitted for `--none`. Deactivation shows `No active change.`. Already-d | No changes exist | "No active changes found. Run /fab-new or /fab-draft." | | Matched folder missing `.status.yaml` | Switch anyway, warn: "Warning: .status.yaml not found — change may be corrupted." | | `fab/changes/` doesn't exist | "fab/changes/ not found. Run /fab-setup." | -| `fab/project/config.yaml` not found | No impact (config not required) | --- diff --git a/src/kit/skills/git-branch.md b/src/kit/skills/git-branch.md index eacb2ae0..0621882a 100644 --- a/src/kit/skills/git-branch.md +++ b/src/kit/skills/git-branch.md @@ -8,6 +8,14 @@ allowed-tools: Bash(git:*) > Branch naming conventions are defined in `_preamble.md` § Naming Conventions. +## Contents + +- Arguments +- Behavior +- Output +- Error Handling +- Key Properties + Create or check out a git branch named `{change-name}` for the active or specified change. When an explicit argument doesn't match any change, falls back to creating a standalone branch with the literal name. Does not modify fab state. --- diff --git a/src/kit/skills/git-pr-review.md b/src/kit/skills/git-pr-review.md index 26500a07..a99bd228 100644 --- a/src/kit/skills/git-pr-review.md +++ b/src/kit/skills/git-pr-review.md @@ -14,13 +14,21 @@ Process GitHub PR review comments on the current branch's PR. Handles feedback f --- +## Contents + +- Behavior +- Rules +- Disposition Reference + +--- + ## Behavior ### Step 0: Start Review-PR Stage -Resolve the change first: +Resolve the change first (`fab change resolve` accepts a 4-char ID, folder substring, or full folder name — see `_cli-fab.md` § fab change): -- **Explicit `` argument provided** → run `fab change resolve 2>/dev/null` (transient override — `.fab-status.yaml` is untouched; accepts 4-char ID, folder substring, or full folder name). On failure, STOP with `Cannot resolve change ''.` — a named target that doesn't resolve is a caller error; do NOT fall back to the active change. +- **Explicit `` argument provided** → run `fab change resolve 2>/dev/null` (transient override — `.fab-status.yaml` is untouched). On failure, STOP with `Cannot resolve change ''.` — a named target that doesn't resolve is a caller error; do NOT fall back to the active change. - **No `` argument** → run `fab change resolve 2>/dev/null` (the active change). On failure, proceed with no change context — every `fab status` step below is skipped silently. On success, capture the output as `{name}` — the change folder name, used wherever later steps reference `` in `fab status` commands and in the Step 6.5 `fab/changes/{name}/…` file paths. @@ -99,13 +107,11 @@ review_tools: If `review_tools.copilot` is `false` (and `--tool copilot` was **not** provided): print `No automated reviewer available. Run /git-pr-review when reviews are added.` and go to Step 6 with outcome **no-reviews** (clean finish). -> **Two distinct logins — do NOT conflate** (getting these backwards is the #1 cause of a poll never seeing a review that has in fact landed): -> - The review-**request** side: you add the reviewer via `gh pr edit --add-reviewer copilot-pull-request-reviewer`, but the entry that then appears under the PR's **requested reviewers** surfaces with login `Copilot`. (Apparent oddity, documented as the empirically-observed reality: the value you `--add-reviewer` with happens to match the *landed-review* author login below, while `requested_reviewers` shows `Copilot`.) -> - The **landed-review** side: once a Copilot review actually lands, the review object in the `reviews` array carries `author.login == "copilot-pull-request-reviewer"` (commonly seen as `copilot-pull-request-reviewer[bot]`). This is the login the poll MUST match. +> **Two distinct logins — do NOT conflate** (getting these backwards is the #1 cause of a poll never seeing a review that has in fact landed): you add the reviewer via `gh pr edit --add-reviewer copilot-pull-request-reviewer`, but the entry that then surfaces under the PR's **requested reviewers** has login `Copilot`; once a review actually lands, the object in the `reviews` array carries `author.login == "copilot-pull-request-reviewer"` (commonly `copilot-pull-request-reviewer[bot]`). (Apparent oddity, recorded as empirical reality: the value you `--add-reviewer` with matches the landed-review author login, while `requested_reviewers` shows `Copilot`.) > -> The poll that detects a **landed review** MUST therefore match `author.login == "copilot-pull-request-reviewer"` (the review-author login on the `reviews` array), **not** `Copilot` (the login that surfaces under `requested_reviewers`) — a predicate keyed on the request-side login never matches a landed review object and the poll would time out even though the review arrived. This matches the established, deliberately-set behavior (`docs/memory/pipeline/execution-skills.md`: n30u documents `"Copilot"` in `requested_reviewers` vs `"copilot-pull-request-reviewer[bot]"` in `reviews`; u1m1 set the Phase 2 `.author.login` filter to `copilot-pull-request-reviewer` so incoming Copilot reviews are detected). Confirming the **request** itself succeeded is a separate question: GitHub's GraphQL `reviewRequests` field **omits bot/app reviewers** like Copilot, so a request confirmation MUST use REST `requested_reviewers` (`gh api repos/{owner}/{repo}/pulls/{number}/requested_reviewers`), never a GraphQL `reviewRequests` field. +> The landed-review poll MUST therefore match `author.login == "copilot-pull-request-reviewer"` (the review-author login on the `reviews` array), **not** `Copilot` (the `requested_reviewers` login) — a predicate keyed on the request-side login never matches a landed review object and would time out even though the review arrived. This matches the established, deliberately-set behavior (`docs/memory/pipeline/execution-skills.md`: n30u documents `"Copilot"` in `requested_reviewers` vs `"copilot-pull-request-reviewer[bot]"` in `reviews`; u1m1 set the Phase 2 `.author.login` filter to `copilot-pull-request-reviewer` so incoming Copilot reviews are detected). Confirming the **request** itself succeeded is separate: GraphQL `reviewRequests` **omits bot/app reviewers** like Copilot, so a request confirmation MUST use REST `requested_reviewers` (`gh api repos/{owner}/{repo}/pulls/{number}/requested_reviewers`), never GraphQL. -> **Synchronous-poll discipline — the subagent MUST NOT yield mid-poll.** When `/git-pr-review` runs as a dispatched subagent (e.g. from `/fab-fff` Step 5), the Copilot poll below MUST run **synchronously to completion within this single invocation**: the subagent MUST NOT yield, return, or hand back control while the poll is pending — it stays in the poll loop until either a Copilot review appears or all 20 attempts (the full 30s × 20 / 10-minute window) are exhausted, and only then proceeds to Step 3 or the timeout exit. This is a permanent, non-negotiable directive: in prior efforts the subagent stalled or died mid-poll **4 times**, leaving `review-pr` stuck `active`. Copilot reviews land ~4.5–6.5 min after the request — comfortably inside the 10-minute window — so the correct behavior is **patience-to-completion**, never an early return. +> **Synchronous-poll discipline — the subagent MUST NOT yield mid-poll.** When `/git-pr-review` runs as a dispatched subagent (e.g. from `/fab-fff` Step 5), the Copilot poll below MUST run **synchronously to completion within this single invocation**: the subagent MUST NOT yield, return, or hand back control while the poll is pending — it stays in the loop until a review appears or all 20 attempts (30s × 20 / 10-minute window) are exhausted, then proceeds to Step 3 or the timeout exit. This is a permanent, non-negotiable directive (prior efforts stalled mid-poll and left `review-pr` stuck `active`). Copilot reviews land ~4.5–6.5 min after the request — inside the window — so patience-to-completion is correct, never an early return. **Copilot request and poll**: @@ -221,7 +227,7 @@ Print: `Replied to {N} comment(s): {F} fix, {D} defer, {S} skip` ### Step 6: Update Review-PR Stage -**Step 6 is the exit point for every terminal path after Step 0, with two exceptions** — Steps 1, 2, and 4 route their terminal conditions here with a named outcome. The exceptions STOP directly without reaching Step 6: **Step 1.5** (invalid `--tool` value — stops before any review processing) and **Step 5** (commit failure — stops after `git reset`, no partial state; push failure — stops keeping the commit, with recovery guidance and no replies posted — the re-run's unpushed-commit gate completes the cycle). +Step 6 is the exit point for every terminal path after Step 0: Steps 1, 2, and 4 route here with a named outcome. The two direct-STOP exceptions that never reach Step 6 are **Step 1.5** (invalid `--tool`) and **Step 5** (commit failure after `git reset`; push failure keeping the commit with recovery guidance and no replies). If a change was resolved in Step 0 (active or explicit), act on the outcome class: diff --git a/src/kit/skills/git-pr.md b/src/kit/skills/git-pr.md index 50079649..f47b62dc 100644 --- a/src/kit/skills/git-pr.md +++ b/src/kit/skills/git-pr.md @@ -14,14 +14,23 @@ Autonomously ship local changes to a GitHub PR. No questions, no prompts — jus --- +## Contents + +- Behavior +- Rules +- PR Type Reference +- Key Properties + +--- + ## Behavior ### Step 0: Resolve Change Context Resolve the change **once** and derive four variables used throughout this skill. Later steps reference these variables and MUST NOT re-run `fab change resolve` — reuse this single resolution to avoid inconsistency. -1. Resolve the change: - - **Explicit `` argument provided** (per the Arguments classification above — any argument that is NOT one of the 7 valid PR types) → run `fab change resolve 2>/dev/null` (transient override — `.fab-status.yaml` is untouched; accepts 4-char ID, folder substring, or full folder name). **Succeeds** → `{has_fab} = true`, `{name}` = resolved change name. **Fails** → STOP with `Cannot resolve change ''.` — a named target that doesn't resolve is a caller error; do NOT fall back to the active change. +1. Resolve the change (`fab change resolve` accepts a 4-char ID, folder substring, or full folder name — see `_cli-fab.md` § fab change): + - **Explicit `` argument provided** (per the Arguments classification above — any argument that is NOT one of the 7 valid PR types) → run `fab change resolve 2>/dev/null` (transient override — `.fab-status.yaml` is untouched). **Succeeds** → `{has_fab} = true`, `{name}` = resolved change name. **Fails** → STOP with `Cannot resolve change ''.` — a named target that doesn't resolve is a caller error; do NOT fall back to the active change. - **No `` argument** → run `fab change resolve 2>/dev/null` (the active change). **Succeeds** → `{has_fab} = true`, `{name}` = resolved change name. **Fails** → `{has_fab} = false`; every step gated on `{has_fab}` is skipped silently. 2. `{has_intake}` — whether `fab/changes/{name}/intake.md` exists *(only when `{has_fab}`)*. 3. `{change_type}` — the `change_type` value from `fab/changes/{name}/.status.yaml` *(only when `{has_fab}`; may be null)*. @@ -51,7 +60,7 @@ Determine the PR type before gathering state. The type controls the PR title pre **Resolution chain** (evaluated in order, first match wins): -1. **Explicit argument**: If the invocation includes an argument that is one of the 7 valid types (case-insensitive), normalize to lowercase and use it. An argument that is not a valid type is the `` argument — it was consumed by Step 0 and does NOT count as a type; fall through to step 2. +1. **Explicit argument**: If the invocation includes an argument that is one of the 7 valid types (case-insensitive), normalize to lowercase and use it (non-type arguments are the `` argument, consumed by Step 0 — see Arguments above); else fall through to step 2. 2. **Read from `.status.yaml`**: If `{has_fab}` (Step 0) and `{change_type}` is non-null and one of the 7 valid types (`feat`, `fix`, `refactor`, `docs`, `test`, `ci`, `chore`), use `{change_type}`. Fall through if `{has_fab}` is false, `{change_type}` is null, or `{change_type}` is not a valid type. @@ -92,13 +101,13 @@ default_branch=$(git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | If `{has_fab}` (Step 0), read issues via `fab status get-issues {name}` and capture the output (one ID per line, may be empty). Determine: -- **branch** — current branch name. An **empty** value means a detached HEAD (`git symbolic-ref -q HEAD` exits 1) — handled by the Step 2 guard before any commit or push -- **has_uncommitted** — whether `git status --porcelain` has output -- **has_unpushed** — whether there are commits ahead of upstream (or no upstream at all) -- **pr_state** — the `state` field from `gh pr view` (`OPEN`, `CLOSED`, or `MERGED`), or `none` when no PR exists. Step 3 branches on this explicitly — a CLOSED or MERGED PR is NOT treated as "the branch already has a PR" -- **number** / **url** — the `number` and `url` fields from `gh pr view` (unset when no PR exists). Interpolated by Step 3's MERGED STOP and the "already shipped" output -- **default_branch** — the resolved default branch from the commands above (symbolic-ref first, `gh repo view` fallback, then the probed literal fallback: `main` when `refs/remotes/origin/main` exists, else `master` — mirroring the operator's strategy). Always non-empty, so every later `{default_branch}` interpolation is meaningful -- **issues** — the issue IDs from `fab status get-issues` (space-joined), or empty if none +- **branch** — current branch name; **empty** = detached HEAD (handled by the Step 2 guard before any commit or push) +- **has_uncommitted** — `git status --porcelain` has output +- **has_unpushed** — commits ahead of upstream, or no upstream at all +- **pr_state** — `state` from `gh pr view` (`OPEN`, `CLOSED`, `MERGED`), or `none` when no PR exists. Step 3 branches on this explicitly — a CLOSED or MERGED PR is NOT treated as "the branch already has a PR" +- **number** / **url** — from `gh pr view` (unset when no PR exists); interpolated by Step 3's MERGED STOP and the "already shipped" output +- **default_branch** — resolved by the commands above (always non-empty), so every later `{default_branch}` interpolation is meaningful +- **issues** — issue IDs from `fab status get-issues` (space-joined), or empty ### Step 2: Branch Guard @@ -108,7 +117,7 @@ Determine: Cannot ship from a detached HEAD — check out a branch first (run /git-branch). ``` -**Default-branch guard**: if the current branch is `{default_branch}` (or literal `main`/`master` when Step 1 fell back to the probed literal — the probe picks one name, so the other literal stays a safety net), STOP immediately. +**Default-branch guard**: if the current branch is `{default_branch}` (or literal `main`/`master` — whichever literal Step 1's probe did not pick stays a safety net), STOP immediately. If `{has_fab}` (Step 0), enhance the message: @@ -201,7 +210,7 @@ fi Print (ONLY when a follow-up commit was actually made): ` ✓ commit — "docs: refresh memory indexes"` -> **Why here, why gated.** This is the first moment `git log` reports the change's real commit date — `fab memory-index` stamps each index row's "Last Updated" cell from `git log` (committed dates only), and 3a's content commit only just landed. The step lives in **ship** (not hydrate) because hydrate is entirely pre-commit, so no in-hydrate regen can see the change's own commit. There is no push here — 3b ("if has_unpushed or just committed") pushes both the 3a content commit and this index-refresh commit together. When `/git-pr` runs standalone outside a fab project (`{has_fab}` false), this sub-step is a **silent no-op**, so general-purpose standalone use is unaffected. +> **Why here, why gated.** This is the first moment `git log` reports the change's real commit date (`fab memory-index` stamps "Last Updated" from committed dates), so the step lives in **ship** not hydrate — hydrate is entirely pre-commit, so no in-hydrate regen can see the change's own commit. There is no push here; 3b pushes both commits together. When `/git-pr` runs standalone (`{has_fab}` false) this sub-step is a **silent no-op**. #### 3b. Push (if has_unpushed or just committed) @@ -348,7 +357,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 (memory-index refresh) is gated on 3a having **just committed this invocation**, so a re-run on the no-commit path skips it entirely; even if reached, `fab memory-index` is byte-stable and the `git diff --quiet -- docs/memory` guard suppresses an empty follow-up commit | +| 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) | | Advances stage? | Yes — ship (start/finish, best-effort) | | Modifies `.fab-status.yaml`? | No | | Modifies git state? | Yes — commit, push, PR creation |