From d2fa696a3084de4f434b83b32e002094d9646ccc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 23:01:15 -0400 Subject: [PATCH] fix(#3479): treat absent default-true mempalace keys as enabled in every prose gate (#3527) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3479): treat absent default-true mempalace keys as enabled in every prose gate The #2982 absent-key fix (capture_artifacts === false) was applied to only one of the sibling gates. Five more hand-written gates in the mempalace skill/command mirrors and the curator agent still used positive presence ('when is true'), silently skipping default-enabled behavior (mirror_kg, diary_journal) whenever the key was absent from .planning/config.json — inverted from the registry-declared defaults. Corrected sites, each now disabled only on an explicit false: - skills/gsd-mempalace-capture/SKILL.md step 3 (mirror_kg) - commands/gsd/mempalace-capture.md step 3 (mirror_kg) - skills/gsd-mempalace-recall/SKILL.md step 3 (mirror_kg) - commands/gsd/mempalace-recall.md step 3 (mirror_kg) - agents/gsd-mempalace-curator.md tasks 1+2 (diary_journal, mirror_kg) Default-false keys (mempalace.enabled, cross_project_tunnels) keep their positive-presence gates. New #3479 regression cases in tests/mempalace-capture-gate-default.test.cjs lock each site's absent/explicit-false boundary and add a registry-parity guard: no gate file may positively gate any mempalace boolean whose registry-declared default is true. * chore(#3479): acknowledge curator size growth from the gate rewording * chore(#3479): add changeset fragment for PR #3527 --------- Co-authored-by: sim --- .../3527-mempalace-default-true-gates.md | 6 + agents/gsd-mempalace-curator.md | 4 +- commands/gsd/mempalace-capture.md | 2 +- commands/gsd/mempalace-recall.md | 2 +- skills/gsd-mempalace-capture/SKILL.md | 2 +- skills/gsd-mempalace-recall/SKILL.md | 2 +- .../3479-mempalace-default-true-gates.json | 8 + tests/mempalace-capture-gate-default.test.cjs | 162 ++++++++++++++++++ 8 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 .changeset/3527-mempalace-default-true-gates.md create mode 100644 tests/emitted-drift-acks/3479-mempalace-default-true-gates.json diff --git a/.changeset/3527-mempalace-default-true-gates.md b/.changeset/3527-mempalace-default-true-gates.md new file mode 100644 index 000000000..336122693 --- /dev/null +++ b/.changeset/3527-mempalace-default-true-gates.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 3527 +--- + +**MemPalace sub-features whose defaults are enabled now run when their config keys are absent** — the earlier `capture_artifacts` absent-key fix (#2982) had been applied to only one of six hand-written config gates; the remaining gates for `mempalace.mirror_kg` (knowledge-graph mirroring in the capture and recall skills, their command mirrors, and the curator agent) and `mempalace.diary_journal` (per-agent diary entries at ship) still required the key to be explicitly present and `true`, so a project that enabled MemPalace without writing every sub-toggle silently never mirrored KG facts or wrote diary entries, with no warning. All six gates now treat an absent key as enabled (matching the capability registry's declared `default: true`) and disable the behavior only on an explicit `false`; default-off switches (`mempalace.enabled`, `cross_project_tunnels`) still require explicit opt-in, and a registry-parity regression test keeps future default-true keys from reintroducing the inversion. (#3479) diff --git a/agents/gsd-mempalace-curator.md b/agents/gsd-mempalace-curator.md index b6e43a46d..813117bf5 100644 --- a/agents/gsd-mempalace-curator.md +++ b/agents/gsd-mempalace-curator.md @@ -27,9 +27,9 @@ If `mempalace.enabled !== true`, do nothing and report `MemPalace disabled — c ## Tasks (each independently best-effort) -1. **Diary entry** (when `mempalace.diary_journal` is true). Write one concise per-agent diary entry summarising the phase outcome: `mempalace_diary_write(agent_name=/, entry=, topic="phase-ship", wing=)` (CLI: `mempalace hook run` / the diary CLI). Namespace `agent_name` by repo+role so diaries don't collide across projects. **Idempotency:** before writing, `mempalace_diary_read` (or list) for an existing entry keyed by `(wing, agent_name, topic, phase-id)`; if one exists for this phase, update it in place rather than appending a second. +1. **Diary entry** (unless `mempalace.diary_journal !== false` — registry default is true, an absent key means enabled). Write one concise per-agent diary entry summarising the phase outcome: `mempalace_diary_write(agent_name=/, entry=, topic="phase-ship", wing=)` (CLI: `mempalace hook run` / the diary CLI). Namespace `agent_name` by repo+role so diaries don't collide across projects. **Idempotency:** before writing, `mempalace_diary_read` (or list) for an existing entry keyed by `(wing, agent_name, topic, phase-id)`; if one exists for this phase, update it in place rather than appending a second. -2. **extract-learnings → KG mirror** (when `mempalace.mirror_kg` is true). For each decision/lesson/pattern/surprise from the phase's learnings, add a typed KG triple with provenance (`source_file`, `source_drawer_id`) and `valid_from` = the phase date. **Idempotency:** the triple `(subject, predicate, object)` is the natural key — `mempalace_kg_query` for it first and skip `mempalace_kg_add` if it already exists with the same `valid_from`, so reruns don't fork duplicate facts. When a prior decision was superseded this phase, call `mempalace_kg_invalidate` to set its `valid_to` rather than deleting it. +2. **extract-learnings → KG mirror** (unless `mempalace.mirror_kg !== false` — registry default is true, an absent key means enabled). For each decision/lesson/pattern/surprise from the phase's learnings, add a typed KG triple with provenance (`source_file`, `source_drawer_id`) and `valid_from` = the phase date. **Idempotency:** the triple `(subject, predicate, object)` is the natural key — `mempalace_kg_query` for it first and skip `mempalace_kg_add` if it already exists with the same `valid_from`, so reruns don't fork duplicate facts. When a prior decision was superseded this phase, call `mempalace_kg_invalidate` to set its `valid_to` rather than deleting it. 3. **Cross-project tunnels** (when `mempalace.cross_project_tunnels` is true). Use `mempalace_find_tunnels` to surface related wings, then `mempalace_create_tunnel(label=…)` only for connections you (or the user) can justify. **Idempotency:** check the `find_tunnels` result first and skip creation if a tunnel with that `(source-wing, target-wing, label)` already exists. Do not mass-create tunnels. diff --git a/commands/gsd/mempalace-capture.md b/commands/gsd/mempalace-capture.md index eaa656b28..7a2537fc5 100644 --- a/commands/gsd/mempalace-capture.md +++ b/commands/gsd/mempalace-capture.md @@ -86,7 +86,7 @@ On any error or timeout, stop and let the phase continue -- capture is best-effo # Mine with --wing only — no --room flag; detect_room() assigns from the folder path mempalace mine "$STAGE" --wing ``` -3. **Mirror KG facts** when `config.mempalace.mirror_kg` is true: extract decision/delivery facts and `mempalace_kg_add` them with `valid_from` = the phase date (e.g. `(, decided, )` from CONTEXT; `(, delivered, )` from SUMMARY). Under `augment` these are an *additive* mirror of GSD's native `.planning/graphs/`. Under `kg_backend`/`replace` the palace KG is the *authoritative* fact store — GSD still produces `.planning/graphs/` through its normal graphify, so an unreachable palace never loses a fact. +3. **Mirror KG facts** unless `config.mempalace.mirror_kg === false` (registry default is true — an absent key means enabled): extract decision/delivery facts and `mempalace_kg_add` them with `valid_from` = the phase date (e.g. `(, decided, )` from CONTEXT; `(, delivered, )` from SUMMARY). Under `augment` these are an *additive* mirror of GSD's native `.planning/graphs/`. Under `kg_backend`/`replace` the palace KG is the *authoritative* fact store — GSD still produces `.planning/graphs/` through its normal graphify, so an unreachable palace never loses a fact. 4. Re-running a phase MUST NOT create duplicate drawers (deterministic ids + `check_duplicate`). ## Step 4 -- Report diff --git a/commands/gsd/mempalace-recall.md b/commands/gsd/mempalace-recall.md index fc3bd367a..f043be0a2 100644 --- a/commands/gsd/mempalace-recall.md +++ b/commands/gsd/mempalace-recall.md @@ -66,7 +66,7 @@ All calls in this step are side-effect-free. On any error or timeout, stop retri 2. **Targeted search:** - Interactive: `mempalace_search(query=, wing=)`. - Headless: `mempalace search "" --wing `. -3. **Knowledge-graph facts** (when `config.mempalace.mirror_kg` is true): `mempalace_kg_query` / `mempalace_kg_timeline` for decisions relevant to the topic and their validity windows. Under `augment` the palace KG *supplements* GSD's native `.planning/graphs/` — combine both, do not treat the palace as the sole source. Under `kg_backend` or `replace` the palace KG is the *primary* graph source — query it first and use `.planning/graphs/` only as a fallback when the palace is unreachable. +3. **Knowledge-graph facts** (unless `config.mempalace.mirror_kg !== false` — registry default is true, an absent key means enabled): `mempalace_kg_query` / `mempalace_kg_timeline` for decisions relevant to the topic and their validity windows. Under `augment` the palace KG *supplements* GSD's native `.planning/graphs/` — combine both, do not treat the palace as the sole source. Under `kg_backend` or `replace` the palace KG is the *primary* graph source — query it first and use `.planning/graphs/` only as a fallback when the palace is unreachable. 4. **Dedup** the returned drawers/facts; keep the top results. ## Step 4 -- Write MEMORY-RECALL.md diff --git a/skills/gsd-mempalace-capture/SKILL.md b/skills/gsd-mempalace-capture/SKILL.md index f7e984b54..3ca959181 100644 --- a/skills/gsd-mempalace-capture/SKILL.md +++ b/skills/gsd-mempalace-capture/SKILL.md @@ -86,7 +86,7 @@ On any error or timeout, stop and let the phase continue -- capture is best-effo # Mine with --wing only — no --room flag; detect_room() assigns from the folder path mempalace mine "$STAGE" --wing ``` -3. **Mirror KG facts** when `config.mempalace.mirror_kg` is true: extract decision/delivery facts and `mempalace_kg_add` them with `valid_from` = the phase date (e.g. `(, decided, )` from CONTEXT; `(, delivered, )` from SUMMARY). Under `augment` these are an *additive* mirror of GSD's native `.planning/graphs/`. Under `kg_backend`/`replace` the palace KG is the *authoritative* fact store — GSD still produces `.planning/graphs/` through its normal graphify, so an unreachable palace never loses a fact. +3. **Mirror KG facts** unless `config.mempalace.mirror_kg === false` (registry default is true — an absent key means enabled): extract decision/delivery facts and `mempalace_kg_add` them with `valid_from` = the phase date (e.g. `(, decided, )` from CONTEXT; `(, delivered, )` from SUMMARY). Under `augment` these are an *additive* mirror of GSD's native `.planning/graphs/`. Under `kg_backend`/`replace` the palace KG is the *authoritative* fact store — GSD still produces `.planning/graphs/` through its normal graphify, so an unreachable palace never loses a fact. 4. Re-running a phase MUST NOT create duplicate drawers (deterministic ids + `check_duplicate`). ## Step 4 -- Report diff --git a/skills/gsd-mempalace-recall/SKILL.md b/skills/gsd-mempalace-recall/SKILL.md index a62169980..40b619f16 100644 --- a/skills/gsd-mempalace-recall/SKILL.md +++ b/skills/gsd-mempalace-recall/SKILL.md @@ -66,7 +66,7 @@ All calls in this step are side-effect-free. On any error or timeout, stop retri 2. **Targeted search:** - Interactive: `mempalace_search(query=, wing=)`. - Headless: `mempalace search "" --wing `. -3. **Knowledge-graph facts** (when `config.mempalace.mirror_kg` is true): `mempalace_kg_query` / `mempalace_kg_timeline` for decisions relevant to the topic and their validity windows. Under `augment` the palace KG *supplements* GSD's native `.planning/graphs/` — combine both, do not treat the palace as the sole source. Under `kg_backend` or `replace` the palace KG is the *primary* graph source — query it first and use `.planning/graphs/` only as a fallback when the palace is unreachable. +3. **Knowledge-graph facts** (unless `config.mempalace.mirror_kg !== false` — registry default is true, an absent key means enabled): `mempalace_kg_query` / `mempalace_kg_timeline` for decisions relevant to the topic and their validity windows. Under `augment` the palace KG *supplements* GSD's native `.planning/graphs/` — combine both, do not treat the palace as the sole source. Under `kg_backend` or `replace` the palace KG is the *primary* graph source — query it first and use `.planning/graphs/` only as a fallback when the palace is unreachable. 4. **Dedup** the returned drawers/facts; keep the top results. ## Step 4 -- Write MEMORY-RECALL.md diff --git a/tests/emitted-drift-acks/3479-mempalace-default-true-gates.json b/tests/emitted-drift-acks/3479-mempalace-default-true-gates.json new file mode 100644 index 000000000..310ae4b01 --- /dev/null +++ b/tests/emitted-drift-acks/3479-mempalace-default-true-gates.json @@ -0,0 +1,8 @@ +{ + "version": 1, + "paths": { + "gsd-mempalace-curator.md": { + "reason": "#3479: the diary_journal and mirror_kg task gates changed from positive presence ('when is true') to disabled-only-on-explicit-false ('unless !== false — registry default is true, an absent key means enabled'), matching the registry-declared defaults. The ~124-byte growth is the two inline absence-semantics clarifications." + } + } +} diff --git a/tests/mempalace-capture-gate-default.test.cjs b/tests/mempalace-capture-gate-default.test.cjs index 9c581b250..ea92b7167 100644 --- a/tests/mempalace-capture-gate-default.test.cjs +++ b/tests/mempalace-capture-gate-default.test.cjs @@ -40,3 +40,165 @@ describe('#2641 — mempalace-capture gate treats absent capture_artifacts as en ); }); }); + +// #3479 — the #2982/#2641 fix was applied to the capture_artifacts gate but not +// to the sibling gates on other mempalace sub-toggles whose registry-declared +// default is `true`. A positive-presence gate ("when `X` is true") treats an +// absent key as disabled — inverted from the registry default. Every gate on a +// default-true key must treat absent as enabled (disabled only on an explicit +// false); every gate on a default-false key must keep requiring positive +// presence. The hand-maintained commands/gsd/*.md mirrors must stay in lockstep +// with their skills/*/SKILL.md originals. + +const RECALL_SKILL = path.join(__dirname, '..', 'skills', 'gsd-mempalace-recall', 'SKILL.md'); +const RECALL_COMMAND = path.join(__dirname, '..', 'commands', 'gsd', 'mempalace-recall.md'); +const CURATOR = path.join(__dirname, '..', 'agents', 'gsd-mempalace-curator.md'); +const REGISTRY_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'capability-registry.cjs'); + +/** Deep-search the mempalace capability for the flat settings schema keyed `mempalace.`. */ +function mempalaceSettings() { + const { capabilities } = require(REGISTRY_PATH); + const stack = [capabilities.mempalace]; + while (stack.length > 0) { + const node = stack.pop(); + if (!node || typeof node !== 'object') continue; + if (Object.prototype.hasOwnProperty.call(node, 'mempalace.mirror_kg')) return node; + for (const value of Object.values(node)) { + if (value && typeof value === 'object') stack.push(value); + } + } + return null; +} + +/** Boolean mempalace. settings partitioned by their registry-declared default. */ +function mempalaceBooleansByDefault() { + const settings = mempalaceSettings(); + assert.ok(settings, 'capability registry must expose a mempalace settings schema'); + const defaultTrue = []; + const defaultFalse = []; + for (const [key, spec] of Object.entries(settings)) { + if (!key.startsWith('mempalace.') || spec.type !== 'boolean') continue; + (spec.default === true ? defaultTrue : defaultFalse).push(key); + } + return { defaultTrue, defaultFalse }; +} + +describe('#3479 — gates on default-true mempalace keys treat an absent key as enabled', () => { + test('capture SKILL.md mirror_kg gate is disabled-only-on-explicit-false', () => { + const text = fs.readFileSync(SKILL, 'utf8'); + assert.ok( + text.includes('config.mempalace.mirror_kg === false'), + 'skills/gsd-mempalace-capture/SKILL.md step 3 must mirror KG facts unless config.mempalace.mirror_kg === false (#3479)', + ); + assert.ok( + !text.includes('config.mempalace.mirror_kg` is true'), + 'skills/gsd-mempalace-capture/SKILL.md must NOT gate mirror_kg on positive presence — absent means enabled per the registry default (#3479)', + ); + }); + + test('commands/gsd/mempalace-capture.md mirror_kg gate matches the skill', () => { + const text = fs.readFileSync(COMMAND, 'utf8'); + assert.ok( + text.includes('config.mempalace.mirror_kg === false'), + 'commands/gsd/mempalace-capture.md is a hand-maintained mirror of the skill — its mirror_kg gate needs the same fix (#3479)', + ); + assert.ok( + !text.includes('config.mempalace.mirror_kg` is true'), + 'commands/gsd/mempalace-capture.md must NOT gate mirror_kg on positive presence (#3479)', + ); + }); + + test('recall SKILL.md mirror_kg gate is disabled-only-on-explicit-false', () => { + const text = fs.readFileSync(RECALL_SKILL, 'utf8'); + assert.ok( + text.includes('config.mempalace.mirror_kg !== false'), + 'skills/gsd-mempalace-recall/SKILL.md KG-facts step must include mirror_kg unless !== false, matching its recall_on_plan gate style (#3479)', + ); + assert.ok( + !text.includes('config.mempalace.mirror_kg` is true'), + 'skills/gsd-mempalace-recall/SKILL.md must NOT gate mirror_kg on positive presence (#3479)', + ); + }); + + test('commands/gsd/mempalace-recall.md mirror_kg gate matches the skill', () => { + const text = fs.readFileSync(RECALL_COMMAND, 'utf8'); + assert.ok( + text.includes('config.mempalace.mirror_kg !== false'), + 'commands/gsd/mempalace-recall.md is a hand-maintained mirror of the skill — its mirror_kg gate needs the same fix (#3479)', + ); + assert.ok( + !text.includes('config.mempalace.mirror_kg` is true'), + 'commands/gsd/mempalace-recall.md must NOT gate mirror_kg on positive presence (#3479)', + ); + }); + + test('curator agent diary_journal and mirror_kg gates are disabled-only-on-explicit-false', () => { + const text = fs.readFileSync(CURATOR, 'utf8'); + assert.ok( + text.includes('mempalace.diary_journal !== false'), + 'agents/gsd-mempalace-curator.md diary gate must run unless mempalace.diary_journal !== false (#3479)', + ); + assert.ok( + text.includes('mempalace.mirror_kg !== false'), + 'agents/gsd-mempalace-curator.md KG-mirror gate must run unless mempalace.mirror_kg !== false (#3479)', + ); + assert.ok( + !text.includes('mempalace.diary_journal` is true'), + 'agents/gsd-mempalace-curator.md must NOT gate diary_journal on positive presence (#3479)', + ); + assert.ok( + !text.includes('mempalace.mirror_kg` is true'), + 'agents/gsd-mempalace-curator.md must NOT gate mirror_kg on positive presence (#3479)', + ); + }); +}); + +describe('#3479 — registry parity guard: no default-true mempalace key is positively gated', () => { + test('every default-true mempalace boolean defaults to enabled in the registry', () => { + // Lock the declared defaults the corrected gates depend on: if one of these + // flips in the registry, the absence semantics of its prose gates must be + // re-audited — fail here so that happens consciously. + const { defaultTrue } = mempalaceBooleansByDefault(); + for (const key of ['mempalace.capture_artifacts', 'mempalace.mirror_kg', 'mempalace.diary_journal']) { + assert.ok(defaultTrue.includes(key), `${key} must declare default: true in the capability registry`); + } + }); + + test('no gate file uses positive presence for a default-true key', () => { + const { defaultTrue } = mempalaceBooleansByDefault(); + const files = [SKILL, COMMAND, RECALL_SKILL, RECALL_COMMAND, CURATOR]; + for (const file of files) { + const text = fs.readFileSync(file, 'utf8'); + for (const key of defaultTrue) { + for (const refix of ['config.' + key, key]) { + assert.ok( + !text.includes('`' + refix + '` is true'), + `${path.relative(process.cwd(), file)} gates \`${key}\` on positive presence, but the registry declares default: true — absent must mean enabled (#3479)`, + ); + } + } + } + }); + + test('default-false keys keep requiring positive presence (no over-correction)', () => { + const { defaultFalse } = mempalaceBooleansByDefault(); + assert.ok( + defaultFalse.includes('mempalace.enabled'), + 'mempalace.enabled must stay default: false — the master switch is opt-in', + ); + const capture = fs.readFileSync(SKILL, 'utf8'); + const curator = fs.readFileSync(CURATOR, 'utf8'); + assert.ok( + capture.includes('config.mempalace.enabled !== true'), + 'capture master gate must keep treating an absent mempalace.enabled as disabled (#3479)', + ); + assert.ok( + curator.includes('mempalace.enabled !== true'), + 'curator master gate must keep treating an absent mempalace.enabled as disabled (#3479)', + ); + assert.ok( + curator.includes('mempalace.cross_project_tunnels` is true'), + 'cross_project_tunnels (default: false) must keep its positive-presence gate — do not over-correct (#3479)', + ); + }); +});