feat(memory): add memory.autoWrite alias for autoMemoryEnabled (#1326) (#1396)

* feat(memory): add memory.autoWrite alias for autoMemoryEnabled (#1326)

The attribution half of #1326 was fixed via #1335 (merged 2026-05-26).
The memory half — '[memory writes should be] explicit and configurable'
with the exact shape `memory.autoWrite` requested by the issue —
remains.

Rather than parallel-tracking a new key, alias `memory.autoWrite` to
the existing `autoMemoryEnabled` opt-out and document the relationship.
Either key opts out; when both are set, the more restrictive (false)
value wins so a parent-scope opt-out can't be silently re-enabled by a
narrower memory.autoWrite: true.

The new `memory` namespace is intentional — future opt-in fields
(approval gates, etc.) can be added under it without claiming a new
top-level key each time.

- types.ts: add `memory.autoWrite` to the settings schema; cross-link
  to autoMemoryEnabled in the description.
- paths.ts isAutoMemoryEnabled: read both keys; opt-out wins on
  conflict; default unchanged (enabled).
- paths.test.ts (new): pins default, both opt-out paths, both opt-in
  paths, opt-out-wins-on-conflict in both directions, env-var still
  overrides settings.

Tests 7/7 green. Default behavior unchanged — this is purely an
additive discoverable alias for governance / regulated / client-
sensitive repos that prefer the namespaced shape called out in the
issue.

* fix(memory): evaluate autoWrite opt-out across raw settings sources (#1326)

isAutoMemoryEnabled() read the already-merged settings object, so source
precedence had already collapsed same-key values before the "false wins"
rule applied: a lower-priority memory.autoWrite/autoMemoryEnabled: false
opt-out was silently overwritten by a higher-priority true, re-enabling
auto-memory against the stated governance guarantee.

Evaluate the opt-out across the raw per-source settings instead, via
getEnabledSettingSources() + getSettingsForSource() (per-source cached, so
the hot path stays cheap). A single false in any source now wins, so a
parent-scope opt-out cannot be re-enabled by a narrower scope flipping the
key to true.

Test now drives per-source fixtures and covers the cross-source precedence
case (lower-priority false beats higher-priority true) for both keys.

* test(memory): stop the autoWrite test leaking settings mocks across files

The previous test mock.module()'d both settings.js and constants.js. bun's
mock.restore() does not undo mock.module(), so the constants.js stub leaked
into later serial test files and broke flagSettings.test.ts (its cache-busted
settings import still resolved the mocked getEnabledSettingSources).

Drive the real getEnabledSettingSources() via setAllowedSettingSources()
instead of mocking constants, stub only getSettingsForSource, and re-register
the real settings module after each test so nothing leaks. Coverage is
unchanged (per-source fixtures + cross-source precedence cases).
This commit is contained in:
Nikhil
2026-06-17 10:25:34 +08:00
committed by GitHub
parent 8f88608055
commit b8c7c3bfac
3 changed files with 161 additions and 9 deletions
+124
View File
@@ -0,0 +1,124 @@
import { afterEach, beforeEach, expect, test, mock } from 'bun:test'
import { setAllowedSettingSources } from '../bootstrap/state.js'
import { SETTING_SOURCES } from '../utils/settings/constants.js'
import * as realSettings from '../utils/settings/settings.js'
import { isAutoMemoryEnabled } from './paths.ts'
// Pin issue #1326: `memory.autoWrite` is a discoverable alias for the legacy
// `autoMemoryEnabled` setting, and either key opts out for governance /
// regulated / client-sensitive repos. The opt-out is evaluated across the raw
// per-source settings (low-to-high priority) rather than the merged object, so
// a `false` in any source survives source-precedence merging and a parent-scope
// opt-out can't be silently re-enabled by a narrower scope flipping the key.
let _originalEnv: Record<string, string | undefined> = {}
type SourceFixture = { source: string; settings: Record<string, unknown> }
let _sources: SourceFixture[] = []
// Drive the raw per-source view that isAutoMemoryEnabled() reads. Sources are
// listed low-to-high priority (userSettings lowest, policySettings highest) —
// the order getEnabledSettingSources() yields. We use the REAL enabled-sources
// list (all sources are allowed below) and only stub getSettingsForSource, so
// no shared module other than settings.js is mocked.
function mockSources(sources: SourceFixture[]): void {
_sources = sources
}
beforeEach(() => {
_originalEnv = {
CLAUDE_CODE_DISABLE_AUTO_MEMORY: process.env.CLAUDE_CODE_DISABLE_AUTO_MEMORY,
CLAUDE_CODE_SIMPLE: process.env.CLAUDE_CODE_SIMPLE,
CLAUDE_CODE_REMOTE: process.env.CLAUDE_CODE_REMOTE,
CLAUDE_CODE_REMOTE_MEMORY_DIR: process.env.CLAUDE_CODE_REMOTE_MEMORY_DIR,
}
delete process.env.CLAUDE_CODE_DISABLE_AUTO_MEMORY
delete process.env.CLAUDE_CODE_SIMPLE
delete process.env.CLAUDE_CODE_REMOTE
delete process.env.CLAUDE_CODE_REMOTE_MEMORY_DIR
_sources = []
// Enable every source so getEnabledSettingSources() returns the full set in
// priority order; the fixtures decide which of them carry a value.
setAllowedSettingSources([...SETTING_SOURCES])
// Stub only the per-source reader. Spread the real module so every other
// export keeps its real binding.
mock.module('../utils/settings/settings.js', () => ({
...realSettings,
getSettingsForSource: (source: string) =>
_sources.find(s => s.source === source)?.settings ?? null,
}))
})
afterEach(() => {
for (const [k, v] of Object.entries(_originalEnv)) {
if (v === undefined) {
delete process.env[k]
} else {
process.env[k] = v
}
}
setAllowedSettingSources([...SETTING_SOURCES])
// mock.restore() undoes spies but NOT mock.module() registrations, which
// otherwise leak into later test files in the same (serial) run. Re-register
// the real settings module so the process is left clean.
mock.module('../utils/settings/settings.js', () => ({ ...realSettings }))
mock.restore()
})
test('defaults to enabled when no source sets the key and no env override', () => {
mockSources([{ source: 'userSettings', settings: {} }])
expect(isAutoMemoryEnabled()).toBe(true)
})
test('memory.autoWrite: false opts out via the new discoverable alias (#1326)', () => {
mockSources([
{ source: 'projectSettings', settings: { memory: { autoWrite: false } } },
])
expect(isAutoMemoryEnabled()).toBe(false)
})
test('memory.autoWrite: true explicitly opts in', () => {
mockSources([
{ source: 'projectSettings', settings: { memory: { autoWrite: true } } },
])
expect(isAutoMemoryEnabled()).toBe(true)
})
test('legacy autoMemoryEnabled: false still opts out (back-compat)', () => {
mockSources([{ source: 'projectSettings', settings: { autoMemoryEnabled: false } }])
expect(isAutoMemoryEnabled()).toBe(false)
})
test('legacy autoMemoryEnabled: true still opts in (back-compat)', () => {
mockSources([{ source: 'projectSettings', settings: { autoMemoryEnabled: true } }])
expect(isAutoMemoryEnabled()).toBe(true)
})
test('a lower-priority memory.autoWrite: false survives a higher-priority true (#1326)', () => {
// The regression the merged-object read missed: source precedence collapses
// same-key values, so getInitialSettings() would keep only the higher-priority
// `true` and silently re-enable auto-memory. Evaluating the raw per-source
// list keeps the parent-scope opt-out authoritative.
mockSources([
{ source: 'userSettings', settings: { memory: { autoWrite: false } } },
{ source: 'localSettings', settings: { memory: { autoWrite: true } } },
])
expect(isAutoMemoryEnabled()).toBe(false)
})
test('a parent autoMemoryEnabled: false is not re-enabled by a narrower memory.autoWrite: true', () => {
mockSources([
{ source: 'projectSettings', settings: { autoMemoryEnabled: false } },
{ source: 'localSettings', settings: { memory: { autoWrite: true } } },
])
expect(isAutoMemoryEnabled()).toBe(false)
})
test('env var still overrides settings', () => {
process.env.CLAUDE_CODE_DISABLE_AUTO_MEMORY = '1'
mockSources([
{ source: 'projectSettings', settings: { memory: { autoWrite: true } } },
])
expect(isAutoMemoryEnabled()).toBe(false)
})
+23 -8
View File
@@ -13,10 +13,8 @@ import {
} from '../utils/envUtils.js'
import { findCanonicalGitRoot } from '../utils/git.js'
import { sanitizePath } from '../utils/path.js'
import {
getInitialSettings,
getSettingsForSource,
} from '../utils/settings/settings.js'
import { getEnabledSettingSources } from '../utils/settings/constants.js'
import { getSettingsForSource } from '../utils/settings/settings.js'
/**
* Whether auto-memory features are enabled (memdir, agent memory, past session search).
@@ -24,7 +22,10 @@ import {
* 1. CLAUDE_CODE_DISABLE_AUTO_MEMORY env var (1/true → OFF, 0/false → ON)
* 2. CLAUDE_CODE_SIMPLE (--bare) → OFF
* 3. CCR without persistent storage → OFF (no CLAUDE_CODE_REMOTE_MEMORY_DIR)
* 4. autoMemoryEnabled in settings.json (supports project-level opt-out)
* 4. settings.json — `memory.autoWrite` and `autoMemoryEnabled` are equivalent
* opt-outs (#1326), evaluated across the raw per-source settings so a
* single `false` in any source wins. A parent-scope opt-out can't be
* silently re-enabled by a narrower scope flipping the same key to `true`.
* 5. Default: enabled
*/
export function isAutoMemoryEnabled(): boolean {
@@ -47,9 +48,23 @@ export function isAutoMemoryEnabled(): boolean {
) {
return false
}
const settings = getInitialSettings()
if (settings.autoMemoryEnabled !== undefined) {
return settings.autoMemoryEnabled
// Evaluate the opt-out across the raw per-source settings rather than the
// merged object. Source precedence collapses same-key values, so a
// lower-priority `false` opt-out would otherwise be silently overwritten by a
// higher-priority `true` (e.g. a shared/project `memory.autoWrite: false`
// beaten by a local/flag `memory.autoWrite: true`). `memory.autoWrite` and
// `autoMemoryEnabled` are equivalent; a single `false` in any source wins, so
// a parent-scope opt-out cannot be re-enabled by a narrower scope (#1326).
// Per-source reads are cached (getSettingsForSource), so this stays cheap on
// the hot path.
for (const source of getEnabledSettingSources()) {
const sourceSettings = getSettingsForSource(source)
if (
sourceSettings?.autoMemoryEnabled === false ||
sourceSettings?.memory?.autoWrite === false
) {
return false
}
}
return true
}
+14 -1
View File
@@ -1015,7 +1015,20 @@ export const SettingsSchema = lazySchema(() =>
.boolean()
.optional()
.describe(
'Enable auto-memory for this project. When false, Claude will not read from or write to the auto-memory directory.',
'Enable auto-memory for this project. When false, Claude will not read from or write to the auto-memory directory. Equivalent to `memory.autoWrite` — see that setting for the governance-focused shape requested in #1326.',
),
memory: z
.object({
autoWrite: z
.boolean()
.optional()
.describe(
'When false, disables auto-memory reads and writes for this project. Discoverable alias for `autoMemoryEnabled`; the two are equivalent and either one can be used to opt out for governance / regulated / client-sensitive repos (issue #1326). When both are set, the more restrictive (false) value wins so a parent-scope opt-out cannot be silently re-enabled by a narrower scope.',
),
})
.optional()
.describe(
'Memory governance settings. Currently exposes `autoWrite` as the discoverable shape requested in #1326; further opt-in fields (e.g. approval gates) may be added under this namespace without taking a new top-level key each time.',
),
autoMemoryDirectory: z
.string()