feat: gate AI-attribution suppression behind typed flag with init wizard - #316
Merged
Conversation
This was referenced Sep 1, 2026
Owner
Author
Resolution SummaryFull summary withheld (public repository).
Full report: /Users/dean/Sandbox/devflow/.devflow/docs/reviews/feat-315-gate-attribution-flag/2026-09-01_1737/resolution-summary.md (not committed; ask the author) Posted by devflow |
…ard (#315) R1 — FLAG_REGISTRY: add `suppress-attribution` boolean flag (setting target, key: `attribution`, onPayload: {commit:'',pr:''}). Extends `BooleanFlagDef` with `onPayload: string | boolean | Record<string,unknown>` 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).
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.
…ole (refs #315) 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<string,unknown> 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`.
…efs #315) - 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 <noreply@anthropic.com>
…(refs #315) - 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 <noreply@anthropic.com>
Remove Co-Authored-By: Claude <noreply@anthropic.com> 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.
… cases (refs #315) 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.
… and drop the vacuous merge case (refs #315) 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).
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).
#315) 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).
… compliance steps (refs #315) Extract PromptOutcome<T>, 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<T> 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<T>) 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.
… the BooleanFlagDef docblock (refs #315) - 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
…d isEnvBooleanFlag and payload-clone tests (refs #315) - 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
…315) 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).
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).
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.
…nd reference docs (refs #315)
…ed git agent (refs #315)
dean0x
force-pushed
the
feat/315-gate-attribution-flag
branch
from
September 1, 2026 20:02
52fdcca to
6cfdb1d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
{"commit":"","pr":""}in settings.json) behind a typed flag (suppress-attribution) in the flag registry, with a wizard question in the Advanceddevflow initpath only (Recommended silently applies the seeded value — fresh install: off)applyFlags/stripFlags) is now the sole writer/removerCo-Authored-By: Claudetrailers from the git skill commit templates; attribution is now governed solely by Claude Code's native mechanism and thesuppress-attributionflagChanges
R1 — Flag registry (
src/core/flags.ts)suppress-attributionboolean flag (setting target, keyattribution,onPayload: {commit:'',pr:''},defaultValue: false)EnvBooleanFlagDef | SettingBooleanFlagDefsplit: env members constrainonPayloadtostring(compile-enforced; env blocks are string maps) and carrysettingDeleteGuard?: neverso the shape-guard path is a compile error on env flags; setting members allowstring | boolean | Record<string,unknown>and the optionalsettingDeleteGuardsettingDeleteGuard?: Record<string,unknown>field toSettingBooleanFlagDefdeepEqualsPlaininternal helper for shape-guarded deletionsapplyFlagsneutral branch andstripFlagsto checksettingDeleteGuardbefore deleting setting keysR2 — Template and merge (
settings.json,post-install.ts)attributionblock fromsrc/targets/claude-code/templates/settings.jsonmergeDevflowSettingsTemplate— attribution is now flags-pipeline-only (D27 comment added)R3 — Attribution wizard (
src/cli/commands/attribution-prompts.ts)compliance-prompts.tspatternshouldRunAttributionStep(gate predicate),AttributionPromptIO(DI seam),buildClackAttributionPrompts,runAttributionStepshouldRunAttributionStep({mode, isTTY})returnsisTTY && mode === 'advanced'— nomodePromptShownorhasCliOverrideparams; the Advanced-only restriction is a deliberate divergence from the compliance gate (which also runs on interactive Recommended) and is test-pinned attests/attribution-prompts.test.tsR4 — Seeding (
src/cli/commands/init-seed.ts)resolveExistingAttributionSuppression(pure, exported): returnstrueonly for the exact{"commit":"","pr":""}shape,undefinedfor custom/absent valuesresolveInitSeedto fold attribution suppression into the seed FlagsRecord (mirrors view-mode priority: settings.json > manifest > false)R5 — init.ts integration
shouldRunAttributionStep({mode:'advanced', isTTY: process.stdin.isTTY})guards the callenabledFlags['suppress-attribution']from seed)R6 — Tests
tests/flags.test.ts: updatesonPayloadvalidity test to allow plain-object for setting targets; addssuppress-attributionto pinned default record; adds 11-test describe block for shape guard (D27/D-ATTR-GUARD), including the ON-over-custom overwrite case (settingDeleteGuard protects deletion only)tests/attribution-prompts.test.ts(new): 14 tests covering gate predicate matrix and step runner with injectable DI seamtests/init-seed.test.ts: addsresolveExistingAttributionSuppressiontests (8) andresolveInitSeedseeding matrix (5)tests/post-install-merge.test.ts: removes old attribution-injection tests; adds tests confirming attribution is NOT injected by merge path (second case uses template WITH an attribution block for falsifiability)tests/init-e2e-flags.test.ts: 5 e2e init convergence cases (off→on, on→off, user-custom survives, re-init seeds from exact shape, PF-018 non-vacuity gate) exercising the fullapplyFlags→ settings.json pipelinetests/uninstall-logic.test.ts: 4 realrunCleanupPhasecases (removes managed block, preserves custom org attribution, preserves block with extra keys, no-op when attribution absent) exercisingstripFlagsthrough the production uninstall pathR7 — CLAUDE.md (see below for diff summary)
suppress-attributionwith D27/D-ATTR-GUARD annotationR8 — Git skill attribution cleanup (user-approved)
Co-Authored-By: Claude <noreply@anthropic.com>trailer from the git skill's commit message format template (src/assets/skills/git/SKILL.md)SKILL.mdCo-Authored-Bytrailer from two commit examples insrc/assets/skills/git/references/patterns.md(fix pattern, breaking-change pattern)Generated with [Claude Code](https://claude.com/claude-code)footer from the PR HEREDOC example inreferences/patterns.mdsuppress-attributionflag instead of always carrying the skill's own trailerCLAUDE.md Changes (markdown-approval)
The following changes were made to
CLAUDE.mdas part of this implementation (R7). Please review and confirm:28 flags total→29 flags total, 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)afterenable-todo-toolsDocs changes (this PR, refs #315):
All 28 flags→All 29 flags; addedsuppress-attributionrow (boolean, settingattribution, defaultfalse) to the flags reference table(28 flags)→(29 flags)Breaking Changes
None. The flag defaults to
false(attribution not suppressed). Existing installs without the flag in their manifest will adoptfalseon nextdevflow init(ADR-014: absent key = adopt default).Users who had
{"commit":"","pr":""}in settings.json from the old template injection will see the flag seeded totrueon next init (settings.json shape detection).Reviewer Focus Areas
applyFlagsandstripFlags— thedeepEqualsPlainhelper must exactly match the devflow-managed shape and leave custom attribution untouchedresolveInitSeed— settings.json exact shape wins over manifest entry; custom attribution falls through to manifest; --reset empties settingsSnapshotshouldRunAttributionStep({mode, isTTY})isisTTY && mode === 'advanced'with nohasCliOverrideormodePromptShownparams; the divergence from the compliance gate is deliberate and test-pinnedconvergeFlagsIntoSettingsCo-Authored-Byno longer appears insrc/assets/skills/git/SKILL.mdorreferences/patterns.md; no tombstone notes presentRelated Issues
Closes #315