From a7289f6eabe64e7c7d544d58e1fdc2db060cd8cf Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 13:29:06 +0300 Subject: [PATCH 01/21] feat: gate AI-attribution suppression behind typed flag with init wizard (#315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1 — FLAG_REGISTRY: add `suppress-attribution` boolean flag (setting target, key: `attribution`, onPayload: {commit:'',pr:''}). Extends `BooleanFlagDef` with `onPayload: string | boolean | Record` and adds an optional `settingDeleteGuard` field for shape-guarded deletion. R2 — Flags pipeline is sole attribution writer/remover: remove the attribution block from src/targets/claude-code/templates/settings.json and remove the attribution injection branch from mergeDevflowSettingsTemplate in post-install.ts. R3 — Attribution wizard: new src/cli/commands/attribution-prompts.ts with shouldRunAttributionStep, AttributionPromptIO DI seam, buildClackAttributionPrompts, and runAttributionStep. Wizard runs in both init paths (Recommended + Advanced) via the same modePromptShown gate as the compliance step (PF-029). R4 — Seeding: resolveExistingAttributionSuppression in init-seed.ts reads the exact devflow attribution shape {"commit":"","pr":""} from settings.json and seeds the flag true; resolveInitSeed encodes the result into FlagsRecord at composition time (mirrors view-mode priority: settings.json > manifest > false). R5 — Shape-guarded deletion (D-ATTR-GUARD): deepEqualsPlain helper in flags.ts; applyFlags and stripFlags both check flag.settingDeleteGuard before deleting a setting key — custom attribution values survive flag disable and uninstall. R6 — Tests: 349 tests across 4 files all pass (flags, attribution-prompts, init-seed, post-install-merge). Full suite: 3980 tests pass; pre-existing flaky timing test in redact-secrets.test.ts unaffected by these changes. R7 — CLAUDE.md: bump flag count 28→29, add suppress-attribution to optional- boolean list with D27/D-ATTR-GUARD annotation, add attribution wizard step description to Two-Mode Init section. Applies PF-015 (toggle fan-out convergence), PF-029 (wizard gate predicate), PF-014 (no process.exit in step runner), ADR-014 (state-aware seeding), ADR-019 (typed flag registry), ADR-020 (no flags editor in init). --- CLAUDE.md | 4 +- src/cli/commands/attribution-prompts.ts | 168 ++++++++++++++++++ src/cli/commands/init-seed.ts | 54 +++++- src/cli/commands/init.ts | 57 ++++++ src/core/flags.ts | 75 +++++++- src/targets/claude-code/post-install.ts | 14 +- .../claude-code/templates/settings.json | 4 - tests/attribution-prompts.test.ts | 167 +++++++++++++++++ tests/flags.test.ts | 90 +++++++++- tests/init-seed.test.ts | 85 +++++++++ tests/post-install-merge.test.ts | 21 ++- 11 files changed, 706 insertions(+), 33 deletions(-) create mode 100644 src/cli/commands/attribution-prompts.ts create mode 100644 tests/attribution-prompts.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6a2fa177..e342afac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. **Debug Tracing**: Single global toggle covering all hooks. Enabled via `devflow debug --enable/--disable/--status` CLI or by setting `DEVFLOW_HOOK_DEBUG=1` in `~/.claude/settings.json` env block (survives reinstalls). All hooks share the `src/assets/scripts/hooks/debug-trace` helper script (sourced via `hook-bootstrap`) so tracing behavior is consistent and updated in one place. Two-phase logging: pre-CWD traces go to global `~/.devflow/logs/.hook-debug.log`; post-CWD traces go to per-project `~/.devflow/logs/{project-slug}/.hook-debug.log`. A 5MB size guard prevents unbounded growth. applies ADR-007 -**Claude Code Flags**: Typed registry (`src/core/flags.ts`) for managing Claude Code feature flags (env vars and top-level settings). Four kinds: `boolean` (on/off), `enum` (validated domain), `number` (bounded integer), `string` (validated with maxLength). 28 flags total: recommended (default ON) — `tui`, `tool-search`, `lsp`, `prompt-caching-1h`, `show-turn-duration`, `clear-context-on-plan`, `disable-bundled-skills`, `pin-sonnet-4-6`, `max-concurrent-subagents` (number, devflow default 40, upstream default 20); optional boolean (default OFF) — `brief`, `thinking-summaries`, `subprocess-env-scrub`, `disable-nonessential-traffic`, `forked-subagents`, `disable-adaptive-thinking`, `always-thinking`, `disable-git-instructions`, `disable-compact`, `disable-1m-context`, `disable-autoupdater`, `agent-teams`, `enable-todo-tools`; valued (default unset) — `subagent-spawn-depth` (number, upstream default 3), `workflow-size-guideline` (enum: `small|medium|large|unrestricted`), `default-model` (string), `goal-checkin-minutes` (number, upstream default 30 min), `spellcheck` (string), `view-mode` (enum: `default|verbose|focus`, devflow default `default`). Stored in manifest `features.flags: Record` — entry-presence = known, `null` = deliberately unset (neutral, deletes the target key), absent = adopt-on-next-init. Pipeline: `applyFlags(settingsJson, FlagsRecord)` / `stripFlags(settingsJson)` — `applyViewMode`/`stripViewMode` retired; view-mode is an enum flag with `neutralValue: 'default'` (the `viewMode` settings.json key is written only when non-default); `resolveExistingViewMode`/`resolveFinalViewMode` remain exported for init.ts external-mode preservation. `devflow flags` bare on TTY launches the interactive flags editor TUI; bare on non-TTY prints a status table to stdout and exits 1. Registry entries carry a `blurb` field (≤30-char per-flag short hint) shown as a dim HINT column in the TUI and in `--status` rows. Display vocabulary via `effectiveDisplay`: booleans render 'on'/'off' (off is dim); neutral/unset enum shows `neutralValue` dim; unset number shows its applicable default dim with ' (default)' suffix; unset string shows '—' dim; an actively set non-boolean renders plain (at devflow default) or bold (deviating); the literal 'unset' is never a displayed value. TUI rendering: `RunTuiSpec.screen?: 'alt' | 'inline'`; flags editor runs inline (renders in-place in the normal scroll buffer, no alt-screen); agents-view defaults to alt. Manageable via `devflow flags --enable/--disable/--set /--unset /--status/--list`; `--enable`/`--disable` are boolean-only — non-boolean flags are redirected to `--set`. +**Claude Code Flags**: Typed registry (`src/core/flags.ts`) for managing Claude Code feature flags (env vars and top-level settings). Four kinds: `boolean` (on/off), `enum` (validated domain), `number` (bounded integer), `string` (validated with maxLength). 29 flags total: recommended (default ON) — `tui`, `tool-search`, `lsp`, `prompt-caching-1h`, `show-turn-duration`, `clear-context-on-plan`, `disable-bundled-skills`, `pin-sonnet-4-6`, `max-concurrent-subagents` (number, devflow default 40, upstream default 20); optional boolean (default OFF) — `brief`, `thinking-summaries`, `subprocess-env-scrub`, `disable-nonessential-traffic`, `forked-subagents`, `disable-adaptive-thinking`, `always-thinking`, `disable-git-instructions`, `disable-compact`, `disable-1m-context`, `disable-autoupdater`, `agent-teams`, `enable-todo-tools`, `suppress-attribution` (writes `{"commit":"","pr":""}` to settings.json — suppresses Claude attribution in git commits and PRs; shape-guarded deletion: only removed on disable when the value exactly matches the devflow-managed shape, never when a user has a custom attribution; D27/D-ATTR-GUARD); valued (default unset) — `subagent-spawn-depth` (number, upstream default 3), `workflow-size-guideline` (enum: `small|medium|large|unrestricted`), `default-model` (string), `goal-checkin-minutes` (number, upstream default 30 min), `spellcheck` (string), `view-mode` (enum: `default|verbose|focus`, devflow default `default`). Stored in manifest `features.flags: Record` — entry-presence = known, `null` = deliberately unset (neutral, deletes the target key), absent = adopt-on-next-init. Pipeline: `applyFlags(settingsJson, FlagsRecord)` / `stripFlags(settingsJson)` — `applyViewMode`/`stripViewMode` retired; view-mode is an enum flag with `neutralValue: 'default'` (the `viewMode` settings.json key is written only when non-default); `resolveExistingViewMode`/`resolveFinalViewMode` remain exported for init.ts external-mode preservation. `devflow flags` bare on TTY launches the interactive flags editor TUI; bare on non-TTY prints a status table to stdout and exits 1. Registry entries carry a `blurb` field (≤30-char per-flag short hint) shown as a dim HINT column in the TUI and in `--status` rows. Display vocabulary via `effectiveDisplay`: booleans render 'on'/'off' (off is dim); neutral/unset enum shows `neutralValue` dim; unset number shows its applicable default dim with ' (default)' suffix; unset string shows '—' dim; an actively set non-boolean renders plain (at devflow default) or bold (deviating); the literal 'unset' is never a displayed value. TUI rendering: `RunTuiSpec.screen?: 'alt' | 'inline'`; flags editor runs inline (renders in-place in the normal scroll buffer, no alt-screen); agents-view defaults to alt. Manageable via `devflow flags --enable/--disable/--set /--unset /--status/--list`; `--enable`/`--disable` are boolean-only — non-boolean flags are redirected to `--set`. **Feature Knowledge Bases**: Per-feature `.devflow/features/` directory containing KNOWLEDGE.md files that capture area-specific patterns, conventions, architecture, and gotchas. Uses a **write-through** model: load = direct file-I/O reading `.devflow/features/index.md` (regenerable cache) with frontmatter-glob fallback over `features/*/KNOWLEDGE.md` (source of truth) + verify-against-code on read; save = in-command write-through via a simplified Knowledge agent that writes `KNOWLEDGE.md` + the `index.md` line directly (no `.create-result.json`, no external scripts, no lock). **Git-tracked & shared (amends ADR-021 for `features/`)**: the root `.gitignore` carve-out (`.devflow/*` + level-by-level `!` re-includes, written byte-identically by `ensure-root-gitignore` / `ensureDevflowGitignore`) un-ignores `.devflow/features/index.md` + every `{slug}/KNOWLEDGE.md` while the rest of `.devflow/` stays local; after writing, the **Knowledge agent commits those two paths to the current worktree branch itself** by running git via its Bash tool (scoped `commit --only` pathspec, never `git add -A`, **never push, never force**, no commit script — per the LLM-vs-plumbing principle the commit is the agent's, not a deterministic helper). A user opts back out by re-adding `.devflow/features/` to their own `.gitignore`. Existing installs upgrade once via the versioned `.root-gitignore-configured-v3` marker (v2→v3 adds the `!.devflow/conventions.md` re-include). Freshness = write-through + verify-on-read (NO git-staleness, NO SessionEnd eval, NO Learning task). `index.md` line format: `- **{slug}** — {areas} — {Use-when description}`; frontmatter is authoritative if the line is lost. MDS module: `src/assets/commands/_partials/_knowledge.mds` (defines/exports `knowledge_load` and `knowledge_writeback` partials) + 9 host `.mds` sources in `src/assets/commands/` compiled to `dist/commands/` by `scripts/build-mds.ts` (`npm run build:mds`). `knowledge_load` is used up-front by: implement, plan, resolve, code-review, self-review, research, bug-analysis. `knowledge_writeback` is used at workflow end by: implement, resolve, self-review, explore, debug. explore/debug do NOT load up-front (intentional asymmetry). Config gate: single `knowledge: true|false` in feature config (default true) — gates write-back only; load is ungated. CLI: `devflow knowledge list` (read index.md / frontmatter glob), `devflow knowledge --enable/--disable/--status` (flip config). Note: `/debug` keeps FEATURE_KNOWLEDGE orchestrator-local (investigation workers examine code without pre-loaded context). Toggleable via `devflow knowledge --enable/--disable/--status` or `devflow init --knowledge/--no-knowledge`. @@ -67,7 +67,7 @@ Knowledge write-back is in-command (not a background pipeline): gated by `devflo **Per-Agent Model Configuration**: User overrides to agent model assignments persist in `~/.devflow/agent-models.json` (deviations only — absent entry = shipped default). `reapplyAgentMapping` runs after every `devflow init` post-install to re-apply user overrides to freshly copied agent files. `revertExternalAgents` reverts all agents to shipped defaults (called on proxy disable and before agent removal on uninstall). GPT model assignments are **dormant** when routing is off — they are stored in `agent-models.json` but not written to agent frontmatter until routing is enabled. Manage via `devflow agents` TUI or `devflow agents --list/--set/--reset`. Core source files: `src/core/agent-frontmatter.ts` (pure rewrite engine), `src/core/agent-models.ts` (schema + apply/revert), `src/core/external-models.ts` (CLAUDE_MODEL_ALIASES, isClaudeModelName, isDormantExternalModel — leaf module), `src/core/model-discovery.ts` (discoverExternalModels, getExternalModelsCached, cache-warming), `src/core/cache.ts` (cache read/write, 0700/0600 permissions, parseRawEnvelope), `src/core/proxy-log.ts` (scrubChildEnv, openProxyLog, relay env allowlisting), `src/core/proxy-state.ts` (state I/O), `src/cli/commands/proxy.ts` (CLI + hook wiring), `src/cli/commands/agents.ts` (CLI), `src/cli/agents-view/` (TUI — state, render, terminal; thin adapter over the shared `src/cli/tui/` driver). -**Two-Mode Init**: `devflow init` offers Recommended (sensible defaults, quick setup) or Advanced (full interactive flow) after plugin selection. `--recommended` / `--advanced` CLI flags for non-interactive use. Recommended applies: ambient ON, memory ON, learning ON, rules ON, HUD ON, default-ON flags, .claudeignore ON, auto-install safe-delete if trash CLI detected, user-mode security deny list, viewMode preserved from existing settings.json. Both init paths apply seeded flag values non-interactively — fresh install: registry defaults; re-init: existing manifest values preserved, defaults adopted only for newly-added flags (ADR-014); `view-mode` resolved from existing settings.json at seed time — and emit an outcome line pointing to `devflow flags` for customization. Advanced path adds a proxy prompt (external model routing — default OFF, requires Codex auth; never part of Recommended defaults). Use `--learning/--no-learning` to toggle the learning agent independently. Use `--rules/--no-rules` to toggle rules independently. Use `--proxy/--no-proxy` to set external model routing (Advanced-only; init runs preflight on enable). Use `--compliance `/`--no-compliance` to set compliance non-interactively (enable with comma-separated framework IDs or disable preserving frameworks; default: off; `--compliance`/`--no-compliance` bypasses the wizard entirely). The compliance wizard step (select which regulatory frameworks to install — GDPR, HIPAA, PCI DSS, SOC 2, ISO 27001, SOX) runs in **both** init paths via `shouldRunComplianceStep`: Advanced always runs it; Recommended only runs it when the user reached the mode-select prompt interactively (`modePromptShown=true`) — `--recommended` flag and non-TTY invocations preserve their promptless contracts. The step shows a "Current setting:" note for re-init legibility, uses a `p.select` (Yes/No) instead of a confirm to avoid Enter-through ambiguity, and emits an outcome line for unambiguous state visibility (per PF-029). **State-aware re-init**: on re-init the wizard reads the prior manifest, config, and settings.json and pre-seeds every prompt with existing values, skipping the Recommended/Advanced question entirely. Use `--reset` for a factory reset that ignores all prior state (mutually exclusive with `--plugin`). +**Two-Mode Init**: `devflow init` offers Recommended (sensible defaults, quick setup) or Advanced (full interactive flow) after plugin selection. `--recommended` / `--advanced` CLI flags for non-interactive use. Recommended applies: ambient ON, memory ON, learning ON, rules ON, HUD ON, default-ON flags, .claudeignore ON, auto-install safe-delete if trash CLI detected, user-mode security deny list, viewMode preserved from existing settings.json. Both init paths apply seeded flag values non-interactively — fresh install: registry defaults; re-init: existing manifest values preserved, defaults adopted only for newly-added flags (ADR-014); `view-mode` resolved from existing settings.json at seed time — and emit an outcome line pointing to `devflow flags` for customization. Advanced path adds a proxy prompt (external model routing — default OFF, requires Codex auth; never part of Recommended defaults). Use `--learning/--no-learning` to toggle the learning agent independently. Use `--rules/--no-rules` to toggle rules independently. Use `--proxy/--no-proxy` to set external model routing (Advanced-only; init runs preflight on enable). Use `--compliance `/`--no-compliance` to set compliance non-interactively (enable with comma-separated framework IDs or disable preserving frameworks; default: off; `--compliance`/`--no-compliance` bypasses the wizard entirely). The compliance wizard step (select which regulatory frameworks to install — GDPR, HIPAA, PCI DSS, SOC 2, ISO 27001, SOX) runs in **both** init paths via `shouldRunComplianceStep`: Advanced always runs it; Recommended only runs it when the user reached the mode-select prompt interactively (`modePromptShown=true`) — `--recommended` flag and non-TTY invocations preserve their promptless contracts. The step shows a "Current setting:" note for re-init legibility, uses a `p.select` (Yes/No) instead of a confirm to avoid Enter-through ambiguity, and emits an outcome line for unambiguous state visibility (per PF-029). **Attribution wizard step** (D27): the `suppress-attribution` wizard question runs in **both** init paths after the compliance step, using the same `shouldRunAttributionStep` gate predicate (same `modePromptShown` logic as compliance, per PF-029) — Yes writes `{"commit":"","pr":""}` to suppress Claude attribution in git history, No preserves attribution labels; seeded from settings.json (exact devflow shape → true) then manifest then false; no CLI override path (toggle via `devflow flags --enable/--disable suppress-attribution`); shape-guarded deletion means a user's custom attribution value is never erased. **State-aware re-init**: on re-init the wizard reads the prior manifest, config, and settings.json and pre-seeds every prompt with existing values, skipping the Recommended/Advanced question entirely. Use `--reset` for a factory reset that ignores all prior state (mutually exclusive with `--plugin`). **Migrations**: Run-once migrations execute automatically on `devflow init`, tracked at `~/.devflow/migrations.json` (scope-independent; single file regardless of user-scope vs local-scope installs). To add a 2.x migration, append an entry to `MIGRATIONS` in `src/core/migrations.ts`. Scopes: `global` (runs once per machine, no project context) vs `per-project` (sweeps all discovered Claude-enabled projects in parallel). Failures are non-fatal — migrations retry on next init. The registry holds 2.x entries only (first: canonicalise-agent-keys-v1); no 1.x upgrade path. diff --git a/src/cli/commands/attribution-prompts.ts b/src/cli/commands/attribution-prompts.ts new file mode 100644 index 00000000..b48fbb40 --- /dev/null +++ b/src/cli/commands/attribution-prompts.ts @@ -0,0 +1,168 @@ +/** + * Attribution prompt helpers for devflow init. + * + * CLI-layer module (ADR-013): prompt-rendering logic lives in src/cli/commands/, + * core business logic stays in src/core/. + * + * Applies PF-029: every wizard gate keys on `modePromptShown`, never on the mode + * name, so --recommended (flag, no prompt) and the non-TTY fallback preserve their + * promptless contracts. + * Applies PF-014: runAttributionStep never calls process.exit() or throws — callers + * own the cancel idiom (p.cancel + process.exit(0)), keeping try/finally cleanup safe. + * + * D27: suppress-attribution flag — gates Claude Code's AI-attribution injection. + */ + +import * as p from '@clack/prompts'; + +// ── Gate predicate ───────────────────────────────────────────────────────────── + +/** + * Determines whether the attribution wizard step should run for a given init invocation. + * + * Gate table (per PF-029: key on modePromptShown, never on the mode name): + * + * --recommended flag / !isTTY fallback → no (promptless contract preserved) + * Interactive mode-prompt → Recommended → yes (modePromptShown=true) + * --advanced flag / re-init (banner path) → yes (mode='advanced', isTTY=true) + * Interactive mode-prompt → Advanced → yes (modePromptShown=true) + * Any path with a hasCliOverride for this step → no (CLI override wins) + * + * Pure predicate — no side effects, fully testable without a TTY. + * Mirrors shouldRunComplianceStep exactly (same gate table per PF-029). + */ +export function shouldRunAttributionStep(input: { + mode: 'recommended' | 'advanced'; + modePromptShown: boolean; + isTTY: boolean; + hasCliOverride: boolean; +}): boolean { + if (input.hasCliOverride) return false; + if (!input.isTTY) return false; + // Advanced path: non-TTY has already exit-1'd, so isTTY=true here → always run. + // Covers: --advanced flag, re-init banner path, interactive-prompt → advanced. + if (input.mode === 'advanced') return true; + // Recommended path: only run when the Setup-mode p.select actually ran + // (user made an active choice). --recommended flag and !isTTY fallback never set + // modePromptShown=true, preserving their promptless contracts. + return input.modePromptShown; +} + +// ── DI seam ──────────────────────────────────────────────────────────────────── + +/** Discriminated union returned by every AttributionPromptIO method. */ +export type PromptOutcome = { kind: 'value'; value: T } | { kind: 'cancel' }; + +/** + * Injectable prompt interface for runAttributionStep. + * Mirrors CompliancePromptIO (src/cli/commands/compliance-prompts.ts). + * Enables unit tests to drive all branches without a real TTY. + */ +export interface AttributionPromptIO { + note: (message: string, title: string) => void; + select: (opts: { + message: string; + options: Array<{ value: boolean; label: string; hint: string }>; + initialValue: boolean; + }) => Promise>; +} + +/** + * Build the real (clack) AttributionPromptIO adapter. + * Translates clack's cancel symbol into the PromptOutcome discriminated union. + */ +export function buildClackAttributionPrompts(): AttributionPromptIO { + return { + note: (message, title) => p.note(message, title), + + select: async (opts) => { + const result = await p.select({ + message: opts.message, + options: opts.options, + initialValue: opts.initialValue, + }); + if (p.isCancel(result)) return { kind: 'cancel' }; + return { kind: 'value', value: result as boolean }; + }, + }; +} + +// ── Step runner ──────────────────────────────────────────────────────────────── + +/** Message emitted after the attribution step resolves. */ +export interface AttributionStepMessage { + level: 'success' | 'info'; + text: string; +} + +/** The attribution step completed (user answered Yes or No). */ +export interface AttributionStepResolved { + kind: 'resolved'; + suppress: boolean; + messages: AttributionStepMessage[]; +} + +/** The attribution step was cancelled (user pressed Escape). */ +export interface AttributionStepCancelled { + kind: 'cancelled'; +} + +export type AttributionStepOutcome = AttributionStepResolved | AttributionStepCancelled; + +/** + * Run the attribution wizard step. + * + * Flow: + * 1. Note — "Current setting: …" header with context about what the flag does. + * 2. Enable select — labeled Yes / No with hints (seeded from prior state); + * p.select is immune to Enter-through muscle memory while still preserving + * the seeded value (ambient-prompt style — per PF-029). + * + * Returns: + * {kind:'resolved', suppress, messages} — step completed; `suppress` is the chosen + * boolean; `messages` are emitted by the caller. + * {kind:'cancelled'} — user pressed Escape; caller runs p.cancel + process.exit(0). + * + * Invariants (PF-014): + * - Never calls process.exit(), never throws. + * - All I/O is routed through the `prompts` parameter (injectable for tests). + */ +export async function runAttributionStep(opts: { + seed: boolean; + prompts: AttributionPromptIO; +}): Promise { + const { seed, prompts } = opts; + + const currentStr = seed ? 'suppressed' : 'shown (default)'; + prompts.note( + `Current setting: ${currentStr}\n\n` + + 'When enabled, writes {"commit":"","pr":""} to settings.json, which\n' + + 'suppresses AI-attribution labels in git commits and pull requests.\n' + + 'Toggle any time with: devflow flags --enable suppress-attribution\n\n' + + 'Note: Only removes the devflow-managed attribution block on disable;\n' + + 'custom attribution values you set manually are never deleted.', + 'AI Attribution', + ); + + const enableOutcome = await prompts.select({ + message: 'Suppress AI attribution in commits and PRs?', + options: [ + { value: true, label: 'Yes', hint: 'hides Claude attribution labels in git history' }, + { value: false, label: 'No', hint: 'keeps Claude attribution labels (default)' }, + ], + initialValue: seed, + }); + + if (enableOutcome.kind === 'cancel') return { kind: 'cancelled' }; + + const suppress = enableOutcome.value; + const text = suppress + ? 'Attribution: suppressed — disable with devflow flags --disable suppress-attribution' + : 'Attribution: shown (default)'; + + return { + kind: 'resolved', + suppress, + messages: [{ level: suppress ? 'success' : 'info', text }], + }; +} diff --git a/src/cli/commands/init-seed.ts b/src/cli/commands/init-seed.ts index eb33c7a0..aa08e81e 100644 --- a/src/cli/commands/init-seed.ts +++ b/src/cli/commands/init-seed.ts @@ -223,12 +223,42 @@ export function resolveSeedPlugins( return { workflowPlugins, languagePlugins }; } +/** + * Extract the attribution suppression state from a settings JSON string. + * + * Returns `true` when the exact devflow-managed attribution shape + * `{"commit":"","pr":""}` is present, signaling that suppress-attribution was + * active at the last install. Returns `undefined` when the block is absent or + * holds a different (user-custom) value — callers fall through to the manifest + * FlagsRecord entry or the registry default. + * + * Mirrors resolveExistingViewMode: returns undefined on malformed JSON, absent + * key, or any non-devflow attribution value. + * + * Pure function — no I/O, no side effects. + */ +export function resolveExistingAttributionSuppression(settingsJson: string): boolean | undefined { + try { + const parsed: unknown = JSON.parse(settingsJson); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return undefined; + const attr = (parsed as Record).attribution; + if (attr === null || typeof attr !== 'object' || Array.isArray(attr)) return undefined; + const a = attr as Record; + // Only match the exact devflow-managed shape: {"commit":"","pr":""} with no extra keys. + if (a['commit'] === '' && a['pr'] === '' && Object.keys(a).length === 2) return true; + return undefined; // custom value — fall through to manifest / default + } catch { + return undefined; + } +} + /** * Compose the full init seed from manifest, project config, settings, and registry. * * view-mode priority: existing settings.json (non-default) → manifest → 'default'. - * The resolved view mode is encoded into flags['view-mode'] so all flag state lives - * in one FlagsRecord (applying PF-015: fold before strip — the fold happens here). + * suppress-attribution priority: settings.json exact devflow shape → manifest → false. + * All flag overrides are encoded into flags so all flag state lives in one FlagsRecord + * (applying PF-015: fold before strip — the fold happens here). * * This is the single composition point; callers (init.ts hoist block) call this * once and pass `seed` down to prompt wiring. @@ -266,9 +296,27 @@ export function resolveInitSeed( resolvedViewMode = 'default'; // fall back to neutral } + // Encode resolved attribution suppression (D27) into flags['suppress-attribution']. + // Priority: settings.json exact devflow shape → manifest FlagsRecord entry → false. + // resolveExistingAttributionSuppression returns true only for the exact managed shape + // {"commit":"","pr":""}; custom values return undefined so the manifest entry wins. + const existingAttr = resolveExistingAttributionSuppression(settingsSnapshot); + const resolvedSuppressAttr: boolean = existingAttr !== undefined + ? existingAttr // settings.json exact shape wins + : ((flags['suppress-attribution'] as boolean) ?? false); // manifest or registry default + // Return a fresh spread rather than mutating flags in place — keeps this function pure // per the module docblock and avoids aliasing if the caller inspects seed.flags. - return { features, flags: { ...flags, 'view-mode': resolvedViewMode }, workflowPlugins, languagePlugins }; + return { + features, + flags: { + ...flags, + 'view-mode': resolvedViewMode, + 'suppress-attribution': resolvedSuppressAttr, + }, + workflowPlugins, + languagePlugins, + }; } /** diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index a1fa1c4c..c73b80c1 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -56,6 +56,11 @@ import { runComplianceStep, buildClackCompliancePrompts, } from './compliance-prompts.js'; +import { + shouldRunAttributionStep, + runAttributionStep, + buildClackAttributionPrompts, +} from './attribution-prompts.js'; import { convergeFromManifest } from '../../targets/claude-code/compliance-install.js'; import { getPendingTurnsPath, getPendingTurnsProcessingPath } from '../../core/project-paths.js'; import * as os from 'os'; @@ -680,6 +685,31 @@ export const initCommand = new Command('init') // prints the Compliance line from complianceSummary via formatComplianceSummary. } + // B5: attribution wizard step — runs only when the Setup-mode prompt actually ran + // (modePromptShown=true), preserving the promptless contracts of --recommended and !isTTY. + // shouldRunAttributionStep gates on modePromptShown rather than the mode name (PF-029). + // No CLI override path exists for attribution on the Recommended path (D27). + let wizardSuppressAttribution: boolean | undefined; + if (shouldRunAttributionStep({ + mode: 'recommended', + modePromptShown, + isTTY: process.stdin.isTTY, + hasCliOverride: false, + })) { + const attributionStep = await runAttributionStep({ + seed: enabledFlags['suppress-attribution'] as boolean, + prompts: buildClackAttributionPrompts(), + }); + if (attributionStep.kind === 'cancelled') { + p.cancel('Installation cancelled.'); + process.exit(0); + } + wizardSuppressAttribution = attributionStep.suppress; + } + if (wizardSuppressAttribution !== undefined) { + enabledFlags = { ...enabledFlags, 'suppress-attribution': wizardSuppressAttribution }; + } + // Apply explicit CLI toggles on top of the seed. // Precedence: explicit CLI flag > wizard result > seed value (prior state > registry default). // proxy is included: --proxy/--no-proxy CLI flags override the seed in non-interactive mode. @@ -947,6 +977,33 @@ export const initCommand = new Command('init') // CLI override (isTTY is guaranteed true by the non-TTY guard above). If it ever // did, the seed values assigned at declaration stand — which is the right default. + // Attribution feature (after compliance, before flags — runs in both Advanced and re-init paths). + // Gated by the same shouldRunAttributionStep predicate as the Recommended path so the + // documented gate table is the single authority for both — the two paths cannot drift. + // Here isTTY is guaranteed true (the non-TTY guard above exit-1'd), so the predicate + // reduces to "no CLI override" — no CLI override exists for attribution (D27). + if (shouldRunAttributionStep({ + mode: 'advanced', + modePromptShown, + isTTY: process.stdin.isTTY, + hasCliOverride: false, + })) { + const attributionStep = await runAttributionStep({ + seed: enabledFlags['suppress-attribution'] as boolean, + prompts: buildClackAttributionPrompts(), + }); + if (attributionStep.kind === 'cancelled') { + p.cancel('Installation cancelled.'); + process.exit(0); + } + enabledFlags = { ...enabledFlags, 'suppress-attribution': attributionStep.suppress }; + // Advanced path emits an outcome line (mirrors compliance step pattern). + for (const msg of attributionStep.messages) { + if (msg.level === 'success') p.log.success(msg.text); + else p.log.info(msg.text); + } + } + /** * D40: init applies seeded flag defaults non-interactively. Flags are customized * exclusively via `devflow flags`; re-init preserves existing values and adopts diff --git a/src/core/flags.ts b/src/core/flags.ts index d4088986..3e804934 100644 --- a/src/core/flags.ts +++ b/src/core/flags.ts @@ -64,10 +64,22 @@ interface FlagDefCommon { /** A boolean on/off flag. `onPayload` is written when the flag is enabled. */ export interface BooleanFlagDef extends FlagDefCommon { readonly kind: 'boolean'; - /** The value written to the target when the flag is ON. Env targets must use strings. */ - readonly onPayload: string | boolean; + /** + * The value written to the target when the flag is ON. + * - Env targets must use strings. + * - Setting targets may use strings, booleans, or plain objects. + */ + readonly onPayload: string | boolean | Record; /** Default value; false = neutral for booleans (key is deleted when false). */ readonly defaultValue: boolean; + /** + * D-ATTR-GUARD: when set, deletion of the target setting key is shape-guarded — + * the key is only removed when its current value deep-equals this shape exactly. + * Prevents erasing user-customized values (e.g. attribution with a real org name) + * when the flag transitions to neutral. + * Only honoured for setting-target boolean flags. Ignored for env targets. + */ + readonly settingDeleteGuard?: Record; } /** An enum flag. `neutralValue` is the value that means "no preference" (key is deleted). */ @@ -411,6 +423,25 @@ export const FLAG_REGISTRY: readonly ClaudeCodeFlag[] = [ defaultValue: false, }, + { + // D27: gates Claude Code's AI-attribution injection into git commits and PRs. + // When true, writes {"commit":"","pr":""} to settings.json which suppresses attribution. + // When false/null (neutral), removes the key ONLY when the current value is the exact + // devflow-managed shape (settingDeleteGuard). A custom attribution value (e.g. an org + // name) is never deleted — shape guard prevents erasure (D-ATTR-GUARD). + id: 'suppress-attribution', + label: 'Suppress AI attribution', + description: 'Remove AI-attribution labels from git commits and pull requests', + hint: 'Writes {"commit":"","pr":""} to settings.json — suppresses Claude attribution in git history', + blurb: 'hide AI attribution labels', + kind: 'boolean', + target: { type: 'setting', key: 'attribution' }, + onPayload: { commit: '', pr: '' }, + settingDeleteGuard: { commit: '', pr: '' }, + recommended: false, + defaultValue: false, + }, + // ── Valued flags (number/enum/string) ──────────────────────────────────── { @@ -960,6 +991,32 @@ export function migrateLegacyFlagsToRecord( // ─── Apply / Strip ──────────────────────────────────────────────────────────── +/** + * Structural deep-equality limited to plain JSON values (objects, arrays, primitives). + * Returns false for non-JSON types (functions, class instances, undefined). + * + * D-ATTR-GUARD: used by the settingDeleteGuard check in applyFlags and stripFlags to + * prevent erasing user-customized settings when a managed flag transitions to neutral. + */ +function deepEqualsPlain(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a === null || b === null) return a === b; + if (typeof a !== 'object' || typeof b !== 'object') return false; + if (Array.isArray(a) !== Array.isArray(b)) return false; + if (Array.isArray(a)) { + const aa = a as unknown[]; + const ba = b as unknown[]; + if (aa.length !== ba.length) return false; + return aa.every((v, i) => deepEqualsPlain(v, ba[i])); + } + const ao = a as Record; + const bo = b as Record; + const aKeys = Object.keys(ao); + const bKeys = Object.keys(bo); + if (aKeys.length !== bKeys.length) return false; + return aKeys.every(k => Object.prototype.hasOwnProperty.call(bo, k) && deepEqualsPlain(ao[k], bo[k])); +} + /** * Return `v` as a `Record` only when it is a plain object. * Returns undefined for arrays, null, or non-objects. @@ -1029,7 +1086,12 @@ export function applyFlags(settingsJson: string, flags: FlagsRecord): string { const env = asPlainObject(settings.env); if (env) delete env[flag.target.key]; } else { - delete settings[flag.target.key]; + // D-ATTR-GUARD: for flags with settingDeleteGuard, only delete when the current + // value deep-equals the guarded shape. Prevents erasing user-customized values. + const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; + if (guard === undefined || deepEqualsPlain(settings[flag.target.key], guard)) { + delete settings[flag.target.key]; + } } } else { const payload = buildPayload(flag, safe as FlagValue); @@ -1074,7 +1136,12 @@ export function stripFlags(settingsJson: string): string { if (flag.target.type === 'env') { if (env) delete env[flag.target.key]; } else { - delete settings[flag.target.key]; + // D-ATTR-GUARD: for flags with settingDeleteGuard, only delete when the current + // value deep-equals the guarded shape. Preserves user-customized values on uninstall. + const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; + if (guard === undefined || deepEqualsPlain(settings[flag.target.key], guard)) { + delete settings[flag.target.key]; + } } } diff --git a/src/targets/claude-code/post-install.ts b/src/targets/claude-code/post-install.ts index 83650e0f..a01d9ad2 100644 --- a/src/targets/claude-code/post-install.ts +++ b/src/targets/claude-code/post-install.ts @@ -811,8 +811,9 @@ function hookCommandsOf(matcher: unknown): string[] { /** * Merge Devflow's template hook entries and top-level fields into an existing * parsed settings object. Idempotent by exact command string — a hook that is - * already present is skipped. Sets `statusLine` and `attribution` only when the - * user has no existing value for them. Mutates `existing` in place. + * already present is skipped. Sets `statusLine` only when the user has no existing + * value. Attribution is NOT injected here — managed by the flags pipeline (D27). + * Mutates `existing` in place. * * D-SETTINGS-1: merge strategy — never replace; only add devflow entries that are absent. * Returns { changed: true } when any field was added. @@ -864,11 +865,10 @@ export function mergeDevflowSettingsTemplate( changed = true; } - // Set attribution only if the user has none - if (existing.attribution === undefined && template.attribution !== undefined) { - existing.attribution = template.attribution; - changed = true; - } + // Attribution is NOT injected here. It is managed exclusively by the flags + // pipeline (suppress-attribution flag, D27). Removing it from merge prevents a + // double-write: applyFlags writes it when the flag is on; stripFlags removes + // it when the flag is off (shape-guarded, D-ATTR-GUARD). return { changed }; } diff --git a/src/targets/claude-code/templates/settings.json b/src/targets/claude-code/templates/settings.json index 151ac876..5e08ed35 100644 --- a/src/targets/claude-code/templates/settings.json +++ b/src/targets/claude-code/templates/settings.json @@ -78,9 +78,5 @@ ] } ] - }, - "attribution": { - "commit": "", - "pr": "" } } diff --git a/tests/attribution-prompts.test.ts b/tests/attribution-prompts.test.ts new file mode 100644 index 00000000..1074a162 --- /dev/null +++ b/tests/attribution-prompts.test.ts @@ -0,0 +1,167 @@ +/** + * Tests for attribution-prompts.ts (D27 / PF-029). + * + * Coverage: + * - shouldRunAttributionStep: gate predicate matrix (PF-029 invariants) + * - runAttributionStep: step runner with injected DI seam (PF-014 invariants) + */ + +import { describe, it, expect } from 'vitest'; +import { + shouldRunAttributionStep, + runAttributionStep, + type AttributionPromptIO, +} from '../src/cli/commands/attribution-prompts.js'; + +// ── shouldRunAttributionStep ────────────────────────────────────────────────── + +describe('shouldRunAttributionStep — gate predicate (PF-029)', () => { + it('--recommended flag (no modePromptShown) → false (promptless contract preserved)', () => { + expect(shouldRunAttributionStep({ + mode: 'recommended', + modePromptShown: false, + isTTY: true, + hasCliOverride: false, + })).toBe(false); + }); + + it('non-TTY → false regardless of mode (promptless contract preserved)', () => { + expect(shouldRunAttributionStep({ + mode: 'advanced', + modePromptShown: true, + isTTY: false, + hasCliOverride: false, + })).toBe(false); + }); + + it('hasCliOverride → false regardless of mode/TTY/modePromptShown', () => { + expect(shouldRunAttributionStep({ + mode: 'advanced', + modePromptShown: true, + isTTY: true, + hasCliOverride: true, + })).toBe(false); + }); + + it('interactive Recommended + modePromptShown=true → true', () => { + expect(shouldRunAttributionStep({ + mode: 'recommended', + modePromptShown: true, + isTTY: true, + hasCliOverride: false, + })).toBe(true); + }); + + it('Advanced mode with TTY → true regardless of modePromptShown', () => { + expect(shouldRunAttributionStep({ + mode: 'advanced', + modePromptShown: false, + isTTY: true, + hasCliOverride: false, + })).toBe(true); + }); + + it('Advanced mode with TTY and modePromptShown=true → true', () => { + expect(shouldRunAttributionStep({ + mode: 'advanced', + modePromptShown: true, + isTTY: true, + hasCliOverride: false, + })).toBe(true); + }); +}); + +// ── runAttributionStep ──────────────────────────────────────────────────────── + +/** Build a no-op AttributionPromptIO that always yields the given select result. */ +function makeIO(selectResult: boolean | 'cancel'): AttributionPromptIO { + return { + note: () => {}, + select: async () => + selectResult === 'cancel' + ? { kind: 'cancel' } + : { kind: 'value', value: selectResult }, + }; +} + +describe('runAttributionStep — step runner (PF-014)', () => { + it('user selects Yes (true) → resolved with suppress:true and success message', async () => { + const result = await runAttributionStep({ seed: false, prompts: makeIO(true) }); + expect(result.kind).toBe('resolved'); + if (result.kind === 'resolved') { + expect(result.suppress).toBe(true); + expect(result.messages).toHaveLength(1); + expect(result.messages[0]?.level).toBe('success'); + expect(result.messages[0]?.text).toContain('suppressed'); + } + }); + + it('user selects No (false) → resolved with suppress:false and info message', async () => { + const result = await runAttributionStep({ seed: true, prompts: makeIO(false) }); + expect(result.kind).toBe('resolved'); + if (result.kind === 'resolved') { + expect(result.suppress).toBe(false); + expect(result.messages).toHaveLength(1); + expect(result.messages[0]?.level).toBe('info'); + expect(result.messages[0]?.text).toContain('shown (default)'); + } + }); + + it('cancel → kind:cancelled (PF-014: never throws)', async () => { + const result = await runAttributionStep({ seed: false, prompts: makeIO('cancel') }); + expect(result.kind).toBe('cancelled'); + }); + + it('note is called with "suppressed" when seed=true', async () => { + let noteMsg = ''; + const io: AttributionPromptIO = { + note: (msg) => { noteMsg = msg; }, + select: async () => ({ kind: 'value', value: false }), + }; + await runAttributionStep({ seed: true, prompts: io }); + expect(noteMsg).toContain('suppressed'); + expect(noteMsg).not.toContain('shown (default)'); + }); + + it('note is called with "shown (default)" when seed=false', async () => { + let noteMsg = ''; + const io: AttributionPromptIO = { + note: (msg) => { noteMsg = msg; }, + select: async () => ({ kind: 'value', value: false }), + }; + await runAttributionStep({ seed: false, prompts: io }); + expect(noteMsg).toContain('shown (default)'); + }); + + it('does not throw — never calls process.exit() (PF-014)', async () => { + // The step runner must return a value, never throw or process.exit. + await expect(runAttributionStep({ seed: false, prompts: makeIO(false) })).resolves.toBeDefined(); + await expect(runAttributionStep({ seed: false, prompts: makeIO('cancel') })).resolves.toBeDefined(); + }); + + it('seeded true → initialValue passed as true to select prompt', async () => { + let capturedInitialValue: boolean | undefined; + const io: AttributionPromptIO = { + note: () => {}, + select: async (opts) => { + capturedInitialValue = opts.initialValue; + return { kind: 'value', value: opts.initialValue }; + }, + }; + await runAttributionStep({ seed: true, prompts: io }); + expect(capturedInitialValue).toBe(true); + }); + + it('seeded false → initialValue passed as false to select prompt', async () => { + let capturedInitialValue: boolean | undefined; + const io: AttributionPromptIO = { + note: () => {}, + select: async (opts) => { + capturedInitialValue = opts.initialValue; + return { kind: 'value', value: opts.initialValue }; + }, + }; + await runAttributionStep({ seed: false, prompts: io }); + expect(capturedInitialValue).toBe(false); + }); +}); diff --git a/tests/flags.test.ts b/tests/flags.test.ts index 0b062102..50570ed7 100644 --- a/tests/flags.test.ts +++ b/tests/flags.test.ts @@ -73,10 +73,15 @@ describe('FLAG_REGISTRY — structural invariants', () => { const boolFlags = FLAG_REGISTRY.filter((f): f is BooleanFlagDef => f.kind === 'boolean'); expect(boolFlags.length).toBeGreaterThan(0); for (const flag of boolFlags) { - expect( - typeof flag.onPayload === 'string' || typeof flag.onPayload === 'boolean', - `${flag.id}: onPayload must be string or boolean`, - ).toBe(true); + // Env targets must use strings; setting targets may also use plain objects (D27/D-ATTR-GUARD). + const isValidPayload = + typeof flag.onPayload === 'string' || + typeof flag.onPayload === 'boolean' || + (flag.target.type === 'setting' && + typeof flag.onPayload === 'object' && + flag.onPayload !== null && + !Array.isArray(flag.onPayload)); + expect(isValidPayload, `${flag.id}: onPayload must be string, boolean, or plain object (setting targets only)`).toBe(true); expect(typeof flag.defaultValue, `${flag.id}: defaultValue must be boolean`).toBe('boolean'); } }); @@ -199,6 +204,9 @@ describe('getDefaultFlagsRecord', () => { // New optional boolean flag expect(record['enable-todo-tools']).toBe(false); + // Attribution suppression flag (off by default — D27) + expect(record['suppress-attribution']).toBe(false); + // view-mode: default is neutralValue, so entry is 'default' expect(record['view-mode']).toBe('default'); }); @@ -1779,3 +1787,77 @@ describe('persistence round-trip: manifest write shape → resolveSeedFlags', () expect(seeded['subagent-spawn-depth']).toBeNull(); }); }); + +// ─── suppress-attribution — shape guard (D27 / D-ATTR-GUARD) ───────────────── + +describe('suppress-attribution flag — shape guard (D27)', () => { + const DEVFLOW_ATTR = { commit: '', pr: '' }; + + it('applyFlags with true writes the attribution block', () => { + const result = JSON.parse(applyFlags(JSON.stringify({}), { 'suppress-attribution': true })); + expect(result.attribution).toEqual(DEVFLOW_ATTR); + }); + + it('applyFlags with false deletes the exact devflow attribution shape', () => { + const input = JSON.stringify({ attribution: DEVFLOW_ATTR }); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })); + expect(result.attribution).toBeUndefined(); + }); + + it('applyFlags with null deletes the exact devflow attribution shape', () => { + const input = JSON.stringify({ attribution: DEVFLOW_ATTR }); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': null })); + expect(result.attribution).toBeUndefined(); + }); + + it('applyFlags with false does NOT delete custom attribution (shape guard)', () => { + const custom = { commit: 'My Org', pr: 'My Org' }; + const input = JSON.stringify({ attribution: custom }); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })); + expect(result.attribution).toEqual(custom); + }); + + it('applyFlags with false does NOT delete string attribution (shape guard)', () => { + const input = JSON.stringify({ attribution: 'My Org' }); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })); + expect(result.attribution).toBe('My Org'); + }); + + it('stripFlags deletes exact devflow attribution shape', () => { + const input = JSON.stringify({ attribution: DEVFLOW_ATTR }); + const result = JSON.parse(stripFlags(input)); + expect(result.attribution).toBeUndefined(); + }); + + it('stripFlags does NOT delete custom attribution (shape guard)', () => { + const custom = { commit: 'My Org', pr: 'My Org' }; + const input = JSON.stringify({ attribution: custom }); + const result = JSON.parse(stripFlags(input)); + expect(result.attribution).toEqual(custom); + }); + + it('stripFlags does NOT delete string attribution', () => { + const input = JSON.stringify({ attribution: 'My Org' }); + const result = JSON.parse(stripFlags(input)); + expect(result.attribution).toBe('My Org'); + }); + + it('suppress-attribution flag entry: id=suppress-attribution, setting target, key=attribution', () => { + const flag = FLAG_REGISTRY.find(f => f.id === 'suppress-attribution')!; + expect(flag).toBeDefined(); + expect(flag.kind).toBe('boolean'); + expect(flag.target.type).toBe('setting'); + expect(flag.target.key).toBe('attribution'); + expect(flag.recommended).toBe(false); + expect(flag.defaultValue).toBe(false); + if (flag.kind === 'boolean') { + expect(flag.onPayload).toEqual(DEVFLOW_ATTR); + expect(flag.settingDeleteGuard).toEqual(DEVFLOW_ATTR); + } + }); + + it('blurb cap: suppress-attribution blurb is ≤ 30 chars', () => { + const flag = FLAG_REGISTRY.find(f => f.id === 'suppress-attribution')!; + expect(flag.blurb.length).toBeLessThanOrEqual(30); + }); +}); diff --git a/tests/init-seed.test.ts b/tests/init-seed.test.ts index 691beee6..c312aad0 100644 --- a/tests/init-seed.test.ts +++ b/tests/init-seed.test.ts @@ -4,6 +4,7 @@ import { resolveSeedFlags, resolveSeedPlugins, resolveInitSeed, + resolveExistingAttributionSuppression, applyCliToggles, resolveResetGatedInputs, FEATURE_DEFAULTS, @@ -733,3 +734,87 @@ describe('compliance seeding', () => { expect(seed.features.compliance).toEqual({ enabled: true, frameworks: ['gdpr'] }); }); }); + +// ── resolveExistingAttributionSuppression ───────────────────────────────────── + +describe('resolveExistingAttributionSuppression (D27)', () => { + it('returns true when exact devflow attribution shape present', () => { + const settings = JSON.stringify({ attribution: { commit: '', pr: '' } }); + expect(resolveExistingAttributionSuppression(settings)).toBe(true); + }); + + it('returns undefined when attribution key is absent', () => { + const settings = JSON.stringify({ other: 'value' }); + expect(resolveExistingAttributionSuppression(settings)).toBeUndefined(); + }); + + it('returns undefined for custom attribution object (shape guard)', () => { + const settings = JSON.stringify({ attribution: { commit: 'My Org', pr: 'My Org' } }); + expect(resolveExistingAttributionSuppression(settings)).toBeUndefined(); + }); + + it('returns undefined for string attribution (legacy/custom)', () => { + const settings = JSON.stringify({ attribution: 'Devflow' }); + expect(resolveExistingAttributionSuppression(settings)).toBeUndefined(); + }); + + it('returns undefined for attribution with extra keys (not exact shape)', () => { + const settings = JSON.stringify({ attribution: { commit: '', pr: '', extra: 'value' } }); + expect(resolveExistingAttributionSuppression(settings)).toBeUndefined(); + }); + + it('returns undefined for attribution with non-empty strings', () => { + const settings = JSON.stringify({ attribution: { commit: 'x', pr: '' } }); + expect(resolveExistingAttributionSuppression(settings)).toBeUndefined(); + }); + + it('returns undefined on malformed JSON', () => { + expect(resolveExistingAttributionSuppression('not json')).toBeUndefined(); + }); + + it('returns undefined for empty settings string', () => { + expect(resolveExistingAttributionSuppression('')).toBeUndefined(); + }); +}); + +// ── resolveInitSeed — suppress-attribution seeding ─────────────────────────── + +describe('resolveInitSeed — suppress-attribution seeding (D27)', () => { + it('fresh install (null manifest) → suppress-attribution defaults to false', () => { + const seed = resolveInitSeed(null, null, JSON.stringify({}), DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(false); + }); + + it('settings.json with exact devflow attribution → seeds true (overrides manifest false)', () => { + const settings = JSON.stringify({ attribution: { commit: '', pr: '' } }); + const manifest = makeManifest({ features: { ...makeManifest().features, flags: { 'suppress-attribution': false } } }); + const seed = resolveInitSeed(manifest, null, settings, DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(true); + }); + + it('manifest has suppress-attribution:true, no exact settings shape → seeds true', () => { + const manifest = makeManifest({ + features: { ...makeManifest().features, flags: { 'suppress-attribution': true } }, + }); + const seed = resolveInitSeed(manifest, null, JSON.stringify({}), DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(true); + }); + + it('custom attribution in settings, manifest flag false → seeds false (shape guard)', () => { + const settings = JSON.stringify({ attribution: { commit: 'My Org', pr: 'My Org' } }); + const manifest = makeManifest({ + features: { ...makeManifest().features, flags: { 'suppress-attribution': false } }, + }); + const seed = resolveInitSeed(manifest, null, settings, DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(false); + }); + + it('--reset → suppress-attribution defaults to false even when settings has exact shape', () => { + const settings = JSON.stringify({ attribution: { commit: '', pr: '' } }); + const { seedManifest, seedConfig, seedSettings } = resolveResetGatedInputs( + true, makeManifest(), null, settings, + ); + const seed = resolveInitSeed(seedManifest, seedConfig, seedSettings, DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(false); + }); +}); diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts index 223ee4bd..9b8f9004 100644 --- a/tests/post-install-merge.test.ts +++ b/tests/post-install-merge.test.ts @@ -11,8 +11,8 @@ * - Hooks absent → changed:true (added) * - Preserves user-owned keys untouched (env, permissions, model, etc.) * - statusLine set only when absent - * - attribution set only when absent * - statusLine preserved when already set by the user + * - attribution is NOT injected (managed by flags pipeline — D27) * - Idempotent — double merge is the same as single merge * - Template hook with no command string is skipped silently * - Empty template → changed:false @@ -35,14 +35,13 @@ function makeHookMatcher(command: string, timeout = 10): HookMatcher { return { hooks: [{ type: 'command', command, timeout }] }; } -function makeTemplate(commands: string[], statusLine = 'devflow: {branch}', attribution = 'Devflow'): Record { +function makeTemplate(commands: string[], statusLine = 'devflow: {branch}'): Record { const matchers = commands.map((cmd) => makeHookMatcher(cmd)); return { hooks: { 'SessionStart': matchers, }, statusLine, - attribution, }; } @@ -67,7 +66,6 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { 'SessionStart': [makeHookMatcher(cmd)], }, statusLine: 'devflow: {branch}', - attribution: 'Devflow', }; const template = makeTemplate([cmd]); const { changed } = mergeDevflowSettingsTemplate(existing, template); @@ -143,16 +141,21 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { expect(existing.statusLine).toBe('my-custom-status-line'); }); - it('sets attribution from template when absent on existing', () => { + it('does NOT inject attribution — attribution is managed by flags pipeline (D27)', () => { + // mergeDevflowSettingsTemplate never writes an attribution key regardless of + // what the template contains. Attribution is written/removed by applyFlags/ + // stripFlags (suppress-attribution flag, D-ATTR-GUARD shape guard). const existing: Record = {}; - const template = { statusLine: 's', attribution: 'Devflow' }; + const template = { statusLine: 's' }; // template has no attribution key (R2) mergeDevflowSettingsTemplate(existing, template); - expect(existing.attribution).toBe('Devflow'); + expect(existing.attribution).toBeUndefined(); }); - it('does NOT overwrite attribution when user already has one', () => { + it('does NOT delete an existing attribution value (preserve user values)', () => { + // Even when attribution appears in template (legacy or edge case), the + // merge function must not touch an existing attribution value. const existing: Record = { attribution: 'my-org' }; - const template = { statusLine: 's', attribution: 'Devflow' }; + const template: Record = { statusLine: 's' }; mergeDevflowSettingsTemplate(existing, template); expect(existing.attribution).toBe('my-org'); }); From c47a6aea509224ab42f8b799b3d7d2b4ba40e0a8 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 13:35:51 +0300 Subject: [PATCH 02/21] refactor: fold wizardSuppressAttribution into if-block (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminate the unnecessary intermediate variable in the Recommended-path attribution wizard block. The variable was assigned inside the if-block and immediately applied in a second if-block outside it with no other references — fold both into the single if-block, matching the Advanced path's pattern. --- src/cli/commands/init.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index c73b80c1..8ceece90 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -689,7 +689,6 @@ export const initCommand = new Command('init') // (modePromptShown=true), preserving the promptless contracts of --recommended and !isTTY. // shouldRunAttributionStep gates on modePromptShown rather than the mode name (PF-029). // No CLI override path exists for attribution on the Recommended path (D27). - let wizardSuppressAttribution: boolean | undefined; if (shouldRunAttributionStep({ mode: 'recommended', modePromptShown, @@ -704,10 +703,8 @@ export const initCommand = new Command('init') p.cancel('Installation cancelled.'); process.exit(0); } - wizardSuppressAttribution = attributionStep.suppress; - } - if (wizardSuppressAttribution !== undefined) { - enabledFlags = { ...enabledFlags, 'suppress-attribution': wizardSuppressAttribution }; + enabledFlags = { ...enabledFlags, 'suppress-attribution': attributionStep.suppress }; + // Step messages not emitted here — the Recommended summary note covers attribution state. } // Apply explicit CLI toggles on top of the seed. From c8438aae8d72b258281ae9448f96307b85206bca Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 13:46:45 +0300 Subject: [PATCH 03/21] fix: restrict attribution prompt to Advanced and close flag payload hole (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review fixes across three areas. Wizard reachability (P0-Design): shouldRunAttributionStep mirrored shouldRunComplianceStep, so interactive Recommended (modePromptShown=true) reached the attribution question. Attribution rewrites git history metadata and must stay Advanced-only; Recommended is a zero-question path that silently applies the seeded value (fresh install: off). The predicate now gates on `mode === 'advanced' && isTTY` — sound because 'advanced' only ever resolves interactively — and the dead modePromptShown/hasCliOverride parameters are gone rather than left as misleading inputs. The Recommended-path call site is removed, leaving a single call site; the divergence from the compliance gate is documented at the predicate so it is not "restored" later. The removed block also claimed the Recommended summary note reported attribution state, which it never did. Flag payload typing (P1-Design): widening BooleanFlagDef.onPayload to admit Record enlarged a pre-existing hole — an env-target boolean flag with an object payload compiled clean and would serialize an object into the settings.json env string map. BooleanFlagDef is now discriminated on target.type (EnvBooleanFlagDef | SettingBooleanFlagDef): env targets are constrained to `onPayload: string` and `settingDeleteGuard?: never`. The invariant is compile-enforced instead of comment-enforced; the registry unit test stays as a runtime backstop and gained a non-vacuity guard. Test coverage (P1-Tests, PF-018): attribution had 100% pure-function coverage and 0% production-path coverage. - tests/uninstall-logic.test.ts: every runCleanupPhase test passed scopesToUninstall: [], making the settings loop a no-op so stripFlags was never reached. Four cases now drive the real phase with scope 'user' against a sandboxed HOME. - tests/init-e2e-flags.test.ts: five subprocess cases over the real init settings pass — off→on materialisation, on→off via --reset, custom value survival across convergeFlagsIntoSettings' strip-then-apply double pass, the pre-D27 upgrade path, and fresh-install default-off. Each asserts manifest and settings.json together (PF-015) behind the existing non-vacuity gate. Falsified: removing settingDeleteGuard fails the custom-value case. Also replaces an unchecked `as boolean` on the wizard seed with `=== true`. --- CLAUDE.md | 2 +- src/cli/commands/attribution-prompts.ts | 44 ++++---- src/cli/commands/init.ts | 42 +++---- src/core/flags.ts | 50 +++++++-- tests/attribution-prompts.test.ts | 73 +++++------- tests/flags.test.ts | 33 ++++++ tests/init-e2e-flags.test.ts | 143 ++++++++++++++++++++++++ tests/uninstall-logic.test.ts | 104 +++++++++++++++++ 8 files changed, 386 insertions(+), 105 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e342afac..57695e13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ Knowledge write-back is in-command (not a background pipeline): gated by `devflo **Per-Agent Model Configuration**: User overrides to agent model assignments persist in `~/.devflow/agent-models.json` (deviations only — absent entry = shipped default). `reapplyAgentMapping` runs after every `devflow init` post-install to re-apply user overrides to freshly copied agent files. `revertExternalAgents` reverts all agents to shipped defaults (called on proxy disable and before agent removal on uninstall). GPT model assignments are **dormant** when routing is off — they are stored in `agent-models.json` but not written to agent frontmatter until routing is enabled. Manage via `devflow agents` TUI or `devflow agents --list/--set/--reset`. Core source files: `src/core/agent-frontmatter.ts` (pure rewrite engine), `src/core/agent-models.ts` (schema + apply/revert), `src/core/external-models.ts` (CLAUDE_MODEL_ALIASES, isClaudeModelName, isDormantExternalModel — leaf module), `src/core/model-discovery.ts` (discoverExternalModels, getExternalModelsCached, cache-warming), `src/core/cache.ts` (cache read/write, 0700/0600 permissions, parseRawEnvelope), `src/core/proxy-log.ts` (scrubChildEnv, openProxyLog, relay env allowlisting), `src/core/proxy-state.ts` (state I/O), `src/cli/commands/proxy.ts` (CLI + hook wiring), `src/cli/commands/agents.ts` (CLI), `src/cli/agents-view/` (TUI — state, render, terminal; thin adapter over the shared `src/cli/tui/` driver). -**Two-Mode Init**: `devflow init` offers Recommended (sensible defaults, quick setup) or Advanced (full interactive flow) after plugin selection. `--recommended` / `--advanced` CLI flags for non-interactive use. Recommended applies: ambient ON, memory ON, learning ON, rules ON, HUD ON, default-ON flags, .claudeignore ON, auto-install safe-delete if trash CLI detected, user-mode security deny list, viewMode preserved from existing settings.json. Both init paths apply seeded flag values non-interactively — fresh install: registry defaults; re-init: existing manifest values preserved, defaults adopted only for newly-added flags (ADR-014); `view-mode` resolved from existing settings.json at seed time — and emit an outcome line pointing to `devflow flags` for customization. Advanced path adds a proxy prompt (external model routing — default OFF, requires Codex auth; never part of Recommended defaults). Use `--learning/--no-learning` to toggle the learning agent independently. Use `--rules/--no-rules` to toggle rules independently. Use `--proxy/--no-proxy` to set external model routing (Advanced-only; init runs preflight on enable). Use `--compliance `/`--no-compliance` to set compliance non-interactively (enable with comma-separated framework IDs or disable preserving frameworks; default: off; `--compliance`/`--no-compliance` bypasses the wizard entirely). The compliance wizard step (select which regulatory frameworks to install — GDPR, HIPAA, PCI DSS, SOC 2, ISO 27001, SOX) runs in **both** init paths via `shouldRunComplianceStep`: Advanced always runs it; Recommended only runs it when the user reached the mode-select prompt interactively (`modePromptShown=true`) — `--recommended` flag and non-TTY invocations preserve their promptless contracts. The step shows a "Current setting:" note for re-init legibility, uses a `p.select` (Yes/No) instead of a confirm to avoid Enter-through ambiguity, and emits an outcome line for unambiguous state visibility (per PF-029). **Attribution wizard step** (D27): the `suppress-attribution` wizard question runs in **both** init paths after the compliance step, using the same `shouldRunAttributionStep` gate predicate (same `modePromptShown` logic as compliance, per PF-029) — Yes writes `{"commit":"","pr":""}` to suppress Claude attribution in git history, No preserves attribution labels; seeded from settings.json (exact devflow shape → true) then manifest then false; no CLI override path (toggle via `devflow flags --enable/--disable suppress-attribution`); shape-guarded deletion means a user's custom attribution value is never erased. **State-aware re-init**: on re-init the wizard reads the prior manifest, config, and settings.json and pre-seeds every prompt with existing values, skipping the Recommended/Advanced question entirely. Use `--reset` for a factory reset that ignores all prior state (mutually exclusive with `--plugin`). +**Two-Mode Init**: `devflow init` offers Recommended (sensible defaults, quick setup) or Advanced (full interactive flow) after plugin selection. `--recommended` / `--advanced` CLI flags for non-interactive use. Recommended applies: ambient ON, memory ON, learning ON, rules ON, HUD ON, default-ON flags, .claudeignore ON, auto-install safe-delete if trash CLI detected, user-mode security deny list, viewMode preserved from existing settings.json. Both init paths apply seeded flag values non-interactively — fresh install: registry defaults; re-init: existing manifest values preserved, defaults adopted only for newly-added flags (ADR-014); `view-mode` resolved from existing settings.json at seed time — and emit an outcome line pointing to `devflow flags` for customization. Advanced path adds a proxy prompt (external model routing — default OFF, requires Codex auth; never part of Recommended defaults). Use `--learning/--no-learning` to toggle the learning agent independently. Use `--rules/--no-rules` to toggle rules independently. Use `--proxy/--no-proxy` to set external model routing (Advanced-only; init runs preflight on enable). Use `--compliance `/`--no-compliance` to set compliance non-interactively (enable with comma-separated framework IDs or disable preserving frameworks; default: off; `--compliance`/`--no-compliance` bypasses the wizard entirely). The compliance wizard step (select which regulatory frameworks to install — GDPR, HIPAA, PCI DSS, SOC 2, ISO 27001, SOX) runs in **both** init paths via `shouldRunComplianceStep`: Advanced always runs it; Recommended only runs it when the user reached the mode-select prompt interactively (`modePromptShown=true`) — `--recommended` flag and non-TTY invocations preserve their promptless contracts. The step shows a "Current setting:" note for re-init legibility, uses a `p.select` (Yes/No) instead of a confirm to avoid Enter-through ambiguity, and emits an outcome line for unambiguous state visibility (per PF-029). **Attribution wizard step** (D27): the `suppress-attribution` question is **Advanced-only** — `shouldRunAttributionStep` returns true only for `mode === 'advanced'` with a TTY, a deliberate divergence from `shouldRunComplianceStep` (which also runs on interactive Recommended). Recommended **never** asks; it silently applies the seeded value (fresh install: off). The step runs after the compliance step: Yes writes `{"commit":"","pr":""}` to suppress Claude attribution in git history, No preserves attribution labels; seeded from settings.json (exact devflow shape → true) then manifest then false; no CLI override path (toggle via `devflow flags --enable/--disable suppress-attribution`); shape-guarded deletion means a user's custom attribution value is never erased. **State-aware re-init**: on re-init the wizard reads the prior manifest, config, and settings.json and pre-seeds every prompt with existing values, skipping the Recommended/Advanced question entirely. Use `--reset` for a factory reset that ignores all prior state (mutually exclusive with `--plugin`). **Migrations**: Run-once migrations execute automatically on `devflow init`, tracked at `~/.devflow/migrations.json` (scope-independent; single file regardless of user-scope vs local-scope installs). To add a 2.x migration, append an entry to `MIGRATIONS` in `src/core/migrations.ts`. Scopes: `global` (runs once per machine, no project context) vs `per-project` (sweeps all discovered Claude-enabled projects in parallel). Failures are non-fatal — migrations retry on next init. The registry holds 2.x entries only (first: canonicalise-agent-keys-v1); no 1.x upgrade path. diff --git a/src/cli/commands/attribution-prompts.ts b/src/cli/commands/attribution-prompts.ts index b48fbb40..62b8afc5 100644 --- a/src/cli/commands/attribution-prompts.ts +++ b/src/cli/commands/attribution-prompts.ts @@ -4,13 +4,14 @@ * CLI-layer module (ADR-013): prompt-rendering logic lives in src/cli/commands/, * core business logic stays in src/core/. * - * Applies PF-029: every wizard gate keys on `modePromptShown`, never on the mode - * name, so --recommended (flag, no prompt) and the non-TTY fallback preserve their - * promptless contracts. + * Applies PF-029: the gate is an exported pure predicate with an explicit isTTY guard, + * so --recommended (flag, no prompt) and the non-TTY fallback keep their promptless + * contracts and the reachability rule is unit-testable without a terminal. * Applies PF-014: runAttributionStep never calls process.exit() or throws — callers * own the cancel idiom (p.cancel + process.exit(0)), keeping try/finally cleanup safe. * * D27: suppress-attribution flag — gates Claude Code's AI-attribution injection. + * The question is ADVANCED-ONLY; Recommended never asks. See shouldRunAttributionStep. */ import * as p from '@clack/prompts'; @@ -20,32 +21,35 @@ import * as p from '@clack/prompts'; /** * Determines whether the attribution wizard step should run for a given init invocation. * - * Gate table (per PF-029: key on modePromptShown, never on the mode name): + * D27 — ADVANCED-ONLY. The attribution question is reachable from the Advanced path + * and nowhere else. This DIVERGES DELIBERATELY from shouldRunComplianceStep, which also + * runs on interactive Recommended: attribution rewrites the user's git history metadata, + * so Recommended stays a zero-question path and silently applies the seeded value + * (fresh install: off). Do not "restore symmetry" with the compliance gate. * - * --recommended flag / !isTTY fallback → no (promptless contract preserved) - * Interactive mode-prompt → Recommended → yes (modePromptShown=true) - * --advanced flag / re-init (banner path) → yes (mode='advanced', isTTY=true) - * Interactive mode-prompt → Advanced → yes (modePromptShown=true) - * Any path with a hasCliOverride for this step → no (CLI override wins) + * Gate table: + * + * --advanced flag / re-init (banner path) / prompt → Advanced → yes + * Interactive mode-prompt → Recommended → NO (D27 divergence) + * --recommended flag → no + * !isTTY (any mode) → no + * + * Gating on the mode name is sound here because 'advanced' is only ever resolved on an + * interactive path (the Advanced branch exit-1s on non-TTY), and the explicit isTTY guard + * keeps the promptless contracts of --recommended and the non-TTY fallback pinned + * regardless (PF-029). + * + * There is no CLI override for attribution — it is toggled post-install via + * `devflow flags --enable/--disable suppress-attribution`. * * Pure predicate — no side effects, fully testable without a TTY. - * Mirrors shouldRunComplianceStep exactly (same gate table per PF-029). */ export function shouldRunAttributionStep(input: { mode: 'recommended' | 'advanced'; - modePromptShown: boolean; isTTY: boolean; - hasCliOverride: boolean; }): boolean { - if (input.hasCliOverride) return false; if (!input.isTTY) return false; - // Advanced path: non-TTY has already exit-1'd, so isTTY=true here → always run. - // Covers: --advanced flag, re-init banner path, interactive-prompt → advanced. - if (input.mode === 'advanced') return true; - // Recommended path: only run when the Setup-mode p.select actually ran - // (user made an active choice). --recommended flag and !isTTY fallback never set - // modePromptShown=true, preserving their promptless contracts. - return input.modePromptShown; + return input.mode === 'advanced'; } // ── DI seam ──────────────────────────────────────────────────────────────────── diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index 8ceece90..b0e83fbf 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -685,27 +685,9 @@ export const initCommand = new Command('init') // prints the Compliance line from complianceSummary via formatComplianceSummary. } - // B5: attribution wizard step — runs only when the Setup-mode prompt actually ran - // (modePromptShown=true), preserving the promptless contracts of --recommended and !isTTY. - // shouldRunAttributionStep gates on modePromptShown rather than the mode name (PF-029). - // No CLI override path exists for attribution on the Recommended path (D27). - if (shouldRunAttributionStep({ - mode: 'recommended', - modePromptShown, - isTTY: process.stdin.isTTY, - hasCliOverride: false, - })) { - const attributionStep = await runAttributionStep({ - seed: enabledFlags['suppress-attribution'] as boolean, - prompts: buildClackAttributionPrompts(), - }); - if (attributionStep.kind === 'cancelled') { - p.cancel('Installation cancelled.'); - process.exit(0); - } - enabledFlags = { ...enabledFlags, 'suppress-attribution': attributionStep.suppress }; - // Step messages not emitted here — the Recommended summary note covers attribution state. - } + // No attribution step here: the suppress-attribution question is Advanced-only (D27). + // Recommended silently carries the seeded value in enabledFlags — fresh installs get + // the registry default (off), re-inits get prior state. See shouldRunAttributionStep. // Apply explicit CLI toggles on top of the seed. // Precedence: explicit CLI flag > wizard result > seed value (prior state > registry default). @@ -974,19 +956,21 @@ export const initCommand = new Command('init') // CLI override (isTTY is guaranteed true by the non-TTY guard above). If it ever // did, the seed values assigned at declaration stand — which is the right default. - // Attribution feature (after compliance, before flags — runs in both Advanced and re-init paths). - // Gated by the same shouldRunAttributionStep predicate as the Recommended path so the - // documented gate table is the single authority for both — the two paths cannot drift. - // Here isTTY is guaranteed true (the non-TTY guard above exit-1'd), so the predicate - // reduces to "no CLI override" — no CLI override exists for attribution (D27). + // Attribution feature (after compliance, before flags). This is the ONLY call site — + // the attribution question is Advanced-only (D27); the Recommended path never asks and + // silently carries the seeded value. The gate stays an explicit predicate call so the + // documented gate table in attribution-prompts.ts remains the single authority. + // isTTY is guaranteed true here (the non-TTY guard above exit-1'd); passing it keeps + // the promptless contract enforced at the predicate rather than by position (PF-029). if (shouldRunAttributionStep({ mode: 'advanced', - modePromptShown, isTTY: process.stdin.isTTY, - hasCliOverride: false, })) { const attributionStep = await runAttributionStep({ - seed: enabledFlags['suppress-attribution'] as boolean, + // resolveInitSeed always emits this key, but `=== true` keeps the seed a real + // boolean rather than an unchecked cast if that contract ever changes — + // an `undefined` reaching p.select's initialValue silently unseeds the prompt. + seed: enabledFlags['suppress-attribution'] === true, prompts: buildClackAttributionPrompts(), }); if (attributionStep.kind === 'cancelled') { diff --git a/src/core/flags.ts b/src/core/flags.ts index 3e804934..458941e2 100644 --- a/src/core/flags.ts +++ b/src/core/flags.ts @@ -34,10 +34,14 @@ export type FlagsRecordValue = FlagValue | null; */ export type FlagsRecord = Record; +/** A target inside the settings.json `env` block. Env values are always strings. */ +export type EnvFlagTarget = { readonly type: 'env'; readonly key: string }; + +/** A top-level settings.json key. May hold any JSON value. */ +export type SettingFlagTarget = { readonly type: 'setting'; readonly key: string }; + /** Where the flag's value is written in settings.json. */ -export type FlagTarget = - | { readonly type: 'env'; readonly key: string } - | { readonly type: 'setting'; readonly key: string }; +export type FlagTarget = EnvFlagTarget | SettingFlagTarget; // ── Per-kind interfaces ──────────────────────────────────────────────────────── @@ -61,27 +65,49 @@ interface FlagDefCommon { readonly target: FlagTarget; } -/** A boolean on/off flag. `onPayload` is written when the flag is enabled. */ -export interface BooleanFlagDef extends FlagDefCommon { +interface BooleanFlagDefCommon extends FlagDefCommon { readonly kind: 'boolean'; - /** - * The value written to the target when the flag is ON. - * - Env targets must use strings. - * - Setting targets may use strings, booleans, or plain objects. - */ - readonly onPayload: string | boolean | Record; /** Default value; false = neutral for booleans (key is deleted when false). */ readonly defaultValue: boolean; +} + +/** + * A boolean flag targeting an env var. `onPayload` is constrained to `string`: + * the settings.json `env` block is a string map, and buildPayload writes + * `onPayload` through verbatim, so a non-string here would serialize an object + * or raw boolean into an env slot that Claude Code reads as a string. + * + * This constraint is COMPILE-ENFORCED — it used to be prose on `onPayload` plus a + * registry unit test, which let `target: {type:'env'}` + an object payload compile + * clean. Do not merge the two members back into one interface. + */ +export interface EnvBooleanFlagDef extends BooleanFlagDefCommon { + readonly target: EnvFlagTarget; + readonly onPayload: string; + /** Env keys are never shape-guarded — `never` makes that a compile error, not a silent no-op. */ + readonly settingDeleteGuard?: never; +} + +/** A boolean flag targeting a top-level settings.json key. */ +export interface SettingBooleanFlagDef extends BooleanFlagDefCommon { + readonly target: SettingFlagTarget; + /** Setting targets may use strings, booleans, or plain objects. */ + readonly onPayload: string | boolean | Record; /** * D-ATTR-GUARD: when set, deletion of the target setting key is shape-guarded — * the key is only removed when its current value deep-equals this shape exactly. * Prevents erasing user-customized values (e.g. attribution with a real org name) * when the flag transitions to neutral. - * Only honoured for setting-target boolean flags. Ignored for env targets. */ readonly settingDeleteGuard?: Record; } +/** + * A boolean on/off flag. `onPayload` is written when the flag is enabled. + * Discriminated on `target.type` so the env-string invariant is enforced by tsc. + */ +export type BooleanFlagDef = EnvBooleanFlagDef | SettingBooleanFlagDef; + /** An enum flag. `neutralValue` is the value that means "no preference" (key is deleted). */ export interface EnumFlagDef extends FlagDefCommon { readonly kind: 'enum'; diff --git a/tests/attribution-prompts.test.ts b/tests/attribution-prompts.test.ts index 1074a162..9568df9f 100644 --- a/tests/attribution-prompts.test.ts +++ b/tests/attribution-prompts.test.ts @@ -15,59 +15,46 @@ import { // ── shouldRunAttributionStep ────────────────────────────────────────────────── -describe('shouldRunAttributionStep — gate predicate (PF-029)', () => { - it('--recommended flag (no modePromptShown) → false (promptless contract preserved)', () => { - expect(shouldRunAttributionStep({ - mode: 'recommended', - modePromptShown: false, - isTTY: true, - hasCliOverride: false, - })).toBe(false); +describe('shouldRunAttributionStep — gate predicate (D27 / PF-029)', () => { + // ── Advanced-only invariant (D27) ─────────────────────────────────────────── + // The attribution question is reachable from the Advanced path ONLY. Unlike the + // compliance step, interactive Recommended never asks — it silently applies the + // seeded value. These tests are the authority for that divergence. + + it('Advanced mode with TTY → true (the only path that asks)', () => { + expect(shouldRunAttributionStep({ mode: 'advanced', isTTY: true })).toBe(true); }); - it('non-TTY → false regardless of mode (promptless contract preserved)', () => { - expect(shouldRunAttributionStep({ - mode: 'advanced', - modePromptShown: true, - isTTY: false, - hasCliOverride: false, - })).toBe(false); + it('interactive Recommended → false (Recommended NEVER asks, D27)', () => { + // Divergence from shouldRunComplianceStep, which returns true here. Interactive + // Recommended silently applies the seeded value (fresh install: off). + expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: true })).toBe(false); }); - it('hasCliOverride → false regardless of mode/TTY/modePromptShown', () => { - expect(shouldRunAttributionStep({ - mode: 'advanced', - modePromptShown: true, - isTTY: true, - hasCliOverride: true, - })).toBe(false); + it('--recommended flag (non-interactive Recommended) → false', () => { + expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: true })).toBe(false); }); - it('interactive Recommended + modePromptShown=true → true', () => { - expect(shouldRunAttributionStep({ - mode: 'recommended', - modePromptShown: true, - isTTY: true, - hasCliOverride: false, - })).toBe(true); + // ── Promptless contracts (PF-029) ─────────────────────────────────────────── + + it('non-TTY → false for Advanced (no prompt without a TTY)', () => { + expect(shouldRunAttributionStep({ mode: 'advanced', isTTY: false })).toBe(false); }); - it('Advanced mode with TTY → true regardless of modePromptShown', () => { - expect(shouldRunAttributionStep({ - mode: 'advanced', - modePromptShown: false, - isTTY: true, - hasCliOverride: false, - })).toBe(true); + it('non-TTY → false for Recommended (promptless contract preserved)', () => { + expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: false })).toBe(false); }); - it('Advanced mode with TTY and modePromptShown=true → true', () => { - expect(shouldRunAttributionStep({ - mode: 'advanced', - modePromptShown: true, - isTTY: true, - hasCliOverride: false, - })).toBe(true); + it('exhaustive gate matrix: only (advanced, TTY) is true', () => { + const matrix: Array<[('recommended' | 'advanced'), boolean, boolean]> = [ + ['advanced', true, true], + ['advanced', false, false], + ['recommended', true, false], + ['recommended', false, false], + ]; + for (const [mode, isTTY, expected] of matrix) { + expect(shouldRunAttributionStep({ mode, isTTY }), `mode=${mode} isTTY=${isTTY}`).toBe(expected); + } }); }); diff --git a/tests/flags.test.ts b/tests/flags.test.ts index 50570ed7..03606179 100644 --- a/tests/flags.test.ts +++ b/tests/flags.test.ts @@ -86,14 +86,47 @@ describe('FLAG_REGISTRY — structural invariants', () => { } }); + /** + * Defense in depth. Since the BooleanFlagDef split into EnvBooleanFlagDef | + * SettingBooleanFlagDef this is ALSO compile-enforced: an env target with an + * object or boolean onPayload no longer typechecks. This test stays as the + * runtime backstop for a registry entry that reaches the array through a cast. + */ it('env boolean flags have string onPayload (env vars are strings)', () => { const envBoolFlags = FLAG_REGISTRY .filter((f): f is BooleanFlagDef => f.kind === 'boolean' && f.target.type === 'env'); + // Non-vacuity: the filter must actually match something or the loop proves nothing. + expect(envBoolFlags.length, 'no env boolean flags matched — assertion would be vacuous').toBeGreaterThan(0); for (const flag of envBoolFlags) { expect( typeof flag.onPayload, `${flag.id}: env boolean flag must have string onPayload`, ).toBe('string'); + // buildPayload writes onPayload verbatim and never consults settingDeleteGuard + // for env targets, so a guard declared here would be a silent no-op. + expect( + flag.settingDeleteGuard, + `${flag.id}: env boolean flags must not declare settingDeleteGuard (it is never honoured)`, + ).toBeUndefined(); + } + }); + + /** + * settingDeleteGuard is only meaningful for a payload that can differ from a + * user's own value. Pinning it to deep-equal onPayload keeps the guard and the + * written shape from drifting apart — a guard that no longer matches what + * applyFlags writes would strand the key in settings.json forever. + */ + it('settingDeleteGuard, where present, deep-equals the flag onPayload', () => { + const guarded = FLAG_REGISTRY.filter( + (f): f is BooleanFlagDef => f.kind === 'boolean' && f.settingDeleteGuard !== undefined, + ); + expect(guarded.length, 'no guarded flags matched — assertion would be vacuous').toBeGreaterThan(0); + for (const flag of guarded) { + expect( + flag.settingDeleteGuard, + `${flag.id}: settingDeleteGuard must match the shape applyFlags writes`, + ).toEqual(flag.onPayload); } }); diff --git a/tests/init-e2e-flags.test.ts b/tests/init-e2e-flags.test.ts index dbd88f8f..b0da41ed 100644 --- a/tests/init-e2e-flags.test.ts +++ b/tests/init-e2e-flags.test.ts @@ -471,3 +471,146 @@ describe('init e2e — flags Phase 6 integration', () => { expect(manifest2.features.flags).toEqual(manifest1.features.flags); }, SUBPROCESS_TIMEOUT_MS); }); + +// --------------------------------------------------------------------------- +// D27 / R3: suppress-attribution through the REAL init settings pass. +// +// PF-018: every other attribution test in the suite is a pure call to applyFlags, +// stripFlags, or resolveExistingAttributionSuppression. None of them proves that +// init writes the block, that the shape guard survives init's strip-then-apply +// double pass (convergeFlagsIntoSettings runs stripFlags THEN applyFlags), that +// the value survives the proxy JSON round-trip and the later security-deny-list +// rewrite, or that the `content !== original` write guard actually fires. +// +// PF-015: each case asserts BOTH artifacts — manifest features.flags and +// settings.json — so a divergence between the two cannot pass. +// --------------------------------------------------------------------------- + +/** Build a current-format manifest with the given flags record. */ +function manifestWithFlags(flags: Record) { + return { + version: '2.0.0', + plugins: ['devflow-implement'], + scope: 'user', + knownPlugins: ['devflow-implement'], + features: { + ambient: false, memory: false, hud: true, knowledge: false, + learning: false, rules: false, proxy: false, + flags, + security: 'user' as const, + compliance: { enabled: false, frameworks: [] }, + }, + installedAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-01T00:00:00.000Z', + }; +} + +const DEVFLOW_ATTRIBUTION = { commit: '', pr: '' }; + +describe('init e2e — suppress-attribution convergence (D27, production path)', () => { + /** Seed manifest + settings, run init, and assert the settings pass did not abort. */ + async function seedAndInit( + flags: Record, + settings: Record, + extraArgs: string[] = [], + ) { + await fs.writeFile( + path.join(tmpHome, '.devflow', 'manifest.json'), + JSON.stringify(manifestWithFlags(flags), null, 2) + '\n', + ); + await fs.writeFile( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify(settings, null, 2) + '\n', + ); + + const result = runInit(tmpHome, extraArgs); + expect(result.status, `init failed:\nstdout: ${result.stdout}\nstderr: ${result.stderr}`).toBe(0); + // PF-018 non-vacuity gate: init swallows settings-pass failures with a warning + // and still exits 0. Without this, every assertion below could pass unrun. + expect( + result.stdout + result.stderr, + 'init warned it could not configure settings.json — the settings pass aborted, ' + + 'so the assertions in this test would be vacuous', + ).not.toContain('Could not configure settings.json'); + + return { + settings: await readSettings(tmpHome), + flags: (await readManifest(tmpHome)).features.flags as Record, + }; + } + + it.skipIf(!CLI_BUILT)('off→on: manifest flag true materialises the attribution block in settings.json', async () => { + const out = await seedAndInit( + { 'suppress-attribution': true }, + { env: { CUSTOM_USER_VAR: 'preserved' } }, // no attribution key on disk + ); + + // On-side convergence: manifest says on, settings must carry the payload. + expect(out.flags['suppress-attribution']).toBe(true); + expect(out.settings.attribution).toEqual(DEVFLOW_ATTRIBUTION); + // Non-vacuity: the settings pass really ran over this file. + expect((out.settings.env as Record).CUSTOM_USER_VAR).toBe('preserved'); + }, SUBPROCESS_TIMEOUT_MS); + + it.skipIf(!CLI_BUILT)('on→off via --reset: the devflow block is removed and the manifest agrees', async () => { + // --reset null-seeds the manifest and empties the settings snapshot, so the flag + // seeds to the registry default (false) even though the block is on disk. This is + // the only init path that turns attribution off — a plain re-init preserves it. + const out = await seedAndInit( + { 'suppress-attribution': true }, + { attribution: { ...DEVFLOW_ATTRIBUTION }, env: { CUSTOM_USER_VAR: 'preserved' } }, + ['--reset'], + ); + + // Off-side convergence: manifest says off, the key must be gone from settings. + expect(out.flags['suppress-attribution']).toBe(false); + expect(out.settings).not.toHaveProperty('attribution'); + }, SUBPROCESS_TIMEOUT_MS); + + it.skipIf(!CLI_BUILT)('a user-customised attribution survives init untouched (shape guard)', async () => { + // The highest-value case: convergeFlagsIntoSettings runs stripFlags THEN + // applyFlags, so the guard has to hold on BOTH passes within a single init. + // Falsification: dropping settingDeleteGuard from the registry entry makes the + // stripFlags pass erase this value and this test fails. + const custom = { commit: 'Acme Corp', pr: 'Acme' }; + const out = await seedAndInit( + { 'suppress-attribution': false }, + { attribution: { ...custom }, env: { CUSTOM_USER_VAR: 'preserved' } }, + ); + + expect(out.settings.attribution).toEqual(custom); + // devflow does not claim a key it did not write. + expect(out.flags['suppress-attribution']).toBe(false); + }, SUBPROCESS_TIMEOUT_MS); + + it.skipIf(!CLI_BUILT)('upgrade path: an existing devflow block seeds the flag ON and is preserved', async () => { + // Every install predating D27 has the block, written by the old template merge, + // with no manifest entry for the flag. Init must adopt it as ON rather than + // silently reverting the user's git attribution behaviour. + const out = await seedAndInit( + {}, // flag absent → adopt-on-init + { attribution: { ...DEVFLOW_ATTRIBUTION }, env: { CUSTOM_USER_VAR: 'preserved' } }, + ); + + expect(out.flags['suppress-attribution']).toBe(true); + expect(out.settings.attribution).toEqual(DEVFLOW_ATTRIBUTION); + }, SUBPROCESS_TIMEOUT_MS); + + it.skipIf(!CLI_BUILT)('fresh install writes no attribution key and records the flag off', async () => { + await fs.writeFile( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ env: { CUSTOM_USER_VAR: 'preserved' } }, null, 2) + '\n', + ); + const result = runInit(tmpHome); // no manifest at all + expect(result.status, `init failed:\n${result.stdout}\n${result.stderr}`).toBe(0); + expect(result.stdout + result.stderr).not.toContain('Could not configure settings.json'); + + const settings = await readSettings(tmpHome); + const flags = (await readManifest(tmpHome)).features.flags as Record; + + // Default OFF: init must not put attribution into a fresh settings.json. + expect(settings).not.toHaveProperty('attribution'); + expect(flags['suppress-attribution']).toBe(false); + expect((settings.env as Record).CUSTOM_USER_VAR).toBe('preserved'); + }, SUBPROCESS_TIMEOUT_MS); +}); diff --git a/tests/uninstall-logic.test.ts b/tests/uninstall-logic.test.ts index dc58645c..1a2777bd 100644 --- a/tests/uninstall-logic.test.ts +++ b/tests/uninstall-logic.test.ts @@ -1550,6 +1550,110 @@ describe('runCleanupPhase (A8)', () => { }); }); +// --------------------------------------------------------------------------- +// R5 / D27: attribution cleanup through the PRODUCTION uninstall path. +// +// PF-018: every other runCleanupPhase test passes `scopesToUninstall: []`, which +// makes the settings.json loop a no-op — so stripFlags was never reached by any +// behavioural test. These drive the real phase with a non-empty scope against a +// sandboxed HOME so the shape guard is exercised where it actually runs, not just +// as a pure applyFlags/stripFlags unit. +// +// Falsification: deleting `settingDeleteGuard` from the suppress-attribution +// registry entry makes the "custom attribution survives" case fail here. +// --------------------------------------------------------------------------- + +describe('R5: uninstall settings cleanup — attribution shape guard (D27, production path)', () => { + let tmpHome: string; + let tmpClaudeDir: string; + let tmpCwd: string; + let settingsPath: string; + + beforeEach(async () => { + tmpHome = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-r5-home-')); + tmpClaudeDir = path.join(tmpHome, '.claude'); + await fs.mkdir(tmpClaudeDir, { recursive: true }); + tmpCwd = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-r5-cwd-')); + settingsPath = path.join(tmpClaudeDir, 'settings.json'); + + // Sandbox every ambient path root runCleanupPhase can reach: getClaudeDirectory() + // honours CLAUDE_CODE_DIR, getDevFlowDirectory() honours DEVFLOW_DIR, and the + // shell-profile probe derives from HOME. Keeping the claude dir INSIDE tmpHome + // also avoids getClaudeDirectory's "outside home directory" console warning. + vi.stubEnv('HOME', tmpHome); + vi.stubEnv('CLAUDE_CODE_DIR', tmpClaudeDir); + vi.stubEnv('DEVFLOW_DIR', path.join(tmpHome, '.devflow')); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(tmpHome, { recursive: true, force: true }); + await fs.rm(tmpCwd, { recursive: true, force: true }); + }); + + /** Drive the real cleanup phase over the sandboxed user scope. */ + async function runCleanup(): Promise { + await runCleanupPhase({ + scopesToUninstall: ['user'], + keepDocs: true, // skips the .devflow/ + security prompt branches + verbose: false, + cwd: tmpCwd, + isTTY: false, // no confirm can fire + }); + } + + it('removes the devflow-managed attribution block', async () => { + await fs.writeFile(settingsPath, JSON.stringify({ + attribution: { commit: '', pr: '' }, + model: 'opus', + }, null, 2), 'utf-8'); + + await runCleanup(); + + const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(after.attribution).toBeUndefined(); + // Non-vacuity: runCleanupPhase swallows all errors in the settings loop, so a + // fixture that never got read would also show `attribution === undefined`. + // An untouched unmanaged key proves the file was actually parsed and rewritten. + expect(after.model, 'unmanaged keys must survive').toBe('opus'); + }); + + it('preserves a user-customized attribution value (shape guard)', async () => { + await fs.writeFile(settingsPath, JSON.stringify({ + attribution: { commit: 'Acme Corp', pr: 'Acme' }, + model: 'opus', + }, null, 2), 'utf-8'); + + await runCleanup(); + + const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(after.attribution).toEqual({ commit: 'Acme Corp', pr: 'Acme' }); + expect(after.model).toBe('opus'); + }); + + it('preserves an attribution object carrying extra keys (not the managed shape)', async () => { + await fs.writeFile(settingsPath, JSON.stringify({ + attribution: { commit: '', pr: '', coAuthor: 'nobody' }, + model: 'opus', + }, null, 2), 'utf-8'); + + await runCleanup(); + + const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(after.attribution).toEqual({ commit: '', pr: '', coAuthor: 'nobody' }); + }); + + it('leaves settings.json without an attribution key untouched', async () => { + await fs.writeFile(settingsPath, JSON.stringify({ model: 'opus' }, null, 2), 'utf-8'); + + await runCleanup(); + + const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(after).not.toHaveProperty('attribution'); + expect(after.model).toBe('opus'); + }); +}); + // --------------------------------------------------------------------------- // F5: settings-hooks cleanup block removal static guard // From 95dd0a76039269d099827b17b938ddb19536cad8 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 13:59:49 +0300 Subject: [PATCH 04/21] test: harden attribution test coverage and fix duplicate gate case (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - post-install-merge: replace second AC3 no-injection test with a fixture that carries an attribution block in the template, so re-introduced injection would be caught (previously the template had no attribution key, making the assertion vacuous for that failure mode) - attribution-prompts: relabel duplicate shouldRunAttributionStep case from '--recommended flag (non-interactive Recommended)' to '--recommended flag (non-TTY, typical non-interactive case)' and change input to {mode:'recommended',isTTY:false} — the predicate has no modePromptShown or hasCliOverride param, so {mode:recommended,isTTY:true} is not a distinct input - flags: add ON-over-custom overwrite test — applyFlags with true replaces a custom attribution value with the devflow shape (settingDeleteGuard only protects deletion, not overwrite), documenting AC2 and the wizard's 'never deleted' wording Co-Authored-By: Claude --- tests/attribution-prompts.test.ts | 7 +++++-- tests/flags.test.ts | 11 +++++++++++ tests/post-install-merge.test.ts | 13 ++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/attribution-prompts.test.ts b/tests/attribution-prompts.test.ts index 9568df9f..2f8bdce8 100644 --- a/tests/attribution-prompts.test.ts +++ b/tests/attribution-prompts.test.ts @@ -31,8 +31,11 @@ describe('shouldRunAttributionStep — gate predicate (D27 / PF-029)', () => { expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: true })).toBe(false); }); - it('--recommended flag (non-interactive Recommended) → false', () => { - expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: true })).toBe(false); + it('--recommended flag (non-TTY, typical non-interactive case) → false', () => { + // The --recommended CLI flag is commonly run without a TTY (CI, scripts). + // The predicate has no modePromptShown or hasCliOverride param — isTTY:false + // is the distinct input that represents this scenario. + expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: false })).toBe(false); }); // ── Promptless contracts (PF-029) ─────────────────────────────────────────── diff --git a/tests/flags.test.ts b/tests/flags.test.ts index 03606179..b0b9ba17 100644 --- a/tests/flags.test.ts +++ b/tests/flags.test.ts @@ -1831,6 +1831,17 @@ describe('suppress-attribution flag — shape guard (D27)', () => { expect(result.attribution).toEqual(DEVFLOW_ATTR); }); + it('applyFlags with true OVERWRITES a custom attribution value (settingDeleteGuard only protects deletion)', () => { + // Documents AC2 and the wizard's "never deleted" wording: settingDeleteGuard only + // guards the DELETE path (flag false/null). Enabling the flag (true) always writes + // the devflow-managed shape regardless of what was on disk. This is intentional — + // the user opted in to attribution suppression, so the managed shape wins. + const custom = { commit: 'My Org', pr: 'My Org PR' }; + const input = JSON.stringify({ attribution: custom }); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': true })); + expect(result.attribution).toEqual(DEVFLOW_ATTR); + }); + it('applyFlags with false deletes the exact devflow attribution shape', () => { const input = JSON.stringify({ attribution: DEVFLOW_ATTR }); const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })); diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts index 9b8f9004..208898b4 100644 --- a/tests/post-install-merge.test.ts +++ b/tests/post-install-merge.test.ts @@ -151,13 +151,16 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { expect(existing.attribution).toBeUndefined(); }); - it('does NOT delete an existing attribution value (preserve user values)', () => { + it('does NOT inject attribution even when template carries an attribution block', () => { // Even when attribution appears in template (legacy or edge case), the - // merge function must not touch an existing attribution value. - const existing: Record = { attribution: 'my-org' }; - const template: Record = { statusLine: 's' }; + // merge function must not write it into a user settings object that lacks one. + // This falsifies re-introduced injection: if mergeDevflowSettingsTemplate were + // ever to merge the attribution key, this test would fail because the template + // carries the block and existing starts empty. + const existing: Record = {}; + const template: Record = { statusLine: 's', attribution: { commit: '', pr: '' } }; mergeDevflowSettingsTemplate(existing, template); - expect(existing.attribution).toBe('my-org'); + expect(existing.attribution).toBeUndefined(); }); it('adds only the missing hooks when some are present and some are not', () => { From d7d2c8e23f89194893a41b26a9e986a0d768c735 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 13:59:55 +0300 Subject: [PATCH 05/21] =?UTF-8?q?docs:=20update=20flag=20count=2028?= =?UTF-8?q?=E2=86=9229=20and=20add=20suppress-attribution=20table=20row=20?= =?UTF-8?q?(refs=20#315)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs/cli-reference.md: 'All 28 flags' → 'All 29 flags'; add suppress-attribution row (boolean, setting attribution, default false) after enable-todo-tools in the flag reference table - docs/reference/file-organization.md: flags.ts comment (28 flags) → (29 flags) Co-Authored-By: Claude --- docs/cli-reference.md | 3 ++- docs/reference/file-organization.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 18c6ee35..db67cf15 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -205,7 +205,7 @@ npx devflow-kit flags --unset # Reset flag(s) to neutral, comma-separ `--enable` and `--disable` accept boolean flags only. Non-boolean flags (enum, number, string) use `--set id=value`. Passing a non-boolean id to `--enable`/`--disable` prints an error and redirects to `--set`. -All 28 flags by kind and devflow default: +All 29 flags by kind and devflow default: | Flag ID | Kind | Target | Devflow Default | |---------|------|--------|-----------------| @@ -231,6 +231,7 @@ All 28 flags by kind and devflow default: | `disable-autoupdater` | boolean | env `DISABLE_AUTOUPDATER` | `false` | | `agent-teams` | boolean | env `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS` | `false` | | `enable-todo-tools` | boolean | env `CLAUDE_CODE_ENABLE_TODO_TOOLS` | `false` | +| `suppress-attribution` | boolean | setting `attribution` | `false` | | `subagent-spawn-depth` | number | env `CLAUDE_CODE_MAX_SUBAGENT_SPAWN_DEPTH` | unset (upstream: 3) | | `workflow-size-guideline` | enum | setting `workflowSizeGuideline` | unset (`small\|medium\|large\|unrestricted`) | | `default-model` | string | env `ANTHROPIC_DEFAULT_MODEL` | unset | diff --git a/docs/reference/file-organization.md b/docs/reference/file-organization.md index 83d8bc9d..05f209ba 100644 --- a/docs/reference/file-organization.md +++ b/docs/reference/file-organization.md @@ -21,7 +21,7 @@ devflow/ │ │ ├── plugins.ts # DEVFLOW_PLUGINS registry — 21 plugin entries │ │ ├── paths.ts # getPackageRoot + asset path helpers │ │ ├── assets.ts # skillsDir, agentsDir, rulesDir, commandsDir, scriptsDir -│ │ ├── flags.ts # Claude Code flag registry (28 flags) +│ │ ├── flags.ts # Claude Code flag registry (29 flags) │ │ ├── fs-atomic.ts # Atomic write helper (D34) │ │ ├── manifest.ts # Manifest read/write │ │ ├── migrations.ts # Run-once migration registry (2.x entries only; first: canonicalise-agent-keys-v1) From ee92e7150ec9e946dc441dc55bf9e38e8e2a4d56 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 14:17:32 +0300 Subject: [PATCH 06/21] docs(knowledge): update installer-shadowing feature knowledge base --- .devflow/features/index.md | 2 +- .../features/installer-shadowing/KNOWLEDGE.md | 57 ++++++++++++++----- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/.devflow/features/index.md b/.devflow/features/index.md index cf1948ce..ccf69532 100644 --- a/.devflow/features/index.md +++ b/.devflow/features/index.md @@ -2,7 +2,7 @@ - **ambient-orchestrator** — src/assets/scripts/hooks, src/cli/commands/ambient.ts, src/core/plugins.ts — Use when modifying the ambient mode hooks (preamble, session-start-orchestrator), the orchestrator charter file (including the feature-knowledge operating rule), the git-marker helper, the ambient CLI toggle, or the plan-handoff fast-path. Keywords: ambient, preamble, orchestrator, charter, plan-handoff, session-start-orchestrator, git-marker, DEVFLOW_BG_UPDATER, devflow ambient, UserPromptSubmit, SessionStart, feature-knowledge. - **dynamic-workflow-engine** — src/assets/commands/dynamic-build.mds, src/assets/commands/dynamic-plan.mds, src/assets/commands/dynamic-tickets.mds, src/assets/commands/dynamic-profile.mds, src/assets/commands/_partials/_engine.mds, src/assets/commands/_partials/_wave.mds, dist/commands, tests/build-mds.test.ts — Use when authoring or modifying the dynamic-* commands (dynamic-build, dynamic-plan, dynamic-tickets, dynamic-profile), the shared engine/wave/preamble/factory MDS partials, or the build-mds test suite that pins doctrine literals. Keywords: dynamic-build, dynamic-plan, dynamic-tickets, dynamic-profile, Workflow tool, agentType, Gate 1, Gate 2, review pass, wave, tickets→plan→build, MDS, _engine.mds, _wave.mds. - **resolve-pipeline** — src/assets/commands/resolve.mds, src/assets/agents/triage.md, src/assets/agents/code.md, src/core/plugins.ts, src/assets/commands/code-review.mds — Use when modifying /resolve or /code-review convergence logic, adding or changing Triage disposition rules (including DUPLICATE collapsing), adjusting Code-agent operating modes (issue-fix/validation-fix), touching the resolution-summary.md parser contract, changing the Verification Gate retry loop, understanding how DIFF_FILES flows from git validate-branch into blast-radius triage, or working on traceability operations (fetch-review-threads, resolve-review-threads, post-resolution-summary, check-merge-readiness, THREAD_MAP). Keywords: resolve, triage, disposition matrix, blast-radius, FIX_NOW, FIX_SEPARATE, TECH_DEBT, FALSE_POSITIVE, BY_DESIGN, ESCALATED, DUPLICATE, duplicate-grouping, duplicates-collapse, duplicate_of, resolution-summary, convergence parser, DIFF_FILES, issue-fix, validation-fix, Verification Gate, manage-debt, COMPLIANCE_SKILL_INSTALLED, TRACEABILITY DEGRADED, fetch-review-threads, THREAD_MAP, post-resolution-summary, Third-Party Threads, check-merge-readiness, ext-N, D7, D9, PF-024. -- **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), or working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. +- **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, suppress-attribution, settingDeleteGuard, deepEqualsPlain, D-ATTR-GUARD, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. - **learning-capture-system** — src/assets/scripts/hooks, src/assets/agents/learning.md, src/cli/commands/learning.ts, src/core/feature-config.ts, src/core/learning-tuning-config.ts, src/hud/components/learning-counts.ts, src/assets/commands/_partials — Use when modifying capture hooks (capture-prompt/capture-turn/capture-question), the learning or memory pending-turns queues, the Learning agent (src/assets/agents/learning.md), the session-start-context learning directive, the feature-config toggles, the learning tuning config, the decisions content files (decisions.md/pitfalls.md/index.md) or their ledger ops, or the devflow learning CLI. Keywords: capture-prompt, capture-turn, capture-question, queue-append, pending-turns, memory-worker, Learning agent, learning directive, LEARNING MAINTENANCE, DEVFLOW_BG_UPDATER, learning-lock, queue_read_gates, decisions_load, DECISIONS_CONTEXT, feature-config, config.json, learning.json, decisions-ledger, assign-anchor, retire-anchor, refresh-anchor, render-decisions, staged-write CAS, WORKING-MEMORY.md.new, segmentDetails, amendments, is-hex-sha, verify_and_swap, compute_commits_since_note, divergence guard, isSafeRawBody. - **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate, subswitch 0.4.0. - **compliance-feature** — src/core/compliance.ts, src/targets/claude-code/compliance-install.ts, src/cli/commands/compliance.ts, src/assets/skills/compliance, src/assets/rules/compliance.md, src/assets/agents/git.md, src/assets/commands/code-review.mds, src/assets/commands/plan.mds, src/assets/commands/implement.mds, src/assets/commands/resolve.mds, src/assets/commands/release.md — Use when adding or modifying the compliance feature (framework registry, converge contract, CLI, rule stamping), changing how host commands resolve COMPLIANCE_SKILL_INSTALLED, modifying traceability operations in the Git agent (learn-conventions, issue-first, thread resolution, shipped markers, release evidence), or extending the D4 DEGRADED contract. Keywords: compliance, COMPLIANCE_SKILL_INSTALLED, convergeComplianceArtifacts, convergeFromManifest, frameworks, FEATURE_OWNED_SKILLS, traceability, D4, D9, gather-release-evidence, conventions.md, resolve-review-threads, ensure-traceable-issue, stamper, manifest-group, ComplianceFeatureState. diff --git a/.devflow/features/installer-shadowing/KNOWLEDGE.md b/.devflow/features/installer-shadowing/KNOWLEDGE.md index 052697a3..57cb1098 100644 --- a/.devflow/features/installer-shadowing/KNOWLEDGE.md +++ b/.devflow/features/installer-shadowing/KNOWLEDGE.md @@ -1,11 +1,11 @@ --- feature: installer-shadowing name: Installer & Skill/Rule Shadowing -description: "Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), or working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline." +description: "Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, suppress-attribution, settingDeleteGuard, deepEqualsPlain, D-ATTR-GUARD, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline." category: architecture -directories: [src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts] +directories: [src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts] created: 2026-07-13 -updated: 2026-08-25 +updated: 2026-09-01 --- # Installer & Skill/Rule Shadowing @@ -211,7 +211,7 @@ When `proxyEnabled` is true entering the install apply pass, `runProxyPreflight` **`revertExternalAgents` on the selective path**: runs **before** `removeSelectedPlugins` — strips GPT model lines from installed agent frontmatter while the files are still present. **Known limitation**: on the selective path it reverts EVERY installed agent, not only those being removed — surviving agents lose GPT frontmatter assignments until the next `devflow init`. On the full-uninstall path it also runs before `removeAllDevFlow`. -**`removeAllDevFlow(claudeDir, devflowScriptsDir, verbose)`** removes `commands/devflow/`, `agents/devflow/`, `rules/devflow/`, `devflowScriptsDir`, and skill dirs via two separate passes (avoids PF-012): **prefixed** (`devflow:name`) for every skill in `getAllSkillNames() ∪ LEGACY_SKILL_NAMES`; **bare** (name or `devflow-name`) for `LEGACY_SKILL_NAMES` only — `~/.claude/skills/` is shared, so a bare dir matching a live-registry skill name is by construction foreign to Devflow. +**`removeAllDevFlow(claudeDir, devflowDir, verbose)`** removes `commands/devflow/`, `agents/devflow/`, `rules/devflow/`, `devflowScriptsDir`, and skill dirs via two separate passes (avoids PF-012): **prefixed** (`devflow:name`) for every skill in `getAllSkillNames() ∪ LEGACY_SKILL_NAMES`; **bare** (name or `devflow-name`) for `LEGACY_SKILL_NAMES` only — `~/.claude/skills/` is shared, so a bare dir matching a live-registry skill name is by construction foreign to Devflow. After `removeAllDevFlow`, scope-specific logic handles the remainder of `devflowDir`. The scope decision lives in `resolveDevflowDirCleanup(opts)`, a **pure exported function** (mirrors `resolveSecurityRemovalDecision`) — no I/O, no side effects, fully testable. @@ -230,7 +230,7 @@ After `removeAllDevFlow`, scope-specific logic handles the remainder of `devflow - **`runDryRunPhase(opts)`** — selective mode: derives plan from `computeAssetsToRemove` + `formatDryRunPlan`; full mode: calls `enumerateDryRunExtras` for each scope (exercises the production enumeration path, not only pure helpers — avoids PF-018). - **`runSelectivePhaseForScope(opts)`** — reverts external agent frontmatter, calls `removeSelectedPlugins` (which calls `sweepDevflowNamespaces`), cleans ambient hook if ambient plugin is removed. - **`runFullPhaseForScope(opts)`** — reverts external agents, calls `removeAllDevFlow`, then scope-aware devflowDir cleanup (local: always artifacts-only; user: `resolveDevflowDirCleanup` gate → prompt or artifacts-only). Takes injected `isTTY` rather than reading `process.stdin.isTTY` directly. -- **`runCleanupPhase(opts)`** — post-loop extras on full uninstall: `.devflow/` project data dir, `.claudeignore`, `settings.json` hooks/flags, security deny list, safe-delete shell function. Takes injected `cwd` and `isTTY` so prompt gates are testable without touching developer files. +- **`runCleanupPhase(opts)`** — post-loop extras on full uninstall: `.devflow/` project data dir, `.claudeignore`, `settings.json` hooks/flags, security deny list, safe-delete shell function. Calls `stripFlags` directly (no record argument) — removes all flag-managed keys including the `attribution` key when its current value is the devflow-managed shape `{"commit":"","pr":""}` (shape guard via `settingDeleteGuard`; a custom attribution value is never deleted). Takes injected `cwd` and `isTTY` so prompt gates are testable without touching developer files. **`enumerateUserDevFlowContent(devflowDir)`** (called BEFORE any removal) checks for: `devflowDir/skills/` (skill shadows), `devflowDir/rules/` (rule shadows), `devflowDir/preference-profile.md`, `devflowDir/learning.json`, and `devflowDir/hud.json`. Returns human-readable labels. `agent-models.json` is **NOT** listed here — it is classified as an install artifact (see `installArtifactPaths`). @@ -250,7 +250,7 @@ A dedicated pure-function module (`src/cli/commands/init-seed.ts`) computes the **Composition point**: `resolveInitSeed(seedManifest, seedConfig, settingsSnapshot, plugins) → InitSeed` -`InitSeed` carries: `features: FeatureSeed`, `flags: FlagsRecord`, `workflowPlugins: string[]`, `languagePlugins: string[]`. `viewMode` is encoded inside `flags['view-mode']` (PF-015: all flag state in FlagsRecord) — there is no separate `viewMode` field. +`InitSeed` carries: `features: FeatureSeed`, `flags: FlagsRecord`, `workflowPlugins: string[]`, `languagePlugins: string[]`. `viewMode` is encoded inside `flags['view-mode']` (PF-015: all flag state in FlagsRecord) — there is no separate `viewMode` field. `suppress-attribution` is similarly encoded inside `flags['suppress-attribution']`. **Feature seeding** (`resolveSeedFeatures`): - `memory / learning / knowledge`: projectConfig wins when present (ADR-001 — config.json is the source of truth); falls back to manifest; then registry defaults (all true). @@ -264,6 +264,8 @@ A dedicated pure-function module (`src/cli/commands/init-seed.ts`) computes the **viewMode resolution**: `resolveInitSeed` resolves view-mode in three-priority order — (1) `resolveExistingViewMode(settingsSnapshot)` (non-`'default'` from current settings.json wins); (2) `readViewMode(flags)` from the spread manifest record (non-`'default'` wins); (3) `'default'`. The resolved value is encoded into `flags['view-mode']` on the returned `InitSeed`. `seedManifest?.features.viewMode` is no longer consulted — that field is retired; view-mode lives entirely in `ManifestData.features.flags['view-mode']`. +**suppress-attribution resolution** (`resolveExistingAttributionSuppression`): an exported pure function in `init-seed.ts` (mirrors `resolveExistingViewMode`). Returns `true` when the settings.json `attribution` key is the exact devflow-managed shape `{"commit":"","pr":""}` (two keys, both empty string, no extras). Returns `undefined` for absent key, custom value, or malformed JSON — callers fall through to the manifest entry. Priority order in `resolveInitSeed`: (1) `resolveExistingAttributionSuppression(settingsSnapshot)` → `true` when exact shape; (2) `flags['suppress-attribution']` from manifest FlagsRecord (boolean); (3) `false` (registry default). The resolved value is encoded into `flags['suppress-attribution']` on the returned `InitSeed`. `--reset` zeroes `settingsSnapshot` and `seedManifest`, so the resolved value is always `false` on a factory reset. + **CLI toggles** (`applyCliToggles`): Applies explicit CLI feature flags (e.g. `--no-learning`, `--proxy`) on top of the resolved seed. Undefined = not specified; seed value is kept. **`--reset --plugin` rejection**: Combining factory reset with a partial install is rejected before reaching seed resolution. @@ -283,6 +285,21 @@ A dedicated CLI-layer module (ADR-013 — CLI-layer prompts; core stays UI-agnos - **`runComplianceStep(opts)`** — pure orchestrator (no `throw`/`process.exit()`/direct I/O; all I/O routed through `opts.prompts`). Flow: note header → labeled enable select → framework multiselect (`required:false`). Disable preserves frameworks (defensive copy; returned arrays never alias the seed). Returns `{kind:'resolved', state, messages}` or `{kind:'cancelled'}`. - **`shouldRunComplianceStep({mode, modePromptShown, isTTY, hasCliOverride})`** — pure gate predicate (PF-029). BOTH wizard paths (Recommended and Advanced) call it. `modePromptShown` is `true` only when the Setup-mode `p.select` actually ran; `--recommended` flag and non-TTY fallback never set it, preserving their promptless contracts. `--compliance`/`--no-compliance` wins via `hasCliOverride`. Recommended threads the result via `applyCliToggles(…, { compliance: cliComplianceOverride ?? wizardCompliance })`. +### Attribution Prompt Module (`src/cli/commands/attribution-prompts.ts`) + +A dedicated CLI-layer module (ADR-013) that owns the attribution wizard UI for the `suppress-attribution` flag (D27). Parallel structure to `compliance-prompts.ts` but with a deliberately different gate predicate. + +Key exports: + +- **`shouldRunAttributionStep({mode, isTTY})`** — pure gate predicate (applies PF-029). Returns `isTTY && mode === 'advanced'`. **Advanced-only — this is a documented divergence from `shouldRunComplianceStep`** (see Gotchas). There is no `modePromptShown` parameter and no CLI override for attribution: the question never runs on Recommended, and post-install toggling is via `devflow flags --enable/--disable suppress-attribution`. +- **`AttributionPromptIO`** — injectable DI seam mirroring `CompliancePromptIO`. Enables unit tests to drive all branches without a real TTY. +- **`buildClackAttributionPrompts()`** — builds the real clack adapter; translates the cancel symbol into `PromptOutcome`. +- **`runAttributionStep({seed, prompts})`** — pure orchestrator (no `throw`/`process.exit()`/direct I/O). Flow: note header (current setting + context) → `p.select` Yes/No (seeded from prior state). Returns `{kind:'resolved', suppress, messages}` or `{kind:'cancelled'}`. + +**Call site in `init.ts`**: a single call in the Advanced path only. The Recommended path has no attribution call and carries the seeded `suppress-attribution` value unchanged from `resolveInitSeed`. After the Advanced step, `enabledFlags` is updated with the wizard answer so the subsequent `convergeFlagsIntoSettings` pipeline writes (or removes) the `attribution` key. + +**Single ownership**: the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags` via `convergeFlagsIntoSettings`). It is absent from `src/targets/claude-code/templates/settings.json` and is NOT injected by `mergeDevflowSettingsTemplate` — adding it to either would create a second writer and could race with the flag pipeline. + ### Migrations (`src/core/migrations.ts`) The `MIGRATIONS` registry (typed `readonly AnyMigration[]`) has one entry: `canonicalise-agent-keys-v1` (scope `'global'`), which renames legacy keys in `~/.devflow/agent-models.json` to their canonical names. @@ -392,6 +409,8 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - **Importing `EXCLUDED` as an oracle in tests** — destroys the test's independent literal check and turns invariant guards into tautologies. Pin an independent literal in the test alongside the production import. - **Dry-run preview using only pure helpers instead of the production enumeration path** — `runDryRunPhase` (full mode) must call `enumerateDryRunExtras`, which itself calls `installArtifactPaths`. A test that exercises only the pure helper (`installArtifactPaths` in isolation) does not catch divergence between the preview and the real removal loop. (avoids PF-018) - **Re-deriving the display vocabulary at a render site instead of calling `effectiveDisplay`** — four render sites (TUI `formatValue`, `--enable/--disable` confirmation, `--status` not-adopted message, `--list` defaultLabel) all route through `effectiveDisplay`. Adding a fifth site that hand-codes 'on'/'off' or shows 'unset' creates vocabulary drift. Always delegate to `effectiveDisplay` (D-EFFDV) or `formatFlagValue` (which does so internally). +- **Mirroring the compliance wizard gate for the attribution wizard gate** — `shouldRunAttributionStep` uses `mode === 'advanced'` directly (no `modePromptShown`), deliberately diverging from `shouldRunComplianceStep`. The compliance gate runs on interactive Recommended (`modePromptShown: true`); the attribution gate never does. Do not "restore symmetry" — the divergence is D27 design intent. +- **Adding `attribution` to `templates/settings.json` or `mergeDevflowSettingsTemplate`** — the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags`). A second writer creates a race: the template merge runs before the flags pipeline, so a template-written value would be immediately overwritten or, on the off path, leave a stale block. Single ownership is enforced by omission from both the template file and the merge function. ## Gotchas @@ -423,12 +442,18 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - **Compliance wizard gate keys on `modePromptShown`, never the mode name.** `shouldRunComplianceStep` uses `modePromptShown` (was the Setup-mode `p.select` actually shown?) rather than checking `mode === 'recommended'`. Gating on the mode name would break the `--recommended` promptless contract: `--recommended` resolves `mode='recommended'` but never shows the prompt, so `modePromptShown` stays `false`. Same applies to the non-TTY fallback. (PF-029) +- **Attribution wizard gate does NOT use `modePromptShown`.** `shouldRunAttributionStep` gates on `mode === 'advanced'` directly — no `modePromptShown` parameter. This is safe because the Advanced branch itself exits non-zero on non-TTY (isTTY is the outer guard), so `mode === 'advanced'` is only ever true in an interactive session. Do not add a `modePromptShown` parameter to "align" it with the compliance gate — the divergence is intentional (D27). (PF-029) + +- **`settingDeleteGuard` protects deletion, not writes.** When `suppress-attribution` is enabled (`true`), `applyFlags` always writes `{"commit":"","pr":""}` to `settings.json`, overwriting any prior value including a custom attribution block. The guard only gates the neutral/off path: when the flag transitions to false/null, the key is deleted ONLY when the current value deep-equals the managed shape. If the user has a custom attribution block (e.g. an org name), a flag-disable preserves it; a flag-enable overwrites it — this behavior is test-pinned. + - **`--set` confirmation echoes literal 'unset' for an explicit null input.** When the user types `--set flag=unset`, `parseFlagValueInput` maps that to `null`. The `handleSet` confirmation special-cases `null → 'unset'` at the call site so the user sees their own word reflected back. Active values route through `formatFlagValue` (D-EFFDV) as normal — this is the only site where 'unset' still appears in user-facing output. - **Blurb hard-cap is enforced by a registry test, not a TypeScript type.** `flag.blurb` is typed as `string` on `FlagDefCommon` (no length constraint in the type). The ≤30-char cap lives in `tests/flags.test.ts` as a registry-walk test — adding a blurb longer than 30 chars will fail CI but not the TypeScript compiler. - **Inline mode (`screen: 'inline'`) does not enter the alt screen.** On exit it cursor-ups to the frame top and `ERASE_BELOW` — the widget is erased and the clack flow continues in the normal scroll buffer. If you attach a flags TUI test expecting `ENTER_ALT` sequences, it will fail for `runFlagsTui` (which passes `screen: 'inline'`) but pass for agents-view tests (which use the default alt mode). Use `screen: 'alt'` explicitly when testing alt-screen behavior. +- **Selective uninstall never strips flags.** `runCleanupPhase` (which calls `stripFlags`) only runs on full uninstall. Selective plugin uninstall (`runSelectivePhaseForScope`) does not invoke `stripFlags` — flag state persists in `settings.json` even when individual plugins are removed. + ## Key Files - `src/core/orphan-sweep.ts` — `sweepOrphanedAssets(dir, knownNames, extractRegistryName) => Promise`; `SweepResult = { scanned, removed, failed }`; `mdFileName` / `mdEntryName` inverse pair; shared by both installer and uninstall; per-item failure isolation on both readdir and rm @@ -436,14 +461,15 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - `src/core/assets.ts` — `skillsDir`, `agentsDir`, `rulesDir`, `scriptsDir`, `commandsDir` accessors; single source of truth for all asset source paths - `src/core/paths.ts` — `getPackageRoot()` with hard `package.json` assertion; 2-level-up resolution from `dist/core/paths.js`; `isContainedIn(parent, candidate)` pure containment predicate (guards path-traversal in reapplyAgentMapping) - `src/targets/claude-code/legacy.ts` — `LEGACY_SKILL_NAMES` (composed from `LEGACY_SKILLS_PRE_V1`, `LEGACY_SKILLS_V2`, `LEGACY_SKILLS_V2X`); target-specific delete lists for upgrade cleanup -- `src/cli/commands/init.ts` — consumes `InstallReport` and `InitSeed`; proxy preflight block using `buildRealPreflightDeps` factory (`swallowSettingsReadError: true`); `reapplyAgentMapping` call (ordering load-bearing, guarded when mapping is empty AND proxy is off); exhaustive `ShadowSkipReason` switch with `never` guard -- `src/cli/commands/init-seed.ts` — pure seeding helpers: `resolveInitSeed`, `resolveSeedFeatures`, `resolveSeedFlags(manifestFlags: FlagsRecord | null, registry)` (two-branch: null→all defaults, non-null→spread+adopt-absent), `resolveSeedPlugins`, `resolveResetGatedInputs`, `applyCliToggles`, `FEATURE_DEFAULTS` (proxy: false); `InitSeed.flags: FlagsRecord` encodes view-mode in `flags['view-mode']` — no separate `viewMode` field -- `src/cli/commands/uninstall.ts` — exported: `removeAllDevFlow`, `removeSelectedPlugins`, `isDevFlowInstalled`, `installArtifactPaths` (SSOT for artifact list), `enumerateDryRunExtras` (derived from installArtifactPaths + skill lists), `sweepDevflowNamespaces` (named selective-path sweep step), `resolveProjectDataCleanup` (pure: cancel→preserve, no process.exit), `enumerateUserDevFlowContent` (skills/rules/preference-profile/learning.json/hud.json — NOT agent-models.json), `removeDevFlowInstallArtifacts` (uses installArtifactPaths; containment guard; `isDir === true` strict equality), `revertExternalAgents` runs on both full and selective paths, `computeAssetsToRemove`, `resolveSecurityRemovalDecision`, `resolveDevflowDirCleanup` (--keep-docs honored); phase runners: `runDryRunPhase`, `runSelectivePhaseForScope`, `runFullPhaseForScope`, `runCleanupPhase` (injected cwd + isTTY) +- `src/cli/commands/init.ts` — consumes `InstallReport` and `InitSeed`; proxy preflight block using `buildRealPreflightDeps` factory (`swallowSettingsReadError: true`); `reapplyAgentMapping` call (ordering load-bearing, guarded when mapping is empty AND proxy is off); exhaustive `ShadowSkipReason` switch with `never` guard; attribution step in Advanced path only (`shouldRunAttributionStep`, `runAttributionStep`) +- `src/cli/commands/init-seed.ts` — pure seeding helpers: `resolveInitSeed`, `resolveSeedFeatures`, `resolveSeedFlags(manifestFlags: FlagsRecord | null, registry)` (two-branch: null→all defaults, non-null→spread+adopt-absent), `resolveSeedPlugins`, `resolveResetGatedInputs`, `applyCliToggles`, `FEATURE_DEFAULTS` (proxy: false); `resolveExistingAttributionSuppression` (mirrors resolveExistingViewMode — returns true for exact devflow shape, undefined otherwise); `InitSeed.flags: FlagsRecord` encodes both view-mode and suppress-attribution — no separate fields +- `src/cli/commands/attribution-prompts.ts` — `shouldRunAttributionStep({mode, isTTY})` (Advanced-only gate; no modePromptShown; D27 divergence from compliance gate); `AttributionPromptIO` (DI seam); `buildClackAttributionPrompts()`; `runAttributionStep({seed, prompts})` (pure orchestrator, no process.exit, no throw) +- `src/cli/commands/uninstall.ts` — exported: `removeAllDevFlow`, `removeSelectedPlugins`, `isDevFlowInstalled`, `installArtifactPaths` (SSOT for artifact list), `enumerateDryRunExtras` (derived from installArtifactPaths + skill lists), `sweepDevflowNamespaces` (named selective-path sweep step), `resolveProjectDataCleanup` (pure: cancel→preserve, no process.exit), `enumerateUserDevFlowContent` (skills/rules/preference-profile/learning.json/hud.json — NOT agent-models.json), `removeDevFlowInstallArtifacts` (uses installArtifactPaths; containment guard; `isDir === true` strict equality), `revertExternalAgents` runs on both full and selective paths, `computeAssetsToRemove`, `resolveSecurityRemovalDecision`, `resolveDevflowDirCleanup` (--keep-docs honored); phase runners: `runDryRunPhase`, `runSelectivePhaseForScope`, `runFullPhaseForScope`, `runCleanupPhase` (injected cwd + isTTY; calls stripFlags with settingDeleteGuard shape-guarded deletion) - `src/core/manifest.ts` — `ManifestData` (`features.flags: FlagsRecord` — key-presence = known, null = neutral, absent = adopt-on-init; `knownPlugins?: string[]`; `features.proxy`); `parseManifestFlags(features, knownFlags)` — three-shape migration: string[]→`migrateLegacyFlagsToRecord`, object→spread, missing→empty; `readManifest` — self-heals legacy `knownFlags` (consumed in migration, not stored), proxy absent→false, applies `sanitizeFlagsRecord`; `writeManifest`, `syncManifestFeature`, `resolvePluginList` (filters `DELETED_PLUGIN_NAMES` via in-memory filter) - `src/core/plugins.ts` — `prefixSkillName`, `unprefixSkillName`, `SKILL_NAMESPACE`, `DEVFLOW_PLUGINS` (21 plugins — no devflow-audit-claude), `buildFullSkillsMap`, `buildRulesMap`, `getAllSkillNames`, `getAllCommandNames`, `getAllAgentNames`, `partitionSelectablePlugins`, `EXCLUDED` (module-level export), `LEGACY_PLUGIN_NAMES`, `LEGACY_COMMAND_NAMES`, `LEGACY_RULE_NAMES`, `DELETED_PLUGIN_NAMES` (['devflow-audit-claude']) - `src/core/migrations.ts` — `MIGRATIONS: readonly AnyMigration[]` (one entry: `canonicalise-agent-keys-v1`, scope `'global'`); `AnyMigration = Migration<'global'> | Migration<'per-project'>` discriminated union; `canonicaliseAgentKeys` returns `{agents, didMutate, renamed, dropped, guardDropped}`; `parseAgentMappingEnvelope` shared with `readAgentMapping`; failure-as-warning means a failed write is permanently skipped (self-healed by `readAgentMapping`) - `src/cli/commands/proxy.ts` — `applyDisableToSettings`, `buildRealPreflightDeps`, `addProxyHooks`, `removeProxyHooks`, `applyProxyEnv`, `stripProxyEnv` -- `src/core/flags.ts` — `FLAG_REGISTRY` (28 flags, each with `blurb: string` on `FlagDefCommon` — ≤30 chars, hard-capped by registry test); `FlagsRecord` (`Record`), `FlagsRecordValue` (`FlagValue | null`); `effectiveDisplay(flag, value): EffectiveDisplay` (D-EFFDV one-definition seam — never returns 'unset': boolean→'on'/'off', enum null→neutralValue, number null→devflow/upstream default, string null→'—'); `formatFlagValue` delegates to `effectiveDisplay`; `getDefaultFlagsRecord`, `sanitizeFlagsRecord`, `migrateLegacyFlagsToRecord`, `coerceFlagValue`, `parseFlagValueInput`, `neutralValueOf`, `isNeutral`, `countActiveFlags`, `readViewMode`; `applyFlags(settingsJson, FlagsRecord)`, `stripFlags`, `resolveExistingViewMode`, `resolveFinalViewMode` +- `src/core/flags.ts` — `FLAG_REGISTRY` (29 flags, each with `blurb: string` on `FlagDefCommon` — ≤30 chars, hard-capped by registry test); `BooleanFlagDef = EnvBooleanFlagDef | SettingBooleanFlagDef` discriminated union — `EnvBooleanFlagDef` compile-constrains `onPayload: string` and `settingDeleteGuard?: never`; `SettingBooleanFlagDef` allows object `onPayload` and optional `settingDeleteGuard: Record`; `suppress-attribution` is a `SettingBooleanFlagDef` (target key `attribution`, onPayload `{commit:'',pr:''}`, settingDeleteGuard same shape); `deepEqualsPlain` (private pure JSON structural equality used by D-ATTR-GUARD); `FlagsRecord` (`Record`), `FlagsRecordValue` (`FlagValue | null`); `effectiveDisplay(flag, value): EffectiveDisplay` (D-EFFDV one-definition seam — never returns 'unset': boolean→'on'/'off', enum null→neutralValue, number null→devflow/upstream default, string null→'—'); `formatFlagValue` delegates to `effectiveDisplay`; `getDefaultFlagsRecord`, `sanitizeFlagsRecord`, `migrateLegacyFlagsToRecord`, `coerceFlagValue`, `parseFlagValueInput`, `neutralValueOf`, `isNeutral`, `countActiveFlags`, `readViewMode`; `applyFlags(settingsJson, FlagsRecord)`, `stripFlags`, `resolveExistingViewMode`, `resolveFinalViewMode` - `src/core/feature-config.ts` — `readConfig`, `readConfigIfPresent`, `writeConfig`, `updateFeature` - `src/cli/commands/flags.ts` — `createFlagsCommand` (bare TTY→TUI inline mode, bare non-TTY→status table+exit 1); `lookupFlag(id)` (null for unknown); `readSettingsSafe(settingsPath)` (Result-returning); `persistFlagConfig(claudeDir, devflowDir, settingsContent, newRecord)` (writes FlagsRecord to manifest + settings.json); `formatStatusRows` uses `effectiveDisplay` for not-adopted rows; `--set` confirmation special-cases null→literal 'unset' - `src/cli/flags-view/state.ts` — `FlagsViewState`, `FlagRow` (includes `blurb: string` sourced from `flag.blurb`); `buildFlagRows(registry, record)`, `collectFlagRecord(rows)`; `buildStops`, `cycleForward`, `cycleBackward`; `recordToTui`/`tuiToRecord` value converters; `reduce(state, key) → {state, done, saved}`; `enterEdit`/`commitEdit`/`insertChar`/`reduceEditMode`; `adjustViewport` @@ -457,11 +483,16 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - ADR-001: Config-only feature gates — governs `readConfigIfPresent` as the init-seed source for memory/learning/knowledge; config.json is the source of truth, manifest is secondary. Note: proxy is NOT in this group — it seeds from the manifest like ambient/hud/rules - ADR-003: End-state not transition — governs removals and legacy cleanup; cancel/decline on uninstall falls through to `removeDevFlowInstallArtifacts` rather than `process.exit()` so cleanup always runs - ADR-010: Shadow tolerance — governs `installViaFileCopy` as sole install path and warn-and-install-source (not hard-fail) for invalid shadows; hard-error policy applies only to declared Devflow sources -- ADR-013: Core/adapter boundary — governs `init-seed.ts` living in `src/cli/commands/` (CLI-init-specific logic) rather than `src/core/` -- ADR-014: State-aware re-init — governs `readManifest` self-heal idiom (`proxy` absent→false), FlagsRecord key-presence as the "known" encoding (absent key = adopt-on-init), and the `knownPlugins` snapshot pattern for detecting newly added plugins +- ADR-013: Core/adapter boundary — governs `init-seed.ts` and `attribution-prompts.ts` living in `src/cli/commands/` (CLI-init-specific logic) rather than `src/core/` +- ADR-014: State-aware re-init — governs `readManifest` self-heal idiom (`proxy` absent→false), FlagsRecord key-presence as the "known" encoding (absent key = adopt-on-init), `--reset` zeroing seedManifest so suppress-attribution always falls back to false on factory reset, and the `knownPlugins` snapshot pattern for detecting newly added plugins +- ADR-019: Typed flag registry — governs the `FLAG_REGISTRY` design including `BooleanFlagDef` as a discriminated union (`EnvBooleanFlagDef | SettingBooleanFlagDef`) enforcing the env-string invariant at compile time +- ADR-020: Flags editor removal from init (D40) — governs that init applies flags non-interactively; `devflow flags` bare on TTY is the sole TUI entry point - PF-009: Per-item failure isolation — per-rule try/catch inside `installRuleFile`; `rules --enable` wraps `installAllRules`; proxy preflight failure warns + forces off without aborting init; `sweepOrphanedAssets` outer/inner independent catches; proxy artifact removal is per-item non-fatal; non-fatal catches can mask systematic TypeErrors when optional properties are not narrowed - PF-012: LEGACY_* lists deletion-risk — lists split between `src/targets/claude-code/legacy.ts` (skill) and `src/core/plugins.ts` (plugin/command/rule); both must be retained across upgrades -- PF-014: process.exit() skips cleanup — governs the cancel/decline path in user-scope uninstall; `removeDevFlowInstallArtifacts` must execute on every non-confirm path; `resolveProjectDataCleanup` maps cancel→false (preserve) instead of process.exit() +- PF-014: process.exit() skips cleanup — governs the cancel/decline path in user-scope uninstall; `removeDevFlowInstallArtifacts` must execute on every non-confirm path; `resolveProjectDataCleanup` maps cancel→false (preserve) instead of process.exit(); `runAttributionStep` also never calls process.exit (callers own the cancel idiom) +- PF-015: Fold-before-strip — governs that `suppress-attribution` (like view-mode) must be encoded into `FlagsRecord` before `convergeFlagsIntoSettings` runs; both flags share the single-record pattern - PF-018: Dry-run regression test must exercise the production output path — the original helper-only test missed a real preview/deletion divergence; `runDryRunPhase` (full mode) calls `enumerateDryRunExtras` which shares `installArtifactPaths` with the removal loop +- PF-029: Wizard gate predicates must be fully wired, seeded, tested — applies to both `shouldRunComplianceStep` and `shouldRunAttributionStep`; the attribution gate diverges deliberately (Advanced-only, no modePromptShown) and the divergence is documented in `attribution-prompts.ts` (D27) +- PF-043: Test fixtures must match runtime shapes — governs the `tests/init-e2e-flags.test.ts` subprocess e2e tests over the real init settings pass, ensuring test fixtures stay in sync with the actual settings.json schema written by `applyFlags` - Feature knowledge: `external-model-routing` — deep proxy mechanics (lifecycle, preflight protocol, ensure-proxy hook, per-agent model mapping, dormancy invariant, agent frontmatter rewriting, TUI); `installer-shadowing` covers only proxy's footprint in the install/uninstall pipeline and init seeding - Feature knowledge: `feature-knowledge-system` — the Knowledge agent writes to `.devflow/features/` which is tracked in git; related to the `.gitignore` carve-out maintained by the installer From db4f1f71cb3d4920a6bb63957f35dc879ada59d4 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 15:10:55 +0300 Subject: [PATCH 07/21] refactor(git-skill): drop hard-coded Co-Authored-By trailer (#315) Remove Co-Authored-By: Claude trailers from the git skill commit templates (SKILL.md message format + HEREDOC example, references/patterns.md fix and breaking-change examples) and remove the Generated-with footer from the PR HEREDOC example. Attribution is now governed solely by Claude Code's native mechanism and the suppress-attribution flag. The skill neither adds nor strips trailers. --- src/assets/skills/git/SKILL.md | 4 ---- src/assets/skills/git/references/patterns.md | 6 ------ 2 files changed, 10 deletions(-) diff --git a/src/assets/skills/git/SKILL.md b/src/assets/skills/git/SKILL.md index 9e2e559e..a7960685 100644 --- a/src/assets/skills/git/SKILL.md +++ b/src/assets/skills/git/SKILL.md @@ -97,8 +97,6 @@ See `references/patterns.md` for extended recovery and stash workflows. - -Co-Authored-By: Claude ``` ### Types @@ -125,8 +123,6 @@ Implement token validation middleware with: - Expiration checking Closes #123 - -Co-Authored-By: Claude EOF )" ``` diff --git a/src/assets/skills/git/references/patterns.md b/src/assets/skills/git/references/patterns.md index a1a8b5fc..46bcbd65 100644 --- a/src/assets/skills/git/references/patterns.md +++ b/src/assets/skills/git/references/patterns.md @@ -177,8 +177,6 @@ Previous implementation assumed UTC, causing off-by-one errors for dates near midnight in non-UTC timezones. Fixes #456 - -Co-Authored-By: Claude EOF )" ``` @@ -193,8 +191,6 @@ BREAKING CHANGE: API responses now follow JSON:API specification. All clients must update to handle new response structure. Migration guide: docs/migration/v2-response-format.md - -Co-Authored-By: Claude EOF )" ``` @@ -255,8 +251,6 @@ gh pr create \ Implements JWT-based authentication... [Full description content] - -Generated with [Claude Code](https://claude.com/claude-code) EOF )" ``` From 722d5c7ad7f0cb4becb3791130769509a413990d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:42:08 +0300 Subject: [PATCH 08/21] =?UTF-8?q?test:=20add=20non-vacuity=20anchors=20to?= =?UTF-8?q?=20uninstall=20shape-guard=20and=20on=E2=86=92off=20e2e=20cases?= =?UTF-8?q?=20(refs=20#315)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit testing-03: seed ENABLE_TOOL_SEARCH (env-target FLAG_REGISTRY key) into three shape-guard fixtures that were vacuous — any early-exit in runCleanupPhase's settings loop would leave the key intact; the new assertion (env flag absent after cleanup) fails RED in that case, making attribution survival and loop execution independently observable (per PF-018). testing-11: add the CUSTOM_USER_VAR survival assertion to the on→off (--reset) e2e case, mirroring the sibling off→on case; without it a deletion assertion passes silently when the file is never rewritten. --- tests/init-e2e-flags.test.ts | 4 ++++ tests/uninstall-logic.test.ts | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/init-e2e-flags.test.ts b/tests/init-e2e-flags.test.ts index b0da41ed..9ea0daf2 100644 --- a/tests/init-e2e-flags.test.ts +++ b/tests/init-e2e-flags.test.ts @@ -565,6 +565,10 @@ describe('init e2e — suppress-attribution convergence (D27, production path)', // Off-side convergence: manifest says off, the key must be gone from settings. expect(out.flags['suppress-attribution']).toBe(false); expect(out.settings).not.toHaveProperty('attribution'); + // Non-vacuity anchor (PF-018): a deletion assertion passes silently when the file + // is never rewritten. The seeded CUSTOM_USER_VAR must survive the init pass, + // proving settings.json was actually rewritten around the deletion. + expect((out.settings.env as Record).CUSTOM_USER_VAR).toBe('preserved'); }, SUBPROCESS_TIMEOUT_MS); it.skipIf(!CLI_BUILT)('a user-customised attribution survives init untouched (shape guard)', async () => { diff --git a/tests/uninstall-logic.test.ts b/tests/uninstall-logic.test.ts index 1a2777bd..3de83542 100644 --- a/tests/uninstall-logic.test.ts +++ b/tests/uninstall-logic.test.ts @@ -1622,6 +1622,7 @@ describe('R5: uninstall settings cleanup — attribution shape guard (D27, produ await fs.writeFile(settingsPath, JSON.stringify({ attribution: { commit: 'Acme Corp', pr: 'Acme' }, model: 'opus', + env: { ENABLE_TOOL_SEARCH: '1' }, }, null, 2), 'utf-8'); await runCleanup(); @@ -1629,28 +1630,43 @@ describe('R5: uninstall settings cleanup — attribution shape guard (D27, produ const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); expect(after.attribution).toEqual({ commit: 'Acme Corp', pr: 'Acme' }); expect(after.model).toBe('opus'); + // Non-vacuity anchor (PF-018): ENABLE_TOOL_SEARCH is a FLAG_REGISTRY env-target that + // stripFlags must remove. If the settings loop never ran, the key would survive and + // this assertion would fail, catching the vacuous-pass case. + expect(after.env?.ENABLE_TOOL_SEARCH, 'managed env flag stripped → settings loop ran').toBeUndefined(); }); it('preserves an attribution object carrying extra keys (not the managed shape)', async () => { await fs.writeFile(settingsPath, JSON.stringify({ attribution: { commit: '', pr: '', coAuthor: 'nobody' }, model: 'opus', + env: { ENABLE_TOOL_SEARCH: '1' }, }, null, 2), 'utf-8'); await runCleanup(); const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); expect(after.attribution).toEqual({ commit: '', pr: '', coAuthor: 'nobody' }); + // Non-vacuity anchor (PF-018): the managed env flag must be stripped, proving + // the settings loop ran; an early-exit would leave ENABLE_TOOL_SEARCH intact. + expect(after.env?.ENABLE_TOOL_SEARCH, 'managed env flag stripped → settings loop ran').toBeUndefined(); }); it('leaves settings.json without an attribution key untouched', async () => { - await fs.writeFile(settingsPath, JSON.stringify({ model: 'opus' }, null, 2), 'utf-8'); + await fs.writeFile(settingsPath, JSON.stringify({ + model: 'opus', + env: { ENABLE_TOOL_SEARCH: '1' }, + }, null, 2), 'utf-8'); await runCleanup(); const after = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); expect(after).not.toHaveProperty('attribution'); expect(after.model).toBe('opus'); + // Non-vacuity anchor (PF-018): without this, asserting 'attribution' absent is + // unconditionally vacuous — the fixture never had it. The managed env flag was + // seeded and must be stripped; an early-exit would leave ENABLE_TOOL_SEARCH '1'. + expect(after.env?.ENABLE_TOOL_SEARCH, 'managed env flag stripped → settings loop ran').toBeUndefined(); }); }); From 9c89479d5b29ed49fabe40377eba1113ab5e22ab Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:43:16 +0300 Subject: [PATCH 09/21] test(post-install): pin flags-owned keys out of the settings template and drop the vacuous merge case (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit consistency-04: replace tombstone comment in mergeDevflowSettingsTemplate with a single end-state invariant line (applies ADR-003). testing-04: delete the unfalsifiable attribution test (template carried no attribution key, so the assertion could never fail — avoids PF-018). The sibling test at 'even when template carries an attribution block' strictly dominates it and is kept. reliability-05: replace the deleted slot with a registry-driven static guard that reads templates/settings.json from disk, enumerates all FLAG_REGISTRY entries with target.type === 'setting', and asserts none of their target.key values appears as a top-level template key. Non-vacuity anchors confirm the template is non-empty and at least one setting-target flag exists. Per-key failure messages name the offending flag ID. Guard applies ADR-024 (one writer per settings.json key class) and D27 (attribution owned by flags pipeline). --- src/targets/claude-code/post-install.ts | 5 +-- tests/post-install-merge.test.ts | 42 +++++++++++++++++++------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/targets/claude-code/post-install.ts b/src/targets/claude-code/post-install.ts index a01d9ad2..1631cdbd 100644 --- a/src/targets/claude-code/post-install.ts +++ b/src/targets/claude-code/post-install.ts @@ -865,10 +865,7 @@ export function mergeDevflowSettingsTemplate( changed = true; } - // Attribution is NOT injected here. It is managed exclusively by the flags - // pipeline (suppress-attribution flag, D27). Removing it from merge prevents a - // double-write: applyFlags writes it when the flag is on; stripFlags removes - // it when the flag is off (shape-guarded, D-ATTR-GUARD). + // attribution: owned exclusively by the flags pipeline (applyFlags/stripFlags, D27) — a second writer here would race it. return { changed }; } diff --git a/tests/post-install-merge.test.ts b/tests/post-install-merge.test.ts index 208898b4..ab6a3ef8 100644 --- a/tests/post-install-merge.test.ts +++ b/tests/post-install-merge.test.ts @@ -21,6 +21,8 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mergeDevflowSettingsTemplate, installSettings } from '../src/targets/claude-code/post-install.js'; import type { HookMatcher } from '../src/targets/claude-code/hooks.js'; +import { FLAG_REGISTRY } from '../src/core/flags.js'; +import type { SettingFlagTarget } from '../src/core/flags.js'; import * as os from 'node:os'; import * as fsp from 'node:fs/promises'; import * as path from 'node:path'; @@ -141,14 +143,36 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { expect(existing.statusLine).toBe('my-custom-status-line'); }); - it('does NOT inject attribution — attribution is managed by flags pipeline (D27)', () => { - // mergeDevflowSettingsTemplate never writes an attribution key regardless of - // what the template contains. Attribution is written/removed by applyFlags/ - // stripFlags (suppress-attribution flag, D-ATTR-GUARD shape guard). - const existing: Record = {}; - const template = { statusLine: 's' }; // template has no attribution key (R2) - mergeDevflowSettingsTemplate(existing, template); - expect(existing.attribution).toBeUndefined(); + /** + * Registry-driven single-ownership guard: no flag with target.type === 'setting' + * may have its target.key present as a top-level key in the settings merge template. + * ADR-024: one writer per settings.json key class. D27: attribution (and all + * other flag-owned keys) are written/removed exclusively by applyFlags/stripFlags. + * A template key managed by a flag creates a double-write on every fresh install. + */ + it('no flag-owned settings key appears as a top-level template key (ADR-024, D27)', async () => { + const templatePath = path.join(REPO_ROOT, 'src/targets/claude-code/templates/settings.json'); + const template = JSON.parse(await fsp.readFile(templatePath, 'utf-8')) as Record; + + // Non-vacuity: template must be a non-empty plain object + expect(Object.keys(template).length).toBeGreaterThan(0); + + // Collect setting-target flags from the registry + const settingFlags = FLAG_REGISTRY.filter( + (f): f is typeof f & { target: SettingFlagTarget } => f.target.type === 'setting', + ); + + // Non-vacuity: at least one setting-target flag must exist in the registry + expect(settingFlags.length).toBeGreaterThan(0); + + // Assert no flag-owned key appears in the template top-level + for (const flag of settingFlags) { + const key = flag.target.key; + expect( + key in template, + `"${key}" (flag: ${flag.id}) is flag-owned — must not appear in the settings merge template (ADR-024, D27)`, + ).toBe(false); + } }); it('does NOT inject attribution even when template carries an attribution block', () => { @@ -221,7 +245,7 @@ describe('mergeDevflowSettingsTemplate — FIX 1 (issue #313)', () => { it('is idempotent — double merge produces same result as single merge', () => { const cmd = '/devflow/scripts/hooks/run-hook memory-worker'; const existing: Record = {}; - const template = makeTemplate([cmd], 'devflow v2', 'Devflow'); + const template = makeTemplate([cmd], 'devflow v2'); mergeDevflowSettingsTemplate(existing, template); const snapshotAfterFirst = JSON.stringify(existing); From d5420747ed58049611187fa406c51116c6d0e108 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:44:01 +0300 Subject: [PATCH 10/21] refactor(flags): single-source the managed-shape guard (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit complexity-03: Replace hand-rolled deepEqualsPlain with node:util.isDeepStrictEqual (node >=22 is pinned; zero-maintenance, correct for all JSON shapes). complexity-04: Extract canDeleteSettingKey(flag, settings) beside asPlainObject — both applyFlags and stripFlags collapse to a single conditional. D-ATTR-GUARD JSDoc moves to the helper, upholding ADR-024 mechanism 3 by construction. consistency-01 / typescript-06: Export settingValueHoldsManagedShape (value-level core) and settingHoldsManagedShape (string-level wrapper) from flags.ts — the ONE place that decides "on-disk value equals the managed shape". Reduce resolveExistingAttributionSuppression in init-seed.ts to a one-liner over this helper; narrow its return type to true | undefined (asymmetry is load-bearing for seeding priority — undefined lets manifest/default win; say so in JSDoc). typescript-01: Remove (flags['suppress-attribution'] as boolean) ?? false cast in resolveInitSeed; replace with === true to safely absorb null/undefined/non-boolean. Applies ADR-003 (leave the end-state; no tombstone comments). --- src/cli/commands/init-seed.ts | 38 +++++----- src/core/flags.ts | 128 +++++++++++++++++++++++++--------- 2 files changed, 112 insertions(+), 54 deletions(-) diff --git a/src/cli/commands/init-seed.ts b/src/cli/commands/init-seed.ts index aa08e81e..a343cb68 100644 --- a/src/cli/commands/init-seed.ts +++ b/src/cli/commands/init-seed.ts @@ -16,6 +16,7 @@ import { resolveExistingViewMode, + settingHoldsManagedShape, FLAG_REGISTRY, defaultValueOf, readViewMode, @@ -226,30 +227,25 @@ export function resolveSeedPlugins( /** * Extract the attribution suppression state from a settings JSON string. * - * Returns `true` when the exact devflow-managed attribution shape - * `{"commit":"","pr":""}` is present, signaling that suppress-attribution was - * active at the last install. Returns `undefined` when the block is absent or - * holds a different (user-custom) value — callers fall through to the manifest - * FlagsRecord entry or the registry default. + * Returns `true` when the exact devflow-managed attribution shape is present — + * delegating to `settingHoldsManagedShape('suppress-attribution')`, which is the + * single place that decides "on-disk value equals the managed shape" (consistency-01). + * Returns `undefined` when the block is absent, holds a custom value, or JSON is + * malformed — callers fall through to the manifest FlagsRecord entry or the registry + * default. * - * Mirrors resolveExistingViewMode: returns undefined on malformed JSON, absent - * key, or any non-devflow attribution value. + * Return type is `true | undefined` (typescript-06): settings.json can signal ON or + * nothing — never explicitly OFF. That asymmetry is load-bearing for seeding priority: + * `undefined` lets the manifest entry or the registry default win, while `true` is an + * unambiguous "this key was devflow-managed and active". + * + * Mirrors resolveExistingViewMode: returns undefined on malformed JSON, absent key, + * or any non-devflow attribution value. * * Pure function — no I/O, no side effects. */ -export function resolveExistingAttributionSuppression(settingsJson: string): boolean | undefined { - try { - const parsed: unknown = JSON.parse(settingsJson); - if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return undefined; - const attr = (parsed as Record).attribution; - if (attr === null || typeof attr !== 'object' || Array.isArray(attr)) return undefined; - const a = attr as Record; - // Only match the exact devflow-managed shape: {"commit":"","pr":""} with no extra keys. - if (a['commit'] === '' && a['pr'] === '' && Object.keys(a).length === 2) return true; - return undefined; // custom value — fall through to manifest / default - } catch { - return undefined; - } +export function resolveExistingAttributionSuppression(settingsJson: string): true | undefined { + return settingHoldsManagedShape(settingsJson, 'suppress-attribution') ? true : undefined; } /** @@ -303,7 +299,7 @@ export function resolveInitSeed( const existingAttr = resolveExistingAttributionSuppression(settingsSnapshot); const resolvedSuppressAttr: boolean = existingAttr !== undefined ? existingAttr // settings.json exact shape wins - : ((flags['suppress-attribution'] as boolean) ?? false); // manifest or registry default + : flags['suppress-attribution'] === true; // manifest or registry default // Return a fresh spread rather than mutating flags in place — keeps this function pure // per the module docblock and avoids aliasing if the caller inspects seed.flags. diff --git a/src/core/flags.ts b/src/core/flags.ts index 458941e2..1a17a900 100644 --- a/src/core/flags.ts +++ b/src/core/flags.ts @@ -12,6 +12,8 @@ * sole API; init.ts works directly with FlagsRecord (no legacy string[] bridge). */ +import { isDeepStrictEqual } from 'node:util'; + // ─── Types ──────────────────────────────────────────────────────────────────── /** Discriminant for the FlagDef union — determines which per-kind fields are present. */ @@ -1018,29 +1020,21 @@ export function migrateLegacyFlagsToRecord( // ─── Apply / Strip ──────────────────────────────────────────────────────────── /** - * Structural deep-equality limited to plain JSON values (objects, arrays, primitives). - * Returns false for non-JSON types (functions, class instances, undefined). + * D-ATTR-GUARD: returns true when it is safe to delete a flag's target key from + * settings. For flags with a settingDeleteGuard the key is only deleted when the + * current on-disk value deep-equals the guarded shape exactly, preventing erasure + * of user-customized values (e.g. an organisation attribution block) on flag + * disable or uninstall. Flags without a guard are always safe to delete. + * + * Uses `isDeepStrictEqual` from node:util — native, correct, and zero-maintenance. * - * D-ATTR-GUARD: used by the settingDeleteGuard check in applyFlags and stripFlags to - * prevent erasing user-customized settings when a managed flag transitions to neutral. + * Single-source invariant (ADR-024 mechanism 3): the guard is symmetric across + * the disable path (applyFlags neutral branch) and the uninstall path (stripFlags), + * upheld by construction since both call sites collapse to this one predicate. */ -function deepEqualsPlain(a: unknown, b: unknown): boolean { - if (a === b) return true; - if (a === null || b === null) return a === b; - if (typeof a !== 'object' || typeof b !== 'object') return false; - if (Array.isArray(a) !== Array.isArray(b)) return false; - if (Array.isArray(a)) { - const aa = a as unknown[]; - const ba = b as unknown[]; - if (aa.length !== ba.length) return false; - return aa.every((v, i) => deepEqualsPlain(v, ba[i])); - } - const ao = a as Record; - const bo = b as Record; - const aKeys = Object.keys(ao); - const bKeys = Object.keys(bo); - if (aKeys.length !== bKeys.length) return false; - return aKeys.every(k => Object.prototype.hasOwnProperty.call(bo, k) && deepEqualsPlain(ao[k], bo[k])); +function canDeleteSettingKey(flag: ClaudeCodeFlag, settings: Record): boolean { + const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; + return guard === undefined || isDeepStrictEqual(settings[flag.target.key], guard); } /** @@ -1112,12 +1106,7 @@ export function applyFlags(settingsJson: string, flags: FlagsRecord): string { const env = asPlainObject(settings.env); if (env) delete env[flag.target.key]; } else { - // D-ATTR-GUARD: for flags with settingDeleteGuard, only delete when the current - // value deep-equals the guarded shape. Prevents erasing user-customized values. - const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; - if (guard === undefined || deepEqualsPlain(settings[flag.target.key], guard)) { - delete settings[flag.target.key]; - } + if (canDeleteSettingKey(flag, settings)) delete settings[flag.target.key]; } } else { const payload = buildPayload(flag, safe as FlagValue); @@ -1162,12 +1151,7 @@ export function stripFlags(settingsJson: string): string { if (flag.target.type === 'env') { if (env) delete env[flag.target.key]; } else { - // D-ATTR-GUARD: for flags with settingDeleteGuard, only delete when the current - // value deep-equals the guarded shape. Preserves user-customized values on uninstall. - const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; - if (guard === undefined || deepEqualsPlain(settings[flag.target.key], guard)) { - delete settings[flag.target.key]; - } + if (canDeleteSettingKey(flag, settings)) delete settings[flag.target.key]; } } @@ -1220,6 +1204,52 @@ export function resolveExistingViewMode(settingsJson: string): ViewMode | undefi return undefined; } +/** + * Returns true when `value` equals the managed shape declared by `flag.settingDeleteGuard`. + * Returns false when: the flag has no guard, is not a setting-target boolean, or the value + * does not deep-equal the guard. + * + * Value-level core for `settingHoldsManagedShape` — both share the single equality decision. + */ +export function settingValueHoldsManagedShape(flag: ClaudeCodeFlag, value: unknown): boolean { + if (flag.kind !== 'boolean' || flag.target.type !== 'setting') return false; + const guard = flag.settingDeleteGuard; + if (guard === undefined) return false; + return isDeepStrictEqual(value, guard); +} + +/** + * D-ATTR-GUARD single-source predicate (consistency-01 / ADR-024). + * + * Returns true when the settings.json string contains a value at the flag's + * target key that equals the flag's managed shape (`settingDeleteGuard`). + * + * This is the ONE place that decides "on-disk value equals the managed shape". + * All consumers — `resolveExistingAttributionSuppression` (seeding priority), + * and the guarded-boolean adoption fold in `convergeFlagsIntoSettings` — delegate + * here rather than hand-rolling their own comparison. + * + * Returns false when: + * - settingsJson is malformed + * - root is not a plain object + * - flagId is not in the registry + * - the flag has no settingDeleteGuard + * - the flag is not a setting-target boolean + * - the on-disk value does not deep-equal the guard + */ +export function settingHoldsManagedShape(settingsJson: string, flagId: string): boolean { + try { + const parsed: unknown = JSON.parse(settingsJson); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + const flag = FLAG_REGISTRY_MAP.get(flagId); + if (!flag) return false; + const value = (parsed as Record)[flag.target.key]; + return settingValueHoldsManagedShape(flag, value); + } catch { + return false; + } +} + /** * Resolve the final view mode to write, combining an existing settings value, * an init-prompt-selected value, and whether the selection was explicit. @@ -1366,6 +1396,38 @@ export function convergeFlagsIntoSettings( } } + // ── Step 2b: adopt guarded boolean settings before the strip ───────────── + // D-ATTR-ADOPT (PF-050 / ADR-024): a delete guard is evidence about the VALUE, + // never about the record — a pre-existing on-disk key whose value matches the + // managed shape means devflow wrote it, so adopt it into the record now, before + // stripFlags can unconditionally remove it on the next line. + // + // Mirror of the view-mode fold (Step 1): adopt only when: + // (a) the flag has a settingDeleteGuard (guarded boolean only) + // (b) the record does NOT already claim the key — a claimed false or null still + // deletes the managed block (the user intentionally disabled it) + // (c) the pre-strip on-disk value holds the managed shape + // + // This fold runs before stripFlags so applyFlags rewrites the block from the + // now-claimed record and the block survives on BOTH the init and flags paths. + for (const flag of FLAG_REGISTRY) { + if (flag.kind !== 'boolean') continue; + if (flag.target.type !== 'setting') continue; + const boolFlag = flag as SettingBooleanFlagDef; + if (boolFlag.settingDeleteGuard === undefined) continue; + + // Skip when the record already claims this flag (claimed false/null deletes the block) + const claimed = + claimedIn !== null && + Object.prototype.hasOwnProperty.call(claimedIn, flag.id); + if (claimed) continue; + + // Adopt only when the on-disk value matches the managed shape exactly + if (settingValueHoldsManagedShape(flag, parsed[flag.target.key])) { + folded[flag.id] = true; + } + } + // ── Step 3: strip all managed keys, then apply the folded record ────────── const stripped = stripFlags(settingsJson); const settings = applyFlags(stripped, folded); From 20f33e100fdd97d59543215fada826dc861a82a6 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:44:18 +0300 Subject: [PATCH 11/21] fix(flags): adopt guarded boolean settings before the strip sweep (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit security-01 / PF-050 / ADR-024: convergeFlagsIntoSettings Step 2b now folds guarded boolean flags that (a) have a settingDeleteGuard, (b) are NOT claimed in the record, and (c) whose pre-strip on-disk value holds the managed shape — setting folded[flag.id] = true. This runs BEFORE stripFlags so applyFlags rewrites the block and it survives on both the init and devflow-flags paths. Root cause: stripFlags iterates FLAG_REGISTRY unconditionally; the exact-shape guard confirmed deletion rather than preventing it; Step 2 skipped booleans entirely; and the flags CLI built newRecord as a bare manifest spread with no backfill — so the first write of ANY flag deleted the pre-existing attribution block unrecoverably while --status reported "off". Mirrors the view-mode fold at Step 1. A claimed false/null still deletes the block (no fold when the record already owns the key). Unguarded booleans are never folded. Records the fix as D-ATTR-ADOPT JSDoc at the fold site. Regression tests in tests/flags.test.ts (RED first by reasoning — the assertions were unsatisfiable pre-fix: Step 2 skipped booleans and stripFlags removed the block, so record['suppress-attribution'] would be undefined and attribution would be absent): - adopts managed block: record gains suppress-attribution:true and block survives - does NOT adopt a custom attribution (shape guard rejects it) - does NOT adopt when the record already claims suppress-attribution:false Registry tests for settingHoldsManagedShape: true for exact shape, false for {commit:'Acme',pr:'Acme'}, {commit:'',pr:'',coAuthor:'x'}, null, [], missing key, malformed JSON, and unknown flag id. Applies ADR-024 (PROVE-YOU-WROTE-IT), PF-050 (adoption fold before strip). --- tests/flags.test.ts | 98 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/tests/flags.test.ts b/tests/flags.test.ts index b0b9ba17..42c0672a 100644 --- a/tests/flags.test.ts +++ b/tests/flags.test.ts @@ -19,6 +19,7 @@ import { applyFlags, stripFlags, convergeFlagsIntoSettings, + settingHoldsManagedShape, // Kept verbatim VIEW_MODES, resolveExistingViewMode, @@ -1905,3 +1906,100 @@ describe('suppress-attribution flag — shape guard (D27)', () => { expect(flag.blurb.length).toBeLessThanOrEqual(30); }); }); + +// ─── settingHoldsManagedShape — consistency-01 single-source predicate ──────── + +describe('settingHoldsManagedShape', () => { + const MANAGED = { commit: '', pr: '' }; + const makeSettings = (attr: unknown): string => + JSON.stringify({ attribution: attr }); + + it('returns true for the exact managed shape', () => { + expect(settingHoldsManagedShape(makeSettings(MANAGED), 'suppress-attribution')).toBe(true); + }); + + it('returns false for a custom attribution object', () => { + expect(settingHoldsManagedShape( + makeSettings({ commit: 'Acme', pr: 'Acme' }), 'suppress-attribution' + )).toBe(false); + }); + + it('returns false for attribution with an extra key', () => { + expect(settingHoldsManagedShape( + makeSettings({ commit: '', pr: '', coAuthor: 'x' }), 'suppress-attribution' + )).toBe(false); + }); + + it('returns false for null attribution', () => { + expect(settingHoldsManagedShape(makeSettings(null), 'suppress-attribution')).toBe(false); + }); + + it('returns false for array attribution', () => { + expect(settingHoldsManagedShape(makeSettings([]), 'suppress-attribution')).toBe(false); + }); + + it('returns false when the key is absent', () => { + expect(settingHoldsManagedShape(JSON.stringify({}), 'suppress-attribution')).toBe(false); + }); + + it('returns false for malformed JSON', () => { + expect(settingHoldsManagedShape('not json', 'suppress-attribution')).toBe(false); + }); + + it('returns false for an unknown flag id', () => { + expect(settingHoldsManagedShape(makeSettings(MANAGED), 'no-such-flag')).toBe(false); + }); +}); + +// ─── convergeFlagsIntoSettings — D-ATTR-ADOPT: guarded boolean adoption ────── +// +// Regression tests for security-01 (PF-050 / ADR-024): +// A pre-existing on-disk managed shape must be adopted into the record BEFORE +// the strip sweep runs, so the block survives on both the init and flags paths. +// RED state was confirmed by reasoning: the assertions are unsatisfiable +// against the pre-fix code because Step 2 skips booleans unconditionally and +// stripFlags deletes the block, meaning result.record['suppress-attribution'] +// would be undefined and the attribution key would be absent from the output. + +describe('convergeFlagsIntoSettings — D-ATTR-ADOPT: guarded boolean adoption', () => { + const MANAGED = { commit: '', pr: '' }; + const SIBLING_KEY = 'unrelatedSetting'; + const SIBLING_VAL = 'preserved'; + + // Baseline: pre-D27 state — no suppress-attribution in record or ownedRecord, + // but the legacy attribution block is on disk plus an unrelated sibling key. + const makePreD27Settings = (): string => + JSON.stringify({ attribution: MANAGED, [SIBLING_KEY]: SIBLING_VAL }); + + it('adopts the managed block: record gains suppress-attribution:true and block survives', () => { + const record: FlagsRecord = {}; + const { settings, record: out } = convergeFlagsIntoSettings( + makePreD27Settings(), record, { viewModeExplicit: false, ownedRecord: null }, + ); + const parsed = JSON.parse(settings) as Record; + expect(parsed['attribution'], 'attribution block must survive').toEqual(MANAGED); + expect(out['suppress-attribution'], 'record must claim suppress-attribution:true').toBe(true); + expect(parsed[SIBLING_KEY], 'unmanaged sibling key must survive').toBe(SIBLING_VAL); + }); + + it('does NOT adopt a custom attribution (shape guard rejects it)', () => { + const custom = { commit: 'Acme', pr: 'Acme' }; + const settings = JSON.stringify({ attribution: custom, [SIBLING_KEY]: SIBLING_VAL }); + const record: FlagsRecord = {}; + const { record: out } = convergeFlagsIntoSettings( + settings, record, { viewModeExplicit: false, ownedRecord: null }, + ); + // Custom shape must not trigger adoption + expect(out['suppress-attribution']).toBeUndefined(); + }); + + it('does NOT adopt when the record already claims suppress-attribution:false', () => { + // A claimed false means the user deliberately disabled it — the managed block must be deleted. + const record: FlagsRecord = { 'suppress-attribution': false }; + const { settings } = convergeFlagsIntoSettings( + makePreD27Settings(), record, { viewModeExplicit: false, ownedRecord: record }, + ); + const parsed = JSON.parse(settings) as Record; + expect(parsed['attribution'], 'claimed false still deletes the managed block').toBeUndefined(); + }); +}); From a813f41c2b53e798d7fc68581d622c70a291c1fc Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:45:25 +0300 Subject: [PATCH 12/21] refactor(cli): share the wizard prompt-IO seam across attribution and compliance steps (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract PromptOutcome, WizardPromptIO, clackNote, and clackSelect into a new prompt-io.ts module. Both attribution-prompts.ts and compliance-prompts.ts were carrying byte-identical PromptOutcome and note/select seam declarations (complexity-01 / architecture-03 / consistency-06). CompliancePromptIO now extends WizardPromptIO; AttributionPromptIO becomes a WizardPromptIO alias kept for backward compatibility. The shared clackSelect is generic over the option value type with no `as T` cast on the result path (typescript-05): p.isCancel narrows symbol | T → T without a cast; the previous `as boolean` was a no-op that would have masked a future widening. The input-side options cast bridges an unresolved conditional type (Option) without losing value-type information. Existing tests and init.ts imports are source-compatible: PromptOutcome is re-exported from compliance-prompts.ts; AttributionPromptIO is a named export from attribution-prompts.ts. --- src/cli/commands/attribution-prompts.ts | 55 +++++++++----------- src/cli/commands/compliance-prompts.ts | 38 ++++++-------- src/cli/commands/prompt-io.ts | 69 +++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 52 deletions(-) create mode 100644 src/cli/commands/prompt-io.ts diff --git a/src/cli/commands/attribution-prompts.ts b/src/cli/commands/attribution-prompts.ts index 62b8afc5..c95b624f 100644 --- a/src/cli/commands/attribution-prompts.ts +++ b/src/cli/commands/attribution-prompts.ts @@ -12,9 +12,12 @@ * * D27: suppress-attribution flag — gates Claude Code's AI-attribution injection. * The question is ADVANCED-ONLY; Recommended never asks. See shouldRunAttributionStep. + * + * Shared DI seam (PromptOutcome, WizardPromptIO, clackNote, clackSelect) lives in + * prompt-io.ts — one definition, both wizard modules import from there (ADR-019). */ -import * as p from '@clack/prompts'; +import { clackNote, clackSelect, type PromptOutcome, type WizardPromptIO } from './prompt-io.js'; // ── Gate predicate ───────────────────────────────────────────────────────────── @@ -54,40 +57,25 @@ export function shouldRunAttributionStep(input: { // ── DI seam ──────────────────────────────────────────────────────────────────── -/** Discriminated union returned by every AttributionPromptIO method. */ -export type PromptOutcome = { kind: 'value'; value: T } | { kind: 'cancel' }; +// Re-export so tests and init.ts continue to compile against these names. +export type { PromptOutcome }; /** * Injectable prompt interface for runAttributionStep. - * Mirrors CompliancePromptIO (src/cli/commands/compliance-prompts.ts). - * Enables unit tests to drive all branches without a real TTY. + * Alias of WizardPromptIO — attribution uses only note + boolean select. + * Kept as a named export for backward compatibility with tests and callers + * that import `AttributionPromptIO` by name. */ -export interface AttributionPromptIO { - note: (message: string, title: string) => void; - select: (opts: { - message: string; - options: Array<{ value: boolean; label: string; hint: string }>; - initialValue: boolean; - }) => Promise>; -} +export type AttributionPromptIO = WizardPromptIO; /** * Build the real (clack) AttributionPromptIO adapter. - * Translates clack's cancel symbol into the PromptOutcome discriminated union. + * Delegates to the shared clackNote / clackSelect adapters (prompt-io.ts). */ export function buildClackAttributionPrompts(): AttributionPromptIO { return { - note: (message, title) => p.note(message, title), - - select: async (opts) => { - const result = await p.select({ - message: opts.message, - options: opts.options, - initialValue: opts.initialValue, - }); - if (p.isCancel(result)) return { kind: 'cancel' }; - return { kind: 'value', value: result as boolean }; - }, + note: clackNote, + select: (opts) => clackSelect(opts), }; } @@ -138,13 +126,20 @@ export async function runAttributionStep(opts: { const { seed, prompts } = opts; const currentStr = seed ? 'suppressed' : 'shown (default)'; + // security-02: name the destructive branch (Yes) BEFORE the user consents. + // ADR-024 corollary (b): turning the flag ON replaces any existing attribution + // value, including a custom one — this is deliberate. Only the exact + // devflow-managed shape {"commit":"","pr":""} is removed on disable. + // security-04: surface the org AI-disclosure-policy dimension as a note (not a gate). prompts.note( `Current setting: ${currentStr}\n\n` + - 'When enabled, writes {"commit":"","pr":""} to settings.json, which\n' + - 'suppresses AI-attribution labels in git commits and pull requests.\n' + - 'Toggle any time with: devflow flags --enable suppress-attribution\n\n' + - 'Note: Only removes the devflow-managed attribution block on disable;\n' + - 'custom attribution values you set manually are never deleted.', + 'Choosing Yes REPLACES any existing \`attribution\` value in settings.json,\n' + + 'including a custom one, with {"commit":"","pr":""}.\n' + + 'Choosing No leaves a custom value untouched — only the exact\n' + + 'devflow-managed block is ever removed on disable.\n' + + 'Some organisations require machine-readable AI-authorship disclosure\n' + + '— check your policy before enabling.\n\n' + + 'Toggle any time with: devflow flags --enable suppress-attribution', 'AI Attribution', ); diff --git a/src/cli/commands/compliance-prompts.ts b/src/cli/commands/compliance-prompts.ts index 32653fea..ca0200c1 100644 --- a/src/cli/commands/compliance-prompts.ts +++ b/src/cli/commands/compliance-prompts.ts @@ -9,10 +9,18 @@ * no prompt) and the non-TTY fallback preserve their promptless contracts. * Applies PF-014: runComplianceStep never calls process.exit() or throws — callers * own the cancel idiom (p.cancel + process.exit(0)), keeping try/finally cleanup safe. + * + * Shared DI seam (PromptOutcome, WizardPromptIO, clackNote, clackSelect) lives in + * prompt-io.ts — one definition, both wizard modules import from there (ADR-019). */ import * as p from '@clack/prompts'; import { COMPLIANCE_FRAMEWORKS, type ComplianceFeatureState } from '../../core/compliance.js'; +import { clackNote, clackSelect, type WizardPromptIO } from './prompt-io.js'; + +// Re-export PromptOutcome so callers that import it from this module continue +// to compile (e.g. tests/compliance-prompts.test.ts). +export type { PromptOutcome } from './prompt-io.js'; // ── Shared prompt content ────────────────────────────────────────────────────── @@ -85,46 +93,32 @@ export function shouldRunComplianceStep(input: { // ── DI seam ──────────────────────────────────────────────────────────────────── -/** Discriminated union returned by every CompliancePromptIO method. */ -export type PromptOutcome = { kind: 'value'; value: T } | { kind: 'cancel' }; - /** * Injectable prompt interface for runComplianceStep. + * Extends WizardPromptIO (from prompt-io.ts) with the compliance-specific + * multiselect prompt. The shared note + select seam is inherited. * Mirrors the ProxyPreflightDeps seam (src/cli/commands/proxy.ts:346). * Enables unit tests to drive all branches without a real TTY. */ -export interface CompliancePromptIO { - note: (message: string, title: string) => void; - select: (opts: { - message: string; - options: Array<{ value: boolean; label: string; hint: string }>; - initialValue: boolean; - }) => Promise>; +export interface CompliancePromptIO extends WizardPromptIO { multiselect: (opts: { message: string; options: Array<{ value: string; label: string; hint: string }>; initialValues: string[]; required: boolean; - }) => Promise>; + }) => Promise>; } /** * Build the real (clack) CompliancePromptIO adapter. + * Delegates the shared note + select to the shared adapters (prompt-io.ts). * Translates clack's cancel symbol into the PromptOutcome discriminated union. */ export function buildClackCompliancePrompts(): CompliancePromptIO { return { - note: (message, title) => p.note(message, title), + note: clackNote, - select: async (opts) => { - const result = await p.select({ - message: opts.message, - options: opts.options, - initialValue: opts.initialValue, - }); - if (p.isCancel(result)) return { kind: 'cancel' }; - return { kind: 'value', value: result as boolean }; - }, + select: (opts) => clackSelect(opts), multiselect: async (opts) => { const result = await p.multiselect({ @@ -134,7 +128,7 @@ export function buildClackCompliancePrompts(): CompliancePromptIO { required: opts.required, }); if (p.isCancel(result)) return { kind: 'cancel' }; - return { kind: 'value', value: result as string[] }; + return { kind: 'value', value: result }; }, }; } diff --git a/src/cli/commands/prompt-io.ts b/src/cli/commands/prompt-io.ts new file mode 100644 index 00000000..b8bb776c --- /dev/null +++ b/src/cli/commands/prompt-io.ts @@ -0,0 +1,69 @@ +/** + * Shared wizard prompt-IO seam for devflow init wizard steps. + * + * ADR-019 corollary (one-definition seam): PromptOutcome and WizardPromptIO + * were byte-identical duplicates across attribution-prompts.ts and + * compliance-prompts.ts (architecture-03 / consistency-06). They are defined + * ONCE here and re-used via import. + * + * D-PROMPT-IO: WizardPromptIO is the base DI seam for all two-action wizard + * steps (note + boolean select). Modules that add a third prompt extend this + * interface with an intersection type (e.g. CompliancePromptIO). + */ + +import * as p from '@clack/prompts'; +import type { SelectOptions } from '@clack/prompts'; + +// ── Shared types ───────────────────────────────────────────────────────────── + +/** Discriminated union returned by every WizardPromptIO method. */ +export type PromptOutcome = { kind: 'value'; value: T } | { kind: 'cancel' }; + +/** + * Base injectable prompt interface for two-action wizard steps. + * + * Carries the shared note + boolean-select seam. Steps that add a third + * prompt (e.g. compliance multiselect) extend this interface: + * export interface CompliancePromptIO extends WizardPromptIO { multiselect: … } + * + * Enables unit tests to drive all branches without a real TTY (mirrors the + * ProxyPreflightDeps pattern in src/cli/commands/proxy.ts). + */ +export interface WizardPromptIO { + note: (message: string, title: string) => void; + select: (opts: { + message: string; + options: Array<{ value: boolean; label: string; hint: string }>; + initialValue: boolean; + }) => Promise>; +} + +// ── Shared clack adapters ───────────────────────────────────────────────────── + +/** Real clack adapter for the note prompt. */ +export function clackNote(message: string, title: string): void { + p.note(message, title); +} + +/** + * Real clack adapter for a select prompt, generic over the option value type T. + * + * typescript-05: no `as T` cast on the result path. `p.select` returns + * `Promise`; `p.isCancel` is a `(value: unknown) => value is symbol` + * guard that narrows away the cancel branch, leaving `result: T` without a cast. + * An `as T` here would mask a future widening of the library's return type. + * + * The `as unknown as SelectOptions` on the input is a safe bridge for the + * unresolved conditional type `Option` — our shape satisfies both branches + * (Primitive: label optional; non-Primitive: label required) and is strictly + * narrower, so no value-type information is lost. + */ +export async function clackSelect(opts: { + message: string; + options: Array<{ value: T; label: string; hint: string }>; + initialValue: T; +}): Promise> { + const result = await p.select(opts as unknown as SelectOptions); + if (p.isCancel(result)) return { kind: 'cancel' }; + return { kind: 'value', value: result }; +} From 164fed33641da498169432e77723df29970a8f0e Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:48:15 +0300 Subject: [PATCH 13/21] =?UTF-8?q?test(init-seed):=20pin=20manifest=20null?= =?UTF-8?q?=20=E2=86=92=20false=20for=20suppress-attribution=20(refs=20#31?= =?UTF-8?q?5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/init-seed.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/init-seed.test.ts b/tests/init-seed.test.ts index c312aad0..3869b185 100644 --- a/tests/init-seed.test.ts +++ b/tests/init-seed.test.ts @@ -817,4 +817,16 @@ describe('resolveInitSeed — suppress-attribution seeding (D27)', () => { const seed = resolveInitSeed(seedManifest, seedConfig, seedSettings, DEVFLOW_PLUGINS); expect(seed.flags['suppress-attribution']).toBe(false); }); + + it('manifest has suppress-attribution: null (ADR-014 deliberate unset) → resolves to false', () => { + // ADR-014: null = known + deliberately unset. For this boolean flag, null and false + // both delete the target settings key; the resolved seed must be exactly false. + const manifest = makeManifest({ + features: { ...makeManifest().features, flags: { 'suppress-attribution': null, tui: true } }, + }); + const seed = resolveInitSeed(manifest, null, JSON.stringify({}), DEVFLOW_PLUGINS); + expect(seed.flags['suppress-attribution']).toBe(false); + // Non-vacuity (PF-018): neighbouring flag from prior manifest survives unchanged. + expect(seed.flags['tui']).toBe(true); + }); }); From 7f11ca7d54c408eda80022e2a8fa4822744279ba Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:50:58 +0300 Subject: [PATCH 14/21] fix(flags): clone object payloads, trim the attribution hint, correct the BooleanFlagDef docblock (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - typescript-03: rewrite BooleanFlagDef docblock to accurately describe what tsc enforces (assignability at declaration sites, not consumer narrowing via target.type); add exported isEnvBooleanFlag type predicate that narrows ClaudeCodeFlag to EnvBooleanFlagDef, and use it in buildPayload to remove the unknown hop into the env string-map - security-03: clone object payloads in buildPayload (D-PAYLOAD-CLONE) so the FLAG_REGISTRY entry is never aliased into the caller's settings tree; primitives and env-targeted boolean payloads (strings) need no clone - consistency-03: trim suppress-attribution hint from 92 to 68 chars so it fits the documented ≤ ~76-col budget --- src/core/flags.ts | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/core/flags.ts b/src/core/flags.ts index 1a17a900..62bad9e5 100644 --- a/src/core/flags.ts +++ b/src/core/flags.ts @@ -106,10 +106,30 @@ export interface SettingBooleanFlagDef extends BooleanFlagDefCommon { /** * A boolean on/off flag. `onPayload` is written when the flag is enabled. - * Discriminated on `target.type` so the env-string invariant is enforced by tsc. + * + * What TypeScript enforces: object-literal assignability at registry declaration + * sites. An entry typed as `EnvBooleanFlagDef` must have `onPayload: string` at + * its declaration; one typed as `SettingBooleanFlagDef` may use an object. + * + * What TypeScript does NOT enforce for consumers: narrowing via a nested + * `target.type` check. After `if (f.target.type === 'env')`, `f` is still typed + * as `BooleanFlagDef` and `f.onPayload` remains `string | boolean | + * Record`. Use `isEnvBooleanFlag(f)` to narrow to + * `EnvBooleanFlagDef` where the string type is needed directly. */ export type BooleanFlagDef = EnvBooleanFlagDef | SettingBooleanFlagDef; +/** + * Type predicate that narrows `ClaudeCodeFlag` to `EnvBooleanFlagDef`. + * Narrows `f.onPayload` to `string` so callers writing to the env string-map + * avoid an `unknown` hop without an unsafe cast. + * + * Use in `buildPayload` and `applyFlags` only where the string type is load-bearing. + */ +export function isEnvBooleanFlag(f: ClaudeCodeFlag): f is EnvBooleanFlagDef { + return f.kind === 'boolean' && f.target.type === 'env'; +} + /** An enum flag. `neutralValue` is the value that means "no preference" (key is deleted). */ export interface EnumFlagDef extends FlagDefCommon { readonly kind: 'enum'; @@ -460,7 +480,7 @@ export const FLAG_REGISTRY: readonly ClaudeCodeFlag[] = [ id: 'suppress-attribution', label: 'Suppress AI attribution', description: 'Remove AI-attribution labels from git commits and pull requests', - hint: 'Writes {"commit":"","pr":""} to settings.json — suppresses Claude attribution in git history', + hint: 'Suppresses Claude attribution labels in git commits and pull requests', blurb: 'hide AI attribution labels', kind: 'boolean', target: { type: 'setting', key: 'attribution' }, @@ -1054,8 +1074,16 @@ function asPlainObject(v: unknown): Record | undefined { /** Compute the value to write to settings.json for an active flag. */ function buildPayload(flag: ClaudeCodeFlag, value: FlagValue): unknown { switch (flag.kind) { - case 'boolean': - return flag.onPayload; + case 'boolean': { + // D-PAYLOAD-CLONE: return a structural clone for object payloads so the + // FLAG_REGISTRY entry is never aliased into the caller's settings tree. + // Primitives (string, boolean) are values and require no clone. + // isEnvBooleanFlag narrows onPayload to string for env flags — no clone needed, + // and the string type avoids an `unknown` hop into the env string-map. + if (isEnvBooleanFlag(flag)) return flag.onPayload; // string — no clone needed + const p = flag.onPayload; + return typeof p === 'object' && p !== null ? structuredClone(p) : p; + } case 'enum': return value as string; case 'number': From 25d21fc86ec162d9309a269367f11555c3cbef24 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:51:09 +0300 Subject: [PATCH 15/21] test(flags): guard edge cases, drop the duplicated blurb-cap case, add isEnvBooleanFlag and payload-clone tests (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - testing-06: delete the single-flag 'blurb cap: suppress-attribution blurb is ≤ 30 chars' test — the registry-walk test (FLAG_REGISTRY — blurb hard-cap) already asserts ≤ 30 for every flag with per-flag failure messages - testing-09: add six applyFlags/stripFlags edge-case guard tests covering null, [], and {commit:'',pr:'',coAuthor:'x'} attribution values; each asserts the key survives a neutral/off pass and that an unrelated sibling key (model:'opus') is intact — non-vacuity anchors per PF-018 - typescript-03: add isEnvBooleanFlag predicate tests (true for tool-search, false for suppress-attribution) plus compile-time narrowing verification - security-03: add D-PAYLOAD-CLONE test via applyFlags; parse the returned JSON, mutate attribution.commit, and assert FLAG_REGISTRY onPayload is unchanged - consistency-03: add registry-walk hint ≤ 76-char test for all flags --- tests/flags.test.ts | 121 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 119 insertions(+), 2 deletions(-) diff --git a/tests/flags.test.ts b/tests/flags.test.ts index 42c0672a..eaeb68e2 100644 --- a/tests/flags.test.ts +++ b/tests/flags.test.ts @@ -20,6 +20,7 @@ import { stripFlags, convergeFlagsIntoSettings, settingHoldsManagedShape, + isEnvBooleanFlag, // Kept verbatim VIEW_MODES, resolveExistingViewMode, @@ -31,6 +32,7 @@ import { type EnumFlagDef, type NumberFlagDef, type StringFlagDef, + type EnvBooleanFlagDef, } from '../src/core/flags.js'; import { resolveSeedFlags } from '../src/cli/commands/init-seed.js'; @@ -1753,6 +1755,27 @@ describe('effectiveDisplay — D-EFFDV one-definition seam', () => { }); }); +// ─── hint column-budget registry test (consistency-03) ─────────────────────── + +describe('FLAG_REGISTRY — hint column budget (≤ 76 chars)', () => { + it('every hint is non-empty and ≤ 76 chars', () => { + for (const flag of FLAG_REGISTRY) { + expect( + typeof flag.hint, + `${flag.id}: hint must be a string`, + ).toBe('string'); + expect( + flag.hint.length, + `${flag.id}: hint "${flag.hint}" is ${flag.hint.length} chars (max 76)`, + ).toBeLessThanOrEqual(76); + expect( + flag.hint.length, + `${flag.id}: hint must not be empty`, + ).toBeGreaterThan(0); + } + }); +}); + // ─── blurb hard-cap registry test ──────────────────────────────────────────── describe('FLAG_REGISTRY — blurb hard-cap (D-BLURB)', () => { @@ -1901,10 +1924,104 @@ describe('suppress-attribution flag — shape guard (D27)', () => { } }); - it('blurb cap: suppress-attribution blurb is ≤ 30 chars', () => { + // ── typescript-03: isEnvBooleanFlag predicate ───────────────────────────── + + it('isEnvBooleanFlag is true for env boolean flags (e.g. tool-search)', () => { + const flag = FLAG_REGISTRY.find(f => f.id === 'tool-search')!; + expect(isEnvBooleanFlag(flag)).toBe(true); + // After the predicate returns true, TypeScript narrows to EnvBooleanFlagDef. + // Runtime-verify that the narrowed type's onPayload is a string. + if (isEnvBooleanFlag(flag)) { + const _: string = (flag as EnvBooleanFlagDef).onPayload; // compile-time check + expect(typeof flag.onPayload).toBe('string'); + } + }); + + it('isEnvBooleanFlag is false for setting boolean flags (e.g. suppress-attribution)', () => { const flag = FLAG_REGISTRY.find(f => f.id === 'suppress-attribution')!; - expect(flag.blurb.length).toBeLessThanOrEqual(30); + expect(isEnvBooleanFlag(flag)).toBe(false); }); + + // ── security-03: object payload clone (D-PAYLOAD-CLONE) ────────────────── + // + // applyFlags returns a JSON string, so the JSON.stringify → JSON.parse boundary + // already prevents a caller from mutating the returned value back into the + // FLAG_REGISTRY. The clone guards against future refactors that might expose + // the intermediate settings object or change buildPayload's return surface. + // + // We verify the invariant through the only available exported surface (applyFlags): + // after applying and round-trip-parsing the JSON, mutating the parsed result must + // leave FLAG_REGISTRY's onPayload unchanged. + + it('mutating the parsed applyFlags result does not affect FLAG_REGISTRY onPayload (D-PAYLOAD-CLONE)', () => { + const flag = FLAG_REGISTRY.find(f => f.id === 'suppress-attribution')!; + const originalPayload = flag.kind === 'boolean' ? { ...(flag.onPayload as object) } : null; + + // Apply the flag ON and parse the returned JSON. + const parsed = JSON.parse( + applyFlags(JSON.stringify({}), { 'suppress-attribution': true }), + ) as Record; + + // Mutate the parsed attribution object. + (parsed['attribution'] as Record)['commit'] = 'MUTATED'; + expect((parsed['attribution'] as Record)['commit']).toBe('MUTATED'); + + // The FLAG_REGISTRY entry's onPayload must be unchanged. + if (flag.kind === 'boolean') { + expect(flag.onPayload).toEqual(originalPayload); + expect((flag.onPayload as Record)['commit']).toBe(''); + } + }); + + // ── testing-09: edge-case attribution values that must NOT be deleted ───── + + it('applyFlags neutral pass: null attribution SURVIVES (guard does not match)', () => { + // null is not the managed shape — the guard must not fire. + const unmanaged = { attribution: null, model: 'opus' }; + const input = JSON.stringify(unmanaged); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })) as Record; + expect(result['attribution']).toBeNull(); + expect(result['model']).toBe('opus'); + }); + + it('stripFlags: null attribution SURVIVES (guard does not match)', () => { + const unmanaged = { attribution: null, model: 'opus' }; + const result = JSON.parse(stripFlags(JSON.stringify(unmanaged))) as Record; + expect(result['attribution']).toBeNull(); + expect(result['model']).toBe('opus'); + }); + + it('applyFlags neutral pass: array attribution SURVIVES (guard does not match)', () => { + const unmanaged = { attribution: [], model: 'opus' }; + const input = JSON.stringify(unmanaged); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })) as Record; + expect(result['attribution']).toEqual([]); + expect(result['model']).toBe('opus'); + }); + + it('stripFlags: array attribution SURVIVES (guard does not match)', () => { + const unmanaged = { attribution: [], model: 'opus' }; + const result = JSON.parse(stripFlags(JSON.stringify(unmanaged))) as Record; + expect(result['attribution']).toEqual([]); + expect(result['model']).toBe('opus'); + }); + + it('applyFlags neutral pass: attribution with extra key SURVIVES (guard does not match)', () => { + // {commit:'',pr:'',coAuthor:'x'} has an extra key — not the managed shape. + const unmanaged = { attribution: { commit: '', pr: '', coAuthor: 'x' }, model: 'opus' }; + const input = JSON.stringify(unmanaged); + const result = JSON.parse(applyFlags(input, { 'suppress-attribution': false })) as Record; + expect(result['attribution']).toEqual({ commit: '', pr: '', coAuthor: 'x' }); + expect(result['model']).toBe('opus'); + }); + + it('stripFlags: attribution with extra key SURVIVES (guard does not match)', () => { + const unmanaged = { attribution: { commit: '', pr: '', coAuthor: 'x' }, model: 'opus' }; + const result = JSON.parse(stripFlags(JSON.stringify(unmanaged))) as Record; + expect(result['attribution']).toEqual({ commit: '', pr: '', coAuthor: 'x' }); + expect(result['model']).toBe('opus'); + }); + }); // ─── settingHoldsManagedShape — consistency-01 single-source predicate ──────── From cf3012a1728ae19fa460e9b5521b980515185fd3 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:52:43 +0300 Subject: [PATCH 16/21] fix(init): bind the attribution gate to the resolved init mode (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hardcoded `mode: 'advanced'` literal in the shouldRunAttributionStep call with `mode: useRecommended ? 'recommended' : 'advanced'` so all four documented gate-table rows are reachable and the predicate — not lexical placement — enforces the D27 Advanced-only invariant (applies PF-029). Extract `attributionSeedFrom(flags)` and `applyAttributionAnswer(flags, outcome)` as pure exported helpers in attribution-prompts.ts, and use them at the call site in init.ts. These helpers are the single merge/seed sites for the wizard answer, enabling isolation testing without driving initCommand (PF-018). --- src/cli/commands/attribution-prompts.ts | 35 +++++++++++++++++++++++++ src/cli/commands/init.ts | 19 +++++++++----- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/cli/commands/attribution-prompts.ts b/src/cli/commands/attribution-prompts.ts index c95b624f..f8e9cf2e 100644 --- a/src/cli/commands/attribution-prompts.ts +++ b/src/cli/commands/attribution-prompts.ts @@ -18,6 +18,7 @@ */ import { clackNote, clackSelect, type PromptOutcome, type WizardPromptIO } from './prompt-io.js'; +import type { FlagsRecord } from '../../core/flags.js'; // ── Gate predicate ───────────────────────────────────────────────────────────── @@ -101,6 +102,40 @@ export interface AttributionStepCancelled { export type AttributionStepOutcome = AttributionStepResolved | AttributionStepCancelled; +// ── Seed / apply helpers ─────────────────────────────────────────────────────── + +/** + * Derive the boolean seed for the attribution prompt from the current FlagsRecord. + * + * Returns true only when `suppress-attribution` is explicitly set to the boolean + * true — undefined, null, and false all map to false, giving `p.select` a real + * boolean rather than an unchecked cast (PF-018: non-vacuous path). + * + * Pure function — no side effects, fully testable without a TTY. + */ +export function attributionSeedFrom(flags: FlagsRecord): boolean { + return flags['suppress-attribution'] === true; +} + +/** + * Apply the resolved wizard answer back onto the FlagsRecord. + * + * Returns a new record with `suppress-attribution` updated to outcome.suppress. + * Never mutates the input (immutability principle). The caller replaces its local + * enabledFlags binding with the return value. + * + * D27: this is the single merge site for the wizard answer; init.ts must not + * duplicate the spread inline. + * + * Pure function — no side effects, fully testable without a TTY. + */ +export function applyAttributionAnswer( + flags: FlagsRecord, + outcome: AttributionStepResolved, +): FlagsRecord { + return { ...flags, 'suppress-attribution': outcome.suppress }; +} + /** * Run the attribution wizard step. * diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index b0e83fbf..7b7bd7b7 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -60,6 +60,8 @@ import { shouldRunAttributionStep, runAttributionStep, buildClackAttributionPrompts, + applyAttributionAnswer, + attributionSeedFrom, } from './attribution-prompts.js'; import { convergeFromManifest } from '../../targets/claude-code/compliance-install.js'; import { getPendingTurnsPath, getPendingTurnsProcessingPath } from '../../core/project-paths.js'; @@ -963,21 +965,26 @@ export const initCommand = new Command('init') // isTTY is guaranteed true here (the non-TTY guard above exit-1'd); passing it keeps // the promptless contract enforced at the predicate rather than by position (PF-029). if (shouldRunAttributionStep({ - mode: 'advanced', + // D27-GATE: bind to the resolved mode so all four documented gate-table rows + // are reachable and the predicate — not lexical placement — enforces Advanced-only. + // Using useRecommended ? 'recommended' : 'advanced' makes the gate testable from + // both sides and prevents the Recommended path from accidentally running the step + // if this block is ever repositioned (applies PF-029). + mode: useRecommended ? 'recommended' : 'advanced', isTTY: process.stdin.isTTY, })) { const attributionStep = await runAttributionStep({ - // resolveInitSeed always emits this key, but `=== true` keeps the seed a real - // boolean rather than an unchecked cast if that contract ever changes — - // an `undefined` reaching p.select's initialValue silently unseeds the prompt. - seed: enabledFlags['suppress-attribution'] === true, + // attributionSeedFrom keeps the seed a real boolean regardless of the stored + // FlagsRecord value type — undefined/null/false all map to false (PF-018). + seed: attributionSeedFrom(enabledFlags), prompts: buildClackAttributionPrompts(), }); if (attributionStep.kind === 'cancelled') { p.cancel('Installation cancelled.'); process.exit(0); } - enabledFlags = { ...enabledFlags, 'suppress-attribution': attributionStep.suppress }; + // D27: applyAttributionAnswer is the single merge site for the wizard answer. + enabledFlags = applyAttributionAnswer(enabledFlags, attributionStep); // Advanced path emits an outcome line (mirrors compliance step pattern). for (const msg of attributionStep.messages) { if (msg.level === 'success') p.log.success(msg.text); From 0de19e8fc8b9f6f76a6750a9736c3bc72dbf1478 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 20:52:51 +0300 Subject: [PATCH 17/21] =?UTF-8?q?test(init):=20cover=20the=20attribution?= =?UTF-8?q?=20wizard=E2=86=92settings=20hand-off=20(refs=20#315)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace five single-cell gate tests with the exhaustive 4-cell matrix test. Add a structural reachability guard that reads init.ts from disk, splits at the `// ── Advanced path: full interactive flow ──` boundary, and asserts: - runAttributionStep( appears exactly once in the Advanced half and zero times in the Recommended half - the attribution call site does NOT contain the bare literal `mode: 'advanced'` (which made three gate rows unreachable) and DOES contain `mode: useRecommended` - non-vacuity: source and both halves are non-empty (PF-018) Add unit tests for applyAttributionAnswer (both booleans, neighbouring entries survive, input not mutated) and attributionSeedFrom (true, false, absent, null — each asserts a real boolean result, PF-018). --- tests/attribution-prompts.test.ts | 199 +++++++++++++++++++++++++----- 1 file changed, 170 insertions(+), 29 deletions(-) diff --git a/tests/attribution-prompts.test.ts b/tests/attribution-prompts.test.ts index 2f8bdce8..b2818715 100644 --- a/tests/attribution-prompts.test.ts +++ b/tests/attribution-prompts.test.ts @@ -2,16 +2,28 @@ * Tests for attribution-prompts.ts (D27 / PF-029). * * Coverage: - * - shouldRunAttributionStep: gate predicate matrix (PF-029 invariants) + * - shouldRunAttributionStep: exhaustive gate matrix (PF-029 invariants) + * - structural reachability guard: init.ts call site must be in Advanced half with + * a non-literal mode binding — test fails when the block is moved or the literal + * is restored (applies PF-029, PF-018) * - runAttributionStep: step runner with injected DI seam (PF-014 invariants) + * - applyAttributionAnswer: immutable merge of wizard answer into FlagsRecord + * - attributionSeedFrom: boolean seed derivation from FlagsRecord (PF-018) */ import { describe, it, expect } from 'vitest'; +import * as path from 'path'; +import * as fs from 'fs'; +import { fileURLToPath } from 'url'; import { shouldRunAttributionStep, runAttributionStep, + applyAttributionAnswer, + attributionSeedFrom, type AttributionPromptIO, + type AttributionStepResolved, } from '../src/cli/commands/attribution-prompts.js'; +import type { FlagsRecord } from '../src/core/flags.js'; // ── shouldRunAttributionStep ────────────────────────────────────────────────── @@ -19,34 +31,7 @@ describe('shouldRunAttributionStep — gate predicate (D27 / PF-029)', () => { // ── Advanced-only invariant (D27) ─────────────────────────────────────────── // The attribution question is reachable from the Advanced path ONLY. Unlike the // compliance step, interactive Recommended never asks — it silently applies the - // seeded value. These tests are the authority for that divergence. - - it('Advanced mode with TTY → true (the only path that asks)', () => { - expect(shouldRunAttributionStep({ mode: 'advanced', isTTY: true })).toBe(true); - }); - - it('interactive Recommended → false (Recommended NEVER asks, D27)', () => { - // Divergence from shouldRunComplianceStep, which returns true here. Interactive - // Recommended silently applies the seeded value (fresh install: off). - expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: true })).toBe(false); - }); - - it('--recommended flag (non-TTY, typical non-interactive case) → false', () => { - // The --recommended CLI flag is commonly run without a TTY (CI, scripts). - // The predicate has no modePromptShown or hasCliOverride param — isTTY:false - // is the distinct input that represents this scenario. - expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: false })).toBe(false); - }); - - // ── Promptless contracts (PF-029) ─────────────────────────────────────────── - - it('non-TTY → false for Advanced (no prompt without a TTY)', () => { - expect(shouldRunAttributionStep({ mode: 'advanced', isTTY: false })).toBe(false); - }); - - it('non-TTY → false for Recommended (promptless contract preserved)', () => { - expect(shouldRunAttributionStep({ mode: 'recommended', isTTY: false })).toBe(false); - }); + // seeded value. This exhaustive matrix is the authority for that divergence. it('exhaustive gate matrix: only (advanced, TTY) is true', () => { const matrix: Array<[('recommended' | 'advanced'), boolean, boolean]> = [ @@ -61,6 +46,91 @@ describe('shouldRunAttributionStep — gate predicate (D27 / PF-029)', () => { }); }); +// ── Structural reachability guard ───────────────────────────────────────────── + +describe('init.ts structural guard — D27 call site must be in Advanced half, non-literal mode (PF-029 / PF-018)', () => { + const __filename = fileURLToPath(import.meta.url); + const __dirname = path.dirname(__filename); + const initTsPath = path.resolve(__dirname, '../src/cli/commands/init.ts'); + + // Robustly split init.ts at the Recommended / Advanced boundary. + // SPLIT_ANCHOR uniquely identifies the start of the Advanced interactive flow. + const SPLIT_ANCHOR = '// ── Advanced path: full interactive flow ──'; + const REC_ANCHOR = 'if (useRecommended) {'; + + it('non-empty corpus: source and both halves must be non-empty (PF-018 non-vacuity)', () => { + const source = fs.readFileSync(initTsPath, 'utf8'); + expect(source.length, 'init.ts is empty — file may have been deleted or renamed').toBeGreaterThan(0); + + const splitIdx = source.indexOf(SPLIT_ANCHOR); + expect(splitIdx, `Split anchor "${SPLIT_ANCHOR}" not found in init.ts — the boundary label was renamed`).not.toBe(-1); + + const recIdx = source.indexOf(REC_ANCHOR); + expect(recIdx, `Recommended anchor "${REC_ANCHOR}" not found in init.ts — the variable was renamed`).not.toBe(-1); + + const recommendedHalf = source.slice(0, splitIdx); + const advancedHalf = source.slice(splitIdx); + expect(recommendedHalf.length, 'Recommended half is empty — split boundary is at position 0').toBeGreaterThan(0); + expect(advancedHalf.length, 'Advanced half is empty — split boundary is at end of file').toBeGreaterThan(0); + }); + + it('runAttributionStep( appears exactly once in Advanced half and zero times in Recommended half', () => { + const source = fs.readFileSync(initTsPath, 'utf8'); + const splitIdx = source.indexOf(SPLIT_ANCHOR); + expect(splitIdx).not.toBe(-1); + + const recommendedHalf = source.slice(0, splitIdx); + const advancedHalf = source.slice(splitIdx); + + const countIn = (haystack: string, needle: string): number => + haystack.split(needle).length - 1; + + expect( + countIn(advancedHalf, 'runAttributionStep('), + 'runAttributionStep( should appear exactly once in the Advanced half', + ).toBe(1); + + expect( + countIn(recommendedHalf, 'runAttributionStep('), + 'runAttributionStep( must not appear in the Recommended half — D27 Advanced-only', + ).toBe(0); + }); + + it('attribution call site does NOT pass a literal mode: "advanced" — must use the ternary (D27-GATE)', () => { + const source = fs.readFileSync(initTsPath, 'utf8'); + const splitIdx = source.indexOf(SPLIT_ANCHOR); + expect(splitIdx).not.toBe(-1); + + const advancedHalf = source.slice(splitIdx); + + // Narrow to the shouldRunAttributionStep call block (not the broader Advanced half, + // which also contains the compliance call site that legitimately uses mode:'advanced'). + const attrCallStart = advancedHalf.indexOf('shouldRunAttributionStep({'); + expect( + attrCallStart, + 'shouldRunAttributionStep({ not found in Advanced half', + ).not.toBe(-1); + const attrCallEnd = advancedHalf.indexOf('})', attrCallStart); + expect(attrCallEnd, '}}) closing not found after shouldRunAttributionStep({').not.toBe(-1); + const attrCallBlock = advancedHalf.slice(attrCallStart, attrCallEnd + 2); + expect(attrCallBlock.length, 'attribution call block is empty').toBeGreaterThan(0); + + // After the fix, the attribution gate call must NOT contain the bare literal + // `mode: 'advanced'` — that pattern is what made the predicate always-true and + // three of the four gate rows unreachable (applies PF-029). + expect( + attrCallBlock.includes("mode: 'advanced'"), + "Found bare literal `mode: 'advanced'` at the attribution call site — must be the ternary `mode: useRecommended ? 'recommended' : 'advanced'`", + ).toBe(false); + + // The ternary that binds to the resolved mode must be present in this block. + expect( + attrCallBlock.includes('mode: useRecommended'), + "Ternary `mode: useRecommended` not found in shouldRunAttributionStep block — attribution gate is not bound to the resolved init mode", + ).toBe(true); + }); +}); + // ── runAttributionStep ──────────────────────────────────────────────────────── /** Build a no-op AttributionPromptIO that always yields the given select result. */ @@ -155,3 +225,74 @@ describe('runAttributionStep — step runner (PF-014)', () => { expect(capturedInitialValue).toBe(false); }); }); + +// ── applyAttributionAnswer ──────────────────────────────────────────────────── + +describe('applyAttributionAnswer — immutable FlagsRecord merge (D27)', () => { + function makeResolved(suppress: boolean): AttributionStepResolved { + return { + kind: 'resolved', + suppress, + messages: [], + }; + } + + it('suppress:true → suppress-attribution written as true', () => { + const flags: FlagsRecord = {}; + const result = applyAttributionAnswer(flags, makeResolved(true)); + expect(result['suppress-attribution']).toBe(true); + }); + + it('suppress:false → suppress-attribution written as false', () => { + const flags: FlagsRecord = { 'suppress-attribution': true }; + const result = applyAttributionAnswer(flags, makeResolved(false)); + expect(result['suppress-attribution']).toBe(false); + }); + + it('neighbouring entries survive the spread (immutable merge)', () => { + const flags: FlagsRecord = { + 'brief': true, + 'thinking-summaries': false, + 'suppress-attribution': false, + }; + const result = applyAttributionAnswer(flags, makeResolved(true)); + expect(result['suppress-attribution']).toBe(true); + expect(result['brief']).toBe(true); + expect(result['thinking-summaries']).toBe(false); + }); + + it('input object is not mutated', () => { + const flags: FlagsRecord = { 'suppress-attribution': false, 'brief': true }; + const before = { ...flags }; + applyAttributionAnswer(flags, makeResolved(true)); + expect(flags).toEqual(before); + }); +}); + +// ── attributionSeedFrom ─────────────────────────────────────────────────────── + +describe('attributionSeedFrom — boolean seed from FlagsRecord (PF-018)', () => { + it('true stored → returns true (real boolean)', () => { + const result = attributionSeedFrom({ 'suppress-attribution': true }); + expect(result).toBe(true); + expect(typeof result).toBe('boolean'); + }); + + it('false stored → returns false (real boolean)', () => { + const result = attributionSeedFrom({ 'suppress-attribution': false }); + expect(result).toBe(false); + expect(typeof result).toBe('boolean'); + }); + + it('absent key → returns false (real boolean)', () => { + const result = attributionSeedFrom({}); + expect(result).toBe(false); + expect(typeof result).toBe('boolean'); + }); + + it('null stored → returns false (real boolean)', () => { + const result = attributionSeedFrom({ 'suppress-attribution': null }); + expect(result).toBe(false); + expect(typeof result).toBe('boolean'); + }); +}); From e15c4beaa74213bbefdd3396ad2c2f39fedcd78b Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 21:04:57 +0300 Subject: [PATCH 18/21] refactor: simplify attribution-flag resolution fixes (refs #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move settingValueHoldsManagedShape and settingHoldsManagedShape to the Apply/Strip section in flags.ts, directly before canDeleteSettingKey. They are shape-guard helpers for the apply/strip pipeline, not viewMode helpers — the prior placement was misleading. Consolidate the duplicate isDeepStrictEqual call: canDeleteSettingKey now delegates to settingValueHoldsManagedShape so there is exactly one equality oracle for managed-shape comparisons across the entire pipeline (canDeleteSettingKey, settingHoldsManagedShape, convergeFlagsIntoSettings Step 2b all route through settingValueHoldsManagedShape). Remove the boolFlag as SettingBooleanFlagDef cast and the now-redundant settingDeleteGuard === undefined guard from Step 2b — settingValueHoldsManagedShape already encapsulates both checks. --- src/core/flags.ts | 114 +++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 56 deletions(-) diff --git a/src/core/flags.ts b/src/core/flags.ts index 62bad9e5..2117c20b 100644 --- a/src/core/flags.ts +++ b/src/core/flags.ts @@ -1039,22 +1039,72 @@ export function migrateLegacyFlagsToRecord( // ─── Apply / Strip ──────────────────────────────────────────────────────────── +/** + * Returns true when `value` equals the managed shape declared by `flag.settingDeleteGuard`. + * Returns false when: the flag has no guard, is not a setting-target boolean, or the value + * does not deep-equal the guard. + * + * Single equality oracle for managed-shape comparisons — used by `canDeleteSettingKey`, + * `settingHoldsManagedShape`, and the Step 2b adoption fold in `convergeFlagsIntoSettings`. + * All three delegate here so there is exactly one `isDeepStrictEqual` call for guard matching. + */ +export function settingValueHoldsManagedShape(flag: ClaudeCodeFlag, value: unknown): boolean { + if (flag.kind !== 'boolean' || flag.target.type !== 'setting') return false; + const guard = flag.settingDeleteGuard; + if (guard === undefined) return false; + return isDeepStrictEqual(value, guard); +} + +/** + * D-ATTR-GUARD single-source predicate (consistency-01 / ADR-024). + * + * Returns true when the settings.json string contains a value at the flag's + * target key that equals the flag's managed shape (`settingDeleteGuard`). + * + * This is the ONE place that decides "on-disk value equals the managed shape". + * All consumers — `resolveExistingAttributionSuppression` (seeding priority), + * and the guarded-boolean adoption fold in `convergeFlagsIntoSettings` — delegate + * here rather than hand-rolling their own comparison. + * + * Returns false when: + * - settingsJson is malformed + * - root is not a plain object + * - flagId is not in the registry + * - the flag has no settingDeleteGuard + * - the flag is not a setting-target boolean + * - the on-disk value does not deep-equal the guard + */ +export function settingHoldsManagedShape(settingsJson: string, flagId: string): boolean { + try { + const parsed: unknown = JSON.parse(settingsJson); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + const flag = FLAG_REGISTRY_MAP.get(flagId); + if (!flag) return false; + const value = (parsed as Record)[flag.target.key]; + return settingValueHoldsManagedShape(flag, value); + } catch { + return false; + } +} + /** * D-ATTR-GUARD: returns true when it is safe to delete a flag's target key from * settings. For flags with a settingDeleteGuard the key is only deleted when the - * current on-disk value deep-equals the guarded shape exactly, preventing erasure - * of user-customized values (e.g. an organisation attribution block) on flag + * current on-disk value deep-equals the managed shape, preventing erasure of + * user-customized values (e.g. an organisation attribution block) on flag * disable or uninstall. Flags without a guard are always safe to delete. * - * Uses `isDeepStrictEqual` from node:util — native, correct, and zero-maintenance. + * Delegates to `settingValueHoldsManagedShape` — the single equality oracle for + * managed-shape comparisons — so there is exactly one `isDeepStrictEqual` call + * across the entire apply/strip pipeline (ADR-024 mechanism 3). * - * Single-source invariant (ADR-024 mechanism 3): the guard is symmetric across - * the disable path (applyFlags neutral branch) and the uninstall path (stripFlags), - * upheld by construction since both call sites collapse to this one predicate. + * Single-source invariant: both the disable path (applyFlags neutral branch) and + * the uninstall path (stripFlags) collapse to this predicate. */ function canDeleteSettingKey(flag: ClaudeCodeFlag, settings: Record): boolean { - const guard = flag.kind === 'boolean' ? flag.settingDeleteGuard : undefined; - return guard === undefined || isDeepStrictEqual(settings[flag.target.key], guard); + if (flag.kind !== 'boolean') return true; + if (flag.settingDeleteGuard === undefined) return true; + return settingValueHoldsManagedShape(flag, settings[flag.target.key]); } /** @@ -1232,52 +1282,6 @@ export function resolveExistingViewMode(settingsJson: string): ViewMode | undefi return undefined; } -/** - * Returns true when `value` equals the managed shape declared by `flag.settingDeleteGuard`. - * Returns false when: the flag has no guard, is not a setting-target boolean, or the value - * does not deep-equal the guard. - * - * Value-level core for `settingHoldsManagedShape` — both share the single equality decision. - */ -export function settingValueHoldsManagedShape(flag: ClaudeCodeFlag, value: unknown): boolean { - if (flag.kind !== 'boolean' || flag.target.type !== 'setting') return false; - const guard = flag.settingDeleteGuard; - if (guard === undefined) return false; - return isDeepStrictEqual(value, guard); -} - -/** - * D-ATTR-GUARD single-source predicate (consistency-01 / ADR-024). - * - * Returns true when the settings.json string contains a value at the flag's - * target key that equals the flag's managed shape (`settingDeleteGuard`). - * - * This is the ONE place that decides "on-disk value equals the managed shape". - * All consumers — `resolveExistingAttributionSuppression` (seeding priority), - * and the guarded-boolean adoption fold in `convergeFlagsIntoSettings` — delegate - * here rather than hand-rolling their own comparison. - * - * Returns false when: - * - settingsJson is malformed - * - root is not a plain object - * - flagId is not in the registry - * - the flag has no settingDeleteGuard - * - the flag is not a setting-target boolean - * - the on-disk value does not deep-equal the guard - */ -export function settingHoldsManagedShape(settingsJson: string, flagId: string): boolean { - try { - const parsed: unknown = JSON.parse(settingsJson); - if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return false; - const flag = FLAG_REGISTRY_MAP.get(flagId); - if (!flag) return false; - const value = (parsed as Record)[flag.target.key]; - return settingValueHoldsManagedShape(flag, value); - } catch { - return false; - } -} - /** * Resolve the final view mode to write, combining an existing settings value, * an init-prompt-selected value, and whether the selection was explicit. @@ -1441,8 +1445,6 @@ export function convergeFlagsIntoSettings( for (const flag of FLAG_REGISTRY) { if (flag.kind !== 'boolean') continue; if (flag.target.type !== 'setting') continue; - const boolFlag = flag as SettingBooleanFlagDef; - if (boolFlag.settingDeleteGuard === undefined) continue; // Skip when the record already claims this flag (claimed false/null deletes the block) const claimed = From a2fdebd053f68db3300b6992b3384b716809c72d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 21:57:16 +0300 Subject: [PATCH 19/21] docs: record suppress-attribution behaviour in CHANGELOG, CLAUDE.md and reference docs (refs #315) --- CHANGELOG.md | 8 ++++++++ CLAUDE.md | 2 +- docs/cli-reference.md | 6 ++++-- docs/reference/file-organization.md | 5 +++-- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe9ebb8..7fd31662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **`suppress-attribution` flag** (optional boolean, default OFF): when enabled, writes `{"commit":"","pr":""}` to the `attribution` key in `settings.json`, suppressing Claude attribution trailers in git commits and PRs. Disabling (or uninstalling) removes the `attribution` key only when its current value exactly matches that managed shape — a custom attribution object is preserved, while enabling always replaces the existing value. Toggle via `devflow flags --enable/--disable suppress-attribution`. +- **Attribution wizard step in Advanced init** (D27): the Advanced-mode `devflow init` wizard asks one attribution question after the compliance step. Recommended init never asks; it silently applies the seeded value (off by default on a fresh install). The non-interactive path is `devflow flags --enable/--disable suppress-attribution` — there is no `--attribution` init flag. + ### Changed +- **`templates/settings.json` no longer ships an `attribution` block**: fresh installs now emit Claude attribution in git commits and PRs by default (where every prior install suppressed it). Existing installs are unaffected — see Upgrade note below. +- **`devflow:git` skill**: commit and PR templates no longer carry a hard-coded `Co-Authored-By: Claude` trailer or `Generated with Claude Code` footer. Attribution suppression is now an opt-in flag (`suppress-attribution`) rather than a hard-coded default. - **Routing runtime pinned to `subswitch@0.4.0`** (from `0.2.0`). Over-window Anthropic-bound request bodies are now streamed upstream instead of being rejected by the relay, so long prompts no longer fail at the proxy; only translated (Codex) routes still return `413 request_too_large`. `buildRoutingConfigJson` no longer injects `anthropic.connectTimeoutMs` — the relay's own default (10 s, connect-only) governs; user-set values are preserved as before. The injection was a 0.2.0-era workaround artifact that outlived its purpose once the key's semantics were narrowed to DNS+TCP connect only. ### Fixed @@ -15,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 **Upgrade**: no action required. If you hand-edited `~/.devflow/proxy-routing.json` to set any of the seven stripped `limits.*` keys, move their values to the 0.4.0 target paths (`limits.maxBufferedBodyBytes`, `anthropic.maxUpstreamSockets`, or the appropriate `providers.codex.*` key); the next `devflow proxy --enable` drops the stale keys for you. +**Upgrade (attribution)**: existing installs that carry the devflow-managed `attribution` block — written by prior versions' `templates/settings.json` — keep their suppression. On the next `devflow init` or any `devflow flags` write, the managed block is detected and adopted into the manifest as `suppress-attribution: true` automatically, so no manual action is required. Installs with a custom `attribution` value are also unaffected — the shape guard prevents any modification. + --- ## [2.3.0] - 2026-08-31 diff --git a/CLAUDE.md b/CLAUDE.md index 57695e13..f16cdafa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,7 +50,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. **Debug Tracing**: Single global toggle covering all hooks. Enabled via `devflow debug --enable/--disable/--status` CLI or by setting `DEVFLOW_HOOK_DEBUG=1` in `~/.claude/settings.json` env block (survives reinstalls). All hooks share the `src/assets/scripts/hooks/debug-trace` helper script (sourced via `hook-bootstrap`) so tracing behavior is consistent and updated in one place. Two-phase logging: pre-CWD traces go to global `~/.devflow/logs/.hook-debug.log`; post-CWD traces go to per-project `~/.devflow/logs/{project-slug}/.hook-debug.log`. A 5MB size guard prevents unbounded growth. applies ADR-007 -**Claude Code Flags**: Typed registry (`src/core/flags.ts`) for managing Claude Code feature flags (env vars and top-level settings). Four kinds: `boolean` (on/off), `enum` (validated domain), `number` (bounded integer), `string` (validated with maxLength). 29 flags total: recommended (default ON) — `tui`, `tool-search`, `lsp`, `prompt-caching-1h`, `show-turn-duration`, `clear-context-on-plan`, `disable-bundled-skills`, `pin-sonnet-4-6`, `max-concurrent-subagents` (number, devflow default 40, upstream default 20); optional boolean (default OFF) — `brief`, `thinking-summaries`, `subprocess-env-scrub`, `disable-nonessential-traffic`, `forked-subagents`, `disable-adaptive-thinking`, `always-thinking`, `disable-git-instructions`, `disable-compact`, `disable-1m-context`, `disable-autoupdater`, `agent-teams`, `enable-todo-tools`, `suppress-attribution` (writes `{"commit":"","pr":""}` to settings.json — suppresses Claude attribution in git commits and PRs; shape-guarded deletion: only removed on disable when the value exactly matches the devflow-managed shape, never when a user has a custom attribution; D27/D-ATTR-GUARD); valued (default unset) — `subagent-spawn-depth` (number, upstream default 3), `workflow-size-guideline` (enum: `small|medium|large|unrestricted`), `default-model` (string), `goal-checkin-minutes` (number, upstream default 30 min), `spellcheck` (string), `view-mode` (enum: `default|verbose|focus`, devflow default `default`). Stored in manifest `features.flags: Record` — entry-presence = known, `null` = deliberately unset (neutral, deletes the target key), absent = adopt-on-next-init. Pipeline: `applyFlags(settingsJson, FlagsRecord)` / `stripFlags(settingsJson)` — `applyViewMode`/`stripViewMode` retired; view-mode is an enum flag with `neutralValue: 'default'` (the `viewMode` settings.json key is written only when non-default); `resolveExistingViewMode`/`resolveFinalViewMode` remain exported for init.ts external-mode preservation. `devflow flags` bare on TTY launches the interactive flags editor TUI; bare on non-TTY prints a status table to stdout and exits 1. Registry entries carry a `blurb` field (≤30-char per-flag short hint) shown as a dim HINT column in the TUI and in `--status` rows. Display vocabulary via `effectiveDisplay`: booleans render 'on'/'off' (off is dim); neutral/unset enum shows `neutralValue` dim; unset number shows its applicable default dim with ' (default)' suffix; unset string shows '—' dim; an actively set non-boolean renders plain (at devflow default) or bold (deviating); the literal 'unset' is never a displayed value. TUI rendering: `RunTuiSpec.screen?: 'alt' | 'inline'`; flags editor runs inline (renders in-place in the normal scroll buffer, no alt-screen); agents-view defaults to alt. Manageable via `devflow flags --enable/--disable/--set /--unset /--status/--list`; `--enable`/`--disable` are boolean-only — non-boolean flags are redirected to `--set`. +**Claude Code Flags**: Typed registry (`src/core/flags.ts`) for managing Claude Code feature flags (env vars and top-level settings). Four kinds: `boolean` (on/off — declared as `EnvBooleanFlagDef | SettingBooleanFlagDef`: env targets constrain `onPayload` to `string` and type `settingDeleteGuard` as `never`, setting targets allow object payloads and an optional `settingDeleteGuard`; enforced at registry declaration sites by object-literal assignability — a `target.type` check does not narrow `onPayload` for consumers, use `isEnvBooleanFlag`), `enum` (validated domain), `number` (bounded integer), `string` (validated with maxLength). 29 flags total: recommended (default ON) — `tui`, `tool-search`, `lsp`, `prompt-caching-1h`, `show-turn-duration`, `clear-context-on-plan`, `disable-bundled-skills`, `pin-sonnet-4-6`, `max-concurrent-subagents` (number, devflow default 40, upstream default 20); optional boolean (default OFF) — `brief`, `thinking-summaries`, `subprocess-env-scrub`, `disable-nonessential-traffic`, `forked-subagents`, `disable-adaptive-thinking`, `always-thinking`, `disable-git-instructions`, `disable-compact`, `disable-1m-context`, `disable-autoupdater`, `agent-teams`, `enable-todo-tools`, `suppress-attribution` (writes `{"commit":"","pr":""}` to settings.json — suppresses Claude attribution in git commits and PRs; shape-guarded deletion: only removed on disable when the value exactly matches the devflow-managed shape, never when a user has a custom attribution — the guard covers deletion only; enabling overwrites any existing `attribution` value (ADR-024 corollary b); D27/D-ATTR-GUARD); valued (default unset) — `subagent-spawn-depth` (number, upstream default 3), `workflow-size-guideline` (enum: `small|medium|large|unrestricted`), `default-model` (string), `goal-checkin-minutes` (number, upstream default 30 min), `spellcheck` (string), `view-mode` (enum: `default|verbose|focus`, devflow default `default`). Stored in manifest `features.flags: Record` — entry-presence = known, `null` = deliberately unset (neutral, deletes the target key), absent = adopt-on-next-init. Pipeline: `applyFlags(settingsJson, FlagsRecord)` / `stripFlags(settingsJson)` — `applyViewMode`/`stripViewMode` retired; view-mode is an enum flag with `neutralValue: 'default'` (the `viewMode` settings.json key is written only when non-default); `resolveExistingViewMode`/`resolveFinalViewMode` remain exported for init.ts external-mode preservation. `devflow flags` bare on TTY launches the interactive flags editor TUI; bare on non-TTY prints a status table to stdout and exits 1. Registry entries carry a `blurb` field (≤30-char per-flag short hint) shown as a dim HINT column in the TUI and in `--status` rows. Display vocabulary via `effectiveDisplay`: booleans render 'on'/'off' (off is dim); neutral/unset enum shows `neutralValue` dim; unset number shows its applicable default dim with ' (default)' suffix; unset string shows '—' dim; an actively set non-boolean renders plain (at devflow default) or bold (deviating); the literal 'unset' is never a displayed value. TUI rendering: `RunTuiSpec.screen?: 'alt' | 'inline'`; flags editor runs inline (renders in-place in the normal scroll buffer, no alt-screen); agents-view defaults to alt. Manageable via `devflow flags --enable/--disable/--set /--unset /--status/--list`; `--enable`/`--disable` are boolean-only — non-boolean flags are redirected to `--set`. **Feature Knowledge Bases**: Per-feature `.devflow/features/` directory containing KNOWLEDGE.md files that capture area-specific patterns, conventions, architecture, and gotchas. Uses a **write-through** model: load = direct file-I/O reading `.devflow/features/index.md` (regenerable cache) with frontmatter-glob fallback over `features/*/KNOWLEDGE.md` (source of truth) + verify-against-code on read; save = in-command write-through via a simplified Knowledge agent that writes `KNOWLEDGE.md` + the `index.md` line directly (no `.create-result.json`, no external scripts, no lock). **Git-tracked & shared (amends ADR-021 for `features/`)**: the root `.gitignore` carve-out (`.devflow/*` + level-by-level `!` re-includes, written byte-identically by `ensure-root-gitignore` / `ensureDevflowGitignore`) un-ignores `.devflow/features/index.md` + every `{slug}/KNOWLEDGE.md` while the rest of `.devflow/` stays local; after writing, the **Knowledge agent commits those two paths to the current worktree branch itself** by running git via its Bash tool (scoped `commit --only` pathspec, never `git add -A`, **never push, never force**, no commit script — per the LLM-vs-plumbing principle the commit is the agent's, not a deterministic helper). A user opts back out by re-adding `.devflow/features/` to their own `.gitignore`. Existing installs upgrade once via the versioned `.root-gitignore-configured-v3` marker (v2→v3 adds the `!.devflow/conventions.md` re-include). Freshness = write-through + verify-on-read (NO git-staleness, NO SessionEnd eval, NO Learning task). `index.md` line format: `- **{slug}** — {areas} — {Use-when description}`; frontmatter is authoritative if the line is lost. MDS module: `src/assets/commands/_partials/_knowledge.mds` (defines/exports `knowledge_load` and `knowledge_writeback` partials) + 9 host `.mds` sources in `src/assets/commands/` compiled to `dist/commands/` by `scripts/build-mds.ts` (`npm run build:mds`). `knowledge_load` is used up-front by: implement, plan, resolve, code-review, self-review, research, bug-analysis. `knowledge_writeback` is used at workflow end by: implement, resolve, self-review, explore, debug. explore/debug do NOT load up-front (intentional asymmetry). Config gate: single `knowledge: true|false` in feature config (default true) — gates write-back only; load is ungated. CLI: `devflow knowledge list` (read index.md / frontmatter glob), `devflow knowledge --enable/--disable/--status` (flip config). Note: `/debug` keeps FEATURE_KNOWLEDGE orchestrator-local (investigation workers examine code without pre-loaded context). Toggleable via `devflow knowledge --enable/--disable/--status` or `devflow init --knowledge/--no-knowledge`. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index db67cf15..0a9e4d11 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -8,7 +8,7 @@ npx devflow-kit init The interactive wizard offers two modes: - **Recommended** (default) — Sensible defaults, quick setup -- **Advanced** — Full interactive flow with all options +- **Advanced** — Full interactive flow with all options, including one attribution question (Recommended never asks); the non-interactive path is `devflow flags --enable/--disable suppress-attribution` — there is no `--attribution` init flag Use `--recommended` or `--advanced` flags for non-interactive setup. @@ -231,7 +231,7 @@ All 29 flags by kind and devflow default: | `disable-autoupdater` | boolean | env `DISABLE_AUTOUPDATER` | `false` | | `agent-teams` | boolean | env `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS` | `false` | | `enable-todo-tools` | boolean | env `CLAUDE_CODE_ENABLE_TODO_TOOLS` | `false` | -| `suppress-attribution` | boolean | setting `attribution` | `false` | +| `suppress-attribution` | boolean | setting `attribution` | `false` ²| | `subagent-spawn-depth` | number | env `CLAUDE_CODE_MAX_SUBAGENT_SPAWN_DEPTH` | unset (upstream: 3) | | `workflow-size-guideline` | enum | setting `workflowSizeGuideline` | unset (`small\|medium\|large\|unrestricted`) | | `default-model` | string | env `ANTHROPIC_DEFAULT_MODEL` | unset | @@ -241,6 +241,8 @@ All 29 flags by kind and devflow default: ¹ Boolean flags targeting an env var write the flag's configured string value when enabled (e.g., `claude-sonnet-4-6` for `pin-sonnet-4-6`), not `1` or `true`. The env var is deleted when the flag is disabled or unset. +² `suppress-attribution` writes the object `{"commit":"","pr":""}` to the `attribution` key in `settings.json` when enabled — not `true`. Disabling or uninstalling removes the `attribution` key only when its current value exactly matches that shape; a custom attribution object is preserved. Enabling always replaces any existing `attribution` value, including a custom one. + ## External Model Routing (Devflow Proxy) Route Devflow agents through GPT models via your OpenAI/Codex subscription. When enabled, a local Devflow proxy relay intercepts agent requests and forwards them to the configured model. diff --git a/docs/reference/file-organization.md b/docs/reference/file-organization.md index 05f209ba..4d353eca 100644 --- a/docs/reference/file-organization.md +++ b/docs/reference/file-organization.md @@ -12,8 +12,9 @@ devflow/ │ │ ├── commands/ # init.ts, init-seed.ts, memory.ts, learning.ts, ambient.ts, │ │ │ # flags.ts, rules.ts, skills.ts, context.ts, hud.ts, │ │ │ # uninstall.ts, safe-delete.ts, security.ts, debug.ts, -│ │ │ # capture.ts, legacy-hooks.ts, compliance.ts, proxy.ts, -│ │ │ # agents.ts, knowledge/ +│ │ │ # capture.ts, legacy-hooks.ts, compliance.ts, +│ │ │ # compliance-prompts.ts, attribution-prompts.ts, +│ │ │ # prompt-io.ts, proxy.ts, agents.ts, knowledge/ │ │ ├── tui/ # Generic TUI shell — runTui driver, normalizeKey, cell helpers │ │ ├── flags-view/ # Claude Code flags editor TUI — standalone `devflow flags` command, inline screen mode (state.ts, render.ts, terminal.ts, index.ts) │ │ └── agents-view/ # Per-agent model config TUI (state.ts, render.ts, terminal.ts) From 8ff4f19a950ab07a02e8bf3aabc48c491adde1ef Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 21:57:24 +0300 Subject: [PATCH 20/21] docs(git-skill): align PR-comment attribution guidance with the shipped git agent (refs #315) --- .devflow/features/installer-shadowing/KNOWLEDGE.md | 2 +- src/assets/skills/git/SKILL.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.devflow/features/installer-shadowing/KNOWLEDGE.md b/.devflow/features/installer-shadowing/KNOWLEDGE.md index 57cb1098..d177efd0 100644 --- a/.devflow/features/installer-shadowing/KNOWLEDGE.md +++ b/.devflow/features/installer-shadowing/KNOWLEDGE.md @@ -211,7 +211,7 @@ When `proxyEnabled` is true entering the install apply pass, `runProxyPreflight` **`revertExternalAgents` on the selective path**: runs **before** `removeSelectedPlugins` — strips GPT model lines from installed agent frontmatter while the files are still present. **Known limitation**: on the selective path it reverts EVERY installed agent, not only those being removed — surviving agents lose GPT frontmatter assignments until the next `devflow init`. On the full-uninstall path it also runs before `removeAllDevFlow`. -**`removeAllDevFlow(claudeDir, devflowDir, verbose)`** removes `commands/devflow/`, `agents/devflow/`, `rules/devflow/`, `devflowScriptsDir`, and skill dirs via two separate passes (avoids PF-012): **prefixed** (`devflow:name`) for every skill in `getAllSkillNames() ∪ LEGACY_SKILL_NAMES`; **bare** (name or `devflow-name`) for `LEGACY_SKILL_NAMES` only — `~/.claude/skills/` is shared, so a bare dir matching a live-registry skill name is by construction foreign to Devflow. +**`removeAllDevFlow(claudeDir, devflowScriptsDir, verbose)`** removes `commands/devflow/`, `agents/devflow/`, `rules/devflow/`, `devflowScriptsDir`, and skill dirs via two separate passes (avoids PF-012): **prefixed** (`devflow:name`) for every skill in `getAllSkillNames() ∪ LEGACY_SKILL_NAMES`; **bare** (name or `devflow-name`) for `LEGACY_SKILL_NAMES` only — `~/.claude/skills/` is shared, so a bare dir matching a live-registry skill name is by construction foreign to Devflow. After `removeAllDevFlow`, scope-specific logic handles the remainder of `devflowDir`. The scope decision lives in `resolveDevflowDirCleanup(opts)`, a **pure exported function** (mirrors `resolveSecurityRemovalDecision`) — no I/O, no side effects, fully testable. diff --git a/src/assets/skills/git/SKILL.md b/src/assets/skills/git/SKILL.md index a7960685..acf1a9ed 100644 --- a/src/assets/skills/git/SKILL.md +++ b/src/assets/skills/git/SKILL.md @@ -201,7 +201,7 @@ sleep 1 # Between each API call - Only lines in the PR diff can receive inline comments - Deduplicate before posting (same file + line = keep one) -- Always include suggested fix and Claude Code attribution footer +- Always include a suggested fix; every comment carries the `` marker, and the visible devflow footer (*Posted by [devflow](https://github.com/dean0x/devflow)*) is appended only on summary comments (see src/assets/agents/git.md) ### Releases From 6cfdb1d9b12dcc5255c8b1c6bc933a04b88d4f6d Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Tue, 1 Sep 2026 22:21:25 +0300 Subject: [PATCH 21/21] docs(knowledge): update installer-shadowing feature knowledge base --- .devflow/features/index.md | 2 +- .../features/installer-shadowing/KNOWLEDGE.md | 59 +++++++++++++------ 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/.devflow/features/index.md b/.devflow/features/index.md index ccf69532..fd2b333e 100644 --- a/.devflow/features/index.md +++ b/.devflow/features/index.md @@ -2,7 +2,7 @@ - **ambient-orchestrator** — src/assets/scripts/hooks, src/cli/commands/ambient.ts, src/core/plugins.ts — Use when modifying the ambient mode hooks (preamble, session-start-orchestrator), the orchestrator charter file (including the feature-knowledge operating rule), the git-marker helper, the ambient CLI toggle, or the plan-handoff fast-path. Keywords: ambient, preamble, orchestrator, charter, plan-handoff, session-start-orchestrator, git-marker, DEVFLOW_BG_UPDATER, devflow ambient, UserPromptSubmit, SessionStart, feature-knowledge. - **dynamic-workflow-engine** — src/assets/commands/dynamic-build.mds, src/assets/commands/dynamic-plan.mds, src/assets/commands/dynamic-tickets.mds, src/assets/commands/dynamic-profile.mds, src/assets/commands/_partials/_engine.mds, src/assets/commands/_partials/_wave.mds, dist/commands, tests/build-mds.test.ts — Use when authoring or modifying the dynamic-* commands (dynamic-build, dynamic-plan, dynamic-tickets, dynamic-profile), the shared engine/wave/preamble/factory MDS partials, or the build-mds test suite that pins doctrine literals. Keywords: dynamic-build, dynamic-plan, dynamic-tickets, dynamic-profile, Workflow tool, agentType, Gate 1, Gate 2, review pass, wave, tickets→plan→build, MDS, _engine.mds, _wave.mds. - **resolve-pipeline** — src/assets/commands/resolve.mds, src/assets/agents/triage.md, src/assets/agents/code.md, src/core/plugins.ts, src/assets/commands/code-review.mds — Use when modifying /resolve or /code-review convergence logic, adding or changing Triage disposition rules (including DUPLICATE collapsing), adjusting Code-agent operating modes (issue-fix/validation-fix), touching the resolution-summary.md parser contract, changing the Verification Gate retry loop, understanding how DIFF_FILES flows from git validate-branch into blast-radius triage, or working on traceability operations (fetch-review-threads, resolve-review-threads, post-resolution-summary, check-merge-readiness, THREAD_MAP). Keywords: resolve, triage, disposition matrix, blast-radius, FIX_NOW, FIX_SEPARATE, TECH_DEBT, FALSE_POSITIVE, BY_DESIGN, ESCALATED, DUPLICATE, duplicate-grouping, duplicates-collapse, duplicate_of, resolution-summary, convergence parser, DIFF_FILES, issue-fix, validation-fix, Verification Gate, manage-debt, COMPLIANCE_SKILL_INSTALLED, TRACEABILITY DEGRADED, fetch-review-threads, THREAD_MAP, post-resolution-summary, Third-Party Threads, check-merge-readiness, ext-N, D7, D9, PF-024. -- **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, suppress-attribution, settingDeleteGuard, deepEqualsPlain, D-ATTR-GUARD, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. +- **installer-shadowing** — src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/commands/compliance-prompts.ts, src/cli/commands/prompt-io.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts — Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the shared wizard prompt-IO seam (prompt-io.ts, WizardPromptIO, PromptOutcome), working on the managed-shape equality oracle (settingValueHoldsManagedShape, settingHoldsManagedShape, isEnvBooleanFlag, canDeleteSettingKey, D-ATTR-ADOPT, convergeFlagsIntoSettings, adoption fold), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, attributionSeedFrom, applyAttributionAnswer, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, attributionSeedFrom, applyAttributionAnswer, suppress-attribution, settingDeleteGuard, settingValueHoldsManagedShape, settingHoldsManagedShape, isEnvBooleanFlag, canDeleteSettingKey, D-ATTR-GUARD, D-ATTR-ADOPT, D-PAYLOAD-CLONE, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, WizardPromptIO, PromptOutcome, clackNote, clackSelect, prompt-io, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline. - **learning-capture-system** — src/assets/scripts/hooks, src/assets/agents/learning.md, src/cli/commands/learning.ts, src/core/feature-config.ts, src/core/learning-tuning-config.ts, src/hud/components/learning-counts.ts, src/assets/commands/_partials — Use when modifying capture hooks (capture-prompt/capture-turn/capture-question), the learning or memory pending-turns queues, the Learning agent (src/assets/agents/learning.md), the session-start-context learning directive, the feature-config toggles, the learning tuning config, the decisions content files (decisions.md/pitfalls.md/index.md) or their ledger ops, or the devflow learning CLI. Keywords: capture-prompt, capture-turn, capture-question, queue-append, pending-turns, memory-worker, Learning agent, learning directive, LEARNING MAINTENANCE, DEVFLOW_BG_UPDATER, learning-lock, queue_read_gates, decisions_load, DECISIONS_CONTEXT, feature-config, config.json, learning.json, decisions-ledger, assign-anchor, retire-anchor, refresh-anchor, render-decisions, staged-write CAS, WORKING-MEMORY.md.new, segmentDetails, amendments, is-hex-sha, verify_and_swap, compute_commits_since_note, divergence guard, isSafeRawBody. - **external-model-routing** — src/core/proxy-state.ts, src/core/external-models.ts, src/core/agent-models.ts, src/core/agent-state.ts, src/core/agent-frontmatter.ts, src/core/codex-auth-inspect.ts, src/core/model-discovery.ts, src/core/cache.ts, src/core/proxy-log.ts, src/cli/commands/proxy.ts, src/cli/commands/agents.ts, src/cli/agents-view, src/cli/tui — Use when working on the proxy lifecycle (enable/disable/status/preflight), the ensure-proxy hook, per-agent model mapping, agent frontmatter rewriting, or the agents TUI. Keywords: proxy, external-model-routing, GPT, agent-models, ensure-proxy, frontmatter, devflow proxy, devflow agents, subswitch, ANTHROPIC_BASE_URL, dormancy, reapplyAgentMapping, proxyJsonExists, applyProxyTeardownToSettings, D-STRIP-1, mergeDevflowSettingsTemplate, subswitch 0.4.0. - **compliance-feature** — src/core/compliance.ts, src/targets/claude-code/compliance-install.ts, src/cli/commands/compliance.ts, src/assets/skills/compliance, src/assets/rules/compliance.md, src/assets/agents/git.md, src/assets/commands/code-review.mds, src/assets/commands/plan.mds, src/assets/commands/implement.mds, src/assets/commands/resolve.mds, src/assets/commands/release.md — Use when adding or modifying the compliance feature (framework registry, converge contract, CLI, rule stamping), changing how host commands resolve COMPLIANCE_SKILL_INSTALLED, modifying traceability operations in the Git agent (learn-conventions, issue-first, thread resolution, shipped markers, release evidence), or extending the D4 DEGRADED contract. Keywords: compliance, COMPLIANCE_SKILL_INSTALLED, convergeComplianceArtifacts, convergeFromManifest, frameworks, FEATURE_OWNED_SKILLS, traceability, D4, D9, gather-release-evidence, conventions.md, resolve-review-threads, ensure-traceable-issue, stamper, manifest-group, ComplianceFeatureState. diff --git a/.devflow/features/installer-shadowing/KNOWLEDGE.md b/.devflow/features/installer-shadowing/KNOWLEDGE.md index d177efd0..f339ff82 100644 --- a/.devflow/features/installer-shadowing/KNOWLEDGE.md +++ b/.devflow/features/installer-shadowing/KNOWLEDGE.md @@ -1,9 +1,9 @@ --- feature: installer-shadowing name: Installer & Skill/Rule Shadowing -description: "Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, suppress-attribution, settingDeleteGuard, deepEqualsPlain, D-ATTR-GUARD, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline." +description: "Use when modifying the install pipeline (installViaFileCopy, installAllRules, composeScripts, InstallReport), adding or changing skill/rule shadow override logic, touching uninstall scope (enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, sweepDevflowNamespaces, resolveProjectDataCleanup) or install-artifact cleanup, extending the CLI skills/rules/flags management commands, working with asset directory accessors (rulesDir, skillsDir, commandsDir) and package-root resolution, modifying the init seeding layer (resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, --reset, FlagsRecord, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, getAllCommandNames, proxy), working on the shared wizard prompt-IO seam (prompt-io.ts, WizardPromptIO, PromptOutcome), working on the managed-shape equality oracle (settingValueHoldsManagedShape, settingHoldsManagedShape, isEnvBooleanFlag, canDeleteSettingKey, D-ATTR-ADOPT, convergeFlagsIntoSettings, adoption fold), working on the flags TUI (FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, effectiveDisplay, blurb, inline mode, RunTuiSpec screen) or the flags CLI (createFlagsCommand, lookupFlag, persistFlagConfig, formatFlagValue), working on the compliance wizard step (shouldRunComplianceStep, runComplianceStep, modePromptShown, CompliancePromptIO), or working on the attribution wizard step (shouldRunAttributionStep, runAttributionStep, AttributionPromptIO, attributionSeedFrom, applyAttributionAnswer, suppress-attribution). Keywords: installViaFileCopy, installAllRules, composeScripts, InstallReport, RuleInstallOutcome, SkillShadowState, RuleShadowState, shadow, unshadow, validateSkillShadow, validateRuleShadow, seedRuleShadow, prefixSkillName, unprefixSkillName, devflow:, skills, rules, uninstall, EISDIR, enumerateUserDevFlowContent, removeDevFlowInstallArtifacts, resolveDevflowDirCleanup, installArtifactPaths, enumerateDryRunExtras, sweepDevflowNamespaces, resolveProjectDataCleanup, runDryRunPhase, runSelectivePhaseForScope, runFullPhaseForScope, runCleanupPhase, getPackageRoot, isContainedIn, rulesDir, skillsDir, agentsDir, commandsDir, scriptsDir, LEGACY_SKILL_NAMES, sweepOrphanedAssets, SweepResult, sweepOrphans, sweepFailures, SweepFailure, mdFileName, mdEntryName, orphan sweep, getAllSkillNames, getAllCommandNames, getAllAgentNames, DELETED_PLUGIN_NAMES, EXCLUDED, resolveInitSeed, resolveSeedFeatures, resolveSeedFlags, resolveSeedPlugins, resolveResetGatedInputs, applyCliToggles, FlagsRecord, FlagsRecordValue, getDefaultFlagsRecord, parseManifestFlags, migrateLegacyFlagsToRecord, sanitizeFlagsRecord, coerceFlagValue, parseFlagValueInput, neutralValueOf, isNeutral, countActiveFlags, readViewMode, knownPlugins, readConfigIfPresent, resolveExistingViewMode, resolveExistingAttributionSuppression, resolveFinalViewMode, reset, init-seed, proxy, reapplyAgentMapping, revertExternalAgents, agent-models.json, proxy.json, proxy-routing.json, proxy.pid, applyDisableToSettings, buildRealPreflightDeps, canonicalise-agent-keys-v1, AnyMigration, migrations.json, compliance-prompts, shouldRunComplianceStep, CompliancePromptIO, runComplianceStep, modePromptShown, attribution-prompts, shouldRunAttributionStep, AttributionPromptIO, runAttributionStep, attributionSeedFrom, applyAttributionAnswer, suppress-attribution, settingDeleteGuard, settingValueHoldsManagedShape, settingHoldsManagedShape, isEnvBooleanFlag, canDeleteSettingKey, D-ATTR-GUARD, D-ATTR-ADOPT, D-PAYLOAD-CLONE, D27, BooleanFlagDef, EnvBooleanFlagDef, SettingBooleanFlagDef, WizardPromptIO, PromptOutcome, clackNote, clackSelect, prompt-io, createFlagsCommand, lookupFlag, persistFlagConfig, FlagsViewState, FlagRow, buildFlagRows, collectFlagRecord, buildStops, cycleForward, cycleBackward, sanitizeCell, padToVisible, truncateVisible, effectiveDisplay, EffectiveDisplay, formatFlagValue, blurb, FlagDefCommon, INLINE_MARGIN, cursorUp, RunTuiSpec, screen, inline." category: architecture -directories: [src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts, src/cli/commands/compliance-prompts.ts] +directories: [src/targets/claude-code/installer.ts, src/targets/claude-code/legacy.ts, src/cli/commands/init.ts, src/cli/commands/init-seed.ts, src/cli/commands/uninstall.ts, src/cli/commands/rules.ts, src/cli/commands/skills.ts, src/cli/commands/flags.ts, src/cli/commands/attribution-prompts.ts, src/cli/commands/compliance-prompts.ts, src/cli/commands/prompt-io.ts, src/cli/flags-view, src/cli/tui, src/core/plugins.ts, src/core/assets.ts, src/core/paths.ts, src/core/manifest.ts, src/core/flags.ts, src/core/feature-config.ts, src/core/orphan-sweep.ts, src/core/migrations.ts] created: 2026-07-13 updated: 2026-09-01 --- @@ -264,7 +264,7 @@ A dedicated pure-function module (`src/cli/commands/init-seed.ts`) computes the **viewMode resolution**: `resolveInitSeed` resolves view-mode in three-priority order — (1) `resolveExistingViewMode(settingsSnapshot)` (non-`'default'` from current settings.json wins); (2) `readViewMode(flags)` from the spread manifest record (non-`'default'` wins); (3) `'default'`. The resolved value is encoded into `flags['view-mode']` on the returned `InitSeed`. `seedManifest?.features.viewMode` is no longer consulted — that field is retired; view-mode lives entirely in `ManifestData.features.flags['view-mode']`. -**suppress-attribution resolution** (`resolveExistingAttributionSuppression`): an exported pure function in `init-seed.ts` (mirrors `resolveExistingViewMode`). Returns `true` when the settings.json `attribution` key is the exact devflow-managed shape `{"commit":"","pr":""}` (two keys, both empty string, no extras). Returns `undefined` for absent key, custom value, or malformed JSON — callers fall through to the manifest entry. Priority order in `resolveInitSeed`: (1) `resolveExistingAttributionSuppression(settingsSnapshot)` → `true` when exact shape; (2) `flags['suppress-attribution']` from manifest FlagsRecord (boolean); (3) `false` (registry default). The resolved value is encoded into `flags['suppress-attribution']` on the returned `InitSeed`. `--reset` zeroes `settingsSnapshot` and `seedManifest`, so the resolved value is always `false` on a factory reset. +**suppress-attribution resolution** (`resolveExistingAttributionSuppression`): an exported pure one-liner in `init-seed.ts`. Returns `settingHoldsManagedShape(settingsSnapshot, 'suppress-attribution') ? true : undefined` — delegates entirely to the single equality oracle in `flags.ts`. Return type is `true | undefined` (not `boolean | undefined`). Priority order in `resolveInitSeed`: (1) `resolveExistingAttributionSuppression(settingsSnapshot)` → `true` when exact shape; (2) `flags['suppress-attribution']` from manifest FlagsRecord (boolean); (3) `false` (registry default). The seed fold uses `=== true` (not `as boolean`). The resolved value is encoded into `flags['suppress-attribution']` on the returned `InitSeed`. `--reset` zeroes `settingsSnapshot` and `seedManifest`, so the resolved value is always `false` on a factory reset. **CLI toggles** (`applyCliToggles`): Applies explicit CLI feature flags (e.g. `--no-learning`, `--proxy`) on top of the resolved seed. Undefined = not specified; seed value is kept. @@ -280,11 +280,23 @@ A dedicated CLI-layer module (ADR-013 — CLI-layer prompts; core stays UI-agnos - **`FRAMEWORK_SELECT_MESSAGE`** — canonical string for the framework multiselect prompt. - **`formatFrameworkCatalogue()`** — padded framework catalogue for the `p.note` body. - **`formatComplianceSummary(enabled, frameworks)`** — pure formatter; canonical home for the compliance state label. `init.ts` re-exports it for backward-compatible test imports. -- **`CompliancePromptIO`** — injectable DI seam (mirrors `ProxyPreflightDeps`). Enables unit tests to drive all branches without a real TTY. +- **`CompliancePromptIO`** — injectable DI seam (extends `WizardPromptIO` with `multiselect`). Enables unit tests to drive all branches without a real TTY. - **`buildClackCompliancePrompts()`** — builds the real clack adapter; translates the cancel symbol into the `PromptOutcome` discriminated union. - **`runComplianceStep(opts)`** — pure orchestrator (no `throw`/`process.exit()`/direct I/O; all I/O routed through `opts.prompts`). Flow: note header → labeled enable select → framework multiselect (`required:false`). Disable preserves frameworks (defensive copy; returned arrays never alias the seed). Returns `{kind:'resolved', state, messages}` or `{kind:'cancelled'}`. - **`shouldRunComplianceStep({mode, modePromptShown, isTTY, hasCliOverride})`** — pure gate predicate (PF-029). BOTH wizard paths (Recommended and Advanced) call it. `modePromptShown` is `true` only when the Setup-mode `p.select` actually ran; `--recommended` flag and non-TTY fallback never set it, preserving their promptless contracts. `--compliance`/`--no-compliance` wins via `hasCliOverride`. Recommended threads the result via `applyCliToggles(…, { compliance: cliComplianceOverride ?? wizardCompliance })`. +### Shared Wizard Prompt-IO Seam (`src/cli/commands/prompt-io.ts`) + +A new module (ADR-019 corollary: one-definition seam) that owns the shared DI types and real clack adapters used by all wizard steps: + +- **`PromptOutcome`** — `{ kind: 'value'; value: T } | { kind: 'cancel' }` (generic cancel-or-value result for all wizard prompts) +- **`WizardPromptIO`** — base injectable interface: `note(message, title): void` + `select(opts): Promise>` +- **`clackNote`** / **`clackSelect`** — real clack adapters. `clackSelect` uses `p.isCancel` as a type guard to narrow away the cancel branch — no `as T` cast on the result path +- `AttributionPromptIO` is a type alias of `WizardPromptIO` (kept as a named export for backward compat with tests and callers that import it by name) +- `CompliancePromptIO = WizardPromptIO & { multiselect }` (intersection extending the base seam for the third prompt) + +A third wizard step should build on this module rather than re-defining `PromptOutcome` or `WizardPromptIO`. + ### Attribution Prompt Module (`src/cli/commands/attribution-prompts.ts`) A dedicated CLI-layer module (ADR-013) that owns the attribution wizard UI for the `suppress-attribution` flag (D27). Parallel structure to `compliance-prompts.ts` but with a deliberately different gate predicate. @@ -292,13 +304,15 @@ A dedicated CLI-layer module (ADR-013) that owns the attribution wizard UI for t Key exports: - **`shouldRunAttributionStep({mode, isTTY})`** — pure gate predicate (applies PF-029). Returns `isTTY && mode === 'advanced'`. **Advanced-only — this is a documented divergence from `shouldRunComplianceStep`** (see Gotchas). There is no `modePromptShown` parameter and no CLI override for attribution: the question never runs on Recommended, and post-install toggling is via `devflow flags --enable/--disable suppress-attribution`. -- **`AttributionPromptIO`** — injectable DI seam mirroring `CompliancePromptIO`. Enables unit tests to drive all branches without a real TTY. -- **`buildClackAttributionPrompts()`** — builds the real clack adapter; translates the cancel symbol into `PromptOutcome`. -- **`runAttributionStep({seed, prompts})`** — pure orchestrator (no `throw`/`process.exit()`/direct I/O). Flow: note header (current setting + context) → `p.select` Yes/No (seeded from prior state). Returns `{kind:'resolved', suppress, messages}` or `{kind:'cancelled'}`. +- **`AttributionPromptIO`** — type alias of `WizardPromptIO` (imported from `prompt-io.ts`). Kept as a named export for backward compatibility with tests and `init.ts` callers. +- **`buildClackAttributionPrompts()`** — builds the real clack adapter; delegates to the shared `clackNote` / `clackSelect` adapters from `prompt-io.ts`. +- **`attributionSeedFrom(flags: FlagsRecord): boolean`** — derives boolean seed from FlagsRecord. Returns `true` only when `suppress-attribution === true` (not `as boolean`); undefined/null/false all map to false. Used by `init.ts` to pass a real boolean to `p.select`. +- **`applyAttributionAnswer(flags, outcome): FlagsRecord`** — immutable merge of the wizard answer onto the FlagsRecord (single merge site; `init.ts` must not duplicate the spread inline). Returns a new record — never mutates the input. +- **`runAttributionStep({seed, prompts})`** — pure orchestrator (no `throw`/`process.exit()`/direct I/O). The wizard note names the destructive enable branch: "Yes REPLACES any existing `attribution` value in settings.json, including a custom one" and includes an org AI-disclosure-policy clause. Returns `{kind:'resolved', suppress, messages}` or `{kind:'cancelled'}`. -**Call site in `init.ts`**: a single call in the Advanced path only. The Recommended path has no attribution call and carries the seeded `suppress-attribution` value unchanged from `resolveInitSeed`. After the Advanced step, `enabledFlags` is updated with the wizard answer so the subsequent `convergeFlagsIntoSettings` pipeline writes (or removes) the `attribution` key. +**Call site in `init.ts`**: `init.ts` passes `mode: useRecommended ? 'recommended' : 'advanced'` (not a string literal). A single call in the Advanced path only via `shouldRunAttributionStep`. `attributionSeedFrom(enabledFlags)` derives the seed; `applyAttributionAnswer(enabledFlags, outcome)` updates `enabledFlags` immutably. The Recommended path has no attribution call and carries the seeded `suppress-attribution` value unchanged from `resolveInitSeed`. -**Single ownership**: the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags` via `convergeFlagsIntoSettings`). It is absent from `src/targets/claude-code/templates/settings.json` and is NOT injected by `mergeDevflowSettingsTemplate` — adding it to either would create a second writer and could race with the flag pipeline. +**Single ownership**: the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags` via `convergeFlagsIntoSettings`). It is absent from `src/targets/claude-code/templates/settings.json` and is NOT injected by `mergeDevflowSettingsTemplate` — adding it to either would create a second writer and could race with the flag pipeline. A registry-driven test in `tests/post-install-merge.test.ts` asserts that no `FLAG_REGISTRY` setting-target key appears as a top-level key in `templates/settings.json` (single-ownership, ADR-024). ### Migrations (`src/core/migrations.ts`) @@ -410,7 +424,9 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - **Dry-run preview using only pure helpers instead of the production enumeration path** — `runDryRunPhase` (full mode) must call `enumerateDryRunExtras`, which itself calls `installArtifactPaths`. A test that exercises only the pure helper (`installArtifactPaths` in isolation) does not catch divergence between the preview and the real removal loop. (avoids PF-018) - **Re-deriving the display vocabulary at a render site instead of calling `effectiveDisplay`** — four render sites (TUI `formatValue`, `--enable/--disable` confirmation, `--status` not-adopted message, `--list` defaultLabel) all route through `effectiveDisplay`. Adding a fifth site that hand-codes 'on'/'off' or shows 'unset' creates vocabulary drift. Always delegate to `effectiveDisplay` (D-EFFDV) or `formatFlagValue` (which does so internally). - **Mirroring the compliance wizard gate for the attribution wizard gate** — `shouldRunAttributionStep` uses `mode === 'advanced'` directly (no `modePromptShown`), deliberately diverging from `shouldRunComplianceStep`. The compliance gate runs on interactive Recommended (`modePromptShown: true`); the attribution gate never does. Do not "restore symmetry" — the divergence is D27 design intent. -- **Adding `attribution` to `templates/settings.json` or `mergeDevflowSettingsTemplate`** — the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags`). A second writer creates a race: the template merge runs before the flags pipeline, so a template-written value would be immediately overwritten or, on the off path, leave a stale block. Single ownership is enforced by omission from both the template file and the merge function. +- **Adding `attribution` to `templates/settings.json` or `mergeDevflowSettingsTemplate`** — the `attribution` settings.json key is owned exclusively by the flags pipeline (`applyFlags`/`stripFlags`). A second writer creates a race: the template merge runs before the flags pipeline, so a template-written value would be immediately overwritten or, on the off path, leave a stale block. Single ownership is enforced by omission from both the template file and the merge function, and by a registry-driven test. +- **Duplicating the managed-shape comparison instead of delegating to `settingHoldsManagedShape`** — `settingValueHoldsManagedShape` (flag + value) and `settingHoldsManagedShape` (settingsJson + flagId) are the single equality oracle; `resolveExistingAttributionSuppression` and the Step 2b adoption fold in `convergeFlagsIntoSettings` both delegate here. Do not hand-roll `isDeepStrictEqual` against the guard at a call site. +- **Defining `PromptOutcome` or `WizardPromptIO` locally in a wizard module** — these types are defined once in `prompt-io.ts`. A new wizard step should import from there, not re-define equivalent types. ## Gotchas @@ -444,16 +460,20 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - **Attribution wizard gate does NOT use `modePromptShown`.** `shouldRunAttributionStep` gates on `mode === 'advanced'` directly — no `modePromptShown` parameter. This is safe because the Advanced branch itself exits non-zero on non-TTY (isTTY is the outer guard), so `mode === 'advanced'` is only ever true in an interactive session. Do not add a `modePromptShown` parameter to "align" it with the compliance gate — the divergence is intentional (D27). (PF-029) -- **`settingDeleteGuard` protects deletion, not writes.** When `suppress-attribution` is enabled (`true`), `applyFlags` always writes `{"commit":"","pr":""}` to `settings.json`, overwriting any prior value including a custom attribution block. The guard only gates the neutral/off path: when the flag transitions to false/null, the key is deleted ONLY when the current value deep-equals the managed shape. If the user has a custom attribution block (e.g. an org name), a flag-disable preserves it; a flag-enable overwrites it — this behavior is test-pinned. +- **`settingDeleteGuard` protects deletion, not writes.** When `suppress-attribution` is enabled (`true`), `applyFlags` always writes `{"commit":"","pr":""}` to `settings.json`, overwriting any prior value including a custom attribution block. The guard only gates the neutral/off path: when the flag transitions to false/null, the key is deleted ONLY when the current value deep-equals the managed shape (via `canDeleteSettingKey` → `settingValueHoldsManagedShape` — the single equality oracle). If the user has a custom attribution block (e.g. an org name), a flag-disable preserves it; a flag-enable overwrites it — this behavior is test-pinned. + +- **`BooleanFlagDef` discriminated union provides declaration-site guarantees only, NOT consumer narrowing.** After `if (f.target.type === 'env')`, `f` is still typed as `BooleanFlagDef` and `f.onPayload` remains `string | boolean | Record`. Use `isEnvBooleanFlag(f)` (exported type predicate) where the string type is load-bearing (e.g. writing to the env string-map). - **`--set` confirmation echoes literal 'unset' for an explicit null input.** When the user types `--set flag=unset`, `parseFlagValueInput` maps that to `null`. The `handleSet` confirmation special-cases `null → 'unset'` at the call site so the user sees their own word reflected back. Active values route through `formatFlagValue` (D-EFFDV) as normal — this is the only site where 'unset' still appears in user-facing output. -- **Blurb hard-cap is enforced by a registry test, not a TypeScript type.** `flag.blurb` is typed as `string` on `FlagDefCommon` (no length constraint in the type). The ≤30-char cap lives in `tests/flags.test.ts` as a registry-walk test — adding a blurb longer than 30 chars will fail CI but not the TypeScript compiler. +- **Blurb and hint caps are enforced by registry tests, not TypeScript types.** `flag.blurb` (≤30 chars) and `flag.hint` (≤76 chars) are both typed as `string` on `FlagDefCommon` — no length constraint in the type. Both caps live in `tests/flags.test.ts` as registry-walk assertions alongside each other — adding a blurb or hint that exceeds its cap fails CI but not the TypeScript compiler. - **Inline mode (`screen: 'inline'`) does not enter the alt screen.** On exit it cursor-ups to the frame top and `ERASE_BELOW` — the widget is erased and the clack flow continues in the normal scroll buffer. If you attach a flags TUI test expecting `ENTER_ALT` sequences, it will fail for `runFlagsTui` (which passes `screen: 'inline'`) but pass for agents-view tests (which use the default alt mode). Use `screen: 'alt'` explicitly when testing alt-screen behavior. - **Selective uninstall never strips flags.** `runCleanupPhase` (which calls `stripFlags`) only runs on full uninstall. Selective plugin uninstall (`runSelectivePhaseForScope`) does not invoke `stripFlags` — flag state persists in `settings.json` even when individual plugins are removed. +- **Step 2b adoption fold runs before `stripFlags` (applies PF-050 / ADR-024).** In `convergeFlagsIntoSettings`, guarded boolean flags (those with `settingDeleteGuard`) whose pre-strip on-disk value matches the managed shape are adopted into the `FlagsRecord` before `stripFlags` runs. Without this fold, a template-written attribution block would be stripped unconditionally on the first init, even when the user never explicitly set the flag. The fold only claims unclaimed flags — a record that already has `suppress-attribution: false` or `null` still deletes the block. + ## Key Files - `src/core/orphan-sweep.ts` — `sweepOrphanedAssets(dir, knownNames, extractRegistryName) => Promise`; `SweepResult = { scanned, removed, failed }`; `mdFileName` / `mdEntryName` inverse pair; shared by both installer and uninstall; per-item failure isolation on both readdir and rm @@ -461,15 +481,16 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - `src/core/assets.ts` — `skillsDir`, `agentsDir`, `rulesDir`, `scriptsDir`, `commandsDir` accessors; single source of truth for all asset source paths - `src/core/paths.ts` — `getPackageRoot()` with hard `package.json` assertion; 2-level-up resolution from `dist/core/paths.js`; `isContainedIn(parent, candidate)` pure containment predicate (guards path-traversal in reapplyAgentMapping) - `src/targets/claude-code/legacy.ts` — `LEGACY_SKILL_NAMES` (composed from `LEGACY_SKILLS_PRE_V1`, `LEGACY_SKILLS_V2`, `LEGACY_SKILLS_V2X`); target-specific delete lists for upgrade cleanup -- `src/cli/commands/init.ts` — consumes `InstallReport` and `InitSeed`; proxy preflight block using `buildRealPreflightDeps` factory (`swallowSettingsReadError: true`); `reapplyAgentMapping` call (ordering load-bearing, guarded when mapping is empty AND proxy is off); exhaustive `ShadowSkipReason` switch with `never` guard; attribution step in Advanced path only (`shouldRunAttributionStep`, `runAttributionStep`) -- `src/cli/commands/init-seed.ts` — pure seeding helpers: `resolveInitSeed`, `resolveSeedFeatures`, `resolveSeedFlags(manifestFlags: FlagsRecord | null, registry)` (two-branch: null→all defaults, non-null→spread+adopt-absent), `resolveSeedPlugins`, `resolveResetGatedInputs`, `applyCliToggles`, `FEATURE_DEFAULTS` (proxy: false); `resolveExistingAttributionSuppression` (mirrors resolveExistingViewMode — returns true for exact devflow shape, undefined otherwise); `InitSeed.flags: FlagsRecord` encodes both view-mode and suppress-attribution — no separate fields -- `src/cli/commands/attribution-prompts.ts` — `shouldRunAttributionStep({mode, isTTY})` (Advanced-only gate; no modePromptShown; D27 divergence from compliance gate); `AttributionPromptIO` (DI seam); `buildClackAttributionPrompts()`; `runAttributionStep({seed, prompts})` (pure orchestrator, no process.exit, no throw) +- `src/cli/commands/init.ts` — consumes `InstallReport` and `InitSeed`; proxy preflight block using `buildRealPreflightDeps` factory (`swallowSettingsReadError: true`); `reapplyAgentMapping` call (ordering load-bearing, guarded when mapping is empty AND proxy is off); exhaustive `ShadowSkipReason` switch with `never` guard; attribution step in Advanced path only (`shouldRunAttributionStep`, `attributionSeedFrom`, `applyAttributionAnswer`, `runAttributionStep`); mode passed as `useRecommended ? 'recommended' : 'advanced'` (not a string literal) +- `src/cli/commands/init-seed.ts` — pure seeding helpers: `resolveInitSeed`, `resolveSeedFeatures`, `resolveSeedFlags(manifestFlags: FlagsRecord | null, registry)` (two-branch: null→all defaults, non-null→spread+adopt-absent), `resolveSeedPlugins`, `resolveResetGatedInputs`, `applyCliToggles`, `FEATURE_DEFAULTS` (proxy: false); `resolveExistingAttributionSuppression` (one-liner over `settingHoldsManagedShape` — returns `true | undefined`; seed fold uses `=== true`); `InitSeed.flags: FlagsRecord` encodes both view-mode and suppress-attribution — no separate fields +- `src/cli/commands/prompt-io.ts` — shared wizard prompt-IO seam: `PromptOutcome` (generic cancel-or-value), `WizardPromptIO` (note + boolean select), `clackNote`, `clackSelect` (no result-side cast); `AttributionPromptIO = WizardPromptIO` (alias); `CompliancePromptIO` extends this interface +- `src/cli/commands/attribution-prompts.ts` — `shouldRunAttributionStep({mode, isTTY})` (Advanced-only gate; no modePromptShown; D27 divergence from compliance gate); `AttributionPromptIO` (alias of `WizardPromptIO` from prompt-io.ts); `buildClackAttributionPrompts()` (delegates to shared adapters); `attributionSeedFrom(flags): boolean` (returns true only for `=== true`); `applyAttributionAnswer(flags, outcome): FlagsRecord` (immutable merge, single call site); `runAttributionStep({seed, prompts})` (note names destructive enable branch + org AI-disclosure clause) - `src/cli/commands/uninstall.ts` — exported: `removeAllDevFlow`, `removeSelectedPlugins`, `isDevFlowInstalled`, `installArtifactPaths` (SSOT for artifact list), `enumerateDryRunExtras` (derived from installArtifactPaths + skill lists), `sweepDevflowNamespaces` (named selective-path sweep step), `resolveProjectDataCleanup` (pure: cancel→preserve, no process.exit), `enumerateUserDevFlowContent` (skills/rules/preference-profile/learning.json/hud.json — NOT agent-models.json), `removeDevFlowInstallArtifacts` (uses installArtifactPaths; containment guard; `isDir === true` strict equality), `revertExternalAgents` runs on both full and selective paths, `computeAssetsToRemove`, `resolveSecurityRemovalDecision`, `resolveDevflowDirCleanup` (--keep-docs honored); phase runners: `runDryRunPhase`, `runSelectivePhaseForScope`, `runFullPhaseForScope`, `runCleanupPhase` (injected cwd + isTTY; calls stripFlags with settingDeleteGuard shape-guarded deletion) - `src/core/manifest.ts` — `ManifestData` (`features.flags: FlagsRecord` — key-presence = known, null = neutral, absent = adopt-on-init; `knownPlugins?: string[]`; `features.proxy`); `parseManifestFlags(features, knownFlags)` — three-shape migration: string[]→`migrateLegacyFlagsToRecord`, object→spread, missing→empty; `readManifest` — self-heals legacy `knownFlags` (consumed in migration, not stored), proxy absent→false, applies `sanitizeFlagsRecord`; `writeManifest`, `syncManifestFeature`, `resolvePluginList` (filters `DELETED_PLUGIN_NAMES` via in-memory filter) - `src/core/plugins.ts` — `prefixSkillName`, `unprefixSkillName`, `SKILL_NAMESPACE`, `DEVFLOW_PLUGINS` (21 plugins — no devflow-audit-claude), `buildFullSkillsMap`, `buildRulesMap`, `getAllSkillNames`, `getAllCommandNames`, `getAllAgentNames`, `partitionSelectablePlugins`, `EXCLUDED` (module-level export), `LEGACY_PLUGIN_NAMES`, `LEGACY_COMMAND_NAMES`, `LEGACY_RULE_NAMES`, `DELETED_PLUGIN_NAMES` (['devflow-audit-claude']) - `src/core/migrations.ts` — `MIGRATIONS: readonly AnyMigration[]` (one entry: `canonicalise-agent-keys-v1`, scope `'global'`); `AnyMigration = Migration<'global'> | Migration<'per-project'>` discriminated union; `canonicaliseAgentKeys` returns `{agents, didMutate, renamed, dropped, guardDropped}`; `parseAgentMappingEnvelope` shared with `readAgentMapping`; failure-as-warning means a failed write is permanently skipped (self-healed by `readAgentMapping`) - `src/cli/commands/proxy.ts` — `applyDisableToSettings`, `buildRealPreflightDeps`, `addProxyHooks`, `removeProxyHooks`, `applyProxyEnv`, `stripProxyEnv` -- `src/core/flags.ts` — `FLAG_REGISTRY` (29 flags, each with `blurb: string` on `FlagDefCommon` — ≤30 chars, hard-capped by registry test); `BooleanFlagDef = EnvBooleanFlagDef | SettingBooleanFlagDef` discriminated union — `EnvBooleanFlagDef` compile-constrains `onPayload: string` and `settingDeleteGuard?: never`; `SettingBooleanFlagDef` allows object `onPayload` and optional `settingDeleteGuard: Record`; `suppress-attribution` is a `SettingBooleanFlagDef` (target key `attribution`, onPayload `{commit:'',pr:''}`, settingDeleteGuard same shape); `deepEqualsPlain` (private pure JSON structural equality used by D-ATTR-GUARD); `FlagsRecord` (`Record`), `FlagsRecordValue` (`FlagValue | null`); `effectiveDisplay(flag, value): EffectiveDisplay` (D-EFFDV one-definition seam — never returns 'unset': boolean→'on'/'off', enum null→neutralValue, number null→devflow/upstream default, string null→'—'); `formatFlagValue` delegates to `effectiveDisplay`; `getDefaultFlagsRecord`, `sanitizeFlagsRecord`, `migrateLegacyFlagsToRecord`, `coerceFlagValue`, `parseFlagValueInput`, `neutralValueOf`, `isNeutral`, `countActiveFlags`, `readViewMode`; `applyFlags(settingsJson, FlagsRecord)`, `stripFlags`, `resolveExistingViewMode`, `resolveFinalViewMode` +- `src/core/flags.ts` — `FLAG_REGISTRY` (29 flags; `blurb: string` ≤30 chars and `hint: string` ≤76 chars on `FlagDefCommon` — both caps enforced by registry-walk tests, not TypeScript types); `BooleanFlagDef = EnvBooleanFlagDef | SettingBooleanFlagDef` discriminated union — provides declaration-site object-literal assignability only (a nested `target.type` check does NOT narrow `onPayload` for consumers; use `isEnvBooleanFlag(f)` instead); `EnvBooleanFlagDef` compile-constrains `onPayload: string` and `settingDeleteGuard?: never`; `SettingBooleanFlagDef` allows object `onPayload` and optional `settingDeleteGuard: Record`; `suppress-attribution` is a `SettingBooleanFlagDef` (target key `attribution`, onPayload `{commit:'',pr:''}`, settingDeleteGuard same shape); `isEnvBooleanFlag(f): f is EnvBooleanFlagDef` (exported type predicate — narrows onPayload to string without unsafe cast); `settingValueHoldsManagedShape(flag, value): boolean` (exported — single equality oracle: returns true iff flag has settingDeleteGuard and value deep-equals it via `node:util.isDeepStrictEqual`); `settingHoldsManagedShape(settingsJson, flagId): boolean` (exported — reads settings.json and delegates to oracle; used by `resolveExistingAttributionSuppression` and Step 2b); `canDeleteSettingKey` (module-private — D-ATTR-GUARD shared by `applyFlags` neutral branch and `stripFlags`, delegates to `settingValueHoldsManagedShape`); `buildPayload` returns `structuredClone` for object payloads (D-PAYLOAD-CLONE — FLAG_REGISTRY entry never aliased into the caller's settings tree); `convergeFlagsIntoSettings` Step 2b (D-ATTR-ADOPT / PF-050 / ADR-024): adoption fold for unclaimed guarded-boolean flags whose pre-strip on-disk value matches the managed shape — runs before stripFlags so the block survives on both the init and flags paths; `FlagsRecord`, `FlagsRecordValue`, `effectiveDisplay`, `formatFlagValue`, `getDefaultFlagsRecord`, `sanitizeFlagsRecord`, `migrateLegacyFlagsToRecord`, `coerceFlagValue`, `parseFlagValueInput`, `neutralValueOf`, `isNeutral`, `countActiveFlags`, `readViewMode`, `applyFlags`, `stripFlags`, `resolveExistingViewMode`, `resolveFinalViewMode` - `src/core/feature-config.ts` — `readConfig`, `readConfigIfPresent`, `writeConfig`, `updateFeature` - `src/cli/commands/flags.ts` — `createFlagsCommand` (bare TTY→TUI inline mode, bare non-TTY→status table+exit 1); `lookupFlag(id)` (null for unknown); `readSettingsSafe(settingsPath)` (Result-returning); `persistFlagConfig(claudeDir, devflowDir, settingsContent, newRecord)` (writes FlagsRecord to manifest + settings.json); `formatStatusRows` uses `effectiveDisplay` for not-adopted rows; `--set` confirmation special-cases null→literal 'unset' - `src/cli/flags-view/state.ts` — `FlagsViewState`, `FlagRow` (includes `blurb: string` sourced from `flag.blurb`); `buildFlagRows(registry, record)`, `collectFlagRecord(rows)`; `buildStops`, `cycleForward`, `cycleBackward`; `recordToTui`/`tuiToRecord` value converters; `reduce(state, key) → {state, done, saved}`; `enterEdit`/`commitEdit`/`insertChar`/`reduceEditMode`; `adjustViewport` @@ -483,16 +504,18 @@ VALUE+BLURB = 46, preserving the prior total from the single VALUE column. All w - ADR-001: Config-only feature gates — governs `readConfigIfPresent` as the init-seed source for memory/learning/knowledge; config.json is the source of truth, manifest is secondary. Note: proxy is NOT in this group — it seeds from the manifest like ambient/hud/rules - ADR-003: End-state not transition — governs removals and legacy cleanup; cancel/decline on uninstall falls through to `removeDevFlowInstallArtifacts` rather than `process.exit()` so cleanup always runs - ADR-010: Shadow tolerance — governs `installViaFileCopy` as sole install path and warn-and-install-source (not hard-fail) for invalid shadows; hard-error policy applies only to declared Devflow sources -- ADR-013: Core/adapter boundary — governs `init-seed.ts` and `attribution-prompts.ts` living in `src/cli/commands/` (CLI-init-specific logic) rather than `src/core/` +- ADR-013: Core/adapter boundary — governs `init-seed.ts`, `attribution-prompts.ts`, `compliance-prompts.ts`, and `prompt-io.ts` living in `src/cli/commands/` (CLI-init-specific logic) rather than `src/core/` - ADR-014: State-aware re-init — governs `readManifest` self-heal idiom (`proxy` absent→false), FlagsRecord key-presence as the "known" encoding (absent key = adopt-on-init), `--reset` zeroing seedManifest so suppress-attribution always falls back to false on factory reset, and the `knownPlugins` snapshot pattern for detecting newly added plugins -- ADR-019: Typed flag registry — governs the `FLAG_REGISTRY` design including `BooleanFlagDef` as a discriminated union (`EnvBooleanFlagDef | SettingBooleanFlagDef`) enforcing the env-string invariant at compile time +- ADR-019: Typed flag registry — governs the `FLAG_REGISTRY` design including `BooleanFlagDef` as a discriminated union (`EnvBooleanFlagDef | SettingBooleanFlagDef`) enforcing the env-string invariant at compile time; corollary: `WizardPromptIO`/`PromptOutcome` defined once in `prompt-io.ts` rather than duplicated across wizard modules - ADR-020: Flags editor removal from init (D40) — governs that init applies flags non-interactively; `devflow flags` bare on TTY is the sole TUI entry point +- ADR-024: Prove-you-wrote-it ownership contract — governs `settingDeleteGuard` (shape-guarded deletion: only remove the key when the value is the devflow-managed shape), the `settingValueHoldsManagedShape` single equality oracle, and the Step 2b adoption fold (D-ATTR-ADOPT) in `convergeFlagsIntoSettings` (PF-050 mechanism) - PF-009: Per-item failure isolation — per-rule try/catch inside `installRuleFile`; `rules --enable` wraps `installAllRules`; proxy preflight failure warns + forces off without aborting init; `sweepOrphanedAssets` outer/inner independent catches; proxy artifact removal is per-item non-fatal; non-fatal catches can mask systematic TypeErrors when optional properties are not narrowed - PF-012: LEGACY_* lists deletion-risk — lists split between `src/targets/claude-code/legacy.ts` (skill) and `src/core/plugins.ts` (plugin/command/rule); both must be retained across upgrades - PF-014: process.exit() skips cleanup — governs the cancel/decline path in user-scope uninstall; `removeDevFlowInstallArtifacts` must execute on every non-confirm path; `resolveProjectDataCleanup` maps cancel→false (preserve) instead of process.exit(); `runAttributionStep` also never calls process.exit (callers own the cancel idiom) - PF-015: Fold-before-strip — governs that `suppress-attribution` (like view-mode) must be encoded into `FlagsRecord` before `convergeFlagsIntoSettings` runs; both flags share the single-record pattern - PF-018: Dry-run regression test must exercise the production output path — the original helper-only test missed a real preview/deletion divergence; `runDryRunPhase` (full mode) calls `enumerateDryRunExtras` which shares `installArtifactPaths` with the removal loop - PF-029: Wizard gate predicates must be fully wired, seeded, tested — applies to both `shouldRunComplianceStep` and `shouldRunAttributionStep`; the attribution gate diverges deliberately (Advanced-only, no modePromptShown) and the divergence is documented in `attribution-prompts.ts` (D27) +- PF-050: Registry adoption of on-disk key — governs the D-ATTR-ADOPT fold: the day a settings key that already ships on disk becomes registry-managed, it must be adopted before the strip pass; `settingDeleteGuard` presence is the signal; `convergeFlagsIntoSettings` Step 2b is the mechanism - PF-043: Test fixtures must match runtime shapes — governs the `tests/init-e2e-flags.test.ts` subprocess e2e tests over the real init settings pass, ensuring test fixtures stay in sync with the actual settings.json schema written by `applyFlags` - Feature knowledge: `external-model-routing` — deep proxy mechanics (lifecycle, preflight protocol, ensure-proxy hook, per-agent model mapping, dormancy invariant, agent frontmatter rewriting, TUI); `installer-shadowing` covers only proxy's footprint in the install/uninstall pipeline and init seeding - Feature knowledge: `feature-knowledge-system` — the Knowledge agent writes to `.devflow/features/` which is tracked in git; related to the `.gitignore` carve-out maintained by the installer