diff --git a/.devflow/decisions/decisions-ledger.jsonl b/.devflow/decisions/decisions-ledger.jsonl index bd5d294f..8e19dfe2 100644 --- a/.devflow/decisions/decisions-ledger.jsonl +++ b/.devflow/decisions/decisions-ledger.jsonl @@ -29,3 +29,4 @@ {"id":"obs_3vt99r","type":"pitfall","pattern":"Assuming a workflow capability does not exist without checking existing agents — the Evaluator already implements intent-vs-implementation comparison","evidence":["are you sure devflow doesn't already do this? isn't it exactly what the evaluator is doing?","You're right to push back — the Evaluator is doing intent-vs-implementation comparison. Let me be precise about what it already does vs what's actually new.","No production tool compares plan/spec intent against implementation. (Confirmed across all 3 research tracks.) — this claim was made before checking devflow's own Evaluator agent"],"details":"area: bug-analysis workflow design, research phase; issue: research concluded no tool performs plan-intent vs implementation comparison, then proceeded to design this as a new capability — without checking whether devflow's own Evaluator agent already does this; impact: wasted design effort and potential duplication; the Evaluator already receives ORIGINAL_REQUEST, EXECUTION_PLAN, FILES_CHANGED, ACCEPTANCE_CRITERIA and performs goal-backward verification; resolution: before designing any new capability that sounds like it overlaps with existing agents (Evaluator, Scrutinizer, Reviewer), explicitly check the existing agent roster and their input contracts first","count":1,"confidence":0.9,"quality_ok":true,"status":"created","created":"2026-05-23T21:17:01.106Z","last_seen":"2026-05-23T21:17:01.106Z","artifact_path":"/Users/dean/Sandbox/devflow/.devflow/decisions/pitfalls.md#PF-005","anchor_id":"PF-005","decisions_status":"Retired"} {"id":"obs_pfyb8b","type":"decision","pattern":"Dynamic workflow plugin ships as pure-instruction command recipes — markdown that teaches the model to author and run a dynamic workflow at runtime, with ZERO authored orchestration code (no parser, scheduler, topo-sort, or formula), now or ever","details":"context: the devflow-dynamic plugin (tickets->plan->build delivery pipeline) needed a build/runtime architecture; an L0 ticket-DAG parser (Kahn topological sort run via Bash and passed to the workflow as args) had been drafted into the design doc as the one programmatic dependency; decision: ship the dynamic commands as pure-instruction command recipes — markdown that instructs the main model how to author and run a Claude Code dynamic workflow at runtime — carrying ZERO deterministic code that devflow authors (no parser, scheduler, topo-sort, FP-ratio/cycle formulas); every judgment (which tickets are independent, wave ordering, parallel vs serial, review-cycle counts) is LLM reasoning at runtime, done by agents that read the GitHub issues and their Depends-on relationships with gh; the recipes are thin orchestrators over devflows ALREADY-installed agents (agentType resolves the real agent identity/skills/per-agent model tier, confirmed by spike F5), so no agent prompts are inlined; rationale: a pure-instruction recipe survives the moving dynamic-workflow API, adapts to arbitrary input, and distributes through the command channel devflow already ships, while any authored parser/formula becomes a brittle deterministic dependency the user categorically rejected (not now, not ever); this extends the LLM-vs-plumbing principle from artifact CONTENT to workflow ORCHESTRATION","anchor_id":"ADR-019","decisions_status":"Accepted","date":"2026-06-12"} {"id":"obs_10svdf","type":"decision","pattern":"In the dynamic-build pipeline, every Coder code mutation runs a post-code quality pipeline in fixed order Validate->Simplify->Scrutinize, the Evaluator runs ONLY when there is an implementation plan (not after fixes), and the Resolver is split — a Coder writes fixes while adversarial verification strips false positives before any fix is attempted","details":"context: defining the agent topology for the /devflow:dynamic-build recipe (a fusion of /implement + /code-review + /resolve); decision: (1) the Simplifier->Scrutinizer->Evaluator order is a load-bearing, non-negotiable invariant; (2) every Coder mutation (initial implement, resolve-fix, alignment-fix, qa-fix) runs a post-code pipeline of Validate->Simplify->Scrutinize, but the Evaluator runs ONLY when there is an implementation plan to verify against — it is skipped after plain fixes; the Tester is part of this gate; (3) the Resolver is split into two halves — its validate-the-issue-is-real half becomes an adversarial verification pass that strips false positives BEFORE any fix, and its write-the-fix half is handled by a Coder (a Coder loads far more relevant context than a Resolver); rationale: the Evaluators job is to confirm the PLAN was implemented properly, so it is meaningless without a plan and wasteful on every fix; a Coder produces better fixes than a Resolver because of the context it loads; gating every fix behind adversarial false-positive verification prevents wasting Coder effort on non-real findings; preserving the Simplify/Scrutinize order on every code mutation keeps the same quality dynamic the static /implement pipeline enforces","anchor_id":"ADR-020","decisions_status":"Accepted","date":"2026-06-12"} +{"id":"obs_cutline1","type":"pitfall","pattern":"Shell cut is line-oriented — using cut to extract a delimited field from a multi-line value silently corrupts the extraction, dropping all lines after the first","details":"area: scripts/hooks/dream-capture, shell string parsing with cut; issue: cut -d -f operates per-line — when the delimiter appears only on the first line of a multi-line string (e.g., SOH joining CWD and a multi-line assistant message), cut extracts the field correctly from line 1 but emits every subsequent line verbatim, corrupting the extracted value; in dream-capture this meant CWD became a multi-line string containing the assistant message body, which failed the directory check silently; impact: systemic — working memory broken machine-wide for ~8 days across all projects; assistant turns silently dropped on every multi-line response; only single-line responses slipped through, making the failure appear intermittent; resolution: replace cut with bash parameter expansion (${PAYLOAD%%$SOH*}) which operates on the whole string regardless of newlines; lesson: never use cut to split fields that may contain newlines — cut is a line-oriented tool and will silently produce wrong results on multi-line values","anchor_id":"PF-013","decisions_status":"Active"} diff --git a/.devflow/decisions/pitfalls.md b/.devflow/decisions/pitfalls.md index d8011828..10bf697e 100644 --- a/.devflow/decisions/pitfalls.md +++ b/.devflow/decisions/pitfalls.md @@ -1,4 +1,4 @@ - + # Known Pitfalls Area-specific gotchas, fragile areas, and past bugs. @@ -83,3 +83,12 @@ Area-specific gotchas, fragile areas, and past bugs. - **Resolution**: drop Guard B, and diagnose hook flakiness by reproducing through the real run-hook preamble path with a truth table that varies only the trailing ? to isolate the actual cause - **Status**: Active - **Source**: self-learning:obs_preambleq1 + +## PF-013: Shell cut is line-oriented — using cut to extract a delimited field from a multi-line value silently corrupts the extraction, dropping all lines after the first + +- **Area**: scripts/hooks/dream-capture, shell string parsing with cut +- **Issue**: cut -d -f operates per-line — when the delimiter appears only on the first line of a multi-line string (e.g., SOH joining CWD and a multi-line assistant message), cut extracts the field correctly from line 1 but emits every subsequent line verbatim, corrupting the extracted value +- **Impact**: systemic — working memory broken machine-wide for ~8 days across all projects +- **Resolution**: replace cut with bash parameter expansion (${PAYLOAD%%$SOH*}) which operates on the whole string regardless of newlines +- **Status**: Active +- **Source**: self-learning:obs_cutline1 diff --git a/.devflow/features/cli-rules/KNOWLEDGE.md b/.devflow/features/cli-rules/KNOWLEDGE.md index 51a60b67..423052c0 100644 --- a/.devflow/features/cli-rules/KNOWLEDGE.md +++ b/.devflow/features/cli-rules/KNOWLEDGE.md @@ -23,7 +23,7 @@ referencedFiles: - shared/rules/quality.md - shared/rules/reliability.md created: 2026-05-10 -updated: 2026-06-13 +updated: 2026-06-16 --- # Rules System CLI @@ -115,6 +115,32 @@ The `devflow-core-skills` plugin's `skills` array in `plugins.ts` registers the **Dual-role bare entries in `LEGACY_SKILLS_V2X`**: the bare entries `dream-decisions`, `dream-knowledge`, and `dream-curation` are still-active skills installed at `devflow:`. These bare entries in `LEGACY_SKILLS_V2X` exist only to clean up pre-namespace V2.x install directories (e.g., `~/.claude/skills/dream-decisions`). On current installs, the `fs.rm` targeting the bare path is a harmless no-op because active skills always install under the `devflow:` prefix. A block comment in `plugins.ts` documents this explicitly — do NOT remove these entries or V2.x upgraders will be left with stale bare-name dirs. +### Manifest: ManifestData and syncManifestFeature + +`ManifestData.features` now includes a `security` field (PR #244): + +```typescript +features: { + ambient: boolean; + memory: boolean; + hud: boolean; + knowledge: boolean; + decisions: boolean; + rules: boolean; + flags: string[]; + viewMode?: ViewMode; + security?: 'none' | 'user' | 'managed'; // Added PR #244 +} +``` + +The `security` field is optional (absent in pre-Phase-F manifests — treated as `undefined`/unknown by `resolveSecurityTriState`). `readManifest` validates it against the `SECURITY_MODES` union and defaults to `undefined` if invalid or absent. + +**`syncManifestFeature`** is the canonical pattern for toggle commands updating the manifest. It reads the existing manifest, updates one feature key, refreshes `updatedAt`, and writes atomically. The pattern: +```typescript +await syncManifestFeature(devflowDir, 'security', targetMode); +``` +Used by: `ambient --enable/--disable`, `decisions --enable/--disable`, `hud --enable/--disable`, `memory --enable/--disable`, `security --enable/--disable`. Never creates a manifest if one doesn't exist — it is a no-op when `readManifest` returns null. + ### Build Pipeline `scripts/build-plugins.ts` extends the skill/agent build to handle rules. The key difference from skills: rules are **flat files** (not directories), so no recursive copy is needed. The build script reads `plugin.json`'s `rules` array, clears and recreates the plugin's `rules/` directory, then copies each `shared/rules/{name}.md` into `plugins/{plugin}/rules/{name}.md`. The build fails with exit 1 if a declared rule is missing from `shared/rules/`. @@ -176,7 +202,7 @@ Two private helpers are top-level named functions in `rules.ts` (not inline): - `isShadowed(devflowDir, ruleName)` — `fs.access` on `~/.devflow/rules/{name}.md`; returns `Promise` - `formatRuleRow(name, devflowDir, ownerMap, suffix)` — builds a colorized display row; both `--status` and `--list` build their own `buildRulesMap(DEVFLOW_PLUGINS)` call locally and pass it in — there is no module-level constant. -## Init Flow Integration (Updated: Two-Step Plugin Selection) +## Init Flow Integration **Scope**: The interactive scope prompt was removed in feat(init). User scope is now the default for all TTY interactive runs. Only `--scope` flag or non-TTY path can set `local` scope. Non-TTY detects and logs "Non-interactive mode detected, using scope: user". @@ -203,8 +229,6 @@ Two exported pure functions power the loop: - `combineSelection(workflowSelected, languageSelected)` → `{ plugins: string[], accepted: boolean }` — merges the two arrays; `accepted` is true iff the union is non-empty. - `shouldRetry(attempt, maxAttempts, accepted)` → `boolean` — returns true iff the selection was empty AND the attempt ceiling has not been reached; returns false when accepted or exhausted (caller exits on false + !accepted). -Both functions are exported from `init.ts` for unit testing (same pattern as `parsePluginSelection`). Tests in `tests/init.test.ts` cover: accept-on-non-empty, empty-both-buckets, retry-exhaustion, and mid-loop iteration. - **WORKFLOW_ORDER is now exported from `plugins.ts`**: ```typescript export const WORKFLOW_ORDER: string[] = [ @@ -216,9 +240,11 @@ export const WORKFLOW_ORDER: string[] = [ ``` `init.ts` imports it from `plugins.ts` rather than keeping a local duplicate. A regression guard test in `tests/plugins.test.ts` verifies every entry has a real backing command in `DEVFLOW_PLUGINS` (bidirectional: WORKFLOW_ORDER ⊆ commands AND commands ⊆ WORKFLOW_ORDER for the non-excluded set). The 5 dynamic commands were added to WORKFLOW_ORDER in the same commit that landed the dynamic plugin — the regression guard catches future omissions. -**Agent Teams init flags removed (PR #240)**: The `--teams`/`--no-teams` init flags and `teamsEnabled` variable no longer exist. Users who want Agent Teams mode use `devflow flags --enable agent-teams` instead. +**Security mode in init (PR #244)**: `initCommand` now has a `--security ` flag (`user`/`managed`/`none`). The init flow detects the current deny list state (`detectDenyState`), reads the manifest mode, and resolves the final action via `resolveSecurityAction()` (a pure resolver). A dedicated security step after the managed-install step handles the actual deny list write to `~/.claude/settings.json`. The `securityMode` is written to `manifest.features.security` so subsequent `devflow list` and `devflow security --status` can surface it. The security step always uses atomic writes (`writeFileAtomicExclusive`) — both the strip and merge paths, not just the merge path. + +**Atomic writes for toggle commands (PR #244)**: `ambient --enable`, `memory --enable/--disable`, `hud --enable/--disable`, `uninstall --security` all now use `writeFileAtomicExclusive` for settings.json writes instead of plain `fs.writeFile`. This ensures Claude Code never reads a partially-written settings.json. -**Learning pipeline removed (PR #238)**: The `--learn`/`--no-learn` init flags, `learnEnabled` variable, and `features.learn` manifest field no longer exist. The self-learning step in the Advanced mode prompt was removed. Legacy hook scripts `eval-learning` and `eval-reinforce` are now in the init's legacy hook cleanup list alongside their removal from `dream-evaluate` sourcing. +**`--no-memory` queue drain (PR #244)**: when `--no-memory` is specified (or memory is disabled via Recommended/Advanced prompts), `init.ts` now drains orphaned queue files (`getPendingTurnsPath` + `getPendingTurnsProcessingPath`) via `Promise.all([fs.unlink, fs.unlink])`, mirroring the `memory.ts --disable` drain. ENOENT is treated as a no-op; other errors propagate. **Excluded from plugin selection buckets**: `devflow-core-skills` (always installed), `devflow-ambient` (always installed), `devflow-audit-claude` (installable via `--plugin` only). @@ -228,14 +254,64 @@ export const WORKFLOW_ORDER: string[] = [ **init → rules**: `rulesEnabled` flows through to `buildRulesMap(pluginsToInstall)` → `installViaFileCopy`. When disabled, post-install removes `~/.claude/rules/devflow/` entirely. -**uninstall → rules**: Full uninstall (`removeAllDevFlow`) includes `~/.claude/rules/devflow/` in its target list. Selective plugin uninstall (`computeAssetsToRemove`) computes which rules to remove using the same "retained by remaining plugins" logic as skills. `uninstall.ts` also calls `stripDevflowTeammateModeFromJson` to clean `teammateMode: "auto"` written by prior Devflow installs. +**uninstall → rules**: Full uninstall (`removeAllDevFlow`) includes `~/.claude/rules/devflow/` in its target list. Selective plugin uninstall (`computeAssetsToRemove`) computes which rules to remove using the same "retained by remaining plugins" logic as skills. `uninstall.ts` also calls `stripDevflowTeammateModeFromJson` to clean `teammateMode: "auto"` written by prior Devflow installs. The uninstall security step now also uses `writeFileAtomicExclusive` (PR #244). -**list → rules**: `devflow list` shows `rules` in the Features line when `manifest.features.rules` is true. +**list → rules**: `devflow list` shows `rules` in the Features line when `manifest.features.rules` is true. `devflow list` also surfaces `security` and `safe-delete` as tri-state values via `formatFeatures(manifest.features, { security, safeDelete })`. **build → install**: Rules are not installed from `shared/rules/` directly at runtime — the installer reads from `plugins/{plugin}/rules/`, which is the build output. Always run `npm run build` after modifying `shared/rules/` before testing install. Similarly, dynamic recipe commands are not installable from `shared/recipes/` — always run `npm run build:recipes` (or `npm run build`) first; the installer reads from `plugins/devflow-dynamic/commands/`. **plugins.ts → init.ts**: `partitionSelectablePlugins`, `WORKFLOW_ORDER`, `combineSelection`, `shouldRetry` are all exported from their respective modules and imported by `init.ts`. `combineSelection` and `shouldRetry` are in `init.ts` (not `plugins.ts`). +## New Commands (PR #244) + +### `devflow security` + +New standalone command in `src/cli/commands/security.ts` for lifecycle management of the security deny list. Subcommands: +- `--status` (default): reports installed locations and entry counts; flags dual-install anomaly; shows manifest mode if available +- `--enable [--managed|--user]`: add-to-target-first, verify, then strip-other (self-healing mode switch); updates manifest to `'user'` or `'managed'`; uses atomic writes +- `--disable`: strips from user settings (must be parseable JSON, hard fails otherwise) and removes managed settings unconditionally; updates manifest to `'none'`; uses atomic writes + +Pure helpers: +- `loadTemplateDenyEntries(rootDir)` — reads from `src/templates/managed-settings.json` bundled with the package +- `countDenyEntries(settingsJson: string)` — count of `permissions.deny` entries; returns 0 on parse error + +Key invariants: `--enable` always writes to only ONE location then strips the other (prevents dual-location installs). `--disable --user` fails hard when settings.json is unparseable (avoids corruption). + +### `devflow safe-delete` + +New command in `src/cli/commands/safe-delete.ts` for shell-profile safe-delete function management. Subcommands: +- `--status` (default): reports tri-state (`installed`/`outdated`/`absent`/`unknown`) + profile path +- `--enable`: idempotent install/upgrade of versioned shell block to profile; checks trash CLI availability first (platform-specific) +- `--disable`: removes block from profile; no-op if not installed + +**`getSafeDeleteStatus()`** is an exported async function that determines install status and profile path: +```typescript +export type SafeDeleteStatus = 'installed' | 'outdated' | 'absent' | 'unknown'; +export async function getSafeDeleteStatus(): Promise<{ status: SafeDeleteStatus; profilePath: string | null }> +``` +Used directly by `devflow list` (no subprocess) for the safe-delete tri-state. + +### `devflow list` — tri-state security + safe-delete + +`list.ts` now exposes `security` and `safe-delete` as tri-state values after regular boolean features: + +```typescript +export type TriState = 'on' | 'off' | 'unknown'; + +export function resolveSecurityTriState( + security: ManifestData['features']['security'], +): TriState // 'user'|'managed' → 'on', 'none' → 'off', absent → 'unknown' + +export function formatFeatures( + features: ManifestData['features'], + extra?: { security?: TriState; safeDelete?: TriState }, +): string +``` + +Order is: `rules → security → safe-delete` (last two are tri-state, appended after standard boolean features). When `extra.security === 'unknown'`, `formatFeatures` emits `security: unknown`; when `'off'`, emits `security: off`; when `'on'`, emits `security`. Same tri-label logic for safe-delete. + +Security fallback probe: if manifest field is absent (`resolveSecurityTriState` returns `'unknown'`), `list.ts` probes `getManagedSettingsPath()` via `fs.access` (wrapped in `try/catch` to survive unsupported platforms); if the path exists, security is considered `'on'`. + ## Constraints - Rules have no namespace prefix (unlike skills which install as `devflow:{name}/`). The directory `~/.claude/rules/devflow/` itself provides the namespace. @@ -255,7 +331,9 @@ export const WORKFLOW_ORDER: string[] = [ - **Unbounded plugin selection loop**: The bounded `while (attempts < MAX_ATTEMPTS)` + `shouldRetry` guard is the pattern — never replace with `while (true)`. - **Long rule files**: Rules should be ~10-15 lines. If a rule grows beyond ~20 lines, extract the detail into a skill's `references/` directory. - **Omitting `rules: []` on a plugin**: The `rules` field is required on `PluginDefinition`. Omitting it causes TypeScript errors at build time. -- **Adding a bespoke feature toggle to `manifest.features`**: Agent Teams was removed partly because it was a bespoke `teamsEnabled` field on the manifest. New optional toggleable Claude Code behaviours belong in `FLAG_REGISTRY` with `defaultEnabled: false`, not as custom manifest fields. +- **Adding a bespoke feature toggle to `manifest.features`**: Agent Teams was removed partly because it was a bespoke `teamsEnabled` field on the manifest. New optional toggleable Claude Code behaviours belong in `FLAG_REGISTRY` with `defaultEnabled: false`, not as custom manifest fields. The `security` field is a justified exception because it stores the selected mode (user/managed/none), not just on/off. +- **Inline read→mutate→write manifest patterns in toggle commands**: All toggle commands must use `syncManifestFeature` instead of the inline pattern. This consolidation (PR #244) eliminates a class of atomic-write omissions. +- **Using plain `fs.writeFile` for settings.json**: Always use `writeFileAtomicExclusive` (temp+rename). Claude Code reads settings.json every turn — a partial write during a crash causes Claude Code to boot with a corrupt settings. ## Gotchas @@ -274,14 +352,13 @@ export const WORKFLOW_ORDER: string[] = [ - **manifest.ts contains a `kb → knowledge` migration self-heal**: `readManifest` detects `features.kb` and migrates it to `features.knowledge` in-place. This is the only backward-compat code in `manifest.ts`; do not add more. For rules, `LEGACY_RULE_NAMES` is the correct pattern when renaming rule files. - **`features.learn` no longer exists in ManifestData**: The learning pipeline was removed in PR #238. `manifest.features.learn`, `--learn`/`--no-learn` init flags, and `learnEnabled` in `init.ts` are all gone. Two migrations (`purge-learning-pipeline-v1` per-project, `purge-learning-global-v1` global) in `src/cli/utils/migrations.ts` sweep legacy learning artifacts on `devflow init`. `eval-learning` and `eval-reinforce` hook scripts are removed and in the legacy hook cleanup list. - **`features.teams` no longer exists in ManifestData**: The agent-teams bespoke manifest field was removed in PR #240. Agent Teams is now a standard flag entry in `FLAG_REGISTRY` (`id: 'agent-teams'`, `defaultEnabled: false`). The env var `CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1` is applied/stripped by the normal `applyFlags`/`stripFlags` machinery. Migration `purge-devflow-teammate-mode-global-v1` (global) and `purge-devflow-teammate-mode-v1` (per-project) clean up stale `teammateMode: "auto"` written by prior installs. -- **`*-teams.md` command files are orphaned, not re-installed**: The blanket sweep in `init.ts` (`f.endsWith('-teams.md')`) runs on every install type including partial installs. This is safe because no `*-teams.md` command file is ever installed by current Devflow. Old installs that had these files will have them cleaned up on the next `devflow init`. -- **`dream-memory` skill is no longer active**: `dream-memory` was removed from `devflow-core-skills` skills array in PR #239 (the `dream-memory` SKILL.md file was removed in PR #238; the skills array entry was removed in PR #239). Memory refresh is handled by `background-memory-update` (detached worker), not a Dream subagent. Both `dream-memory` and `devflow:dream-memory` are in `LEGACY_SKILLS_V2X` and are swept by `devflow init`. Migration `purge-stale-memory-markers-v1` (added in PR #239) sweeps `dream/memory.*` marker files from old installations. +- **`manifest.features.security` is absent in pre-Phase-F manifests**: `readManifest` returns `undefined` for missing or invalid values. `resolveSecurityTriState(undefined)` returns `'unknown'`. All consumers of `security` must handle `undefined`. +- **`getSafeDeleteStatus()` never throws**: Wrapped in try/catch. Returns `{ status: 'unknown', profilePath: null }` on any error. Safe to call from `devflow list` without additional error handling. +- **`getWorkingMemoryDisabledSentinel` removed from `project-paths.cjs`**: The function was deleted (PR #244 cleanup). Any reference to `getWorkingMemoryDisabledSentinel` in hooks or plumbing is stale. +- **Security `--disable` hard-fails on unparseable JSON**: Unlike `--enable`, which can create a new settings.json, `--disable` must parse the existing settings.json to safely strip entries. If the file is present but unparseable, the command exits with an error — avoids corrupting the file by writing a stripped version that drops all settings. +- **`HUD --disable` self-heals lingering statusLine**: `hud --disable` always attempts to read and strip the statusLine from settings.json, regardless of `hud.json` config state. This self-heals drift where hud.json says disabled but a Devflow statusLine still lingers from a partial prior state. - **`DreamConfig` tracks 4 features**: `{memory, decisions, knowledge, autoCommit}` — the `learning` key was removed in PR #238; `autoCommit` was added in PR #241 (default `true`; governs whether Dream tasks auto-commit maintenance writes). Do not add `learning` back or reference `features.learn`. -- **New migration `sync-devflow-gitignore-v2`**: Per-project migration added in PR #238 to re-sync `.devflow/.gitignore` to the ignore-by-default allowlist policy. Overwrites existing `.gitignore` if content differs from canonical template. ENOENT-safe (no-op if `.devflow/` does not exist). -- **New migration `sync-devflow-gitignore-v3`**: Per-project migration added in PR #241 to push the gitignore update that explicitly allows `decisions-ledger.jsonl` (the committed ledger). Without this, projects that had `.gitignore` from `sync-devflow-gitignore-v2` would not track the ledger file. -- **New migration `decisions-ledger-unify-v1`**: Per-project migration added in PR #241. Backfills `decisions-ledger.jsonl` from the existing `decisions.md`/`pitfalls.md` and `decisions-log.jsonl`. Preserves every existing body verbatim via `raw_body`, synthesizes rows whose source obs is missing, marks hand-deleted anchors `Retired` (numbers reserved). Idempotent + crash-safe (always reconciles `.md` from ledger after write). Located in `src/cli/utils/decisions-ledger-migration.ts`. - **`devflow-dynamic` plugin is optional**: It installs only when the user selects it at `devflow init` (Step 1 — Workflow plugins) or passes `--plugin=dynamic`. It is not pre-selected. Its 5 recipe commands (`/dynamic-tickets`, `/dynamic-plan`, `/dynamic-build`, `/dynamic-wave`, `/dynamic-profile`) are compiled from `shared/recipes/*.mds` — not hand-authored `.md` files. -- **`COMMAND_REFS` in `tests/skill-references.test.ts` includes dynamic commands**: The test allowlist `COMMAND_REFS` contains `dynamic-tickets`, `dynamic-plan`, `dynamic-build`, `dynamic-profile`, `dynamic-wave`. When adding new recipe commands, add them to this set or the skill-references test will fail. ## Key Files @@ -289,20 +366,27 @@ export const WORKFLOW_ORDER: string[] = [ - `shared/recipes/` — source of truth for dynamic plugin recipe commands; `.mds` files compiled at build time; partials prefixed with `_` are not compiled to commands - `scripts/build-recipes.ts` — compiles `shared/recipes/*.mds` → `plugins/devflow-dynamic/commands/*.md`; hard-fails on any compile error; skips partials - `src/cli/plugins.ts` — `DEVFLOW_PLUGINS` `rules` field, `buildRulesMap()`, `getAllRuleNames()`, `isValidRuleName()`, `LEGACY_RULE_NAMES`, `WORKFLOW_ORDER` (now includes 5 dynamic commands), `partitionSelectablePlugins()`; active Dream skills: `dream-decisions`, `dream-knowledge`, `dream-curation` (NOT `dream-memory`); `LEGACY_SKILLS_V2X` includes `dream-memory`, `devflow:dream-memory`, and `devflow:agent-teams` for cleanup; `devflow-dynamic` is optional, command-bearing, workflow bucket -- `src/cli/commands/init.ts` — `rulesEnabled` flag; two-step plugin selection with `partitionSelectablePlugins`; `combineSelection`, `shouldRetry` pure helpers (exported for tests); `WORKFLOW_ORDER` import; Recommended-mode silent apply vs Advanced-mode note+confirm; `buildRulesMap(pluginsToInstall)`; `LEGACY_RULE_NAMES` stale-file cleanup loop; blanket `*-teams.md` command sweep; no `--learn`/`--no-learn` or `learnEnabled` (removed PR #238); no `--teams`/`--no-teams` or `teamsEnabled` (removed PR #240) +- `src/cli/commands/init.ts` — `rulesEnabled` flag; two-step plugin selection with `partitionSelectablePlugins`; `combineSelection`, `shouldRetry` pure helpers (exported for tests); `WORKFLOW_ORDER` import; Recommended-mode silent apply vs Advanced-mode note+confirm; `buildRulesMap(pluginsToInstall)`; `LEGACY_RULE_NAMES` stale-file cleanup loop; blanket `*-teams.md` command sweep; `--security ` flag + dedicated security step after managed install; `--no-memory` pending queue drain - `src/cli/commands/rules.ts` — `devflow rules` command (enable/disable/status/list) -- `src/cli/commands/ambient.ts` — purges legacy `commands.md` via `COMMANDS_RULE_PATH` / `removeLegacyCommandsRule()`; called unconditionally from `addAmbientHook` and `removeAmbientHook` so stale files are cleaned up on every enable/disable/init +- `src/cli/commands/ambient.ts` — purges legacy `commands.md` via `COMMANDS_RULE_PATH` / `removeLegacyCommandsRule()`; `ambient --enable` now uses `writeFileAtomicExclusive` + `syncManifestFeature` +- `src/cli/commands/security.ts` — NEW: `devflow security` command (status/enable/disable); `loadTemplateDenyEntries`, `countDenyEntries` pure helpers; atomic writes throughout +- `src/cli/commands/safe-delete.ts` — NEW: `devflow safe-delete` command (status/enable/disable); `getSafeDeleteStatus()` exported for `devflow list` +- `src/cli/commands/list.ts` — `TriState`, `resolveSecurityTriState()`, `formatFeatures(features, extra?)` with tri-state support for security + safe-delete; security fallback probe via `getManagedSettingsPath` +- `src/cli/commands/hud.ts` — `hud --disable` self-heals lingering statusLine unconditionally; uses `writeFileAtomicExclusive` + `syncManifestFeature` +- `src/cli/commands/memory.ts` — `memory --enable/--disable` use `writeFileAtomicExclusive` + `syncManifestFeature`; `memory --disable` drains pending queue files +- `src/cli/commands/decisions.ts` — `decisions --enable/--disable` use `syncManifestFeature` - `src/cli/utils/installer.ts` — `installRuleFile` (exported); `installViaFileCopy` rules section -- `src/cli/commands/uninstall.ts` — `computeAssetsToRemove` includes rules; `removeAllDevFlow` removes rules dir; `removeSelectedPlugins` removes per-rule files; calls `stripDevflowTeammateModeFromJson` to clean `teammateMode` -- `src/cli/utils/manifest.ts` — `ManifestData.features.rules` with `true` self-heal default; `features.learn` removed in PR #238; no `features.teams` (removed PR #240) +- `src/cli/commands/uninstall.ts` — `computeAssetsToRemove` includes rules; `removeAllDevFlow` removes rules dir; `removeSelectedPlugins` removes per-rule files; calls `stripDevflowTeammateModeFromJson` to clean `teammateMode`; unified security strip uses `writeFileAtomicExclusive` +- `src/cli/utils/manifest.ts` — `ManifestData.features.rules` with `true` self-heal default; `ManifestData.features.security?: 'none'|'user'|'managed'`; `syncManifestFeature` helper for atomic single-field updates; `features.learn` removed in PR #238; no `features.teams` (removed PR #240) - `src/cli/utils/flags.ts` — `FLAG_REGISTRY` with 18 entries including `agent-teams` (defaultEnabled: false); `applyFlags`/`stripFlags`/`getDefaultFlags`; `applyViewMode`/`stripViewMode` - `src/cli/utils/teammate-mode-cleanup.ts` — `stripDevflowTeammateModeFromJson` (pure, tolerant) and `stripDevflowTeammateMode` (file I/O wrapper); used by both migrations and uninstall -- `src/cli/utils/migrations.ts` — `purge-learning-pipeline-v1` (per-project) + `purge-learning-global-v1` (global) sweep legacy learning artifacts; `sync-devflow-gitignore-v2` re-syncs `.devflow/.gitignore` (PR #238); `sync-devflow-gitignore-v3` pushes gitignore change for decisions-ledger allowlist (PR #241); `purge-stale-memory-markers-v1` removes stale `dream/memory.*` markers; `purge-devflow-teammate-mode-global-v1` (global) + `purge-devflow-teammate-mode-v1` (per-project) remove `teammateMode: "auto"` from settings.json files; `decisions-ledger-unify-v1` (per-project) backfills `decisions-ledger.jsonl` from existing `.md` + log, preserving every body verbatim (`raw_body`), marking hand-deleted anchors `Retired`; applies ADR-002 +- `src/cli/utils/migrations.ts` — PR #238: `purge-learning-pipeline-v1` + `purge-learning-global-v1` + `sync-devflow-gitignore-v2`; PR #239: `purge-stale-memory-markers-v1`; PR #240: `purge-devflow-teammate-mode-global-v1` + `purge-devflow-teammate-mode-v1`; PR #241: `decisions-ledger-unify-v1` + `sync-devflow-gitignore-v3` - `scripts/build-plugins.ts` — build-time distribution from `shared/rules/` → `plugins/*/rules/` - `tests/plugins.test.ts` — `partitionSelectablePlugins` (8 cases) + `WORKFLOW_ORDER` regression guard (4 cases, bidirectional) + `LEGACY_SKILL_NAMES consistency` guard; `allowedOptional` set includes `devflow-dynamic` - `tests/init.test.ts` — `combineSelection` and `shouldRetry` unit tests -- `tests/skill-references.test.ts` — `COMMAND_REFS` set includes dynamic commands (`dynamic-tickets`, `dynamic-plan`, `dynamic-build`, `dynamic-profile`, `dynamic-wave`) -- `tests/teammate-mode-cleanup.test.ts` — `stripDevflowTeammateModeFromJson` and `stripDevflowTeammateMode` tests +- `tests/list-logic.test.ts` — `resolveSecurityTriState` suite (on/off/unknown cases); `formatFeatures` with extras (8 cases covering all tri-states); order-asserting test for rules→security→safe-delete ordering +- `tests/safe-delete-command.test.ts` — `getSafeDeleteStatus` and safe-delete command tests +- `tests/manifest.test.ts` — `readManifest` security field round-trip and self-heal tests ## Related @@ -310,10 +394,9 @@ export const WORKFLOW_ORDER: string[] = [ - ADR-012: `.devflow` knowledge committed to git — governs feature knowledge storage; rules themselves install outside the repo to `~/.claude/rules/devflow/` - Skills system (parallel architecture): `src/cli/utils/installer.ts` `installViaFileCopy` skills section is the model rules followed - Feature flags: `src/cli/utils/flags.ts` — another toggleable feature using the same manifest.features pattern; now the home for `agent-teams` after bespoke pipeline removal -- Ambient simplification (c51114d): introduced `commands.md` rule + ambient-managed separation -- Init flow simplification (5143d73–154899b): two-step selection, `partitionSelectablePlugins`, `WORKFLOW_ORDER` export, `combineSelection`/`shouldRetry` - PR #238 (learning removal): removed `dream-memory` SKILL.md file, removed `features.learn`, removed `--learn`/`--no-learn`, added `purge-learning-pipeline-v1` + `sync-devflow-gitignore-v2` migrations; note — `dream-memory` was removed from the `devflow-core-skills` skills ARRAY in PR #239 (not #238) - PR #239 (eager memory refresh): removed `dream-memory` from `devflow-core-skills` skills array, added `background-memory-update` worker, added `purge-stale-memory-markers-v1` migration; `background-memory-update` is NOT in `LEGACY_HOOK_FILES` (fixed in `8c157db`) - PR #240 (agent-teams removal): removed bespoke Agent Teams machinery; re-exposed as `agent-teams` flag in `FLAG_REGISTRY`; added `purge-devflow-teammate-mode-global-v1` + `purge-devflow-teammate-mode-v1` migrations; blanket `*-teams.md` sweep in `init.ts`; `devflow:agent-teams` skill in `LEGACY_SKILLS_V2X` - PR #241 (decisions ledger + deterministic render): added `decisions-ledger.jsonl` as committed render source of truth; new `assign-anchor`/`retire-anchor`/`rotate-observations` ops in `json-helper.cjs`; `dream-commit` shell helper for attributable maintenance commits; `autoCommit: boolean` added to `DreamConfig`; `devflow decisions --status` now surfaces auto-commit state; `decisions-ledger-unify-v1` + `sync-devflow-gitignore-v3` per-project migrations added - PR #242 (devflow-dynamic plugin): added `devflow-dynamic` optional plugin with 5 recipe commands compiled from `shared/recipes/*.mds` via `scripts/build-recipes.ts`; `WORKFLOW_ORDER` extended with 5 dynamic commands; `devflow-dynamic` added to `allowedOptional` in `tests/plugins.test.ts`; `COMMAND_REFS` in `tests/skill-references.test.ts` updated with dynamic command names +- PR #244 (close feature-toggle gaps): added `devflow security` + `devflow safe-delete` commands; `manifest.features.security`; `syncManifestFeature` helper; `TriState` + `resolveSecurityTriState` in `list.ts`; `formatFeatures` extended with extra params; `hud --disable` self-healing; `ambient`/`memory`/`hud`/`decisions` toggle commands converted to `writeFileAtomicExclusive` + `syncManifestFeature`; `--no-memory` init queue drain; `--security` init flag + dedicated security step; `getWorkingMemoryDisabledSentinel` removed from `project-paths.cjs` diff --git a/.devflow/features/decisions/KNOWLEDGE.md b/.devflow/features/decisions/KNOWLEDGE.md index fbd24e68..34a1d44d 100644 --- a/.devflow/features/decisions/KNOWLEDGE.md +++ b/.devflow/features/decisions/KNOWLEDGE.md @@ -17,7 +17,7 @@ referencedFiles: - shared/skills/dream-decisions/SKILL.md - shared/skills/dream-curation/SKILL.md created: 2026-06-10 -updated: 2026-06-11 +updated: 2026-06-16 --- # Decisions & Pitfalls Ledger @@ -121,7 +121,7 @@ Three ledger-mutating operations (all run from `process.cwd()` as project root): - Caller-locked: the Dream agent acquires `.observations.lock` externally before calling this - Passthrough for ledger fields: `anchor_id`, `date`, `decisions_status`, `amendments`, `raw_body` -**`count-active `** — Reads ledger; returns count of active anchored rows. Note: unlike `assign-anchor`, `retire-anchor`, and `rotate-observations` (which derive project root from `process.cwd()`), `count-active` requires the worktree path as `args[0]` and the type as `args[1]` — always call as `count-active "$(pwd)" "decision"` or `count-active "$(pwd)" "pitfall"`. The `devflow:dream-curation` SKILL.md example shows `count-active "decision"` (single arg), which would resolve `"decision"` as a filesystem path — use `"$(pwd)"` as the first arg in practice. +**`count-active `** — Reads ledger; returns count of active anchored rows. Unlike `assign-anchor`, `retire-anchor`, and `rotate-observations` (which derive project root from `process.cwd()`), `count-active` requires the worktree path as `args[0]` and the type as `args[1]` — always call as `node "$HOME/.devflow/scripts/hooks/json-helper.cjs" count-active "$(pwd)" "decision"` or `"$(pwd)" "pitfall"`. The `devflow:dream-curation` SKILL.md example shows `count-active "decision"` (single arg), which would resolve `"decision"` as a filesystem path — use `"$(pwd)"` as the first arg in practice. ### Locking Discipline (ADR-017) @@ -159,6 +159,8 @@ The Dream SKILL for curation (`shared/skills/dream-curation/SKILL.md`) defines p - Calls `retire-anchor` once per entry (each self-locks `.decisions.lock` — do NOT hold lock across multiple calls, that would deadlock) - Calls `dream-commit curation "" ` after all retirements +**Curation depends on decisions** (PR #244): `eval-curation` is now gated by `DECISIONS_ENABLED` — it returns immediately (using `return 0`, not `exit`) without touching `.curation-last` when decisions is disabled. `dream-collect-tasks` Pass 1 also sweeps curation markers when decisions is disabled. This prevents: (a) stray curation markers triggering an opus spawn when decisions is off, and (b) disabling decisions burning the 7-day curation suppression window on re-enable. + ### dream-commit Shell helper that stages only the allowed paths and commits with structured trailers: diff --git a/.devflow/features/hooks/KNOWLEDGE.md b/.devflow/features/hooks/KNOWLEDGE.md index c5f7985a..90195a67 100644 --- a/.devflow/features/hooks/KNOWLEDGE.md +++ b/.devflow/features/hooks/KNOWLEDGE.md @@ -1,9 +1,9 @@ --- feature: hooks name: Dream & Hooks System -description: "Use when modifying dream hooks, background maintenance, marker lifecycle, memory/decisions/knowledge/curation processing, or per-task dream skills. Keywords: dream, hooks, background processor, merge-observation, assign-anchor, retire-anchor, rotate-observations, render-decisions, decisions-ledger, marker, .processing, SessionStart, dream-capture, background-memory-update, dream-evaluate, dream-decisions, dream-knowledge, dream-curation." +description: "Use when modifying dream hooks, background maintenance, marker lifecycle, memory/decisions/knowledge/curation processing, or per-task dream skills. Keywords: dream, hooks, background processor, merge-observation, assign-anchor, retire-anchor, rotate-observations, render-decisions, decisions-ledger, marker, .processing, SessionStart, dream-capture, background-memory-update, dream-evaluate, dream-decisions, dream-knowledge, dream-curation, dream-commit, autoCommit." category: architecture -directories: ["scripts/hooks/", "shared/agents/"] +directories: ["scripts/hooks/", "shared/agents/", "shared/skills/"] referencedFiles: - scripts/hooks/lib/feature-knowledge.cjs - scripts/hooks/lib/decisions-index.cjs @@ -19,13 +19,15 @@ referencedFiles: - scripts/hooks/eval-curation - scripts/hooks/session-start-memory - scripts/hooks/session-start-context + - scripts/hooks/dream-commit - shared/agents/dream.md - shared/skills/dream-decisions/SKILL.md - shared/skills/dream-knowledge/SKILL.md - shared/skills/dream-curation/SKILL.md - src/cli/commands/decisions.ts + - src/cli/utils/dream-config.ts created: 2026-06-01 -updated: 2026-06-11 +updated: 2026-06-16 --- # Dream & Hooks System @@ -44,7 +46,7 @@ Three active task types can be pending at any session start: `decisions`, `knowl | Hook / Worker | Trigger | Role | |---------------|---------|------| -| `dream-capture` | Stop (per turn) | Append assistant turn to queue; spawn `background-memory-update` worker after 120s throttle | +| `dream-capture` | Stop (per turn) | Append assistant turn to queue; spawn `background-memory-update` worker after 120s throttle; run decisions usage scanner | | `background-memory-update` | Spawned by dream-capture | Drain queue → `claude -p haiku` → rewrite WORKING-MEMORY.md with git stamp | | `dream-evaluate` | SessionEnd | Source eval-* modules; write per-session markers for decisions/knowledge/curation | | `session-start-context` | SessionStart | Recover stale `.processing`, collect pending markers, emit per-task DREAM MAINTENANCE directives | @@ -136,7 +138,37 @@ No raw UNPROCESSED TURNS dump in session-start-memory (removed). The background Queue overflow: capped at 200 lines, truncated to 100 under a mkdir-based lock to prevent multi-session truncation races. -Decisions usage scanner (`decisions-usage-scan.cjs`) also runs in `dream-capture` — it increments cite counts in `.decisions-usage.json` when the assistant response contains `ADR-NNN` or `PF-NNN` references. This is independent of memory state. +Decisions usage scanner (`decisions-usage-scan.cjs`) also runs in `dream-capture` — it increments cite counts in `.decisions-usage.json` when the assistant response contains `ADR-NNN` or `PF-NNN` references. **Dual-signal gate (PR #244)**: the scanner is now skipped when decisions is disabled via EITHER dream config (`decisions: false`) OR the `.disabled` sentinel — mirrors the same dual-signal check used in `dream-evaluate`. A supplementary guard (D29) skips the Node subprocess entirely when no `ADR-[0-9]+|PF-[0-9]+` pattern is found in the response (~50-100ms savings). + +## Curation Gate: Decisions-Dependency (PR #244) + +Curation depends on the decisions pipeline — it curates ADR/PF entries that decisions writes. Two new gates enforce this dependency: + +**`eval-curation` gate**: before writing the `.curation-last` throttle file, `eval-curation` checks `DECISIONS_ENABLED`. If decisions is disabled, it returns immediately (using `return 0`, never `exit` — it is sourced under `set -e` inside the orchestrator). This prevents disabling decisions from accidentally burning the 7-day curation suppression window. + +**`dream-collect-tasks` curation sweep**: Pass 1 now includes a `curation)` branch — when decisions is disabled (`dec_enabled != "true"`), curation markers are deleted unconditionally, same as `learning.*` and `memory.*`. This prevents stale curation markers from triggering an opus spawn when decisions is disabled. + +**`dream-evaluate` dual-signal gate**: after reading `DECISIONS_ENABLED` from dream config, a `[ -f "$DECISIONS_DIR_DATA/.disabled" ]` check can override it to `"false"`. This ensures both config-based disabling and sentinel-based disabling suppress marker writes — the same gate is mirrored in `dream-capture` for the usage scanner. + +## dream-capture: SOH-Delimited Field Parsing + +`dream-capture` parses `cwd` and `last_assistant_message` from the Stop hook JSON input in a single subprocess, using ASCII SOH (U+0001, `$'\001'`) as the binary delimiter between fields: + +```bash +_FIELDS=$(printf '%s' "$INPUT" | jq -r '(.cwd // "") + "" + (.last_assistant_message // "")') +CWD="${_FIELDS%%$'\001'*}" +ASSISTANT_MSG="${_FIELDS#*$'\001'}" +``` + +**Why SOH and not `cut`**: `cut` is line-oriented — a multi-line `last_assistant_message` would split across newlines, causing the second and subsequent lines to be misinterpreted as `CWD`. This caused a silent drop of every multi-line assistant turn (the `[ ! -d "$CWD" ]` guard would fail on the garbled value). The fix (commit 495b2d0) replaced `cut` with parameter expansion, which is newline-safe and bash 3.2 compatible. + +`dream-dispatch` (UserPromptSubmit) uses the same pattern via `json_extract_cwd_prompt()` in `json-parse`, which outputs `cwd + SOH + prompt`. Both hooks now use the identical split idiom: +```bash +CWD="${FIELDS%%$'\001'*}" +PROMPT_OR_MSG="${FIELDS#*$'\001'}" +``` + +**Diagnostic signal**: if `dream-capture` silently drops all multi-line assistant turns (memory queue stays empty despite active sessions), suspect a regression to line-oriented parsing. Look for `cut -d$'\001'` or `@tsv` patterns near the field-split. ## Transcript Channels (transcript-filter.cjs) @@ -182,13 +214,13 @@ Two key plumbing operations in `json-helper.cjs` handle all observation writes: - Caller holds `.devflow/dream/.observations.lock` (mkdir-based) EXTERNALLY — lock acquired by the per-task skill around the Bash call, then released. Never held across tool calls. - Writes atomically via temp+mv with O_EXCL flag -**`assign-anchor `** — anchor an observation into the committed ledger (replaces the removed `decisions-append`): +**`assign-anchor `** — anchor an observation into the committed ledger (replaces the removed `decisions-append`): - Assigns the next `ADR-NNN` or `PF-NNN` anchor (max+1 over all anchored rows incl. Retired; ADR/PF numbered independently; 3-digit pad). Retired numbers are reserved, never resurrected. -- Appends a projected row (`toLedgerRow`: id, type, pattern, details, anchor_id, decisions_status, date?, raw_body?, amendments?) to `decisions-ledger.jsonl`, then deterministically renders `decisions.md`/`pitfalls.md` from the ledger via `render-decisions.cjs` (active entries only — Deprecated/Superseded/Retired are dropped from the `.md`). +- Appends a projected row to `decisions-ledger.jsonl`, then deterministically renders `decisions.md`/`pitfalls.md` from the ledger via `render-decisions.cjs` (active entries only — Deprecated/Superseded/Retired are dropped from the `.md`). - Acquires `.devflow/decisions/.decisions.lock` INTERNALLY — self-locking; asserts the anchor isn't already present and the obs isn't already anchored. - Never call from a context that already holds `.decisions.lock` (deadlock). -**`retire-anchor `** — recoverable removal: +**`retire-anchor `** — recoverable removal: - Flips the row's `decisions_status` (e.g. → `Retired`/`Deprecated`/`Superseded`) and re-renders. The ledger row is never deleted, so the entry is recoverable; the rendered `.md` simply drops it. Errors if the anchor_id isn't found. Self-locks `.decisions.lock`. **`rotate-observations`** — log bound: @@ -217,7 +249,7 @@ The boundary is strict: ## Curation (eval-curation + dream-curation skill) -`eval-curation` (sourced by `dream-evaluate`) writes a `curation.{session}.json` marker, throttled to once every 7 days via `.devflow/dream/.curation-last` epoch file. The `devflow:dream-curation` skill (loaded by the Dream agent) then: +`eval-curation` (sourced by `dream-evaluate`) writes a `curation.{session}.json` marker, throttled to once every 7 days via `.devflow/dream/.curation-last` epoch file. Curation is now **gated by `DECISIONS_ENABLED`** (PR #244): if decisions is disabled, `eval-curation` returns immediately (using `return 0` not `exit`) without touching `.curation-last`, so the suppression window is not burned. The `devflow:dream-curation` skill (loaded by the Dream agent) then: - Reads `.decisions-usage.json` directly for cite counts (never calls `decisions-usage-scan.cjs`) - Retires/deprecates/supersedes by calling `retire-anchor ` — flips `decisions_status` on the ledger row and re-renders both `.md` files atomically. The `.md` files are rendered views of the ledger and are NEVER hand-edited; retired entries vanish from the `.md` but survive (recoverable) in the committed ledger. @@ -242,7 +274,7 @@ Note: `.curation-last` lives in `.devflow/dream/` (not `.devflow/decisions/`), c ## Dream Config -The sole source of truth for feature enabled-state is `.devflow/dream/config.json` (ADR-001 clean break — there is no runtime fallback). `DreamConfig` is `{memory, decisions, knowledge, autoCommit}` (`src/cli/utils/dream-config.ts`); the legacy `learning` field has been removed, and `coerceConfig` silently drops it when reading old configs. The `autoCommit` field (added PR #241, default `true`) controls whether Dream tasks auto-commit maintenance writes; `dream-commit` reads it at runtime via jq or `json-helper.cjs get-field-file`. Legacy `.devflow/sidecar/config.json` files are migrated to `dream/config.json` once at `devflow init` time by the `rename-sidecar-to-dream-v1` migration — the hooks do **not** read the sidecar path at runtime. (The old transitional `# dream-fallback: REMOVE after one release` read of `sidecar/config.json` has been removed from all hooks; do not reintroduce it.) +The sole source of truth for feature enabled-state is `.devflow/dream/config.json` (ADR-001 clean break — there is no runtime fallback). `DreamConfig` is `{memory, decisions, knowledge, autoCommit}` (`src/cli/utils/dream-config.ts`); the legacy `learning` field has been removed, and `coerceConfig` silently drops it when reading old configs. The `autoCommit` field (added PR #241, default `true`) controls whether Dream tasks auto-commit maintenance writes; `dream-commit` reads it at runtime via jq or `json-helper.cjs get-field-file`. Legacy `.devflow/sidecar/config.json` files are migrated to `dream/config.json` once at `devflow init` time by the `rename-sidecar-to-dream-v1` migration — the hooks do **not** read the sidecar path at runtime. ## Anti-Patterns @@ -255,11 +287,15 @@ The sole source of truth for feature enabled-state is `.devflow/dream/config.jso - **Using `decisions-usage-scan.cjs` to read cite counts** — it is a write-path tool that increments counts from session text. Read `.decisions-usage.json` directly for reporting or curation decisions. - **Writing artifact content in deterministic scripts** — observations, ADR/PF bodies, and knowledge bases must be authored by the LLM Dream agent via per-task skills; WORKING-MEMORY.md is authored by the LLM inside the `background-memory-update` worker; plumbing scripts handle only structural writes (ADR-008). - **Expecting USER_SIGNALS to feed any active pipeline** — the learning pipeline is gone. `transcript-filter.cjs` still emits a `user-signals` op but nothing consumes it. Only `dialog-pairs` (decisions) is active. +- **Using line-oriented `cut` to split multi-field jq output** — `cut` splits on newlines, so a multi-line field value (e.g. `last_assistant_message`) will bleed body lines into the preceding field. Use SOH delimiter with bash parameter expansion instead (fixed in 495b2d0). +- **Calling `eval-curation` return 0 as exit** — `eval-curation` is sourced under `set -e` inside `dream-evaluate`. Using `exit` would kill the whole orchestrator process. Always use `return 0` when short-circuiting from a sourced file. +- **Expecting curation to run when decisions is disabled** — curation is not an independent feature. Both `eval-curation` (write-time gate) and `dream-collect-tasks` (collect-time sweep) enforce this dependency. Disabling decisions suppresses curation too. ## Gotchas - **PF-006**: Claude Code renamed `response_text` → `last_assistant_message` in the Stop hook JSON silently (mid-May 2026). All 3+ projects had frozen memory for weeks. After any Claude Code version update, verify hook input field names against current docs. `dream-capture` now reads `last_assistant_message`; if Claude Code changes the API again, this will break silently — always test hook field presence after upgrades. - **PF-007**: Source is `scripts/hooks/`; installed is `~/.devflow/scripts/hooks/`. Editing installed copies creates repo divergence and the changes are overwritten on next `devflow init`. Always source-first. +- **Multi-line assistant message parsing (fixed 495b2d0)**: `dream-capture` previously used `cut -d$'\001'` to split `cwd + SOH + last_assistant_message`. `cut` is line-oriented: multi-line messages produced multiple lines of output, making the second line of `last_assistant_message` the apparent CWD. The `-d "$CWD"` guard would fail silently, dropping every multi-line turn from the memory queue. Fixed by switching to `${_FIELDS%%$'\001'*}` / `${_FIELDS#*$'\001'}` parameter expansion (bash 3.2 compatible, newline-safe). - **set -e / no-abort discipline** — `dream-capture` and `dream-evaluate` use `set -e`. However, `session-start-context` intentionally omits `set -e` because its two sections (1.5 decisions TL;DR and 2 dream pending-work) are independent — a failure in one must not prevent the other from running. Never add `set -e` to `session-start-context`. - **Background session guards** — all three hooks check `DEVFLOW_BG_UPDATER`, `DEVFLOW_BG_LEARNER`, `DEVFLOW_BG_KNOWLEDGE_REFRESH` env vars at the top and exit immediately to prevent feedback loops when the hook fires inside a background agent's subprocess. - **Atomic writes everywhere** — all marker and state file writes use `tmp.$$` (PID-unique) + atomic `mv`. The `json-helper.cjs` uses `writeExclusive` (O_EXCL flag) for temp files to prevent TOCTOU symlink attacks. @@ -268,20 +304,24 @@ The sole source of truth for feature enabled-state is `.devflow/dream/config.jso - **Per-task skill model override** — `shared/agents/dream.md` has `model: sonnet` in its frontmatter, but `session-start-context` overrides this per spawn via `dream_build_spawn_directive`. The agent frontmatter model is never actually used in production; the spawn-time model is authoritative. - **`dream_build_spawn_directive` communicates via global** — uses `_DREAM_DIRECTIVE` global variable (not stdout) so exact directive bytes including trailing newlines survive intact; command substitution would strip them. - **eval-decisions daily cap blocks late-session writes** — at 3 runs/day (default), decisions markers stop being written mid-session. No marker = no Dream agent spawn for decisions that session. Configurable via `.devflow/decisions/decisions.json` → `max_daily_runs`. +- **Decisions usage scanner dual-signal (PR #244)** — the scanner in `dream-capture` checks both config (`decisions: false`) AND the `.disabled` sentinel. Previously it only checked the sentinel. Any code reading the usage scanner gate must check both signals. +- **Curation gate is `return 0`, not `exit 0`** — `eval-curation` is sourced (not executed) under `set -e` inside `dream-evaluate`. Using `exit` would kill the entire `dream-evaluate` orchestrator, not just the curation eval. All short-circuits in sourced eval modules must use `return`. +- **`dream-collect-tasks` curation pass-through gone** — curation no longer blindly passes through when decisions is disabled. The prior comment "curation and unknown types: pass through unchanged" is outdated. A curation marker found when `dec_enabled != "true"` is now deleted in Pass 1. ## Key Files -- `scripts/hooks/dream-capture` — Stop hook; queue append + spawns `background-memory-update` worker (120s throttle); decisions usage scanner (independent of memory) +- `scripts/hooks/dream-capture` — Stop hook; queue append + spawns `background-memory-update` worker (120s throttle); decisions usage scanner with dual-signal gate (config OR sentinel); SOH-delimited `cwd`+`last_assistant_message` parsing via parameter expansion - `scripts/hooks/background-memory-update` — detached `claude -p haiku` worker: claims `.pending-turns.jsonl` → `.pending-turns.processing` atomically, calls `claude -p` with Write permission, verifies stamp on line 1, touches `.last-refresh-ok` on success; uses 300s-stale mkdir lock at `.working-memory.lock/` -- `scripts/hooks/dream-dispatch` — UserPromptSubmit hook; capture-only (user turn append to pending-turns queue); no directive emission -- `scripts/hooks/dream-evaluate` — SessionEnd hook; orchestrator sourcing eval-helpers + eval-decisions + eval-knowledge + eval-curation (eval-learning and eval-reinforce removed); exports shared vars to eval modules via orchestrator contract +- `scripts/hooks/dream-dispatch` — UserPromptSubmit hook; capture-only (user turn append to pending-turns queue); no directive emission; uses `json_extract_cwd_prompt()` from `json-parse` for SOH-delimited field split +- `scripts/hooks/dream-evaluate` — SessionEnd hook; orchestrator sourcing eval-helpers + eval-decisions + eval-knowledge + eval-curation; dual-signal gate for `DECISIONS_ENABLED` (config + sentinel override) - `scripts/hooks/eval-decisions` — sourced by dream-evaluate; daily-cap check (default 3/day via `.decisions-runs-today`); extracts dialog pairs via transcript-filter.cjs; writes `decisions.{session}.json` marker - `scripts/hooks/eval-knowledge` — sourced by dream-evaluate; 2-hour throttle (`.knowledge-last-refresh`); queries `feature-knowledge.cjs stale-slugs`; writes `knowledge.{session}.json` marker; optimistically updates throttle -- `scripts/hooks/eval-curation` — sourced by dream-evaluate; writes `curation.{session}.json` marker on 7-day throttle; `.curation-last` in `.devflow/dream/` +- `scripts/hooks/eval-curation` — sourced by dream-evaluate; gated by `DECISIONS_ENABLED` (returns immediately without burning `.curation-last` when decisions disabled); writes `curation.{session}.json` marker on 7-day throttle - `scripts/hooks/dream-recover` — sourced helper; stale `.processing` recovery per-type thresholds; JUST_RECOVERED guard; orphaned pending-turns recovery -- `scripts/hooks/dream-collect-tasks` — 3-arg sourced helper; two-pass design: Pass 1 unconditional sweep (deletes `learning.*` + `memory.*` + disabled-feature markers), Pass 2 type accumulation; `dream_build_spawn_directive` function; COLLECT_LIMIT=50 FIFO +- `scripts/hooks/dream-collect-tasks` — 3-arg sourced helper; two-pass design: Pass 1 unconditional sweep (deletes `learning.*` + `memory.*` + disabled-feature markers + curation markers when decisions disabled), Pass 2 type accumulation; `dream_build_spawn_directive` function; COLLECT_LIMIT=50 FIFO - `scripts/hooks/session-start-context` — SessionStart hook (no set -e); two independent sections: 1.5 decisions TL;DR, 2 per-task dream spawn directives (calls `dream_build_spawn_directive`) - `scripts/hooks/json-helper.cjs` — plumbing ops: `merge-observation`, `assign-anchor`, `retire-anchor`, `rotate-observations`, `read-dream`, atomic writes; does NOT contain judgment logic +- `scripts/hooks/json-parse` — sourced by all hooks; provides `json_field`, `json_field_file`, `json_extract_cwd_prompt` and friends; `json_extract_cwd_prompt` uses SOH delimiter + node/jq fallback for safe multi-field extraction - `scripts/hooks/lib/transcript-filter.cjs` — two-channel filter: USER_SIGNALS (orphaned, unused) + DIALOG_PAIRS (active, decisions only) - `scripts/hooks/lib/staleness.cjs` — annotates log entries with `mayBeStale` based on file existence; signal-only (no CLI display surface) - `scripts/hooks/lib/feature-knowledge.cjs` — KB index, staleness checks (`checkAllStaleness` batches all KBs in one git log call), `updateIndex`, `stale-slugs` CLI op, slug validation diff --git a/.devflow/features/index.json b/.devflow/features/index.json index 8f7a4b2e..16093727 100644 --- a/.devflow/features/index.json +++ b/.devflow/features/index.json @@ -29,7 +29,7 @@ "shared/rules/quality.md", "shared/rules/reliability.md" ], - "lastUpdated": "2026-06-13T09:31:19.557Z", + "lastUpdated": "2026-06-16T12:28:06.273Z", "createdBy": "devflow-knowledge" }, "hooks": { @@ -63,7 +63,7 @@ "src/cli/commands/decisions.ts", "src/cli/utils/dream-config.ts" ], - "lastUpdated": "2026-06-11T20:00:13.078Z", + "lastUpdated": "2026-06-16T12:27:57.174Z", "createdBy": "implement" }, "decisions": { @@ -88,7 +88,7 @@ "shared/skills/dream-decisions/SKILL.md", "shared/skills/dream-curation/SKILL.md" ], - "lastUpdated": "2026-06-11T20:00:20.339Z", + "lastUpdated": "2026-06-16T12:28:13.817Z", "createdBy": "implement" } } diff --git a/CLAUDE.md b/CLAUDE.md index 772770e3..a626b325 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,7 +43,7 @@ Plugin marketplace with 22 plugins (12 core + 9 optional language/ecosystem + 1 **LLM-vs-plumbing principle**: The LLM does all detection, semantic matching, materialization, and curation. Deterministic code is plumbing only: hooks, locks, throttles, file I/O, id-keyed JSONL records, `assign-anchor`/`retire-anchor` ledger numbering, `render-decisions` rendering, and `merge-observation` writes. No detection or judgment logic lives in shell or TypeScript. -**Working Memory**: Three shell-script hooks (`scripts/hooks/`) replace the old 8-hook system with a background-maintenance (Dream) architecture. Toggleable via `devflow memory --enable/--disable/--status` or `devflow init --memory/--no-memory`. Feature state is stored in `.devflow/dream/config.json` (primary source of truth); runtime sentinel `.devflow/memory/.working-memory-disabled` provides defense-in-depth. `dream-capture` (Stop hook) — captures user/assistant turns to `.devflow/memory/.pending-turns.jsonl` queue; after the 120s throttle (keyed by `.working-memory-last-trigger` mtime), spawns `background-memory-update` as a detached `nohup` worker (`claude -p --model haiku`); touches `.working-memory-last-trigger` BEFORE spawning to prevent double-spawn; uses mkdir-based locking for queue overflow truncation across concurrent sessions. `background-memory-update` (Stop-hook worker, not a hook itself) — drains `.pending-turns.jsonl`, calls `claude -p` (prompt on stdin, never argv), rewrites `WORKING-MEMORY.md` with `` on line 1, touches `.last-refresh-ok` on success; holds a 300s-stale worker lock; user-only queue truncated without LLM run. `dream-dispatch` (UserPromptSubmit hook) — **capture-only**: appends the user turn to `.pending-turns.jsonl`; it does NOT emit any directive. `dream-evaluate` (SessionEnd hook) — orchestrator that sources `eval-helpers` + 3 feature modules (`eval-decisions`, `eval-knowledge`, `eval-curation`) after shared setup; each module uses `${VAR:?}` fail-fast guards and `_MODULENAME_` variable prefixes for namespace isolation; evaluates whether to write decisions, knowledge, or curation dream markers; writes per-session marker files using atomic temp+mv; uses mkdir-based locking (`dream-lock`) to serialize operations across concurrent sessions. Always-on SessionStart hook (`session-start-context`) — recovers stale `.processing` markers (via `dream-recover` → `dream_recover_stale`), collects pending markers (via `dream-collect-tasks`), and emits a **DREAM MAINTENANCE** directive instructing the main model to spawn background `Dream` agents; directive emission is throttled to 120s. `dream-collect-tasks` unconditionally deletes orphaned `learning.*` AND `memory.*` markers (both pipelines removed from Dream subagent). The Dream agent processes decisions/knowledge/curation only — memory is NOT a Dream task. SessionStart hook (`session-start-memory`) → injects previous memory with git-reconciled header (3-state: A in-sync / B drifted / C refresh-failing) + optional pre-compact snapshot as `additionalContext`; stamp `` on line 1 drives drift detection; no raw-turns dump. PreCompact hook → saves git state + WORKING-MEMORY.md snapshot. Memory sections: `## Now`, `## Progress`, `## Decisions`, `## Context`, `## Session Log`. The background-memory-update worker uses rename-to-claim for queue consumption (atomically renames `.pending-turns.jsonl` → `.pending-turns.processing`). Disabling memory writes `memory: false` to dream config — hooks remain registered (shared across features). `removeMemoryHooks` (used by `devflow init --no-memory`) also removes pre-dream legacy hooks. Use `devflow memory --clear` to clean up pending queue files across projects. Zero-ceremony context preservation. +**Working Memory**: Three shell-script hooks (`scripts/hooks/`) replace the old 8-hook system with a background-maintenance (Dream) architecture. Toggleable via `devflow memory --enable/--disable/--status` or `devflow init --memory/--no-memory`. Feature state is stored in `.devflow/dream/config.json` (config-only; dream config is the sole source of truth per ADR-001). `dream-capture` (Stop hook) — captures user/assistant turns to `.devflow/memory/.pending-turns.jsonl` queue; after the 120s throttle (keyed by `.working-memory-last-trigger` mtime), spawns `background-memory-update` as a detached `nohup` worker (`claude -p --model haiku`); touches `.working-memory-last-trigger` BEFORE spawning to prevent double-spawn; uses mkdir-based locking for queue overflow truncation across concurrent sessions. `background-memory-update` (Stop-hook worker, not a hook itself) — drains `.pending-turns.jsonl`, calls `claude -p` (prompt on stdin, never argv), rewrites `WORKING-MEMORY.md` with `` on line 1, touches `.last-refresh-ok` on success; holds a 300s-stale worker lock; user-only queue truncated without LLM run. `dream-dispatch` (UserPromptSubmit hook) — **capture-only**: appends the user turn to `.pending-turns.jsonl`; it does NOT emit any directive. `dream-evaluate` (SessionEnd hook) — orchestrator that sources `eval-helpers` + 3 feature modules (`eval-decisions`, `eval-knowledge`, `eval-curation`) after shared setup; each module uses `${VAR:?}` fail-fast guards and `_MODULENAME_` variable prefixes for namespace isolation; evaluates whether to write decisions, knowledge, or curation dream markers; writes per-session marker files using atomic temp+mv; uses mkdir-based locking (`dream-lock`) to serialize operations across concurrent sessions. Always-on SessionStart hook (`session-start-context`) — recovers stale `.processing` markers (via `dream-recover` → `dream_recover_stale`), collects pending markers (via `dream-collect-tasks`), and emits a **DREAM MAINTENANCE** directive instructing the main model to spawn background `Dream` agents; directive emission is throttled to 120s. `dream-collect-tasks` unconditionally deletes orphaned `learning.*` AND `memory.*` markers (both pipelines removed from Dream subagent). The Dream agent processes decisions/knowledge/curation only — memory is NOT a Dream task. SessionStart hook (`session-start-memory`) → injects previous memory with git-reconciled header (3-state: A in-sync / B drifted / C refresh-failing) + optional pre-compact snapshot as `additionalContext`; stamp `` on line 1 drives drift detection; no raw-turns dump. PreCompact hook → saves git state + WORKING-MEMORY.md snapshot. Memory sections: `## Now`, `## Progress`, `## Decisions`, `## Context`, `## Session Log`. The background-memory-update worker uses rename-to-claim for queue consumption (atomically renames `.pending-turns.jsonl` → `.pending-turns.processing`). Disabling memory writes `memory: false` to dream config — hooks remain registered (shared across features). `removeMemoryHooks` (used by `devflow init --no-memory`) also removes pre-dream legacy hooks. Use `devflow memory --clear` to clean up pending queue files across projects. Zero-ceremony context preservation. **Ambient Mode**: Single-component system for zero-overhead session enhancement. UserPromptSubmit hook (`preamble`) uses two coexisting detection paths, both controlled by the same single toggle (`devflow ambient --enable/--disable/--status` or `devflow init`). **First-word keyword detection** — when a prompt's first word (case-insensitive) is one of `implement`, `explore`, `research`, `debug`, or `plan`, followed by at least one additional word, the hook outputs a directive instructing the model to briefly announce the workflow then invoke the matching `devflow:` skill via the Skill tool. **3-marker plan detection** — when a prompt contains `## Goal`, `## Steps`, and `## Files` markers (and the keyword path did not fire), it outputs a directive to invoke `devflow:implement`. Zero overhead for normal prompts — hook outputs nothing. Any legacy `commands.md` rule left by prior installs is auto-removed on every `devflow ambient --enable/--disable` or `devflow init`. @@ -65,7 +65,7 @@ Debug logs stored at `~/.devflow/logs/{project-slug}/`. **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, decisions 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. Advanced path adds a view mode selector (default/verbose/focus) after Claude Code flags. Use `--decisions/--no-decisions` to toggle the decisions agent independently. Use `--rules/--no-rules` to toggle rules independently. -**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). Registry: append an entry to `MIGRATIONS` in `src/cli/utils/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. Currently registered per-project migrations include `purge-legacy-knowledge-v2` (removes 4 hardcoded pre-v2 ADR/PF IDs and orphan `PROJECT-PATTERNS.md`), `purge-legacy-knowledge-v3` (v3: sweeps all remaining pre-v2 seeded entries using the `- **Source**: self-learning:` format discriminator — any ADR/PF section lacking this marker is removed; entries the user edited to include the marker survive), `purge-orphaned-sidecar-judgment-state` (per-project; removes orphaned `.learning-manifest.json`, `.decisions-manifest.json`, `.decisions-notifications.json` — judgment-state files written by the now-removed deterministic render/reconcile layer), `purge-learning-pipeline-v1` (per-project; removes `.devflow/learning/` directory, learning dream markers, `learning` key from dream/sidecar config, `.claude/commands/self-learning/`, and auto-generated skills), `purge-stale-memory-markers-v1` (per-project; removes stale `dream/memory.*` markers left by the old Dream-subagent memory pipeline now that `background-memory-update` handles memory refresh — ENOENT-idempotent, rethrows non-ENOENT errors). Global migration `purge-learning-global-v1` removes `~/.devflow/learning.json`. **D37 edge case**: a project cloned *after* migrations have run won't be swept (the marker is global, not per-project). Recovery: `rm ~/.devflow/migrations.json` forces a re-sweep on next `devflow init`. +**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). Registry: append an entry to `MIGRATIONS` in `src/cli/utils/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. Currently registered per-project migrations include `purge-legacy-knowledge-v2` (removes 4 hardcoded pre-v2 ADR/PF IDs and orphan `PROJECT-PATTERNS.md`), `purge-legacy-knowledge-v3` (v3: sweeps all remaining pre-v2 seeded entries using the `- **Source**: self-learning:` format discriminator — any ADR/PF section lacking this marker is removed; entries the user edited to include the marker survive), `purge-orphaned-sidecar-judgment-state` (per-project; removes orphaned `.learning-manifest.json`, `.decisions-manifest.json`, `.decisions-notifications.json` — judgment-state files written by the now-removed deterministic render/reconcile layer), `purge-learning-pipeline-v1` (per-project; removes `.devflow/learning/` directory, learning dream markers, `learning` key from dream/sidecar config, `.claude/commands/self-learning/`, and auto-generated skills), `purge-stale-memory-markers-v1` (per-project; removes stale `dream/memory.*` markers left by the old Dream-subagent memory pipeline now that `background-memory-update` handles memory refresh — ENOENT-idempotent, rethrows non-ENOENT errors), `purge-dead-working-memory-sentinel-v1` (per-project; removes the stale `.devflow/memory/.working-memory-disabled` sentinel now that the memory gate is config-only per ADR-001 — ENOENT-tolerant, rethrows non-ENOENT errors). Global migration `purge-learning-global-v1` removes `~/.devflow/learning.json`. **D37 edge case**: a project cloned *after* migrations have run won't be swept (the marker is global, not per-project). Recovery: `rm ~/.devflow/migrations.json` forces a re-sweep on next `devflow init`. ## Project Structure @@ -155,7 +155,6 @@ Per-project runtime files live under `.devflow/`: ├── memory/ │ ├── WORKING-MEMORY.md # Auto-maintained by background-memory-update worker (claude -p haiku) │ ├── backup.json # Pre-compact git state snapshot -│ ├── .working-memory-disabled # Runtime sentinel — memory hooks skip when present │ ├── .pending-turns.jsonl # Queue of captured user/assistant turns (JSONL, ephemeral) │ ├── .pending-turns.processing # Atomic handoff during background processing (transient, D56c) │ ├── .working-memory-last-trigger # Mtime = last worker spawn time (120s throttle key, transient) diff --git a/README.md b/README.md index 5f80d41f..e0894fb6 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Context ████░░░░ 42% · 5h ████░░░░ 45% (2h 15m) Opus 4.6 (1M) · 3 MCPs 2 rules · $1.42 · $18.50/wk · $62.30/mo ``` -**Security.** Deny lists block dangerous tool patterns out of the box — configurable during init. +**Security.** Deny lists block dangerous tool patterns out of the box — configurable during init and toggleable any time with `devflow security` (`--enable`/`--disable`/`--status`). ## Quick Start @@ -117,6 +117,8 @@ npx devflow-kit list # List available plugins npx devflow-kit ambient --enable # Toggle ambient mode npx devflow-kit decisions --enable # Toggle decision/pitfall tracking npx devflow-kit rules --status # Show installed rules +npx devflow-kit security --status # Show / manage the security deny list +npx devflow-kit safe-delete --enable # Install rm -> trash safe-delete npx devflow-kit uninstall # Remove Devflow ``` diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 1bd8e6ce..6e171c5a 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -25,6 +25,7 @@ Use `--recommended` or `--advanced` flags for non-interactive setup. | `--rules` / `--no-rules` | Enable/disable rules (default: on) | | `--hud` / `--no-hud` | Enable/disable HUD status line (default: on) | | `--hud-only` | Install only the HUD (no plugins, hooks, or extras) | +| `--security ` | Security deny list location (default: user) | | `--verbose` | Show detailed output | ### Scopes @@ -120,6 +121,23 @@ npx devflow-kit hud --detail # Show tool/agent descriptions npx devflow-kit hud --no-detail # Hide tool/agent descriptions ``` +## Security (Deny List) + +```bash +npx devflow-kit security --status # Show deny list state + entry counts + location +npx devflow-kit security --enable # Install deny list (user settings, default) +npx devflow-kit security --enable --managed # Install into system managed settings +npx devflow-kit security --disable # Remove the deny list from all locations +``` + +## Safe-Delete + +```bash +npx devflow-kit safe-delete --status # Show install state (installed/outdated/absent/unknown) +npx devflow-kit safe-delete --enable # Install the rm -> trash shell function +npx devflow-kit safe-delete --disable # Remove the safe-delete shell function +``` + ## Skill Shadowing Override any Devflow skill with your own version. Shadowed skills survive `devflow init` — your version is installed instead of Devflow's. diff --git a/scripts/hooks/dream-capture b/scripts/hooks/dream-capture index 66bc49cb..17a6e22b 100755 --- a/scripts/hooks/dream-capture +++ b/scripts/hooks/dream-capture @@ -85,10 +85,18 @@ if [ ${#ASSISTANT_MSG} -gt 2000 ]; then fi # --- Decisions usage scanner (independent of memory state) --- -# D29: Supplementary — skip Node spawn when no ADR/PF citations present (~50-100ms savings). +# D29: Grep-first reorder — cheap in-process citation check gates the expensive config parse. +# Dual-signal: skip when decisions is disabled via config OR sentinel (mirror of dream-evaluate). +# On the common case (no ADR/PF citations) the grep exits false and the config subprocess is +# never forked, preserving the D29 optimization (~5-20ms jq / ~50-100ms node per turn saved). SCANNER="$SCRIPT_DIR/decisions-usage-scan.cjs" -if [ -f "$SCANNER" ] && [ ! -f "$DEVFLOW_DIR/decisions/.disabled" ]; then - if printf '%s' "$ASSISTANT_MSG" | grep -qE 'ADR-[0-9]+|PF-[0-9]+'; then +if [ -f "$SCANNER" ] && printf '%s' "$ASSISTANT_MSG" | grep -qE 'ADR-[0-9]+|PF-[0-9]+'; then + _DEC_ENABLED_CAPTURE="true" + if [ -f "$DREAM_CONFIG" ]; then + _DEC_ENABLED_CAPTURE=$(json_field_file "$DREAM_CONFIG" "decisions" "true") + fi + [ -f "$DEVFLOW_DIR/decisions/.disabled" ] && _DEC_ENABLED_CAPTURE="false" + if [ "$_DEC_ENABLED_CAPTURE" = "true" ]; then dbg "Running decisions usage scanner" printf '%s' "$ASSISTANT_MSG" | node "$SCANNER" --cwd "$CWD" 2>/dev/null || true fi diff --git a/scripts/hooks/dream-collect-tasks b/scripts/hooks/dream-collect-tasks index dc0dcb78..085054ac 100644 --- a/scripts/hooks/dream-collect-tasks +++ b/scripts/hooks/dream-collect-tasks @@ -21,7 +21,9 @@ # - Skips config.json (reserved). # - Pass 1: unconditional sweep — deletes learning.* and memory.* markers always # (both pipelines removed from Dream subagent); deletes disabled decisions/knowledge -# markers. curation and unknown types pass through unchanged (no flag gates them). +# markers. When decisions is disabled, curation markers are also swept (curation +# depends on decisions data and should not run when decisions is disabled). +# Unknown types pass through unchanged. # - Pass 1 runs over ALL markers regardless of cap, so no orphan survives by # being pushed past position 50. # - COLLECT_LIMIT=50: when >50 kept markers remain after pass 1, orders by @@ -122,7 +124,16 @@ dream_collect_tasks() { continue fi ;; - # curation and unknown types: pass through unchanged + curation) + # Curation depends on decisions data — sweep when decisions is disabled + # so stray curation markers don't trigger Dream agent spawns when disabled. + if [ "$dec_enabled" != "true" ]; then + rm -f "$_f" 2>/dev/null || true + dbg "dream_collect_tasks: deleted disabled-feature curation marker: $_base" + continue + fi + ;; + # unknown types: pass through unchanged esac # Marker kept — accumulate diff --git a/scripts/hooks/dream-evaluate b/scripts/hooks/dream-evaluate index 001c8fd3..0cb90df4 100755 --- a/scripts/hooks/dream-evaluate +++ b/scripts/hooks/dream-evaluate @@ -51,6 +51,11 @@ if [ -f "$DREAM_CONFIG" ]; then KNOWLEDGE_ENABLED=$(json_field_file "$DREAM_CONFIG" "knowledge" "true") fi +# Dual-signal gate: config OR sentinel disables decisions (and transitively curation). +# This ensures session-start-context and dream-evaluate honour the same signal so +# stray markers are never written when decisions is disabled via the sentinel alone. +[ -f "$DECISIONS_DIR_DATA/.disabled" ] && DECISIONS_ENABLED="false" + # Log + dream-lock source "$SCRIPT_DIR/hook-log-init" "dream-evaluate" source "$SCRIPT_DIR/dream-lock" || { log "failed to source dream-lock"; exit 1; } diff --git a/scripts/hooks/eval-curation b/scripts/hooks/eval-curation index bb066c51..663abc1e 100644 --- a/scripts/hooks/eval-curation +++ b/scripts/hooks/eval-curation @@ -1,11 +1,14 @@ #!/bin/bash # dream-evaluate: Curation evaluation module. # Writes a throttled curation marker at most once per 7 days. -# Curation has no feature-enabled flag — it always runs when the throttle allows. +# Curation is gated by DECISIONS_ENABLED — when decisions is disabled (via +# dream config or .disabled sentinel), curation is also skipped so that +# disabling decisions does not stamp .curation-last and cause a 7-day +# suppression window after re-enable. # Source from dream-evaluate orchestrator after eval-helpers. # # Requires (from orchestrator namespace): -# DREAM_DIR, NOW, MARKER_SUFFIX, +# DECISIONS_ENABLED, DREAM_DIR, NOW, MARKER_SUFFIX, # _HAS_JQ, log(), dbg() # # D58: 7-day curation throttle. @@ -20,12 +23,23 @@ # next window will pick it up. This is intentional: a failed curation is less # costly than repeatedly spawning curation work in a retry storm. +: "${DECISIONS_ENABLED:?eval-curation: DECISIONS_ENABLED must be set by orchestrator}" : "${DREAM_DIR:?eval-curation: DREAM_DIR must be set by orchestrator}" : "${NOW:?eval-curation: NOW must be set by orchestrator}" : "${MARKER_SUFFIX:?eval-curation: MARKER_SUFFIX must be set by orchestrator}" : "${_HAS_JQ:?eval-curation: _HAS_JQ must be set by orchestrator}" -# Curation is always-on (no feature-enabled flag). +# Gate: curation inherits decisions enabled-state. When decisions is disabled, +# skip immediately — BEFORE reading or writing .curation-last — so disabling +# decisions does not burn a 7-day suppression window. +# Use return (not exit): this file is sourced under set -e in dream-evaluate; +# exit would kill the whole orchestrator process. +if [ "$DECISIONS_ENABLED" != "true" ]; then + log "Curation skipped: decisions disabled" + dbg "eval-curation: DECISIONS_ENABLED=$DECISIONS_ENABLED — returning" + return 0 +fi + # Throttle: at most once per 7 days (604800 seconds). _CUR_THROTTLE=604800 _CUR_LAST_FILE="$DREAM_DIR/.curation-last" diff --git a/scripts/hooks/eval-decisions b/scripts/hooks/eval-decisions index df019bab..f033c19c 100644 --- a/scripts/hooks/eval-decisions +++ b/scripts/hooks/eval-decisions @@ -19,8 +19,6 @@ if [ "$DECISIONS_ENABLED" = "true" ] && [ "$SESSION_DEEP" = "true" ]; then log "Evaluating decisions..." - # Note: .devflow/decisions/.disabled is for session-start-context only — this hook uses DECISIONS_ENABLED. - # Daily cap check _DEC_RUNS_FILE="$DREAM_DIR/.decisions-runs-today" _DEC_RUNS_TODAY=$(read_daily_cap "$_DEC_RUNS_FILE" 0) diff --git a/scripts/hooks/lib/project-paths.cjs b/scripts/hooks/lib/project-paths.cjs index 4f1c5311..d83febd9 100644 --- a/scripts/hooks/lib/project-paths.cjs +++ b/scripts/hooks/lib/project-paths.cjs @@ -138,11 +138,6 @@ function getDecisionsBatchIdsPath(projectRoot) { // Memory / working-memory files // --------------------------------------------------------------------------- -/** .devflow/memory/.working-memory-disabled — sentinel that gates all 4 memory hooks */ -function getWorkingMemoryDisabledSentinel(projectRoot) { - return path.join(projectRoot, '.devflow', 'memory', '.working-memory-disabled'); -} - /** .devflow/memory/WORKING-MEMORY.md */ function getWorkingMemoryPath(projectRoot) { return path.join(projectRoot, '.devflow', 'memory', 'WORKING-MEMORY.md'); @@ -303,7 +298,6 @@ module.exports = { getDecisionsRunsTodayPath, getDecisionsBatchIdsPath, // Memory files - getWorkingMemoryDisabledSentinel, getWorkingMemoryPath, getBackupPath, getPendingTurnsPath, diff --git a/src/cli/cli.ts b/src/cli/cli.ts index 88c4e041..b1e65735 100644 --- a/src/cli/cli.ts +++ b/src/cli/cli.ts @@ -16,6 +16,8 @@ import { knowledgeCommand } from './commands/knowledge/index.js'; import { decisionsCommand } from './commands/decisions.js'; import { rulesCommand } from './commands/rules.js'; import { debugCommand } from './commands/debug.js'; +import { securityCommand } from './commands/security.js'; +import { safeDeleteCommand } from './commands/safe-delete.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); @@ -32,7 +34,7 @@ program .description('Agentic Development Toolkit for Claude Code\n\nEnhance your AI-assisted development with intelligent commands and workflows.') .version(packageJson.version, '-v, --version', 'Display version number') .helpOption('-h, --help', 'Display help information') - .addHelpText('after', '\nExamples:\n $ devflow init Install all Devflow plugins\n $ devflow init --plugin=implement Install specific plugin\n $ devflow init --plugin=implement,review Install multiple plugins\n $ devflow list List available plugins\n $ devflow ambient --enable Enable always-on ambient mode\n $ devflow memory --status Check working memory state\n $ devflow hud --configure Configure HUD preset\n $ devflow uninstall Remove Devflow from Claude Code\n $ devflow --version Show version\n $ devflow --help Show help\n\nDocumentation:\n https://github.com/dean0x/devflow#readme'); + .addHelpText('after', '\nExamples:\n $ devflow init Install all Devflow plugins\n $ devflow init --plugin=implement Install specific plugin\n $ devflow init --plugin=implement,review Install multiple plugins\n $ devflow list List available plugins\n $ devflow ambient --enable Enable always-on ambient mode\n $ devflow memory --status Check working memory state\n $ devflow hud --configure Configure HUD preset\n $ devflow security --status Check security deny list state\n $ devflow security --disable Remove the security deny list\n $ devflow safe-delete --status Check safe-delete shell function state\n $ devflow safe-delete --enable Install safe-delete shell function\n $ devflow uninstall Remove Devflow from Claude Code\n $ devflow --version Show version\n $ devflow --help Show help\n\nDocumentation:\n https://github.com/dean0x/devflow#readme'); // Register commands program.addCommand(initCommand); @@ -47,6 +49,8 @@ program.addCommand(knowledgeCommand); program.addCommand(decisionsCommand); program.addCommand(rulesCommand); program.addCommand(debugCommand); +program.addCommand(securityCommand); +program.addCommand(safeDeleteCommand); // Handle no command program.action(() => { diff --git a/src/cli/commands/ambient.ts b/src/cli/commands/ambient.ts index de96a546..6c0d4db7 100644 --- a/src/cli/commands/ambient.ts +++ b/src/cli/commands/ambient.ts @@ -5,6 +5,8 @@ import * as os from 'os'; import * as p from '@clack/prompts'; import color from 'picocolors'; import { getClaudeDirectory, getDevFlowDirectory } from '../utils/paths.js'; +import { syncManifestFeature } from '../utils/manifest.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; import type { HookMatcher, Settings } from '../utils/hooks.js'; const PREAMBLE_HOOK_MARKER = 'preamble'; @@ -218,7 +220,8 @@ export const ambientCommand = new Command('ambient') p.log.info('Ambient mode already enabled'); return; } - await fs.writeFile(settingsPath, updated, 'utf-8'); + await writeFileAtomicExclusive(settingsPath, updated); + await syncManifestFeature(getDevFlowDirectory(), 'ambient', true); p.log.success('Ambient mode enabled — detection hook registered'); p.log.info(color.dim('Keyword prompts and structured plans auto-run their workflow')); } @@ -229,7 +232,8 @@ export const ambientCommand = new Command('ambient') p.log.info('Ambient mode already disabled'); return; } - await fs.writeFile(settingsPath, updated, 'utf-8'); + await writeFileAtomicExclusive(settingsPath, updated); + await syncManifestFeature(getDevFlowDirectory(), 'ambient', false); p.log.success('Ambient mode disabled — hook removed'); } }); diff --git a/src/cli/commands/decisions.ts b/src/cli/commands/decisions.ts index 2820d3e9..16e8d8e0 100644 --- a/src/cli/commands/decisions.ts +++ b/src/cli/commands/decisions.ts @@ -17,6 +17,8 @@ import { getDecisionsDisabledSentinel, } from '../utils/project-paths.js'; import { updateFeature, isFeatureEnabled, readConfig } from '../utils/dream-config.js'; +import { syncManifestFeature } from '../utils/manifest.js'; +import { getDevFlowDirectory } from '../utils/paths.js'; import { getGitRoot } from '../utils/git.js'; import { type DecisionsEntryStatus, @@ -352,6 +354,7 @@ export const decisionsCommand = new Command('decisions') try { await fs.unlink(getDecisionsDisabledSentinel(gitRoot)); } catch { /* may not exist */ } + await syncManifestFeature(getDevFlowDirectory(), 'decisions', true); p.log.success('Decisions learning enabled — configuration updated'); p.log.info(color.dim('Architectural decisions and pitfalls will be detected from your sessions')); } else { @@ -368,6 +371,7 @@ export const decisionsCommand = new Command('decisions') const decisionsDir = getDecisionsDir(gitRoot); await fs.mkdir(decisionsDir, { recursive: true }); await fs.writeFile(getDecisionsDisabledSentinel(gitRoot), '', 'utf-8'); + await syncManifestFeature(getDevFlowDirectory(), 'decisions', false); p.log.success('Decisions learning disabled — configuration updated'); } else { p.log.warn('Could not resolve git root — configuration not updated'); diff --git a/src/cli/commands/hud.ts b/src/cli/commands/hud.ts index 4b7ab980..cc1adbbe 100644 --- a/src/cli/commands/hud.ts +++ b/src/cli/commands/hud.ts @@ -4,6 +4,8 @@ import * as path from 'node:path'; import * as p from '@clack/prompts'; import color from 'picocolors'; import { getClaudeDirectory, getDevFlowDirectory } from '../utils/paths.js'; +import { syncManifestFeature } from '../utils/manifest.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; import { HUD_COMPONENTS, loadConfig, @@ -169,6 +171,7 @@ export const hudCommand = new Command('hud') if (options.enable) { const claudeDir = getClaudeDirectory(); + const devflowDir = getDevFlowDirectory(); const settingsPath = path.join(claudeDir, 'settings.json'); let settingsContent: string; try { @@ -203,9 +206,8 @@ export const hudCommand = new Command('hud') } } - const devflowDir = getDevFlowDirectory(); const updated = addHudStatusLine(settingsContent, devflowDir); - await fs.writeFile(settingsPath, updated, 'utf-8'); + await writeFileAtomicExclusive(settingsPath, updated); } // Update config @@ -216,18 +218,44 @@ export const hudCommand = new Command('hud') } saveConfig({ ...config, enabled: true }); + await syncManifestFeature(devflowDir, 'hud', true); p.log.success('HUD enabled'); p.log.info(color.dim('Restart Claude Code to see the HUD')); } if (options.disable) { const config = loadConfig(); - if (!config.enabled) { + const wasEnabled = config.enabled; + + // Always update config to disabled (idempotent). + if (wasEnabled) { + saveConfig({ ...config, enabled: false }); + } + + // D1: Always attempt to hard-remove the statusLine from settings.json, + // regardless of config.enabled. This self-heals drift where hud.json says + // disabled but a Devflow statusLine still lingers in settings.json from a + // partial prior state (e.g. crash between config-write and settings-write). + const claudeDir = getClaudeDirectory(); + const settingsPath = path.join(claudeDir, 'settings.json'); + let statusLineRemoved = false; + try { + const settingsContent = await fs.readFile(settingsPath, 'utf-8'); + const updated = removeHudStatusLine(settingsContent); + if (updated !== settingsContent) { + await writeFileAtomicExclusive(settingsPath, updated); + statusLineRemoved = true; + } + } catch { + // settings.json may not exist — non-fatal + } + + if (!wasEnabled && !statusLineRemoved) { p.log.info('HUD already disabled'); return; } - saveConfig({ ...config, enabled: false }); + + await syncManifestFeature(getDevFlowDirectory(), 'hud', false); p.log.success('HUD disabled'); - p.log.info(color.dim('Version upgrade notifications will still appear')); } }); diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index 14f7a4a9..229a5080 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -16,6 +16,14 @@ import { discoverProjectGitRoots, updateGitignore, createDocsStructure, + applyUserSecurityDenyList, + stripUserDenyList, + detectDenyState, + resolveSecurityAction, + assertHistoricalDenySuperset, + DEVFLOW_HISTORICAL_DENY, + loadTemplateDenyEntries, + stripUserSecurityDenyList, type SecurityMode, } from '../utils/post-install.js'; import { DEVFLOW_PLUGINS, LEGACY_PLUGIN_NAMES, LEGACY_SKILL_NAMES, LEGACY_COMMAND_NAMES, LEGACY_RULE_NAMES, buildAssetMaps, buildFullSkillsMap, buildRulesMap, partitionSelectablePlugins, WORKFLOW_ORDER, type PluginDefinition } from '../plugins.js'; @@ -30,8 +38,9 @@ import { readManifest, writeManifest, resolvePluginList, detectUpgrade } from '. import { getDefaultFlags, applyFlags, stripFlags, applyViewMode, stripViewMode, FLAG_REGISTRY, ViewMode, VIEW_MODES } from '../utils/flags.js'; import { addContextHook, removeContextHook, hasContextHook } from './context.js'; import { manageSentinel } from '../utils/sentinel.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; import { writeConfig as writeDreamConfig } from '../utils/dream-config.js'; -import { getFeaturesDir, getFeaturesIndexPath, getFeaturesDisabledSentinel, getDecisionsDisabledSentinel } from '../utils/project-paths.js'; +import { getFeaturesDir, getFeaturesIndexPath, getFeaturesDisabledSentinel, getDecisionsDisabledSentinel, getPendingTurnsPath, getPendingTurnsProcessingPath } from '../utils/project-paths.js'; import * as os from 'os'; // Re-export pure functions for tests (canonical source is post-install.ts) @@ -159,6 +168,7 @@ interface InitOptions { knowledge?: boolean; decisions?: boolean; rules?: boolean; + security?: SecurityMode; hudOnly?: boolean; recommended?: boolean; advanced?: boolean; @@ -181,6 +191,7 @@ export const initCommand = new Command('init') .option('--no-decisions', 'Disable decision/pitfall tracking') .option('--rules', 'Enable rules (always-on engineering principles)') .option('--no-rules', 'Disable rules') + .option('--security ', 'Security deny list location: user, managed, or none', /^(user|managed|none)$/i) .option('--hud-only', 'Install only the HUD (no plugins, hooks, or extras)') .option('--recommended', 'Apply recommended defaults after plugin selection (skip advanced prompts)') .option('--advanced', 'Show all configuration prompts') @@ -437,10 +448,9 @@ export const initCommand = new Command('init') let discoveredProjects: string[] = []; let safeDeleteAction: 'install' | 'upgrade' | 'skip' = 'skip'; let safeDeleteBlock: string | null = null; - // Default to 'user' mode: recommended path skips managed-settings to avoid a - // sudo prompt in non-interactive / quick-setup contexts. Advanced mode offers - // the managed option explicitly with a confirmation step. - let securityMode: SecurityMode = 'user'; + // Security mode is resolved from flag + manifest + detected reality via resolveSecurityAction. + // The final value is written to the manifest and consumed by the dedicated security step. + let securityMode: SecurityMode = 'user'; // placeholder; overwritten below by resolve let managedSettingsConfirmed = false; // Safe-delete detection (both paths need this) @@ -875,6 +885,50 @@ export const initCommand = new Command('init') } } + // Detect current deny list state in user settings (read-only; write happens in security step) + { + const userSettingsPath = path.join(claudeDir, 'settings.json'); + let userSettingsJson: string | null = null; + try { userSettingsJson = await fs.readFile(userSettingsPath, 'utf-8'); } catch { /* absent */ } + + let managedExists = false; + let managedContentJson: string | null = null; + try { + const { getManagedSettingsPath: getMgdPath } = await import('../utils/paths.js'); + const mgdPath = getMgdPath(); + managedContentJson = await fs.readFile(mgdPath, 'utf-8'); + managedExists = true; + } catch { /* absent or unsupported platform */ } + + const detected = detectDenyState(userSettingsJson, managedExists, managedContentJson); + + const flagValue = options.security as SecurityMode | undefined; + const manifestMode = existingManifest?.features.security as SecurityMode | undefined; + const resolution = resolveSecurityAction(flagValue, manifestMode, detected, process.stdin.isTTY); + + if (resolution.warn) { + p.log.warn(resolution.warn); + } + + // In TTY + CONFLICT, prompt the user (the pure fn returned prompt descriptor) + if (resolution.prompt && process.stdin.isTTY) { + // Default: keep detected reality (the safe choice — don't remove protection silently) + const keep = await p.confirm({ message: resolution.prompt, initialValue: true }); + if (p.isCancel(keep)) { + p.cancel('Installation cancelled.'); + process.exit(0); + } + // If user declines to keep, switch to the manifest mode + if (!keep && manifestMode !== undefined) { + securityMode = manifestMode === 'none' ? 'none' : manifestMode as SecurityMode; + } else { + securityMode = resolution.target === 'none' ? 'none' : resolution.target; + } + } else { + securityMode = resolution.target === 'none' ? 'none' : resolution.target; + } + } + // Validate target directory s.message('Validating target directory'); @@ -1070,12 +1124,7 @@ export const initCommand = new Command('init') // === Settings & hooks (all automatic based on collected choices) === s.message('Configuring settings'); - // Determine effective security mode (managed settings executed later, after safe-delete) - let effectiveSecurityMode = securityMode; - if (securityMode === 'managed' && !managedSettingsConfirmed) { - effectiveSecurityMode = 'user'; - } - await installSettings(claudeDir, rootDir, devflowDir, verbose, effectiveSecurityMode); + await installSettings(claudeDir, rootDir, devflowDir, verbose); const settingsPath = path.join(claudeDir, 'settings.json'); @@ -1171,6 +1220,15 @@ export const initCommand = new Command('init') knowledge: knowledgeEnabled, autoCommit: existingDreamConfig.autoCommit, }); + + // Drain orphaned queue files when memory is disabled so stale turns + // don't process on a future re-enable. Mirrors memory.ts --disable drain. + if (!memoryEnabled) { + await Promise.all([ + fs.unlink(getPendingTurnsPath(gitRoot)).catch((e: NodeJS.ErrnoException) => { if (e.code !== 'ENOENT') throw e; }), + fs.unlink(getPendingTurnsProcessingPath(gitRoot)).catch((e: NodeJS.ErrnoException) => { if (e.code !== 'ENOENT') throw e; }), + ]); + } } // Configure HUD @@ -1208,14 +1266,81 @@ export const initCommand = new Command('init') await installToProfile(profilePath, safeDeleteBlock); } - // Managed settings (last install step — sudo may prompt for password) - if (securityMode === 'managed' && managedSettingsConfirmed) { - s.stop('Configuring managed settings (may prompt for sudo password)...'); - const managed = await installManagedSettings(rootDir, verbose); - if (!managed) { - p.log.warn('Managed settings write failed — falling back to user settings'); + // ── Dedicated security step (always targets ~/.claude/settings.json for user mode) ── + // Runs AFTER managed-install so effective mode is known. + // Reads the current template deny list, asserts historical superset at install time. + { + const userSettingsPath = path.join(claudeDir, 'settings.json'); + + // Use canonical loadTemplateDenyEntries (avoids duplicating parse logic here). + const templateDeny = await loadTemplateDenyEntries(rootDir); + if (templateDeny.length > 0) { + // Catch any drift where a new template entry was not added to DEVFLOW_HISTORICAL_DENY + try { assertHistoricalDenySuperset(templateDeny); } catch (e) { + p.log.warn(`Security template drift: ${e instanceof Error ? e.message : e}`); + } + } else if (verbose) { + p.log.warn('Could not read managed-settings template; deny list unchanged'); + } + + if (securityMode === 'managed') { + if (managedSettingsConfirmed) { + // Managed path: attempt sudo write, fall back to user on failure + s.stop('Configuring managed settings (may prompt for sudo password)...'); + const managed = await installManagedSettings(rootDir, verbose); + if (!managed) { + // Real fallback: actually write to user settings (not just a warning) + p.log.warn('Managed settings write failed — deny list written to user settings instead'); + try { + await applyUserSecurityDenyList(userSettingsPath, templateDeny); + securityMode = 'user'; // update so manifest reflects reality + if (verbose) p.log.success('Security deny list written to ~/.claude/settings.json (fallback)'); + } catch (e) { + p.log.warn(`Could not write deny list to user settings either: ${e instanceof Error ? e.message : e}`); + } + } else { + // Managed write succeeded — strip from user settings to avoid duplication. + // Uses the canonical helper (atomic temp+rename; ENOENT-safe; only-write-if-changed). + const stripResult = await stripUserSecurityDenyList(userSettingsPath); + if (stripResult && verbose) p.log.info('Removed deny list from user settings (now in managed settings)'); + } + s.start('Finalizing installation...'); + } else { + // applies ADR-010: user declined sudo and chose the settings.json fallback. + // securityMode is 'managed' (from resolveSecurityAction or interactive choice) but + // managedSettingsConfirmed is false — honor the "fall back to settings.json" label + // by writing to user settings. Manifest will record 'user' to match reality. + if (templateDeny.length > 0) { + try { + await applyUserSecurityDenyList(userSettingsPath, templateDeny); + securityMode = 'user'; // manifest must reflect where the deny list actually landed + if (verbose) p.log.success('Security deny list written to ~/.claude/settings.json (declined managed)'); + } catch (e) { + p.log.warn(`Could not write deny list to user settings: ${e instanceof Error ? e.message : e}`); + } + } + } + } else if (securityMode === 'user') { + // User mode (default): merge deny list into ~/.claude/settings.json + if (templateDeny.length > 0) { + try { + await applyUserSecurityDenyList(userSettingsPath, templateDeny); + if (verbose) p.log.success('Security deny list applied to ~/.claude/settings.json'); + } catch (e) { + if (verbose) p.log.warn(`Could not apply security deny list: ${e instanceof Error ? e.message : e}`); + } + } + } else if (securityMode === 'none') { + // None: strip Devflow deny entries from user settings. + // Uses the canonical helper (atomic temp+rename; ENOENT-safe; only-write-if-changed). + const stripResult = await stripUserSecurityDenyList(userSettingsPath); + if (stripResult && verbose) p.log.info(`Security deny list removed (${stripResult.removed.length} entries stripped)`); + } else { + // Exhaustive guard — if TypeScript reaches here, a new SecurityMode variant was added + // without a matching branch. avoids PF-009 (stale references after rename/refactor). + const _exhaustive: never = securityMode; + void _exhaustive; } - s.start('Finalizing installation...'); } s.stop('Installation complete'); @@ -1295,7 +1420,7 @@ export const initCommand = new Command('init') version, plugins: resolvePluginList(installedPluginNames, existingManifest, !!options.plugin), scope, - features: { ambient: ambientEnabled, memory: memoryEnabled, hud: hudEnabled, knowledge: knowledgeEnabled, decisions: decisionsEnabled, rules: rulesEnabled, flags: enabledFlags, viewMode }, + features: { ambient: ambientEnabled, memory: memoryEnabled, hud: hudEnabled, knowledge: knowledgeEnabled, decisions: decisionsEnabled, rules: rulesEnabled, flags: enabledFlags, viewMode, security: securityMode }, installedAt: existingManifest?.installedAt ?? now, updatedAt: now, }; diff --git a/src/cli/commands/list.ts b/src/cli/commands/list.ts index 6a6fce9a..03013579 100644 --- a/src/cli/commands/list.ts +++ b/src/cli/commands/list.ts @@ -2,16 +2,51 @@ import { Command } from 'commander'; import * as p from '@clack/prompts'; import color from 'picocolors'; import { DEVFLOW_PLUGINS, type PluginDefinition } from '../plugins.js'; -import { getDevFlowDirectory } from '../utils/paths.js'; +import { getDevFlowDirectory, getManagedSettingsPath } from '../utils/paths.js'; import { getGitRoot } from '../utils/git.js'; import { readManifest, type ManifestData } from '../utils/manifest.js'; +import { getSafeDeleteStatus } from './safe-delete.js'; import * as path from 'path'; +import { promises as fs } from 'fs'; + +/** + * Tri-state status for features that have live detection beyond the manifest boolean. + */ +export type TriState = 'on' | 'off' | 'unknown'; + +/** + * Resolve security tri-state from the manifest security field. + * 'user' or 'managed' → 'on'; 'none' → 'off'; absent/undefined → 'unknown'. + */ +export function resolveSecurityTriState(security: ManifestData['features']['security']): TriState { + if (security === 'user' || security === 'managed') return 'on'; + if (security === 'none') return 'off'; + return 'unknown'; +} /** * Format manifest feature flags into a human-readable comma-separated string. * Returns 'none' when no features are enabled. + * + * @param extra - Optional tri-state values for security and safe-delete. + * Tri-state entries are only appended when provided (not undefined). */ -export function formatFeatures(features: ManifestData['features']): string { +export function formatFeatures( + features: ManifestData['features'], + extra?: { security?: TriState; safeDelete?: TriState }, +): string { + const triLabel = (label: string, state: TriState): string => { + switch (state) { + case 'on': return label; + case 'off': return `${label}: off`; + case 'unknown': return `${label}: unknown`; + default: { + const _exhaustive: never = state; + return `${label}: ${_exhaustive}`; + } + } + }; + const parts = [ features.ambient ? 'ambient' : null, features.memory ? 'memory' : null, @@ -20,6 +55,8 @@ export function formatFeatures(features: ManifestData['features']): string { features.hud ? 'hud' : null, features.rules ? 'rules' : null, features.flags?.length ? `flags: ${features.flags.length}` : null, + extra?.security !== undefined ? triLabel('security', extra.security) : null, + extra?.safeDelete !== undefined ? triLabel('safe-delete', extra.safeDelete) : null, ].filter(Boolean); return parts.join(', ') || 'none'; } @@ -58,22 +95,51 @@ export const listCommand = new Command('list') .action(async () => { p.intro(color.bgCyan(color.black(' Devflow Plugins '))); - // Resolve user manifest and git root in parallel (independent I/O) + // Resolve user manifest, git root, and tri-state status in parallel (independent I/O) const userDevflowDir = getDevFlowDirectory(); - const [gitRoot, userManifest] = await Promise.all([ + const [gitRoot, userManifest, safeDeleteResult] = await Promise.all([ getGitRoot(), readManifest(userDevflowDir), + // getSafeDeleteStatus reads only profile file — no subprocess + getSafeDeleteStatus().catch(() => ({ status: 'unknown' as const, profilePath: null })), ]); const localDevflowDir = gitRoot ? path.join(gitRoot, '.devflow') : null; const localManifest = localDevflowDir ? await readManifest(localDevflowDir) : null; const manifest = localManifest ?? userManifest; + // Resolve security tri-state — wrap fs.access in try/catch (throws on unsupported platforms) + let securityTriState: TriState = 'unknown'; + if (manifest) { + securityTriState = resolveSecurityTriState(manifest.features.security); + // If manifest says unknown, probe managed settings path as live fallback + if (securityTriState === 'unknown') { + try { + await fs.access(getManagedSettingsPath()); + securityTriState = 'on'; + } catch { + // managed settings absent or platform threw — leave as 'unknown' + } + } + } + + // safe-delete tri-state from profile detection (no subprocess) + let safeDeleteTriState: TriState; + switch (safeDeleteResult.status) { + case 'installed': safeDeleteTriState = 'on'; break; + case 'absent': + case 'outdated': safeDeleteTriState = 'off'; break; + default: safeDeleteTriState = 'unknown'; break; + } + // Show install status if manifest exists if (manifest) { const installedAt = new Date(manifest.installedAt).toLocaleDateString(); const updatedAt = new Date(manifest.updatedAt).toLocaleDateString(); const scope = resolveScope(localManifest); - const features = formatFeatures(manifest.features); + const features = formatFeatures(manifest.features, { + security: securityTriState, + safeDelete: safeDeleteTriState, + }); p.note( `${color.dim('Version:')} ${color.cyan(`v${manifest.version}`)}\n` + diff --git a/src/cli/commands/memory.ts b/src/cli/commands/memory.ts index 7ef7905d..71480144 100644 --- a/src/cli/commands/memory.ts +++ b/src/cli/commands/memory.ts @@ -4,6 +4,8 @@ import * as path from 'path'; import * as p from '@clack/prompts'; import color from 'picocolors'; import { getClaudeDirectory, getDevFlowDirectory } from '../utils/paths.js'; +import { syncManifestFeature } from '../utils/manifest.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; import { discoverProjectGitRoots } from '../utils/post-install.js'; import { getGitRoot } from '../utils/git.js'; import { @@ -362,7 +364,7 @@ export const memoryCommand = new Command('memory') p.log.info(color.dim('Session context will be automatically preserved across conversations')); } else { const updated = addMemoryHooks(settingsContent, devflowDir); - await fs.writeFile(settingsPath, updated, 'utf-8'); + await writeFileAtomicExclusive(settingsPath, updated); p.log.success('Working memory enabled — hooks registered'); p.log.info(color.dim('Session context will be automatically preserved across conversations')); } @@ -370,6 +372,7 @@ export const memoryCommand = new Command('memory') if (gitRoot) { await updateFeature(gitRoot, 'memory', true); } + await syncManifestFeature(getDevFlowDirectory(), 'memory', true); return; } @@ -383,6 +386,7 @@ export const memoryCommand = new Command('memory') fs.unlink(getPendingTurnsPath(gitRoot)).catch((e: NodeJS.ErrnoException) => { if (e.code !== 'ENOENT') throw e; }), fs.unlink(getPendingTurnsProcessingPath(gitRoot)).catch((e: NodeJS.ErrnoException) => { if (e.code !== 'ENOENT') throw e; }), ]); + await syncManifestFeature(getDevFlowDirectory(), 'memory', false); p.log.success('Working memory disabled — configuration updated'); } else { p.log.warn('Could not resolve git root — configuration not updated'); diff --git a/src/cli/commands/safe-delete.ts b/src/cli/commands/safe-delete.ts new file mode 100644 index 00000000..ac0c4f23 --- /dev/null +++ b/src/cli/commands/safe-delete.ts @@ -0,0 +1,184 @@ +import { Command } from 'commander'; +import * as p from '@clack/prompts'; +import color from 'picocolors'; +import { + detectPlatform, + detectShell, + getProfilePath, + getSafeDeleteInfo, + hasSafeDelete, +} from '../utils/safe-delete.js'; +import { + generateSafeDeleteBlock, + installToProfile, + removeFromProfile, + getInstalledVersion, + SAFE_DELETE_BLOCK_VERSION, +} from '../utils/safe-delete-install.js'; + +/** + * Tri-state status for the safe-delete block in the user's shell profile. + * + * - 'installed' — block present at the current SAFE_DELETE_BLOCK_VERSION + * - 'outdated' — block present but at an older version (upgrade available) + * - 'absent' — block not installed + * - 'unknown' — could not determine (unsupported platform or $SHELL unset) + */ +export type SafeDeleteStatus = 'installed' | 'outdated' | 'absent' | 'unknown'; + +/** + * Determine the current safe-delete install status and resolved profile path. + * Returns { status, profilePath } — profilePath may be null on unsupported platforms. + * + * Wrapped in try/catch so callers never crash regardless of environment. + */ +export async function getSafeDeleteStatus(): Promise<{ status: SafeDeleteStatus; profilePath: string | null }> { + try { + const shell = detectShell(); + const profilePath = getProfilePath(shell); + + if (shell === 'unknown' || profilePath === null) { + return { status: 'unknown', profilePath: null }; + } + + const version = await getInstalledVersion(profilePath); + if (version === 0) { + return { status: 'absent', profilePath }; + } + if (version < SAFE_DELETE_BLOCK_VERSION) { + return { status: 'outdated', profilePath }; + } + return { status: 'installed', profilePath }; + } catch { + return { status: 'unknown', profilePath: null }; + } +} + +interface SafeDeleteOptions { + enable?: boolean; + disable?: boolean; + status?: boolean; +} + +export const safeDeleteCommand = new Command('safe-delete') + .description('Enable or disable the safe-delete shell function (routes rm → trash)') + .option('--enable', 'Install safe-delete shell function') + .option('--disable', 'Remove safe-delete shell function') + .option('--status', 'Show current safe-delete state') + .action(async (options: SafeDeleteOptions) => { + const hasFlag = options.enable || options.disable || options.status; + if (!hasFlag) { + // No flag → show usage (matches ambient/memory/hud/decisions pattern) + p.note( + `${color.cyan('devflow safe-delete --enable')} Install safe-delete shell function\n` + + `${color.cyan('devflow safe-delete --disable')} Remove safe-delete shell function\n` + + `${color.cyan('devflow safe-delete --status')} Show current safe-delete state`, + 'Usage', + ); + return; + } + + if (options.status) { + // --status → tri-state report + const { status, profilePath } = await getSafeDeleteStatus(); + + let statusLabel: string; + switch (status) { + case 'installed': + statusLabel = color.green('installed'); + break; + case 'outdated': + statusLabel = color.yellow('outdated (upgrade available)'); + break; + case 'absent': + statusLabel = color.dim('absent'); + break; + case 'unknown': + statusLabel = color.dim('unknown'); + break; + default: { + const _exhaustive: never = status; + statusLabel = color.dim(_exhaustive); + } + } + + p.log.info(`Safe-delete: ${statusLabel}`); + if (profilePath) { + p.log.info(`Profile: ${color.dim(profilePath)}`); + } else { + p.log.info(`Profile: ${color.dim('unknown (unsupported platform or $SHELL not set)')}`); + } + + if (status === 'outdated') { + p.log.info(color.dim('Run devflow safe-delete --enable to upgrade to the current version')); + } + return; + } + + if (options.enable) { + const platform = detectPlatform(); + const shell = detectShell(); + const profilePath = getProfilePath(shell); + + if (shell === 'unknown' || profilePath === null) { + p.log.warn('Cannot determine shell profile path — $SHELL not set or platform not supported'); + p.log.info(color.dim('Set $SHELL and retry, or add the safe-delete function manually')); + return; + } + + // Check trash CLI availability (Windows always has Recycle Bin) + if (platform !== 'windows' && !hasSafeDelete(platform)) { + const info = getSafeDeleteInfo(platform); + p.log.warn(`safe-delete requires a trash CLI — not found`); + p.log.info(color.dim(`Install with: ${info.installHint ?? 'see your package manager'}`)); + p.log.info(color.dim('Then retry: devflow safe-delete --enable')); + return; + } + + const info = getSafeDeleteInfo(platform); + const block = generateSafeDeleteBlock(shell, process.platform, info.command); + if (!block) { + p.log.warn(`Shell '${shell}' is not supported for safe-delete`); + return; + } + + // Upgrade-in-place if an outdated block exists; idempotent if already current + const installedVersion = await getInstalledVersion(profilePath); + if (installedVersion === SAFE_DELETE_BLOCK_VERSION) { + p.log.info('Safe-delete already installed at the current version — no change'); + return; + } + + if (installedVersion > 0) { + // Outdated block — remove then reinstall (upgrade in place, no duplicate) + await removeFromProfile(profilePath); + await installToProfile(profilePath, block); + p.log.success(`Safe-delete upgraded to v${SAFE_DELETE_BLOCK_VERSION} in ${color.dim(profilePath)}`); + } else { + // Not installed — append + await installToProfile(profilePath, block); + p.log.success(`Safe-delete installed in ${color.dim(profilePath)}`); + } + + p.log.info(color.dim('Restart your shell or run: source ' + profilePath)); + return; + } + + if (options.disable) { + const shell = detectShell(); + const profilePath = getProfilePath(shell); + + if (shell === 'unknown' || profilePath === null) { + p.log.warn('Cannot determine shell profile path — $SHELL not set or platform not supported'); + return; + } + + const removed = await removeFromProfile(profilePath); + if (removed) { + p.log.success(`Safe-delete removed from ${color.dim(profilePath)}`); + } else { + p.log.info('Safe-delete is not installed — nothing to remove'); + } + return; + } + }); diff --git a/src/cli/commands/security.ts b/src/cli/commands/security.ts new file mode 100644 index 00000000..99be3706 --- /dev/null +++ b/src/cli/commands/security.ts @@ -0,0 +1,216 @@ +import { Command } from 'commander'; +import * as path from 'path'; +import * as p from '@clack/prompts'; +import color from 'picocolors'; +import { getClaudeDirectory, getDevFlowDirectory, getManagedSettingsPath } from '../utils/paths.js'; +import { readManifest, syncManifestFeature } from '../utils/manifest.js'; +import { + mergeDenyList, + stripUserDenyList, + detectDenyState, + DEVFLOW_HISTORICAL_DENY, + removeManagedSettings, + installManagedSettings, + loadTemplateDenyEntries, + stripUserSecurityDenyList, +} from '../utils/post-install.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; +import { promises as fs } from 'fs'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +interface SecurityOptions { + status?: boolean; + enable?: boolean; + managed?: boolean; + user?: boolean; + disable?: boolean; +} + +// ─── Pure helpers ───────────────────────────────────────────────────────────── + +/** + * Count deny entries in a parsed settings JSON string. + * Returns 0 on parse error or when no deny array is present. + */ +export function countDenyEntries(settingsJson: string): number { + try { + const obj = JSON.parse(settingsJson) as Record; + const rawDeny = (obj.permissions as Record | undefined)?.deny; + return Array.isArray(rawDeny) ? rawDeny.length : 0; + } catch { + return 0; + } +} + +// ─── Command ────────────────────────────────────────────────────────────────── + +export const securityCommand = new Command('security') + .description('Manage the security deny list (permissions.deny in Claude Code settings)') + .option('--status', 'Show current deny list state and entry counts') + .option('--enable', 'Install the deny list (use --managed or --user to select location)') + .option('--managed', 'Target system-level managed settings (requires sudo on some platforms)') + .option('--user', 'Target ~/.claude/settings.json (default)') + .option('--disable', 'Remove the deny list from all locations') + .addHelpText('after', '\nExamples:\n $ devflow security --status\n $ devflow security --enable\n $ devflow security --enable --managed\n $ devflow security --disable') + .action(async (options: SecurityOptions) => { + const claudeDir = getClaudeDirectory(); + const devflowDir = getDevFlowDirectory(); + const userSettingsPath = path.join(claudeDir, 'settings.json'); + const rootDir = path.resolve(__dirname, '../..'); + + // ── Load current state ────────────────────────────────────────────────── + let userSettingsJson: string | null = null; + try { userSettingsJson = await fs.readFile(userSettingsPath, 'utf-8'); } catch { /* absent */ } + + let managedPath: string | null = null; + let managedExists = false; + let managedContentJson: string | null = null; + try { + managedPath = getManagedSettingsPath(); + managedContentJson = await fs.readFile(managedPath, 'utf-8'); + managedExists = true; + } catch { /* absent or unsupported platform */ } + + const detected = detectDenyState(userSettingsJson, managedExists, managedContentJson); + + // Default (no flag) → show Usage note (mirrors memory.ts:242-253 incumbent pattern) + if (!options.enable && !options.disable && !options.status) { + p.note( + `${color.cyan('devflow security --status')} Show current deny list state\n` + + `${color.cyan('devflow security --enable')} Install deny list (user settings)\n` + + `${color.cyan('devflow security --enable --managed')} Install deny list (managed settings)\n` + + `${color.cyan('devflow security --disable')} Remove the deny list from all locations`, + 'Usage', + ); + return; + } + + // ── --status ──────────────────────────────────────────────────────────── + if (options.status) { + const both = detected.user && detected.managed; + + if (!detected.user && !detected.managed) { + p.log.info('Security deny list: ' + color.dim('none (not installed)')); + } else { + if (detected.user) { + const count = userSettingsJson ? countDenyEntries(userSettingsJson) : 0; + p.log.info(`Security deny list: ${color.green('installed')} in user settings (${count} entries)`); + p.log.info(` Location: ${color.dim(userSettingsPath)}`); + } + if (detected.managed && managedPath) { + const count = managedContentJson ? countDenyEntries(managedContentJson) : 0; + p.log.info(`Security deny list: ${color.green('installed')} in managed settings (${count} entries)`); + p.log.info(` Location: ${color.dim(managedPath)}`); + } + if (both) { + p.log.warn('Deny list found in BOTH locations. Run --disable and --enable to consolidate.'); + } + } + + if (detected.unknown) { + p.log.warn('User settings.json is present but unparseable — cannot determine entry count'); + } + + // Show manifest mode if available + const manifest = await readManifest(devflowDir); + if (manifest?.features.security) { + p.log.info(`Manifest mode: ${color.dim(manifest.features.security)}`); + } + + return; + } + + // ── --enable ──────────────────────────────────────────────────────────── + if (options.enable) { + const useManaged = options.managed === true; + const targetMode = useManaged ? 'managed' : 'user'; + + const templateDeny = await loadTemplateDenyEntries(rootDir); + if (templateDeny.length === 0) { + p.log.error('Could not load deny list template — no entries to install'); + return; + } + + if (targetMode === 'managed') { + // Add to managed first, verify, then strip from user + const managed = await installManagedSettings(rootDir, true); + if (!managed) { + p.log.error('Managed settings write failed — re-run without --managed to use user settings'); + return; + } + p.log.success('Security deny list written to managed settings'); + + // Strip from user settings (self-heal) — adopts canonical stripUserSecurityDenyList (SF9) + const stripResult = await stripUserSecurityDenyList(userSettingsPath); + if (stripResult && stripResult.removed.length > 0) { + p.log.info(`Removed ${stripResult.removed.length} entries from user settings (now in managed)`); + } + + } else { + // User mode: merge into ~/.claude/settings.json + let existing: string; + try { + existing = await fs.readFile(userSettingsPath, 'utf-8'); + } catch { + existing = '{}'; + } + const merged = mergeDenyList(existing, templateDeny); + await writeFileAtomicExclusive(userSettingsPath, merged); + const count = countDenyEntries(merged); + p.log.success(`Security deny list applied to user settings (${count} entries)`); + p.log.info(` Location: ${color.dim(userSettingsPath)}`); + } + + await syncManifestFeature(devflowDir, 'security', targetMode); + + return; + } + + // ── --disable ─────────────────────────────────────────────────────────── + if (options.disable) { + // User settings: must be parseable to strip — hard fail if not + if (userSettingsJson !== null) { + try { + JSON.parse(userSettingsJson); + } catch { + p.log.error( + 'settings.json is unparseable — cannot safely remove the deny list. ' + + 'Fix the JSON syntax manually and re-run.', + ); + process.exit(1); + } + + const { json: stripped, removed } = stripUserDenyList(userSettingsJson, DEVFLOW_HISTORICAL_DENY); + if (removed.length > 0) { + await writeFileAtomicExclusive(userSettingsPath, stripped); + p.log.success(`Removed ${removed.length} entries from user settings:`); + for (const entry of removed) { + p.log.info(` ${color.dim(entry)}`); + } + } else { + p.log.info('No Devflow deny entries found in user settings'); + } + } + + // Managed settings: gate on detected.managed (parse-safe signal from detectDenyState), + // not raw managedExists — avoids raw JSON.parse crash inside removeManagedSettings + // when the managed file exists but is corrupt JSON. (SF5 — applies engineering.md + // "never throw in business logic"; avoids PF partial-failure: user settings already + // stripped but manifest sync would be skipped on uncaught throw.) + if (detected.managed) { + await removeManagedSettings(rootDir, true); + } else if (managedExists && !detected.managed) { + // File exists but is unparseable or contains no Devflow entries — warn and skip. + p.log.warn('Managed settings file exists but contains no Devflow deny entries — skipping'); + } else { + p.log.info('No managed settings to remove'); + } + + await syncManifestFeature(devflowDir, 'security', 'none'); + + return; + } + }); diff --git a/src/cli/commands/uninstall.ts b/src/cli/commands/uninstall.ts index c2d2a07a..4c066106 100644 --- a/src/cli/commands/uninstall.ts +++ b/src/cli/commands/uninstall.ts @@ -16,7 +16,8 @@ import { removeContextHook } from './context.js'; import { listShadowed } from './skills.js'; import { detectShell, getProfilePath } from '../utils/safe-delete.js'; import { isAlreadyInstalled, removeFromProfile } from '../utils/safe-delete-install.js'; -import { removeManagedSettings } from '../utils/post-install.js'; +import { removeManagedSettings, stripUserDenyList, detectDenyState, DEVFLOW_HISTORICAL_DENY } from '../utils/post-install.js'; +import { writeFileAtomicExclusive } from '../utils/fs-atomic.js'; import { stripFlags, stripViewMode } from '../utils/flags.js'; import { stripDevflowTeammateModeFromJson } from '../utils/teammate-mode-cleanup.js'; @@ -91,6 +92,33 @@ export function formatDryRunPlan( return lines.join('\n'); } +/** + * Determine whether the security deny list should prompt for removal in an + * interactive session, and what the non-interactive default is. + * + * PURE — no I/O, fully testable. + * + * Safety invariant: non-interactive mode NEVER removes the deny list + * (protective-by-default). `keepDocs` suppresses the entire section. + * + * Returns: + * - "skip" — nothing present, or keepDocs is set; no action needed + * - "preserve" — non-interactive (isTTY=false); deny list preserved + * - "prompt" — interactive (isTTY=true); caller should ask the user + * + * @D1 Non-interactive-preserve invariant: when isTTY is false the result is + * always "preserve", never "prompt" — avoids PF-004 half-applied-state hazard. + */ +export function resolveSecurityRemovalDecision(opts: { + anySecurityPresent: boolean; + keepDocs: boolean; + isTTY: boolean; +}): 'skip' | 'preserve' | 'prompt' { + if (!opts.anySecurityPresent || opts.keepDocs) return 'skip'; + if (!opts.isTTY) return 'preserve'; + return 'prompt'; +} + /** * Uninstall plugin using Claude CLI */ @@ -409,32 +437,79 @@ export const uninstallCommand = new Command('uninstall') } } - // 6. Managed settings (security deny list) - let managedSettingsExist = false; + // 6. Security deny list (user settings + managed settings) + const uninstallRootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + + // Detect what's installed + let userSettingsJsonForSecurity: string | null = null; + const userSettingsPathForSecurity = path.join(getClaudeDirectory(), 'settings.json'); + try { userSettingsJsonForSecurity = await fs.readFile(userSettingsPathForSecurity, 'utf-8'); } catch { /* absent */ } + + let managedExistsForSecurity = false; + let managedContentForSecurity: string | null = null; try { const managedPath = getManagedSettingsPath(); - await fs.access(managedPath); - managedSettingsExist = true; - } catch { - // Managed settings don't exist or platform unsupported - } - - if (managedSettingsExist) { - if (process.stdin.isTTY) { - const removeManagedConfirm = await p.confirm({ - message: 'Remove Devflow security deny list from managed settings?', - initialValue: false, + managedContentForSecurity = await fs.readFile(managedPath, 'utf-8'); + managedExistsForSecurity = true; + } catch { /* absent or unsupported platform */ } + + const detectedSecurity = detectDenyState( + userSettingsJsonForSecurity, + managedExistsForSecurity, + managedContentForSecurity, + ); + + const anySecurityPresent = detectedSecurity.user || detectedSecurity.managed; + if (anySecurityPresent) { + const bothLocations = detectedSecurity.user && detectedSecurity.managed; + const locationLabel = bothLocations ? 'user settings + managed settings' + : detectedSecurity.user ? 'user settings' : 'managed settings'; + + const securityDecision = resolveSecurityRemovalDecision({ + anySecurityPresent, + keepDocs: !!options.keepDocs, + isTTY: process.stdin.isTTY, + }); + + let shouldRemoveSecurity = false; + + if (securityDecision === 'prompt') { + const removeDenyConfirm = await p.confirm({ + message: `Remove Devflow security deny list from ${locationLabel}?`, + initialValue: false, // Deny list is protective — default to keeping it }); - if (!p.isCancel(removeManagedConfirm) && removeManagedConfirm) { - // Resolve rootDir for the template path — use the dist directory - const uninstallRootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); - await removeManagedSettings(uninstallRootDir, verbose); + if (!p.isCancel(removeDenyConfirm) && removeDenyConfirm) { + shouldRemoveSecurity = true; } else { - p.log.info('Managed settings preserved'); + p.log.info(`Security deny list preserved (${locationLabel})`); + } + } else if (securityDecision === 'preserve') { + p.log.info(`Security deny list preserved (${locationLabel}, non-interactive mode)`); + } + // securityDecision === 'skip' when keepDocs is set — no message, no action + + if (shouldRemoveSecurity) { + // Strip from user settings (ENOENT-tolerant; atomic write via temp+rename). + // NOTE: unparseable user settings are detected as user=false by detectDenyState + // (JSON.parse failure sets unknown=true but not user=true), so this branch is + // only reached when the JSON is valid — stripUserDenyList will not throw. + // Unlike `devflow security --disable`, uninstall does NOT hard-fail on corrupt + // JSON; it silently skips the strip instead (user=false → guard below is false). + if (detectedSecurity.user && userSettingsJsonForSecurity !== null) { + const { json: stripped, removed } = stripUserDenyList( + userSettingsJsonForSecurity, + DEVFLOW_HISTORICAL_DENY, + ); + if (removed.length > 0) { + await writeFileAtomicExclusive(userSettingsPathForSecurity, stripped); + p.log.success(`Security deny list removed from user settings (${removed.length} entries)`); + } + } + // Strip from managed settings (ENOENT-tolerant via removeManagedSettings) + if (managedExistsForSecurity) { + await removeManagedSettings(uninstallRootDir, verbose); } - } else { - p.log.info('Managed settings preserved (non-interactive mode)'); } } diff --git a/src/cli/utils/manifest.ts b/src/cli/utils/manifest.ts index b8b416f5..35e6b6de 100644 --- a/src/cli/utils/manifest.ts +++ b/src/cli/utils/manifest.ts @@ -3,6 +3,16 @@ import * as path from 'path'; import { LEGACY_PLUGIN_NAMES } from '../plugins.js'; import { VIEW_MODES, ViewMode } from './flags.js'; +/** + * Where the Devflow security deny list is installed. + * 'user' = ~/.claude/settings.json (default) + * 'managed' = system-level managed settings + * 'none' = not installed + * + * Canonical definition — imported by post-install.ts and init.ts. + */ +export type SecurityMode = 'managed' | 'user' | 'none'; + /** * Manifest data tracked for each Devflow installation. */ @@ -19,6 +29,12 @@ export interface ManifestData { rules: boolean; flags: string[]; viewMode?: ViewMode; + /** + * Security deny list location. 'user' = ~/.claude/settings.json, + * 'managed' = system-level managed settings, 'none' = not installed. + * Absent in pre-Phase-F manifests — readManifest defaults to undefined (unknown). + */ + security?: SecurityMode; }; installedAt: string; updatedAt: string; @@ -52,6 +68,8 @@ export async function readManifest(devflowDir: string): Promise( + devflowDir: string, + key: K, + value: ManifestData['features'][K], +): Promise { + const manifest = await readManifest(devflowDir); + if (!manifest) return; + manifest.features[key] = value; + manifest.updatedAt = new Date().toISOString(); + await writeManifest(devflowDir, manifest); +} + /** * Merge new plugins into existing plugin list (union, no duplicates). * Preserves order: existing plugins first, then new ones appended. diff --git a/src/cli/utils/migrations.ts b/src/cli/utils/migrations.ts index 9466777b..37d0e3ec 100644 --- a/src/cli/utils/migrations.ts +++ b/src/cli/utils/migrations.ts @@ -1021,6 +1021,36 @@ const MIGRATION_DECISIONS_LEDGER_UNIFY: Migration<'per-project'> = { }, }; +/** + * Per-project: unlink the stale `.devflow/memory/.working-memory-disabled` + * sentinel that was previously used as a defense-in-depth gate for memory + * hooks but is now dead code (dream config is the sole source of truth, + * per ADR-001 clean-break philosophy). + * + * Best-effort cleanup of a runtime-inert file — ENOENT is success (ADR-001). + * Non-fatal on any other error (PF-004 idempotency: failed cleanup is invisible + * at runtime because no hook reads this path anymore). + * + * Applies ADR-001 (clean-break; minimal migration footprint). + * Mirrors purge-stale-memory-markers-v1 in shape. + */ +const MIGRATION_PURGE_DEAD_WORKING_MEMORY_SENTINEL: Migration<'per-project'> = { + id: 'purge-dead-working-memory-sentinel-v1', + description: 'Remove stale .devflow/memory/.working-memory-disabled sentinel (config-only gate now)', + scope: 'per-project', + async run(ctx: PerProjectMigrationContext): Promise { + const sentinelPath = path.join(ctx.projectRoot, '.devflow', 'memory', '.working-memory-disabled'); + try { + await fs.unlink(sentinelPath); + return { infos: ['Removed stale .working-memory-disabled sentinel'], warnings: [] }; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return { infos: [], warnings: [] }; // already gone — no-op + throw err; // unexpected — surface to runner + } + }, +}; + export const MIGRATIONS: readonly Migration[] = [ MIGRATION_SHADOW_OVERRIDES, MIGRATION_PURGE_LEGACY_KNOWLEDGE, @@ -1039,6 +1069,7 @@ export const MIGRATIONS: readonly Migration[] = [ MIGRATION_PURGE_TEAMMATE_MODE_PER_PROJECT, MIGRATION_SYNC_DEVFLOW_GITIGNORE_V3, MIGRATION_DECISIONS_LEDGER_UNIFY, + MIGRATION_PURGE_DEAD_WORKING_MEMORY_SENTINEL, ]; const MIGRATIONS_FILE = 'migrations.json'; diff --git a/src/cli/utils/post-install.ts b/src/cli/utils/post-install.ts index 78f4d066..37cc7a2f 100644 --- a/src/cli/utils/post-install.ts +++ b/src/cli/utils/post-install.ts @@ -5,6 +5,8 @@ import * as os from 'os'; import * as p from '@clack/prompts'; import { getManagedSettingsPath } from './paths.js'; import { getGitignoreEntries, getDocsDir } from './project-paths.js'; +import { writeFileAtomicExclusive } from './fs-atomic.js'; +import type { SecurityMode } from './manifest.js'; /** * Type guard for Node.js system errors with error codes. @@ -38,17 +40,414 @@ export function computeGitignoreAppend(existingContent: string, entries: string[ } /** - * Merge Devflow deny entries into an existing managed settings object. - * Preserves existing entries, deduplicates, and returns the merged JSON string. + * Merge Devflow deny entries into an existing settings JSON object. + * Preserves existing entries (including allow and sibling keys), deduplicates, + * and returns the merged JSON string with trailing newline. + * + * PURE + idempotent: calling with the same inputs always yields byte-equal output. + * Non-array `deny` (e.g. a string, null) is treated as empty — neither throws nor spreads chars. + * + * @throws {SyntaxError} on malformed JSON — callers must pre-validate (e.g. via detectDenyState) + * or wrap in try/catch. */ export function mergeDenyList(existingJson: string, newDenyEntries: string[]): string { - const existing = JSON.parse(existingJson); - const currentDeny: string[] = existing.permissions?.deny ?? []; + const existing = JSON.parse(existingJson) as Record; + const rawDeny = (existing.permissions as Record | undefined)?.deny; + const currentDeny: string[] = Array.isArray(rawDeny) ? rawDeny as string[] : []; const merged = [...new Set([...currentDeny, ...newDenyEntries])]; - existing.permissions = { ...existing.permissions, deny: merged }; + existing.permissions = { ...(existing.permissions as Record ?? {}), deny: merged }; return JSON.stringify(existing, null, 2) + '\n'; } +/** + * Historical superset of every deny entry Devflow has ever shipped. + * Append every future entry here; never remove entries. + * Used by stripUserDenyList to identify Devflow-managed entries in legacy installs. + * + * Load-time assertion below verifies this is a superset of the current template. + */ +// D-SECURITY-01: frozen at module load — any future template entry must appear here too. +export const DEVFLOW_HISTORICAL_DENY: ReadonlySet = Object.freeze(new Set([ + // v1 batch — 154 entries shipped in src/templates/managed-settings.json + 'Bash(rm -rf /*)', + 'Bash(rm -rf ~*)', + 'Bash(rm -rf .*)', + 'Bash(* rm -rf /*)', + 'Bash(rm -r /*)', + 'Bash(rm -r ~*)', + 'Bash(rm -r .*)', + 'Bash(rm -fr /*)', + 'Bash(rm -fr ~*)', + 'Bash(rm -fr .*)', + 'Bash(rm -f /*)', + 'Bash(rm -f ~*)', + 'Bash(rm -f .*)', + 'Bash(dd if=*)', + 'Bash(dd*of=/dev/*)', + 'Bash(mkfs*)', + 'Bash(fdisk*)', + 'Bash(parted*)', + 'Bash(shred*)', + 'Bash(> /dev/sda*)', + 'Bash(> /dev/nvme*)', + 'Bash(sh -c *)', + 'Bash(bash -c *)', + 'Bash(curl * | bash*)', + 'Bash(curl * | sh*)', + 'Bash(wget * | bash*)', + 'Bash(wget * | sh*)', + 'Bash(fetch | sh*)', + 'Bash(lynx -source | bash*)', + 'Bash(base64 -d | bash*)', + 'Bash(base64 -d | sh*)', + 'Bash(base64 --decode | bash*)', + 'Bash(eval *)', + 'Bash(exec *)', + 'Bash(sudo *)', + 'Bash(su *)', + 'Bash(doas *)', + 'Bash(pkexec *)', + 'Bash(passwd*)', + 'Bash(useradd*)', + 'Bash(userdel*)', + 'Bash(usermod*)', + 'Bash(groupadd*)', + 'Bash(chmod*777*)', + 'Bash(chmod*666*)', + 'Bash(chmod -R 666*)', + 'Bash(chmod a+w*)', + 'Bash(chown*root*)', + 'Bash(chgrp*root*)', + 'Bash(kill -9 *)', + 'Bash(kill -KILL *)', + 'Bash(killall *)', + 'Bash(pkill -9 *)', + 'Bash(pkill *)', + 'Bash(xkill*)', + 'Bash(reboot*)', + 'Bash(shutdown*)', + 'Bash(halt*)', + 'Bash(poweroff*)', + 'Bash(init 0*)', + 'Bash(init 6*)', + 'Bash(systemctl*stop*)', + 'Bash(systemctl*disable*)', + 'Bash(systemctl*mask*)', + 'Bash(service*stop*)', + 'Bash(nc -l*)', + 'Bash(nc -e*)', + 'Bash(netcat -l*)', + 'Bash(ncat -l*)', + 'Bash(socat*)', + 'Bash(telnet*)', + 'Bash(python -c *)', + 'Bash(python3 -c *)', + 'Bash(php -r *)', + 'Bash(perl -e *)', + 'Bash(ruby -rsocket*)', + 'Bash(nmap*)', + 'Bash(masscan*)', + 'Bash(ufw disable*)', + 'Bash(iptables -F*)', + 'Bash(iptables --flush*)', + 'Bash(insmod*)', + 'Bash(rmmod*)', + 'Bash(modprobe*)', + 'Bash(sysctl -w *)', + 'Bash(docker run --privileged*)', + 'Bash(docker run -v /:/host*)', + 'Bash(docker run --pid=host*)', + 'Bash(docker run --net=host*)', + 'Bash(nsenter*)', + 'Bash(crontab*)', + 'Bash(rm /var/log*)', + 'Bash(rm -rf /var/log*)', + 'Bash(rm -r /var/log*)', + 'Bash(rm -f /var/log*)', + 'Bash(rm -fr /var/log*)', + 'Bash(> /var/log*)', + 'Bash(truncate /var/log*)', + 'Bash(history -c*)', + 'Bash(history -w*)', + 'Bash(rm ~/.bash_history*)', + 'Bash(rm -f ~/.bash_history*)', + 'Bash(rm ~/.zsh_history*)', + 'Bash(rm -f ~/.zsh_history*)', + 'Bash(unset HISTFILE*)', + 'Bash(curl 169.254.169.254*)', + 'Bash(wget 169.254.169.254*)', + 'Bash(rsync --daemon*)', + 'Bash(sftp *)', + 'Bash(ssh -o *)', + 'Bash(xmrig*)', + 'Bash(cgminer*)', + 'Bash(bfgminer*)', + 'Bash(ethminer*)', + 'Bash(minerd*)', + 'Bash(npm install -g *)', + 'Bash(npm i -g *)', + 'Bash(pip install --system*)', + 'Bash(pip3 install --system*)', + 'Bash(apt*install*)', + 'Bash(yum*install*)', + 'Bash(brew*install*)', + 'Bash(mount *)', + 'Bash(umount *)', + 'Bash(> /etc/*)', + 'Bash(> /usr/*)', + 'Bash(> /bin/*)', + 'Bash(> /sys/*)', + 'Bash(> /proc/*)', + 'Read(.env)', + 'Read(.env.*)', + 'Read(**/.env)', + 'Read(**/.env.*)', + 'Read(**/secrets/**)', + 'Read(**/credentials/**)', + 'Read(~/.ssh/id_*)', + 'Read(~/.ssh/*.pem)', + 'Read(~/.ssh/config)', + 'Read(~/.aws/credentials)', + 'Read(~/.aws/config)', + 'Read(~/.config/gcloud/**)', + 'Read(**/*.pem)', + 'Read(**/*.key)', + 'Read(**/*.pfx)', + 'Read(**/*.p12)', + 'Read(**/private.key)', + 'Read(**/privkey.pem)', + 'Read(**/id_rsa)', + 'Read(**/id_ed25519)', + 'Read(**/id_ecdsa)', + 'Read(**/id_dsa)', + 'Read(/etc/shadow)', + 'Read(/etc/sudoers)', + 'Read(/etc/passwd)', +])); + +/** + * Assert that DEVFLOW_HISTORICAL_DENY is a superset of the provided template entries. + * Throws if any template entry is missing from the historical set. + * Call this after loading the template to catch drift where a new template entry + * was not also added to DEVFLOW_HISTORICAL_DENY. + * + * @param templateEntries - the deny array from the current managed-settings template + */ +export function assertHistoricalDenySuperset(templateEntries: string[]): void { + const missing = templateEntries.filter(e => !DEVFLOW_HISTORICAL_DENY.has(e)); + if (missing.length > 0) { + throw new Error( + `DEVFLOW_HISTORICAL_DENY is missing ${missing.length} template entries. ` + + `Add these to DEVFLOW_HISTORICAL_DENY in post-install.ts:\n${missing.join('\n')}`, + ); + } +} + +/** + * Strip Devflow-managed deny entries from a user settings JSON string. + * PURE + idempotent — mirror of removeMemoryHooks in shape. + * + * - Parses the JSON; if permissions.deny is absent or not an array → returns input unchanged. + * - Removes entries that are in historicalSet; preserves user-only entries. + * - When remaining is empty: deletes permissions.deny; if permissions then has no keys, deletes permissions. + * - Never deletes the file — may return `{}\n`. + * - Preserves allow and all sibling keys. + * - Returns { json, removed[] } where removed is the list of entries actually stripped. + * + * @throws {SyntaxError} on malformed JSON — callers must pre-validate (e.g. via detectDenyState) + * or wrap in try/catch. + */ +export function stripUserDenyList( + existingJson: string, + historicalSet: ReadonlySet, +): { json: string; removed: string[] } { + const obj = JSON.parse(existingJson) as Record; + const perms = obj.permissions as Record | undefined; + if (!perms) return { json: existingJson, removed: [] }; + + const rawDeny = perms.deny; + if (!Array.isArray(rawDeny)) return { json: existingJson, removed: [] }; + + const currentDeny = rawDeny as string[]; + const removed = currentDeny.filter(e => historicalSet.has(e)); + const remaining = currentDeny.filter(e => !historicalSet.has(e)); + + if (remaining.length === 0) { + delete perms.deny; + if (Object.keys(perms).length === 0) { + delete obj.permissions; + } + } else { + perms.deny = remaining; + } + + return { + json: JSON.stringify(obj, null, 2) + '\n', + removed, + }; +} + +/** + * Detect where the Devflow deny list currently lives. + * + * - user: ANY historical-set entry in user settings permissions.deny → true (subset installs count). + * - managed: managedExists AND ANY historical entry in managed content's deny → true. + * - unknown: user settings is present but unparseable → true (caller skips strip). + * + * Guards getManagedSettingsPath() — may throw on unsupported platforms; caught → managed absent. + */ +export function detectDenyState( + userSettingsJson: string | null, + managedExists: boolean, + managedContentJson: string | null, +): { user: boolean; managed: boolean; unknown: boolean } { + let user = false; + let unknown = false; + let managed = false; + + if (userSettingsJson !== null) { + try { + const obj = JSON.parse(userSettingsJson) as Record; + const rawDeny = (obj.permissions as Record | undefined)?.deny; + if (Array.isArray(rawDeny)) { + user = (rawDeny as string[]).some(e => DEVFLOW_HISTORICAL_DENY.has(e)); + } + } catch { + unknown = true; + } + } + + if (managedExists && managedContentJson !== null) { + try { + const obj = JSON.parse(managedContentJson) as Record; + const rawDeny = (obj.permissions as Record | undefined)?.deny; + if (Array.isArray(rawDeny)) { + managed = (rawDeny as string[]).some(e => DEVFLOW_HISTORICAL_DENY.has(e)); + } + } catch { + // Unparseable managed file — treat as not present + } + } + + return { user, managed, unknown }; +} + +/** Internal 4-way classification of what detectDenyState found on disk. */ +type DetectedMode = 'user' | 'managed' | 'both' | 'none'; + +/** + * Collapse a DetectedMode to the canonical SecurityMode target to keep. + * 'both' and 'user' prefer user-settings (user-settings-first convention). + * Called from the three distinct collapse sites in resolveSecurityAction. + */ +function keepTargetFor(m: DetectedMode): SecurityMode { + return m === 'managed' ? 'managed' : m === 'none' ? 'none' : 'user'; +} + +/** + * Derive DetectedMode from the raw boolean flags returned by detectDenyState. + * Extracted so the 4-way ternary is named and reusable (pairs with keepTargetFor). + */ +function classifyDetected(detected: { user: boolean; managed: boolean; unknown: boolean }): DetectedMode { + return detected.user && detected.managed ? 'both' + : detected.user ? 'user' + : detected.managed ? 'managed' + : 'none'; +} + +/** + * Describe the action required for the security deny list. + * PURE — no I/O, no prompting. + * + * Resolution order: + * 1. Explicit `flag` always wins. + * 2. Manifest field present AND matches detected reality → proceed (merge for enabled, strip for none/disabled). + * 3. Manifest field disagrees with detected reality (CONFLICT): + * - TTY → return a prompt descriptor. + * - Non-TTY → keep detected reality + signal a warning. + * 4. Fresh (no manifest field) → seed from detected state; default 'user' when nothing detected. + */ +export function resolveSecurityAction( + flag: SecurityMode | undefined, + manifestMode: SecurityMode | undefined, + detected: { user: boolean; managed: boolean; unknown: boolean }, + isTTY: boolean, +): { + target: SecurityMode; + action: 'merge' | 'strip' | 'noop'; + prompt?: string; + warn?: string; +} { + // 1. Explicit CLI flag wins + if (flag !== undefined) { + if (flag === 'none') return { target: 'none', action: 'strip' }; + if (flag === 'user') return { target: 'user', action: 'merge' }; + if (flag === 'managed') return { target: 'managed', action: 'merge' }; + // Exhaustiveness guard — TypeScript ensures SecurityMode is exhausted above. + const _exhaustive: never = flag; + void _exhaustive; + } + + const detectedMode = classifyDetected(detected); + + // 2. Manifest field present — check alignment with detected reality + if (manifestMode !== undefined) { + const manifestEnabled = manifestMode !== 'none'; + const detectedEnabled = detectedMode !== 'none'; + + if (manifestEnabled && detectedEnabled) { + // Both agree something is active — merge/upgrade with current template + return { target: manifestMode === 'managed' ? 'managed' : 'user', action: 'merge' }; + } + if (!manifestEnabled && !detectedEnabled) { + // Both agree it's off — nothing to do + return { target: 'none', action: 'noop' }; + } + // CONFLICT: manifest disagrees with reality + if (isTTY) { + return { + target: keepTargetFor(detectedMode), + action: 'noop', + prompt: `Security deny list is ${detectedEnabled ? 'present' : 'absent'} but manifest says ${manifestMode}. Keep current state?`, + }; + } + // Non-TTY: preserve detected reality + warn + return { + target: keepTargetFor(detectedMode), + action: detectedEnabled ? 'merge' : 'strip', + warn: `Security deny list state (manifest=${manifestMode}, detected=${detectedMode}) — keeping detected reality`, + }; + } + + // 4. Fresh (no manifest field) — seed from detected state + if (detectedMode === 'none') { + // Nothing detected — apply user mode as default + return { target: 'user', action: 'merge' }; + } + // Something already installed — keep it, upgrade with current template + // detectedMode !== 'none' here (handled above), so keepTargetFor yields 'user' | 'managed'. + return { target: keepTargetFor(detectedMode), action: 'merge' }; +} + +/** + * Load the deny entry array from the managed-settings.json template. + * Canonical single-source helper used by installManagedSettings, removeManagedSettings, + * init.ts's security step, and security.ts's --enable/--disable paths. + * + * Defensive read: treats file as `Record`, guards with Array.isArray, + * coerces each element to string. Returns [] on any read or parse failure (never throws). + */ +export function loadTemplateDenyEntries(rootDir: string): Promise { + const sourceManaged = path.join(rootDir, 'src', 'templates', 'managed-settings.json'); + return fs.readFile(sourceManaged, 'utf-8') + .then((raw) => { + const tmpl = JSON.parse(raw) as Record; + const rawDeny = (tmpl.permissions as Record | undefined)?.deny; + return Array.isArray(rawDeny) ? rawDeny.map(String) : []; + }) + .catch(() => []); +} + /** * Attempt to install managed settings (security deny list) to the system path. * Managed settings have highest precedence in Claude Code and cannot be overridden. @@ -70,13 +469,9 @@ export async function installManagedSettings( } const managedDir = path.dirname(managedPath); - const sourceManaged = path.join(rootDir, 'src', 'templates', 'managed-settings.json'); - let newDenyEntries: string[]; - try { - const template = JSON.parse(await fs.readFile(sourceManaged, 'utf-8')); - newDenyEntries = template.permissions?.deny ?? []; - } catch { + const newDenyEntries = await loadTemplateDenyEntries(rootDir); + if (newDenyEntries.length === 0) { if (verbose) { p.log.warn('Could not read managed settings template'); } @@ -162,12 +557,8 @@ export async function removeManagedSettings( } // Load our deny entries to identify which to remove - const sourceManaged = path.join(rootDir, 'src', 'templates', 'managed-settings.json'); - let devflowDenyEntries: string[]; - try { - const template = JSON.parse(await fs.readFile(sourceManaged, 'utf-8')); - devflowDenyEntries = template.permissions?.deny ?? []; - } catch { + const devflowDenyEntries = await loadTemplateDenyEntries(rootDir); + if (devflowDenyEntries.length === 0) { return false; } @@ -255,45 +646,88 @@ export async function removeManagedSettings( } } -export type SecurityMode = 'managed' | 'user'; +// Re-export canonical SecurityMode for callers that import from post-install.ts +// (canonical definition lives in manifest.ts — avoids re-declaration in 7 places). +export type { SecurityMode } from './manifest.js'; + +/** + * Apply the Devflow deny list atomically to the user settings file (~/.claude/settings.json). + * Called by init's dedicated security step when target mode is 'user' (or as a fallback when + * the managed write fails). + * + * Merges currentTemplateDeny into the existing settings file. Idempotent. + * Returns the merged JSON string written to disk. + */ +export async function applyUserSecurityDenyList( + settingsPath: string, + currentTemplateDeny: string[], +): Promise { + let existing: string; + try { + existing = await fs.readFile(settingsPath, 'utf-8'); + } catch { + existing = '{}'; + } + const merged = mergeDenyList(existing, currentTemplateDeny); + await writeFileAtomicExclusive(settingsPath, merged); + return merged; +} + +/** + * Strip Devflow deny entries from the user settings file (~/.claude/settings.json). + * Colocated with applyUserSecurityDenyList — the remove-side counterpart. + * + * Sequence: read → stripUserDenyList → guard (stripped !== existing) → + * writeFileAtomicExclusive → return { removed }. + * Atomic write (temp+rename) upholds the never-truncate-on-crash invariant. + * ENOENT is swallowed (file absent = nothing to strip). Other errors propagate. + * + * @returns { removed: string[] } listing the stripped entries, + * or null when the file is absent (ENOENT) or nothing changed. + * + * Callers: init.ts (managed-write success and 'none' branches) and security.ts + * (--enable --managed self-heal) use this instead of open-coding the sequence. + */ +export async function stripUserSecurityDenyList( + settingsPath: string, +): Promise<{ removed: string[] } | null> { + let existing: string; + try { + existing = await fs.readFile(settingsPath, 'utf-8'); + } catch (error: unknown) { + if (isNodeSystemError(error) && error.code === 'ENOENT') { + return null; + } + throw error; + } + const { json: stripped, removed } = stripUserDenyList(existing, DEVFLOW_HISTORICAL_DENY); + if (stripped === existing) { + return null; + } + await writeFileAtomicExclusive(settingsPath, stripped); + return { removed }; +} /** * Install or update settings.json with Devflow configuration. * Prompts interactively in TTY mode when settings already exist. * In non-TTY mode, skips override (safe default). * - * When securityMode is 'managed', the deny list goes to system-level managed - * settings and is excluded from user settings.json. When 'user', the deny list - * is included in settings.json (original behavior). + * The deny list is handled by init's dedicated security step + * (applyUserSecurityDenyList / installManagedSettings) after installSettings completes. */ export async function installSettings( claudeDir: string, rootDir: string, devflowDir: string, verbose: boolean, - securityMode: SecurityMode = 'user', ): Promise { const settingsPath = path.join(claudeDir, 'settings.json'); const sourceSettingsPath = path.join(rootDir, 'src', 'templates', 'settings.json'); try { const settingsTemplate = await fs.readFile(sourceSettingsPath, 'utf-8'); - let settingsContent = substituteSettingsTemplate(settingsTemplate, devflowDir); - - // When securityMode is 'user', inject deny list from managed-settings template - if (securityMode === 'user') { - const managedTemplatePath = path.join(rootDir, 'src', 'templates', 'managed-settings.json'); - try { - const managedTemplate = JSON.parse(await fs.readFile(managedTemplatePath, 'utf-8')); - const settings = JSON.parse(settingsContent); - settings.permissions = managedTemplate.permissions; - settingsContent = JSON.stringify(settings, null, 2) + '\n'; - } catch { - if (verbose) { - p.log.warn('Could not load security deny list — settings will be written without it'); - } - } - } + const settingsContent = substituteSettingsTemplate(settingsTemplate, devflowDir); let settingsExists = false; try { diff --git a/src/cli/utils/project-paths.ts b/src/cli/utils/project-paths.ts index 0b512694..0452ef0b 100644 --- a/src/cli/utils/project-paths.ts +++ b/src/cli/utils/project-paths.ts @@ -139,11 +139,6 @@ export function getDecisionsBatchIdsPath(projectRoot: string): string { // Memory / working-memory files // --------------------------------------------------------------------------- -/** .devflow/memory/.working-memory-disabled — sentinel that gates all 4 memory hooks */ -export function getWorkingMemoryDisabledSentinel(projectRoot: string): string { - return path.join(projectRoot, '.devflow', 'memory', '.working-memory-disabled'); -} - /** .devflow/memory/WORKING-MEMORY.md */ export function getWorkingMemoryPath(projectRoot: string): string { return path.join(projectRoot, '.devflow', 'memory', 'WORKING-MEMORY.md'); diff --git a/tests/hud.test.ts b/tests/hud.test.ts index e911c06c..e17f3529 100644 --- a/tests/hud.test.ts +++ b/tests/hud.test.ts @@ -141,6 +141,69 @@ describe('hasHudStatusLine', () => { }); }); +/** + * Drift self-healing contract tests. + * + * The --disable command is now idempotent end-to-end: it always attempts to + * strip a lingering Devflow statusLine from settings.json even when hud.json + * already says disabled. The pure-function layer that --disable delegates to + * (removeHudStatusLine) must satisfy these properties for the self-healing + * to be correct. + */ +describe('--disable drift self-healing (via removeHudStatusLine)', () => { + it('removes a lingering Devflow hud.sh statusLine when config was already disabled', () => { + // Simulates: hud.json says enabled=false but settings.json still has the statusLine + // (drift from a partial prior state such as a crash between config-write and settings-write). + const driftedSettings = JSON.stringify({ + statusLine: { type: 'command', command: '/home/user/.devflow/scripts/hud.sh' }, + env: { FOO: 'bar' }, + }); + + const result = removeHudStatusLine(driftedSettings); + const settings = JSON.parse(result); + + expect(settings.statusLine).toBeUndefined(); + // Other settings must survive + expect(settings.env.FOO).toBe('bar'); + }); + + it('removes a lingering legacy statusline.sh when config was already disabled', () => { + const driftedSettings = JSON.stringify({ + statusLine: { type: 'command', command: '/old/path/.devflow/scripts/statusline.sh' }, + }); + + const result = removeHudStatusLine(driftedSettings); + const settings = JSON.parse(result); + + expect(settings.statusLine).toBeUndefined(); + }); + + it('does NOT remove a non-Devflow statusLine — third-party statusLine must survive --disable', () => { + // A user may have their own statusLine from another tool. + // --disable must never remove it (removeHudStatusLine returns unchanged JSON). + const input = JSON.stringify({ + statusLine: { type: 'command', command: '/usr/local/bin/my-custom-statusbar.sh' }, + }); + + const result = removeHudStatusLine(input); + + expect(result).toBe(input); + expect(JSON.parse(result).statusLine.command).toBe('/usr/local/bin/my-custom-statusbar.sh'); + }); + + it('is a no-op (returns identical JSON) when no statusLine is present', () => { + // When config is already disabled AND settings has no statusLine, + // removeHudStatusLine must return the same string (content unchanged). + // The --disable command uses changed-content detection (updated !== settingsContent) + // to decide whether to write; unchanged means no write. + const input = JSON.stringify({ env: { FOO: 'bar' } }); + + const result = removeHudStatusLine(input); + + expect(result).toBe(input); + }); +}); + describe('hasNonDevFlowStatusLine', () => { it('returns true for external statusLine', () => { const input = JSON.stringify({ diff --git a/tests/init-logic.test.ts b/tests/init-logic.test.ts index 9993ae60..f4e4c57a 100644 --- a/tests/init-logic.test.ts +++ b/tests/init-logic.test.ts @@ -14,7 +14,17 @@ import { runMigrationsWithFallback, } from '../src/cli/commands/init.js'; import { getManagedSettingsPath } from '../src/cli/utils/paths.js'; -import { installManagedSettings, installClaudeignore } from '../src/cli/utils/post-install.js'; +import { + installManagedSettings, + installClaudeignore, + stripUserDenyList, + detectDenyState, + resolveSecurityAction, + assertHistoricalDenySuperset, + DEVFLOW_HISTORICAL_DENY, + applyUserSecurityDenyList, + loadTemplateDenyEntries, +} from '../src/cli/utils/post-install.js'; import { installViaFileCopy, type Spinner } from '../src/cli/utils/installer.js'; import { DEVFLOW_PLUGINS, buildAssetMaps, buildRulesMap, prefixSkillName } from '../src/cli/plugins.js'; import type { RunMigrationsResult } from '../src/cli/utils/migrations.js'; @@ -245,6 +255,315 @@ describe('mergeDenyList', () => { const result = JSON.parse(mergeDenyList(existing, [])); expect(result.permissions.deny).toEqual(['Bash(rm -rf /*)']); }); + + it('handles non-array deny (string) without throwing or spreading chars', () => { + const existing = JSON.stringify({ permissions: { deny: 'oops-a-string' } }); + // Must not throw and must treat string as empty, then add newEntries + const result = JSON.parse(mergeDenyList(existing, ['Bash(sudo *)'])); + expect(result.permissions.deny).toEqual(['Bash(sudo *)']); + }); + + it('handles null deny without throwing', () => { + const existing = JSON.stringify({ permissions: { deny: null } }); + const result = JSON.parse(mergeDenyList(existing, ['Bash(sudo *)'])); + expect(result.permissions.deny).toEqual(['Bash(sudo *)']); + }); + + it('produces byte-equal output on idempotent re-run', () => { + const existing = JSON.stringify({ permissions: { deny: ['Bash(sudo *)'], allow: ['Read(*)'] } }); + const first = mergeDenyList(existing, ['Bash(sudo *)']); + const second = mergeDenyList(first, ['Bash(sudo *)']); + expect(first).toBe(second); + }); + + it('always ends with trailing newline', () => { + const existing = JSON.stringify({}); + const result = mergeDenyList(existing, ['Bash(sudo *)']); + expect(result.endsWith('\n')).toBe(true); + }); + + it('preserves allow alongside deny', () => { + const existing = JSON.stringify({ permissions: { allow: ['Read(**)'], deny: [] } }); + const result = JSON.parse(mergeDenyList(existing, ['Bash(sudo *)'])); + expect(result.permissions.allow).toEqual(['Read(**)']); + expect(result.permissions.deny).toEqual(['Bash(sudo *)']); + }); +}); + +describe('stripUserDenyList', () => { + const historical = new Set(['Bash(rm -rf /*)', 'Bash(sudo *)', 'Bash(eval *)']); + + it('removes historical entries', () => { + const json = JSON.stringify({ + permissions: { deny: ['Bash(rm -rf /*)', 'Bash(sudo *)', 'my-custom-rule'] }, + }); + const { json: out, removed } = stripUserDenyList(json, historical); + const parsed = JSON.parse(out); + expect(parsed.permissions.deny).toEqual(['my-custom-rule']); + expect(removed).toEqual(['Bash(rm -rf /*)', 'Bash(sudo *)']); + }); + + it('preserves user-only entries', () => { + const json = JSON.stringify({ + permissions: { deny: ['user-only-1', 'user-only-2'] }, + }); + const { json: out, removed } = stripUserDenyList(json, historical); + const parsed = JSON.parse(out); + expect(parsed.permissions.deny).toEqual(['user-only-1', 'user-only-2']); + expect(removed).toEqual([]); + }); + + it('removes permissions.deny when remaining is empty', () => { + const json = JSON.stringify({ + permissions: { deny: ['Bash(rm -rf /*)'] }, + }); + const { json: out, removed } = stripUserDenyList(json, historical); + const parsed = JSON.parse(out); + expect(parsed.permissions).toBeUndefined(); + expect(removed).toHaveLength(1); + }); + + it('removes permissions object when it becomes empty', () => { + const json = JSON.stringify({ + permissions: { deny: ['Bash(sudo *)'] }, + }); + const { json: out } = stripUserDenyList(json, historical); + const parsed = JSON.parse(out); + expect(Object.keys(parsed)).not.toContain('permissions'); + }); + + it('preserves allow alongside deny after strip', () => { + const json = JSON.stringify({ + permissions: { allow: ['Read(**)'], deny: ['Bash(sudo *)'] }, + }); + const { json: out } = stripUserDenyList(json, historical); + const parsed = JSON.parse(out); + expect(parsed.permissions.allow).toEqual(['Read(**)']); + expect(parsed.permissions.deny).toBeUndefined(); + }); + + it('returns {}\n when the only key was permissions with only historical deny', () => { + const json = JSON.stringify({ permissions: { deny: ['Bash(rm -rf /*)'] } }); + const { json: out } = stripUserDenyList(json, historical); + expect(out).toBe('{}\n'); + }); + + it('is idempotent (second strip returns same JSON)', () => { + const json = JSON.stringify({ + permissions: { deny: ['Bash(rm -rf /*)', 'Bash(sudo *)'] }, + }); + const { json: first } = stripUserDenyList(json, historical); + const { json: second } = stripUserDenyList(first, historical); + expect(first).toBe(second); + }); + + it('no-ops when permissions is absent', () => { + const json = JSON.stringify({ otherKey: 'val' }); + const { json: out, removed } = stripUserDenyList(json, historical); + expect(out).toBe(json); + expect(removed).toEqual([]); + }); + + it('no-ops when deny is not an array', () => { + const json = JSON.stringify({ permissions: { deny: 'bad' } }); + const { json: out, removed } = stripUserDenyList(json, historical); + expect(out).toBe(json); + expect(removed).toEqual([]); + }); + + it('ends with trailing newline', () => { + const json = JSON.stringify({ permissions: { deny: ['Bash(rm -rf /*)'] } }); + const { json: out } = stripUserDenyList(json, historical); + expect(out.endsWith('\n')).toBe(true); + }); +}); + +describe('detectDenyState', () => { + const smallHistorical = new Set(['Bash(rm -rf /*)', 'Bash(sudo *)']); + + it('returns user=true when user settings has a historical entry', () => { + const userJson = JSON.stringify({ permissions: { deny: ['Bash(rm -rf /*)'] } }); + const state = detectDenyState(userJson, false, null); + expect(state.user).toBe(true); + expect(state.managed).toBe(false); + expect(state.unknown).toBe(false); + }); + + it('treats subset install (older entries) as user=true', () => { + // DEVFLOW_HISTORICAL_DENY has all 154 entries; even one match → user=true + const entry = [...DEVFLOW_HISTORICAL_DENY][0]; + const userJson = JSON.stringify({ permissions: { deny: [entry] } }); + const state = detectDenyState(userJson, false, null); + expect(state.user).toBe(true); + }); + + it('returns user=false when user settings has no historical entries', () => { + const userJson = JSON.stringify({ permissions: { deny: ['custom-rule'] } }); + const state = detectDenyState(userJson, false, null); + expect(state.user).toBe(false); + }); + + it('returns unknown=true when user settings is unparseable', () => { + const state = detectDenyState('not-json{{{', false, null); + expect(state.unknown).toBe(true); + expect(state.user).toBe(false); + }); + + it('returns managed=true when managed file has a historical entry', () => { + const managedJson = JSON.stringify({ permissions: { deny: ['Bash(rm -rf /*)'] } }); + const state = detectDenyState(null, true, managedJson); + expect(state.managed).toBe(true); + expect(state.user).toBe(false); + }); + + it('returns managed=false when managedExists=false even if content is present', () => { + const managedJson = JSON.stringify({ permissions: { deny: ['Bash(rm -rf /*)'] } }); + const state = detectDenyState(null, false, managedJson); + expect(state.managed).toBe(false); + }); + + it('treats unparseable managed file as managed=false (never throws)', () => { + expect(() => detectDenyState(null, true, 'bad-json')).not.toThrow(); + const state = detectDenyState(null, true, 'bad-json'); + expect(state.managed).toBe(false); + }); + + it('returns none when user settings is null', () => { + const state = detectDenyState(null, false, null); + expect(state.user).toBe(false); + expect(state.managed).toBe(false); + expect(state.unknown).toBe(false); + }); + + it('reports both user and managed as true simultaneously', () => { + const entry = [...DEVFLOW_HISTORICAL_DENY][0]; + const userJson = JSON.stringify({ permissions: { deny: [entry] } }); + const managedJson = JSON.stringify({ permissions: { deny: [entry] } }); + const state = detectDenyState(userJson, true, managedJson); + expect(state.user).toBe(true); + expect(state.managed).toBe(true); + }); +}); + +describe('resolveSecurityAction', () => { + const noneDetected = { user: false, managed: false, unknown: false }; + const userDetected = { user: true, managed: false, unknown: false }; + const managedDetected = { user: false, managed: true, unknown: false }; + const bothDetected = { user: true, managed: true, unknown: false }; + const unknownDetected = { user: false, managed: false, unknown: true }; + + it('explicit flag=user always wins', () => { + const r = resolveSecurityAction('user', 'none', noneDetected, false); + expect(r.target).toBe('user'); + expect(r.action).toBe('merge'); + }); + + it('explicit flag=managed always wins', () => { + const r = resolveSecurityAction('managed', 'user', userDetected, false); + expect(r.target).toBe('managed'); + expect(r.action).toBe('merge'); + }); + + it('explicit flag=none strips', () => { + const r = resolveSecurityAction('none', 'user', userDetected, false); + expect(r.target).toBe('none'); + expect(r.action).toBe('strip'); + }); + + it('manifest agrees with detected (both enabled) → merge', () => { + const r = resolveSecurityAction(undefined, 'user', userDetected, false); + expect(r.action).toBe('merge'); + }); + + it('manifest=none and detected=none → noop', () => { + const r = resolveSecurityAction(undefined, 'none', noneDetected, false); + expect(r.target).toBe('none'); + expect(r.action).toBe('noop'); + }); + + it('CONFLICT: manifest=none but detected user is present (TTY) → returns prompt', () => { + const r = resolveSecurityAction(undefined, 'none', userDetected, true); + expect(r.prompt).toBeTruthy(); + }); + + it('CONFLICT: manifest=none but detected user is present (non-TTY) → keeps reality + warn', () => { + const r = resolveSecurityAction(undefined, 'none', userDetected, false); + expect(r.warn).toBeTruthy(); + // Keeps detected reality + expect(r.target).toBe('user'); + expect(r.action).toBe('merge'); + }); + + it('fresh install (no manifest, no detected) → default user + merge', () => { + const r = resolveSecurityAction(undefined, undefined, noneDetected, false); + expect(r.target).toBe('user'); + expect(r.action).toBe('merge'); + }); + + it('fresh install (no manifest, user detected) → keeps user + merge', () => { + const r = resolveSecurityAction(undefined, undefined, userDetected, false); + expect(r.target).toBe('user'); + expect(r.action).toBe('merge'); + }); + + it('fresh install (no manifest, managed detected) → keeps managed + merge', () => { + const r = resolveSecurityAction(undefined, undefined, managedDetected, false); + expect(r.target).toBe('managed'); + expect(r.action).toBe('merge'); + }); + + it('both detected → target=user (user-settings-first preference)', () => { + const r = resolveSecurityAction(undefined, undefined, bothDetected, false); + expect(r.target).toBe('user'); + }); + + it('unknown does not perturb explicit flag resolution', () => { + // unknownDetected should not block an explicit flag from winning + const r = resolveSecurityAction('user', undefined, unknownDetected, false); + expect(r.target).toBe('user'); + expect(r.action).toBe('merge'); + }); + + it('CONFLICT: manifest=user but nothing detected (non-TTY) → strip + warn', () => { + const r = resolveSecurityAction(undefined, 'user', noneDetected, false); + expect(r.warn).toBeTruthy(); + expect(r.action).toBe('strip'); + }); + + it('CONFLICT: manifest=none but managed is present (non-TTY) → keeps managed + warn', () => { + const r = resolveSecurityAction(undefined, 'none', managedDetected, false); + expect(r.warn).toBeTruthy(); + expect(r.target).toBe('managed'); + }); +}); + +describe('assertHistoricalDenySuperset', () => { + it('passes when template entries are all in historical set', () => { + const templateEntries = ['Bash(rm -rf /*)', 'Bash(sudo *)']; + expect(() => assertHistoricalDenySuperset(templateEntries)).not.toThrow(); + }); + + it('throws when a template entry is missing from historical set', () => { + const templateEntries = ['Bash(rm -rf /*)', 'Bash(new-future-entry-not-in-historical)']; + expect(() => assertHistoricalDenySuperset(templateEntries)).toThrow(/missing.*entries/i); + }); + + it('passes for empty template', () => { + expect(() => assertHistoricalDenySuperset([])).not.toThrow(); + }); + + it('DEVFLOW_HISTORICAL_DENY is superset of actual template (154 entries covered)', () => { + // The actual template has 154 entries — all must be in the historical set + // We test a representative sample here since the full template is a file I/O concern + const sampleEntries = [ + 'Bash(rm -rf /*)', + 'Read(/etc/shadow)', + 'Read(/etc/sudoers)', + 'Read(/etc/passwd)', + 'Bash(sudo *)', + ]; + expect(() => assertHistoricalDenySuperset(sampleEntries)).not.toThrow(); + }); }); describe('installManagedSettings', () => { @@ -887,3 +1206,124 @@ describe('buildRulesMap', () => { expect(map.get('shared-rule')).toBe('devflow-a'); // first plugin wins }); }); + +describe('applyUserSecurityDenyList', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-apply-deny-test-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + const templateDeny = ['Bash(rm -rf /*)', 'Bash(sudo *)', 'Read(/etc/shadow)']; + + it('merges template deny into an existing settings file, preserving sibling keys', async () => { + const settingsPath = path.join(tmpDir, 'settings.json'); + const initial = { theme: 'dark', model: 'claude-opus-4-5', permissions: { allow: ['Read(~/notes)'] } }; + await fs.writeFile(settingsPath, JSON.stringify(initial, null, 2) + '\n', 'utf-8'); + + const result = await applyUserSecurityDenyList(settingsPath, templateDeny); + + const written = JSON.parse(result); + // Deny entries are present + for (const entry of templateDeny) { + expect(written.permissions.deny).toContain(entry); + } + // Sibling keys are preserved + expect(written.theme).toBe('dark'); + expect(written.model).toBe('claude-opus-4-5'); + // Allow list is preserved + expect(written.permissions.allow).toEqual(['Read(~/notes)']); + + // File on disk matches + const fromDisk = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(fromDisk.permissions.deny).toEqual(written.permissions.deny); + }); + + it('creates settings from {} when the file is absent (ENOENT branch)', async () => { + const settingsPath = path.join(tmpDir, 'nonexistent', 'settings.json'); + // Parent directory does not exist — applyUserSecurityDenyList will hit ENOENT on + // the read (falling back to '{}') and then writeFileAtomicExclusive must create it. + // writeFileAtomicExclusive writes to a .tmp sibling and renames — the parent must exist. + // So we create the parent dir but leave the file absent. + await fs.mkdir(path.join(tmpDir, 'nonexistent'), { recursive: true }); + + const result = await applyUserSecurityDenyList(settingsPath, templateDeny); + + const written = JSON.parse(result); + for (const entry of templateDeny) { + expect(written.permissions.deny).toContain(entry); + } + // File was actually written to disk + const fromDisk = JSON.parse(await fs.readFile(settingsPath, 'utf-8')); + expect(fromDisk.permissions.deny).toEqual(written.permissions.deny); + }); + + it('is idempotent — two applies produce byte-equal output, no duplication', async () => { + const settingsPath = path.join(tmpDir, 'settings.json'); + await fs.writeFile(settingsPath, JSON.stringify({ theme: 'light' }, null, 2) + '\n', 'utf-8'); + + const first = await applyUserSecurityDenyList(settingsPath, templateDeny); + const second = await applyUserSecurityDenyList(settingsPath, templateDeny); + + // Returned strings are identical + expect(second).toBe(first); + + // No duplication in the deny array + const written = JSON.parse(second); + const uniqueEntries = new Set(written.permissions.deny); + expect(written.permissions.deny.length).toBe(uniqueEntries.size); + // All template entries still present exactly once + for (const entry of templateDeny) { + expect(written.permissions.deny.filter((e: string) => e === entry).length).toBe(1); + } + }); +}); + +describe('loadTemplateDenyEntries', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-load-deny-test-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it('returns the template deny entries for a valid template', async () => { + const denyEntries = ['Bash(rm -rf /*)', 'Bash(sudo *)', 'Read(/etc/shadow)']; + const templateContent = JSON.stringify({ permissions: { deny: denyEntries } }, null, 2); + await fs.mkdir(path.join(tmpDir, 'src', 'templates'), { recursive: true }); + await fs.writeFile( + path.join(tmpDir, 'src', 'templates', 'managed-settings.json'), + templateContent, + 'utf-8', + ); + + const result = await loadTemplateDenyEntries(tmpDir); + expect(result).toEqual(denyEntries); + }); + + it('returns [] when permissions.deny is a non-array (e.g. a string)', async () => { + const templateContent = JSON.stringify({ permissions: { deny: 'not-an-array' } }); + await fs.mkdir(path.join(tmpDir, 'src', 'templates'), { recursive: true }); + await fs.writeFile( + path.join(tmpDir, 'src', 'templates', 'managed-settings.json'), + templateContent, + 'utf-8', + ); + + const result = await loadTemplateDenyEntries(tmpDir); + expect(result).toEqual([]); + }); + + it('returns [] on read/parse error (missing template file)', async () => { + // No template file created — the directory is empty + const result = await loadTemplateDenyEntries(tmpDir); + expect(result).toEqual([]); + }); +}); diff --git a/tests/list-logic.test.ts b/tests/list-logic.test.ts index bf90c1d3..79da2b59 100644 --- a/tests/list-logic.test.ts +++ b/tests/list-logic.test.ts @@ -4,6 +4,8 @@ import { resolveScope, getPluginInstallStatus, formatPluginCommands, + resolveSecurityTriState, + type TriState, } from '../src/cli/commands/list.js'; import type { ManifestData } from '../src/cli/utils/manifest.js'; @@ -52,6 +54,14 @@ describe('formatFeatures', () => { expect(formatFeatures(features)).toBe('memory, knowledge, decisions, hud, rules'); }); + it('preserves feature order: ..., rules, security, safe-delete', () => { + const features: ManifestData['features'] = { + ...allOff, memory: true, knowledge: true, decisions: true, hud: true, rules: true, + }; + expect(formatFeatures(features, { security: 'on', safeDelete: 'on' })) + .toBe('memory, knowledge, decisions, hud, rules, security, safe-delete'); + }); + it('includes flags count when flags are present', () => { const features: ManifestData['features'] = { ...allOff, ambient: true, @@ -114,6 +124,66 @@ describe('getPluginInstallStatus', () => { }); }); +describe('resolveSecurityTriState', () => { + it('returns "on" for user mode', () => { + expect(resolveSecurityTriState('user')).toBe('on'); + }); + + it('returns "on" for managed mode', () => { + expect(resolveSecurityTriState('managed')).toBe('on'); + }); + + it('returns "off" for none mode', () => { + expect(resolveSecurityTriState('none')).toBe('off'); + }); + + it('returns "unknown" for undefined (old manifest)', () => { + expect(resolveSecurityTriState(undefined)).toBe('unknown'); + }); +}); + +describe('formatFeatures with tri-state extras', () => { + it('shows security: off when security is off', () => { + const features: ManifestData['features'] = { ...allOff, ambient: true }; + expect(formatFeatures(features, { security: 'off' })).toBe('ambient, security: off'); + }); + + it('shows security: unknown when security is unknown', () => { + const features: ManifestData['features'] = { ...allOff, ambient: true }; + expect(formatFeatures(features, { security: 'unknown' })).toBe('ambient, security: unknown'); + }); + + it('shows security without suffix when security is on', () => { + const features: ManifestData['features'] = { ...allOff, ambient: true }; + expect(formatFeatures(features, { security: 'on' })).toBe('ambient, security'); + }); + + it('shows safe-delete: off when safe-delete is off', () => { + const features: ManifestData['features'] = { ...allOff }; + expect(formatFeatures(features, { safeDelete: 'off' })).toBe('safe-delete: off'); + }); + + it('shows safe-delete: unknown when safe-delete is unknown', () => { + const features: ManifestData['features'] = { ...allOff }; + expect(formatFeatures(features, { safeDelete: 'unknown' })).toBe('safe-delete: unknown'); + }); + + it('omits extra fields when not provided', () => { + const features: ManifestData['features'] = { ...allOff, ambient: true }; + expect(formatFeatures(features)).toBe('ambient'); + }); + + it('omits extra fields when extra object is empty', () => { + const features: ManifestData['features'] = { ...allOff, ambient: true }; + expect(formatFeatures(features, {})).toBe('ambient'); + }); + + it('returns "none" when all off and extras are off/unknown', () => { + expect(formatFeatures(allOff, { security: 'off', safeDelete: 'unknown' })) + .toBe('security: off, safe-delete: unknown'); + }); +}); + describe('formatPluginCommands', () => { it('returns comma-separated commands', () => { expect(formatPluginCommands(['/implement'])).toBe('/implement'); diff --git a/tests/manifest.test.ts b/tests/manifest.test.ts index 9a30a036..2af10ac3 100644 --- a/tests/manifest.test.ts +++ b/tests/manifest.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fs } from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { readManifest, writeManifest, mergeManifestPlugins, resolvePluginList, detectUpgrade, type ManifestData } from '../src/cli/utils/manifest.js'; +import { readManifest, writeManifest, mergeManifestPlugins, resolvePluginList, detectUpgrade, syncManifestFeature, type ManifestData } from '../src/cli/utils/manifest.js'; describe('readManifest', () => { let tmpDir: string; @@ -253,6 +253,97 @@ describe('readManifest', () => { expect(result).not.toBeNull(); expect(result!.features.viewMode).toBeUndefined(); }); + + it('parses manifest with security=user', async () => { + const data = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [], security: 'user' }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await fs.writeFile(path.join(tmpDir, 'manifest.json'), JSON.stringify(data), 'utf-8'); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + expect(result!.features.security).toBe('user'); + }); + + it('parses manifest with security=managed', async () => { + const data = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [], security: 'managed' }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await fs.writeFile(path.join(tmpDir, 'manifest.json'), JSON.stringify(data), 'utf-8'); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + expect(result!.features.security).toBe('managed'); + }); + + it('parses manifest with security=none', async () => { + const data = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [], security: 'none' }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await fs.writeFile(path.join(tmpDir, 'manifest.json'), JSON.stringify(data), 'utf-8'); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + expect(result!.features.security).toBe('none'); + }); + + it('normalizes absent security field to undefined (back-compat with pre-Phase-F manifests)', async () => { + const data = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [] }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await fs.writeFile(path.join(tmpDir, 'manifest.json'), JSON.stringify(data), 'utf-8'); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + // Pre-Phase-F manifests have no security field → reads as undefined + expect(result!.features.security).toBeUndefined(); + }); + + it('normalizes invalid security value to undefined', async () => { + const data = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [], security: 'invalid-value' }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await fs.writeFile(path.join(tmpDir, 'manifest.json'), JSON.stringify(data), 'utf-8'); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + expect(result!.features.security).toBeUndefined(); + }); + + it('security field round-trips through write/read', async () => { + const data: ManifestData = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { ambient: true, memory: true, hud: false, knowledge: false, decisions: false, rules: true, flags: [], security: 'user' }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-13T00:00:00.000Z', + }; + await writeManifest(tmpDir, data); + const result = await readManifest(tmpDir); + expect(result).not.toBeNull(); + expect(result!.features.security).toBe('user'); + }); }); describe('writeManifest', () => { @@ -525,3 +616,79 @@ describe('resolvePluginList', () => { expect(result).toEqual(['devflow-core-skills', 'devflow-ui-design', 'devflow-code-review']); }); }); + +describe('syncManifestFeature', () => { + let tmpDir: string; + + const baseManifest: ManifestData = { + version: '1.4.0', + plugins: ['devflow-core-skills'], + scope: 'user', + features: { + ambient: true, + memory: true, + hud: false, + knowledge: false, + decisions: false, + rules: true, + flags: [], + }, + installedAt: '2026-03-01T00:00:00.000Z', + updatedAt: '2026-03-01T00:00:00.000Z', + }; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-sync-feature-test-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it('is a no-op when manifest.json is absent — file must NOT be created', async () => { + // No manifest.json in tmpDir + await syncManifestFeature(tmpDir, 'ambient', false); + + // The invariant: toggles never fabricate a manifest from nothing + let exists = false; + try { + await fs.access(path.join(tmpDir, 'manifest.json')); + exists = true; + } catch { + exists = false; + } + expect(exists).toBe(false); + }); + + it('updates the target feature field and bumps updatedAt when manifest is present', async () => { + await writeManifest(tmpDir, baseManifest); + + await syncManifestFeature(tmpDir, 'ambient', false); + + const updated = await readManifest(tmpDir); + expect(updated).not.toBeNull(); + expect(updated!.features.ambient).toBe(false); + // updatedAt is refreshed (strictly after the initial value) + expect(updated!.updatedAt > baseManifest.updatedAt).toBe(true); + // Other fields are untouched + expect(updated!.features.memory).toBe(true); + expect(updated!.features.hud).toBe(false); + expect(updated!.features.rules).toBe(true); + expect(updated!.version).toBe('1.4.0'); + expect(updated!.plugins).toEqual(['devflow-core-skills']); + }); + + it('can update the security field in a manifest that has it', async () => { + const withSecurity: ManifestData = { + ...baseManifest, + features: { ...baseManifest.features, security: 'none' }, + }; + await writeManifest(tmpDir, withSecurity); + + await syncManifestFeature(tmpDir, 'security', 'user'); + + const updated = await readManifest(tmpDir); + expect(updated).not.toBeNull(); + expect(updated!.features.security).toBe('user'); + }); +}); diff --git a/tests/migrations.test.ts b/tests/migrations.test.ts index ff7eae84..5763752e 100644 --- a/tests/migrations.test.ts +++ b/tests/migrations.test.ts @@ -2003,3 +2003,101 @@ describe('purge-devflow-teammate-mode-v1 migration', () => { expect(result?.warnings ?? []).toEqual([]); }); }); + +// --------------------------------------------------------------------------- +// purge-dead-working-memory-sentinel-v1 (per-project) +// applies ADR-001 (clean-break; dream config is sole source of truth) +// avoids PF-004 (idempotency means a buggy run is never re-swept — the +// rethrow contract must be correct on the first attempt) +// --------------------------------------------------------------------------- + +describe('purge-dead-working-memory-sentinel-v1 migration', () => { + let tmpDir: string; + let projectRoot: string; + let devflowDir: string; + let fakeHome: string; + let originalHome: string | undefined; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'devflow-purge-dead-sentinel-test-')); + projectRoot = path.join(tmpDir, 'project'); + devflowDir = path.join(projectRoot, '.devflow'); + await fs.mkdir(devflowDir, { recursive: true }); + originalHome = process.env.HOME; + process.env.HOME = path.join(tmpDir, 'home'); + fakeHome = path.join(tmpDir, 'home', '.devflow'); + await fs.mkdir(fakeHome, { recursive: true }); + }); + + afterEach(async () => { + if (originalHome !== undefined) { + process.env.HOME = originalHome; + } else { + delete process.env.HOME; + } + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + function getMigration(): Migration<'per-project'> { + const m = MIGRATIONS.find(m => m.id === 'purge-dead-working-memory-sentinel-v1'); + if (!m) throw new Error('purge-dead-working-memory-sentinel-v1 migration not found'); + return m as Migration<'per-project'>; + } + + function makeCtx(): import('../src/cli/utils/migrations.js').PerProjectMigrationContext { + return { + scope: 'per-project', + devflowDir: fakeHome, + memoryDir: path.join(devflowDir, 'memory'), + projectRoot, + }; + } + + it('is registered in MIGRATIONS with per-project scope', () => { + // applies ADR-001: per-project scope ensures cleanup targets the correct project tree + const m = MIGRATIONS.find(m => m.id === 'purge-dead-working-memory-sentinel-v1'); + expect(m).toBeDefined(); + expect(m?.scope).toBe('per-project'); + }); + + it('removes .working-memory-disabled sentinel when present and returns info', async () => { + // applies ADR-001: file is dead code — dream config is now the sole gate + const memoryDir = path.join(devflowDir, 'memory'); + await fs.mkdir(memoryDir, { recursive: true }); + const sentinelPath = path.join(memoryDir, '.working-memory-disabled'); + await fs.writeFile(sentinelPath, '', 'utf-8'); + + const result = await getMigration().run(makeCtx()); + + await expect(fs.access(sentinelPath)).rejects.toThrow(); + expect(result?.infos?.length).toBeGreaterThan(0); + expect(result?.warnings ?? []).toEqual([]); + }); + + it('is a no-op when sentinel is absent (ENOENT) without throwing', async () => { + // avoids PF-004: idempotency — sentinel already gone is a valid success state + await expect(getMigration().run(makeCtx())).resolves.not.toThrow(); + const result = await getMigration().run(makeCtx()); + expect(result?.infos ?? []).toEqual([]); + expect(result?.warnings ?? []).toEqual([]); + }); + + it('rethrows non-ENOENT errors (avoids PF-004 silent-swallow)', async () => { + // avoids PF-004: migration idempotency means a buggy run is never re-swept; + // the rethrow contract must be correct on the first attempt so callers know it failed. + // Point unlink at a path that yields ENOTDIR by putting a file where the dir should be. + const memoryParent = path.join(devflowDir, 'memory'); + // Write a plain file where the memory directory should be so that + // path.join(memoryParent, '.working-memory-disabled') produces ENOTDIR. + await fs.writeFile(memoryParent, 'not-a-dir', 'utf-8'); + + await expect(getMigration().run(makeCtx())).rejects.toThrow(); + }); + + it('appears after purge-stale-memory-markers-v1 in MIGRATIONS array', () => { + const staleIdx = MIGRATIONS.findIndex(m => m.id === 'purge-stale-memory-markers-v1'); + const deadIdx = MIGRATIONS.findIndex(m => m.id === 'purge-dead-working-memory-sentinel-v1'); + expect(staleIdx).toBeGreaterThanOrEqual(0); + expect(deadIdx).toBeGreaterThan(staleIdx); + }); +}); diff --git a/tests/project-paths.test.ts b/tests/project-paths.test.ts index 04b63b71..7a4cc672 100644 --- a/tests/project-paths.test.ts +++ b/tests/project-paths.test.ts @@ -36,7 +36,6 @@ import { getDecisionsNotificationsPath, getDecisionsRunsTodayPath, getDecisionsBatchIdsPath, - getWorkingMemoryDisabledSentinel, getWorkingMemoryPath, getBackupPath, getPendingTurnsPath, @@ -144,10 +143,6 @@ describe('project-paths TypeScript module', () => { }); describe('memory / working-memory files', () => { - it('getWorkingMemoryDisabledSentinel returns .devflow/memory/.working-memory-disabled', () => { - expect(getWorkingMemoryDisabledSentinel(ROOT)).toBe('/some/project/.devflow/memory/.working-memory-disabled'); - }); - it('getWorkingMemoryPath returns .devflow/memory/WORKING-MEMORY.md', () => { expect(getWorkingMemoryPath(ROOT)).toBe('/some/project/.devflow/memory/WORKING-MEMORY.md'); }); @@ -296,7 +291,6 @@ describe('CJS project-paths parity', () => { { name: 'getDecisionsNotificationsPath', ts: getDecisionsNotificationsPath, cjs: cjsPaths.getDecisionsNotificationsPath }, { name: 'getDecisionsRunsTodayPath', ts: getDecisionsRunsTodayPath, cjs: cjsPaths.getDecisionsRunsTodayPath }, { name: 'getDecisionsBatchIdsPath', ts: getDecisionsBatchIdsPath, cjs: cjsPaths.getDecisionsBatchIdsPath }, - { name: 'getWorkingMemoryDisabledSentinel', ts: getWorkingMemoryDisabledSentinel, cjs: cjsPaths.getWorkingMemoryDisabledSentinel }, { name: 'getWorkingMemoryPath', ts: getWorkingMemoryPath, cjs: cjsPaths.getWorkingMemoryPath }, { name: 'getBackupPath', ts: getBackupPath, cjs: cjsPaths.getBackupPath }, { name: 'getPendingTurnsPath', ts: getPendingTurnsPath, cjs: cjsPaths.getPendingTurnsPath }, diff --git a/tests/safe-delete-command.test.ts b/tests/safe-delete-command.test.ts new file mode 100644 index 00000000..97f94c96 --- /dev/null +++ b/tests/safe-delete-command.test.ts @@ -0,0 +1,275 @@ +/** + * Tests for the safe-delete command (src/cli/commands/safe-delete.ts). + * + * Tests operate against the pure helpers and status detection logic. + * All tests use HOME/SHELL overrides and tmp directories — no mutation of + * the real developer shell profile. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as os from 'os'; +import * as path from 'path'; +import * as fsSync from 'fs'; +import { promises as fs } from 'fs'; +import { + getSafeDeleteStatus, + type SafeDeleteStatus, +} from '../src/cli/commands/safe-delete.js'; +import { + getInstalledVersion, + installToProfile, + removeFromProfile, + generateSafeDeleteBlock, + SAFE_DELETE_BLOCK_VERSION, +} from '../src/cli/utils/safe-delete-install.js'; +import { + detectShell, + getProfilePath, + hasSafeDelete, +} from '../src/cli/utils/safe-delete.js'; + +// ─── helpers ───────────────────────────────────────────────────────────────── + +function makeTmpDir(): string { + return fsSync.mkdtempSync(path.join(os.tmpdir(), 'devflow-sd-test-')); +} + +async function cleanup(dir: string): Promise { + await fs.rm(dir, { recursive: true, force: true }); +} + +// ─── getSafeDeleteStatus ───────────────────────────────────────────────────── + +describe('getSafeDeleteStatus', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + it('returns unknown when $SHELL is unset', async () => { + vi.stubEnv('SHELL', ''); + vi.stubEnv('PSModulePath', ''); + const { status, profilePath } = await getSafeDeleteStatus(); + expect(status).toBe('unknown'); + expect(profilePath).toBeNull(); + }); + + it('returns absent when profile has no block installed', async () => { + const tmpDir = makeTmpDir(); + try { + vi.stubEnv('HOME', tmpDir); + vi.stubEnv('SHELL', '/bin/zsh'); + vi.stubEnv('PSModulePath', ''); + // Profile file doesn't exist → absent + const { status } = await getSafeDeleteStatus(); + expect(status).toBe('absent'); + } finally { + await cleanup(tmpDir); + } + }); + + it('returns installed when block is at current version', async () => { + const tmpDir = makeTmpDir(); + try { + vi.stubEnv('HOME', tmpDir); + vi.stubEnv('SHELL', '/bin/zsh'); + vi.stubEnv('PSModulePath', ''); + const profilePath = path.join(tmpDir, '.zshrc'); + const block = generateSafeDeleteBlock('zsh', 'darwin', 'trash'); + expect(block).not.toBeNull(); + await installToProfile(profilePath, block!); + const { status } = await getSafeDeleteStatus(); + expect(status).toBe('installed'); + } finally { + await cleanup(tmpDir); + } + }); + + it('returns outdated when block is at an older version', async () => { + const tmpDir = makeTmpDir(); + try { + vi.stubEnv('HOME', tmpDir); + vi.stubEnv('SHELL', '/bin/zsh'); + vi.stubEnv('PSModulePath', ''); + const profilePath = path.join(tmpDir, '.zshrc'); + // Write a v1 block manually (no version stamp = v1 per getInstalledVersion) + const legacyBlock = '# >>> Devflow safe-delete >>>\nrm() { trash "$@"; }\n# <<< Devflow safe-delete <<<'; + await fs.writeFile(profilePath, legacyBlock, 'utf-8'); + const version = await getInstalledVersion(profilePath); + expect(version).toBe(1); // sanity-check: reads as v1 + const { status } = await getSafeDeleteStatus(); + expect(status).toBe('outdated'); + } finally { + await cleanup(tmpDir); + } + }); + + it('never crashes — returns unknown on internal error', async () => { + // Stub getProfilePath to throw + vi.stubEnv('SHELL', '/bin/zsh'); + vi.stubEnv('PSModulePath', ''); + vi.stubEnv('HOME', '/nonexistent/__devflow_test__/'); // forces file read error, still returns absent or unknown + const { status } = await getSafeDeleteStatus(); + // Either absent (ENOENT is swallowed by getInstalledVersion) or unknown + expect(['absent', 'unknown']).toContain(status); + }); +}); + +// ─── enable — no-trash-CLI guard ───────────────────────────────────────────── + +describe('hasSafeDelete (no trash CLI)', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns true for windows (always has Recycle Bin, no CLI check)', () => { + // Windows fast-path never checks for a trash CLI binary. + const result = hasSafeDelete('windows'); + expect(result).toBe(true); + }); +}); + +// ─── enable — upgrade-no-duplicate ─────────────────────────────────────────── + +describe('upgrade-no-duplicate', () => { + it('does not stack blocks — remove then reinstall leaves exactly one block', async () => { + const tmpDir = makeTmpDir(); + try { + const profilePath = path.join(tmpDir, '.zshrc'); + // Write a v1 legacy block + const legacyBlock = '# >>> Devflow safe-delete >>>\nrm() { trash "$@"; }\n# <<< Devflow safe-delete <<<'; + await fs.writeFile(profilePath, legacyBlock + '\n', 'utf-8'); + + // Simulate upgrade: remove old, install new + await removeFromProfile(profilePath); + const newBlock = generateSafeDeleteBlock('zsh', 'darwin', 'trash'); + expect(newBlock).not.toBeNull(); + await installToProfile(profilePath, newBlock!); + + // Verify only one block marker pair exists + const content = await fs.readFile(profilePath, 'utf-8'); + const openCount = (content.match(/# >>> Devflow safe-delete >>>/g) ?? []).length; + const closeCount = (content.match(/# <<< Devflow safe-delete << { + const tmpDir = makeTmpDir(); + try { + const profilePath = path.join(tmpDir, '.zshrc'); + const block = generateSafeDeleteBlock('zsh', 'darwin', 'trash')!; + // Install once + await installToProfile(profilePath, block); + // The command's enable path checks version and no-ops if current — simulate by checking version + const v1 = await getInstalledVersion(profilePath); + expect(v1).toBe(SAFE_DELETE_BLOCK_VERSION); + // Attempting a second install via installToProfile would stack, but the command + // guards against this via the version check in the action handler. + // Here we only verify the helper itself — the command is tested via getSafeDeleteStatus. + const content = await fs.readFile(profilePath, 'utf-8'); + const openCount = (content.match(/# >>> Devflow safe-delete >>>/g) ?? []).length; + expect(openCount).toBe(1); + } finally { + await cleanup(tmpDir); + } + }); +}); + +// ─── disable ───────────────────────────────────────────────────────────────── + +describe('disable (removeFromProfile)', () => { + it('removes only the marker block, preserving other content', async () => { + const tmpDir = makeTmpDir(); + try { + const profilePath = path.join(tmpDir, '.zshrc'); + const before = 'export PATH="$PATH:/usr/local/bin"'; + const block = generateSafeDeleteBlock('zsh', 'darwin', 'trash')!; + await fs.writeFile(profilePath, before + '\n', 'utf-8'); + await installToProfile(profilePath, block); + + await removeFromProfile(profilePath); + + const content = await fs.readFile(profilePath, 'utf-8'); + expect(content).not.toContain('Devflow safe-delete'); + expect(content).toContain('export PATH'); + } finally { + await cleanup(tmpDir); + } + }); + + it('returns false and does not crash when block not installed', async () => { + const tmpDir = makeTmpDir(); + try { + const profilePath = path.join(tmpDir, '.zshrc'); + await fs.writeFile(profilePath, 'export FOO=bar\n', 'utf-8'); + const removed = await removeFromProfile(profilePath); + expect(removed).toBe(false); + } finally { + await cleanup(tmpDir); + } + }); + + it('deletes the fish function file when it becomes empty', async () => { + const tmpDir = makeTmpDir(); + try { + const fishDir = path.join(tmpDir, '.config', 'fish', 'functions'); + await fs.mkdir(fishDir, { recursive: true }); + const profilePath = path.join(fishDir, 'rm.fish'); + const block = generateSafeDeleteBlock('fish', 'linux', 'trash-put')!; + // Write only the block (no other content → file becomes empty after remove) + await fs.writeFile(profilePath, block + '\n', 'utf-8'); + await removeFromProfile(profilePath); + // File should be deleted when empty + const exists = await fs.access(profilePath).then(() => true).catch(() => false); + expect(exists).toBe(false); + } finally { + await cleanup(tmpDir); + } + }); +}); + +// ─── $SHELL unset → status unknown ─────────────────────────────────────────── + +describe('$SHELL unset handling', () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it('detectShell returns unknown when SHELL is not set', () => { + vi.stubEnv('SHELL', ''); + vi.stubEnv('PSModulePath', ''); + expect(detectShell()).toBe('unknown'); + }); + + it('getProfilePath returns null for unknown shell', () => { + expect(getProfilePath('unknown')).toBeNull(); + }); + + it('getSafeDeleteStatus returns unknown when SHELL unset', async () => { + vi.stubEnv('SHELL', ''); + vi.stubEnv('PSModulePath', ''); + const { status, profilePath } = await getSafeDeleteStatus(); + expect(status).toBe('unknown'); + expect(profilePath).toBeNull(); + }); +}); diff --git a/tests/security-command.test.ts b/tests/security-command.test.ts new file mode 100644 index 00000000..c2a05997 --- /dev/null +++ b/tests/security-command.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from 'vitest'; +import { countDenyEntries } from '../src/cli/commands/security.js'; + +describe('countDenyEntries', () => { + it('returns the length of a valid deny array', () => { + const settings = JSON.stringify({ + permissions: { + deny: ['Bash(rm -rf /*)', 'Bash(sudo *)', 'Read(.env)'], + }, + }); + expect(countDenyEntries(settings)).toBe(3); + }); + + it('returns 0 on JSON parse error', () => { + expect(countDenyEntries('{ not valid json')).toBe(0); + }); + + it('returns 0 when the deny key is absent', () => { + const settings = JSON.stringify({ permissions: { allow: [] } }); + expect(countDenyEntries(settings)).toBe(0); + }); + + it('returns 0 when deny is not an array (string value)', () => { + const settings = JSON.stringify({ permissions: { deny: 'Bash(rm -rf /*)' } }); + expect(countDenyEntries(settings)).toBe(0); + }); + + it('returns 0 when permissions key is absent', () => { + const settings = JSON.stringify({ hooks: {} }); + expect(countDenyEntries(settings)).toBe(0); + }); + + it('returns 0 for an empty deny array', () => { + const settings = JSON.stringify({ permissions: { deny: [] } }); + expect(countDenyEntries(settings)).toBe(0); + }); + + it('returns 0 when deny is null', () => { + const settings = JSON.stringify({ permissions: { deny: null } }); + expect(countDenyEntries(settings)).toBe(0); + }); +}); diff --git a/tests/sentinel.test.ts b/tests/sentinel.test.ts index 599c585f..1d7a1e70 100644 --- a/tests/sentinel.test.ts +++ b/tests/sentinel.test.ts @@ -504,7 +504,7 @@ describe('manageSentinel utility', () => { afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); it('creates sentinel file when disabled=false', async () => { - const sentinelPath = path.join(tmpDir, '.devflow', 'memory', '.working-memory-disabled'); + const sentinelPath = path.join(tmpDir, '.devflow', 'decisions', '.disabled'); await manageSentinel(sentinelPath, false); expect(fs.existsSync(sentinelPath)).toBe(true); }); @@ -523,21 +523,21 @@ describe('manageSentinel utility', () => { }); it('is idempotent when enabling with no sentinel present', async () => { - const sentinelPath = path.join(tmpDir, '.devflow', 'memory', '.working-memory-disabled'); + const sentinelPath = path.join(tmpDir, '.devflow', 'decisions', '.disabled'); // No sentinel exists — enabling again should not throw await expect(manageSentinel(sentinelPath, true)).resolves.toBeUndefined(); expect(fs.existsSync(sentinelPath)).toBe(false); }); it('is idempotent when disabling with sentinel already present', async () => { - const sentinelPath = path.join(tmpDir, '.devflow', 'memory', '.working-memory-disabled'); + const sentinelPath = path.join(tmpDir, '.devflow', 'decisions', '.disabled'); writeDisabledSentinel(sentinelPath); await expect(manageSentinel(sentinelPath, false)).resolves.toBeUndefined(); expect(fs.existsSync(sentinelPath)).toBe(true); }); it('disable then enable removes the sentinel', async () => { - const sentinelPath = path.join(tmpDir, '.devflow', 'memory', '.working-memory-disabled'); + const sentinelPath = path.join(tmpDir, '.devflow', 'decisions', '.disabled'); await manageSentinel(sentinelPath, false); expect(fs.existsSync(sentinelPath)).toBe(true); await manageSentinel(sentinelPath, true); diff --git a/tests/shell-hooks.test.ts b/tests/shell-hooks.test.ts index d0878b34..16fd8cf0 100644 --- a/tests/shell-hooks.test.ts +++ b/tests/shell-hooks.test.ts @@ -2321,21 +2321,23 @@ describe('dream-collect-tasks: single-pass scan', () => { expect(fs.existsSync(path.join(dreamDir, 'learning.sess2.json'))).toBe(false); }); - // AC-3: memory always swept (unconditional); disabled decisions/knowledge deleted; type never in _DREAM_TASKS - it('AC-3: memory always swept; disabled decisions/knowledge markers deleted; not in _DREAM_TASKS', () => { + // AC-3: memory always swept (unconditional); disabled decisions/knowledge/curation deleted; type never in _DREAM_TASKS + // Curation is now also swept when decisions is disabled (curation depends on decisions data). + it('AC-3: memory always swept; disabled decisions/knowledge/curation markers deleted when decisions disabled', () => { fs.writeFileSync(path.join(dreamDir, 'memory.sess1.json'), '{}'); // swept unconditionally fs.writeFileSync(path.join(dreamDir, 'decisions.sess1.json'), '{}'); fs.writeFileSync(path.join(dreamDir, 'knowledge.sess1.json'), '{}'); fs.writeFileSync(path.join(dreamDir, 'curation.sess1.json'), '{}'); - // Memory markers are always swept unconditionally (memory is no longer a Dream task) + // Memory markers are always swept unconditionally (memory is no longer a Dream task). + // Curation markers are also swept when decisions is disabled — curation inherits decisions state. const { tasks } = runCollectTasks(dreamDir, { decEnabled: false, knowEnabled: false }); - expect(tasks).toBe('curation'); + expect(tasks).toBe(''); expect(fs.existsSync(path.join(dreamDir, 'memory.sess1.json'))).toBe(false); expect(fs.existsSync(path.join(dreamDir, 'decisions.sess1.json'))).toBe(false); expect(fs.existsSync(path.join(dreamDir, 'knowledge.sess1.json'))).toBe(false); - // curation survives - expect(fs.existsSync(path.join(dreamDir, 'curation.sess1.json'))).toBe(true); + // curation swept because decisions is disabled + expect(fs.existsSync(path.join(dreamDir, 'curation.sess1.json'))).toBe(false); }); // AC-3b: memory markers always swept unconditionally (not flag-gated — ADR-016) diff --git a/tests/uninstall-logic.test.ts b/tests/uninstall-logic.test.ts index bd627d0b..980a40c9 100644 --- a/tests/uninstall-logic.test.ts +++ b/tests/uninstall-logic.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { computeAssetsToRemove, formatDryRunPlan } from '../src/cli/commands/uninstall.js'; +import { computeAssetsToRemove, formatDryRunPlan, resolveSecurityRemovalDecision } from '../src/cli/commands/uninstall.js'; import { DEVFLOW_PLUGINS, type PluginDefinition } from '../src/cli/plugins.js'; describe('computeAssetsToRemove', () => { @@ -192,3 +192,82 @@ describe('formatDryRunPlan', () => { expect(plan).toContain('Rules (2)'); }); }); + +describe('resolveSecurityRemovalDecision', () => { + // === Non-interactive preserve invariant (SAFETY PROPERTY) === + // When isTTY is false the deny list must NEVER be removed — avoids PF-004 + // half-applied-state hazard during scripted/CI uninstalls. + + it('returns preserve when security is present and isTTY is false (non-interactive invariant)', () => { + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: true, + keepDocs: false, + isTTY: false, + })).toBe('preserve'); + }); + + it('returns preserve regardless of keepDocs when isTTY is false and security is present', () => { + // keepDocs wins over isTTY only when keepDocs is true — tested separately below + // This asserts the priority order: skip (keepDocs) > preserve (non-TTY) > prompt + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: true, + keepDocs: false, + isTTY: false, + })).toBe('preserve'); + }); + + // === keepDocs gate === + + it('returns skip when keepDocs is true even if isTTY is true', () => { + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: true, + keepDocs: true, + isTTY: true, + })).toBe('skip'); + }); + + it('returns skip when keepDocs is true and isTTY is false', () => { + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: true, + keepDocs: true, + isTTY: false, + })).toBe('skip'); + }); + + // === nothing-present gate === + + it('returns skip when no security is present regardless of TTY or keepDocs', () => { + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: false, + keepDocs: false, + isTTY: true, + })).toBe('skip'); + + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: false, + keepDocs: false, + isTTY: false, + })).toBe('skip'); + }); + + // === interactive prompt path === + + it('returns prompt when security is present, keepDocs is false, and isTTY is true', () => { + expect(resolveSecurityRemovalDecision({ + anySecurityPresent: true, + keepDocs: false, + isTTY: true, + })).toBe('prompt'); + }); + + // === exhaustiveness — all three outcomes are reachable === + + it('covers all three return values', () => { + const skip = resolveSecurityRemovalDecision({ anySecurityPresent: false, keepDocs: false, isTTY: true }); + const preserve = resolveSecurityRemovalDecision({ anySecurityPresent: true, keepDocs: false, isTTY: false }); + const prompt = resolveSecurityRemovalDecision({ anySecurityPresent: true, keepDocs: false, isTTY: true }); + expect(skip).toBe('skip'); + expect(preserve).toBe('preserve'); + expect(prompt).toBe('prompt'); + }); +});