From be1b76dddd82297eb8ee236c4e1bcd286c91219d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 17 Sep 2026 06:30:36 -0400 Subject: [PATCH] fix(#4700): queue the headless mempalace mine on the palace lock (#4821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4699): add failing-first coverage for skipping complete phases in next_phase * fix(#4699): skip already-complete phases in the next_phase cascade Both next-phase scans selected the numerically lowest phase above N without consulting completion state, so completing a reopened phase persisted an already-[x] phase as STATE.md current_phase while roadmap.analyze correctly named the outstanding one (issue repro: completing 2 with phases 1 and 3 already [x] returned next_phase 03). The cascade collects the complete phase numbers from the roadmap checkboxes (milestone-scoped, comparePhaseNum-deduped) and skips them in both the disk scan and the roadmap scan; a [x] checkbox row and its heading sibling both name a phase that is never next. Heading-only and checkbox-less roadmaps behave exactly as before. * test(#4699): align the negative-control expectation with the disk spelling * test(#4699): pin the STATE.md persistence and the all-later-complete tail corner Review findings: the regression never asserted STATE.md current_phase (the issue's actual harm), and the all-later-phases-[x] corner (is_last_phase true, next_phase null) was unpinned. A changeset fragment is included. * docs(#4699): backfill changeset PR number * test(#4700): add failing-first coverage for the queued headless mine * fix(#4700): queue the headless mempalace mine and surface skipped captures The capture's mine ran in the foreground with no lock handling: MemPalace wraps every mine in a per-palace lock, so any concurrent writer (two phases finishing a stage at once, a git-hook refresh mining the same palace) made it exit 1 (MineAlreadyRunning) and the onError: skip step silently dropped the capture — unlost for CONTEXT/PLAN/SUMMARY files that can be re-filed, unrecoverable for execute:wave:post problem-fix pairs. The mine now queues via --daemon --background (MemPalace #2029: the daemon holds a job refused the lock and runs it when the holder exits), and the report step gains the queued and skipped outcomes per #4700's requirement that a skipped capture never stay silent. Option 2 (write_routing.cli) is unreleased at MemPalace 3.9.0; option 3 (retry) re-enters the same lock race — both declined in the PR body. * fix(#4700): queue the wave:post problems fragment's headless mine too The issue names the execute:wave:post problem-fix pair as the unrecoverable loss (no source file to re-file later); the capture-problems fragment's headless mine ran foreground like the capture capability's did. Same fix: --daemon --background, with the lock-deferral rationale inline. * docs(#4700): backfill changeset PR number * fix(#4682): register the stale-reverification part in the capability registry The new steps/ part is a shipped workflow file; gen-capability-registry --check requires it in the committed registry. --------- Co-authored-by: sim --- .changeset/serene-deer-squeak.md | 5 ++ .../mempalace/fragments/capture-problems.md | 2 +- commands/gsd/mempalace-capture.md | 10 ++-- gsd-core/bin/lib/capability-registry.cjs | 4 +- skills/gsd-mempalace-capture/SKILL.md | 10 ++-- ...alace-capture-headless-invocation.test.cjs | 52 +++++++++++++++++++ 6 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 .changeset/serene-deer-squeak.md diff --git a/.changeset/serene-deer-squeak.md b/.changeset/serene-deer-squeak.md new file mode 100644 index 000000000..a82f5eaff --- /dev/null +++ b/.changeset/serene-deer-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4821 +--- +**mempalace capture queues instead of dropping writes** — the headless `mempalace mine` ran in the foreground, so a concurrent writer holding the palace lock made it exit 1 and the onError: skip step silently dropped the capture. The mine now queues via `--daemon --background` (MemPalace runs it when the lock frees), and the capture report surfaces the queued or skipped outcome instead of staying silent. (#4700) diff --git a/capabilities/mempalace/fragments/capture-problems.md b/capabilities/mempalace/fragments/capture-problems.md index 4a86a4071..ff413f795 100644 --- a/capabilities/mempalace/fragments/capture-problems.md +++ b/capabilities/mempalace/fragments/capture-problems.md @@ -13,7 +13,7 @@ For each confirmed bug/issue resolved in this wave: 1. **Resolve the wing** (`mempalace.wing`, else `project_code`, else project dir) and target `room: problems`. 2. **Dedupe first.** Call `mempalace_check_duplicate` (interactive) before filing so re-runs don't create duplicate drawers. -3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine` (no `--room` flag — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id). +3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine --daemon --background` (no `--room` flag; the background queue defers the write when another writer holds the palace lock, #4700 — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id). 4. **Mirror the KG fact** when `mempalace.mirror_kg` is on: add `(, fixed_by, )` with `valid_from` = the phase date via `mempalace_kg_add`. 5. **Mode awareness** (`mempalace.memory_mode`). Under `augment` the fact is an *additive* mirror alongside `.planning/graphs/`. Under `kg_backend`/`replace` the palace is the *authoritative* store for the fact; GSD still writes `.planning/graphs/` through its normal graphify, so an unreachable palace never loses it. diff --git a/commands/gsd/mempalace-capture.md b/commands/gsd/mempalace-capture.md index 2cc5a9e01..47420c98c 100644 --- a/commands/gsd/mempalace-capture.md +++ b/commands/gsd/mempalace-capture.md @@ -84,15 +84,19 @@ On any error or timeout, stop and let the phase continue -- capture is best-effo ROOM_DIR="$STAGE//" mkdir -p "$ROOM_DIR" cp "" "$ROOM_DIR/" - # Mine with --wing only — no --room flag; detect_room() assigns from the folder path - mempalace mine "$STAGE" --wing + # Mine with --wing only — no --room flag; detect_room() assigns from the folder path. + # --daemon --background (#4700): MemPalace wraps every mine in a per-palace + # lock; a concurrent writer would make a foreground mine exit 1 + # (MineAlreadyRunning) and this onError: skip step would silently drop the + # capture. The background queue defers the job until the lock frees. + mempalace mine "$STAGE" --wing --daemon --background ``` 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 -Print a one-line summary: `Filed → / ( KG facts)` or `MemPalace unavailable — capture skipped`. +Print a one-line summary: `Filed → / ( KG facts)`, `Capture queued — palace busy; MemPalace will file it when the lock frees` (the background mine was deferred behind another writer), or `MemPalace capture skipped — ` (#4700: a skipped capture is never silent). ## Anti-Patterns diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 229ab5fd1..af443b529 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -2731,7 +2731,7 @@ const capabilities = { "into": "verifier", "fragment": { "path": "fragments/capture-problems.md", - "inline": "\n### Capture problems → fixes (MemPalace)\n\n**Gate first.** Read `.planning/config.json`. If `mempalace.enabled` is not `true`, or `mempalace.capture_artifacts` is `false`, **skip this entire section** and let the wave complete unchanged. (This contribution is only injected when the capability is enabled; the `capture_artifacts` check lets you turn capture off without disabling the rest of the capability.)\n\nOtherwise — after verifying this wave, persist any *confirmed* problem→fix pairs into the palace so they are recalled in future phases. This is best-effort; if MemPalace is unreachable, skip silently — capture never fails a wave.\n\nFor each confirmed bug/issue resolved in this wave:\n\n1. **Resolve the wing** (`mempalace.wing`, else `project_code`, else project dir) and target `room: problems`.\n2. **Dedupe first.** Call `mempalace_check_duplicate` (interactive) before filing so re-runs don't create duplicate drawers.\n3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine` (no `--room` flag — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id).\n4. **Mirror the KG fact** when `mempalace.mirror_kg` is on: add `(, fixed_by, )` with `valid_from` = the phase date via `mempalace_kg_add`.\n5. **Mode awareness** (`mempalace.memory_mode`). Under `augment` the fact is an *additive* mirror alongside `.planning/graphs/`. Under `kg_backend`/`replace` the palace is the *authoritative* store for the fact; GSD still writes `.planning/graphs/` through its normal graphify, so an unreachable palace never loses it.\n\nCaptures are idempotent: deterministic drawer IDs + `check_duplicate` mean re-running the wave re-files the same content without duplication. On any error, skip and let the wave complete normally.\n" + "inline": "\n### Capture problems → fixes (MemPalace)\n\n**Gate first.** Read `.planning/config.json`. If `mempalace.enabled` is not `true`, or `mempalace.capture_artifacts` is `false`, **skip this entire section** and let the wave complete unchanged. (This contribution is only injected when the capability is enabled; the `capture_artifacts` check lets you turn capture off without disabling the rest of the capability.)\n\nOtherwise — after verifying this wave, persist any *confirmed* problem→fix pairs into the palace so they are recalled in future phases. This is best-effort; if MemPalace is unreachable, skip silently — capture never fails a wave.\n\nFor each confirmed bug/issue resolved in this wave:\n\n1. **Resolve the wing** (`mempalace.wing`, else `project_code`, else project dir) and target `room: problems`.\n2. **Dedupe first.** Call `mempalace_check_duplicate` (interactive) before filing so re-runs don't create duplicate drawers.\n3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine --daemon --background` (no `--room` flag; the background queue defers the write when another writer holds the palace lock, #4700 — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id).\n4. **Mirror the KG fact** when `mempalace.mirror_kg` is on: add `(, fixed_by, )` with `valid_from` = the phase date via `mempalace_kg_add`.\n5. **Mode awareness** (`mempalace.memory_mode`). Under `augment` the fact is an *additive* mirror alongside `.planning/graphs/`. Under `kg_backend`/`replace` the palace is the *authoritative* store for the fact; GSD still writes `.planning/graphs/` through its normal graphify, so an unreachable palace never loses it.\n\nCaptures are idempotent: deterministic drawer IDs + `check_duplicate` mean re-running the wave re-files the same content without duplication. On any error, skip and let the wave complete normally.\n" }, "produces": [], "consumes": [], @@ -4587,7 +4587,7 @@ const byLoopPoint = { "into": "verifier", "fragment": { "path": "fragments/capture-problems.md", - "inline": "\n### Capture problems → fixes (MemPalace)\n\n**Gate first.** Read `.planning/config.json`. If `mempalace.enabled` is not `true`, or `mempalace.capture_artifacts` is `false`, **skip this entire section** and let the wave complete unchanged. (This contribution is only injected when the capability is enabled; the `capture_artifacts` check lets you turn capture off without disabling the rest of the capability.)\n\nOtherwise — after verifying this wave, persist any *confirmed* problem→fix pairs into the palace so they are recalled in future phases. This is best-effort; if MemPalace is unreachable, skip silently — capture never fails a wave.\n\nFor each confirmed bug/issue resolved in this wave:\n\n1. **Resolve the wing** (`mempalace.wing`, else `project_code`, else project dir) and target `room: problems`.\n2. **Dedupe first.** Call `mempalace_check_duplicate` (interactive) before filing so re-runs don't create duplicate drawers.\n3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine` (no `--room` flag — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id).\n4. **Mirror the KG fact** when `mempalace.mirror_kg` is on: add `(, fixed_by, )` with `valid_from` = the phase date via `mempalace_kg_add`.\n5. **Mode awareness** (`mempalace.memory_mode`). Under `augment` the fact is an *additive* mirror alongside `.planning/graphs/`. Under `kg_backend`/`replace` the palace is the *authoritative* store for the fact; GSD still writes `.planning/graphs/` through its normal graphify, so an unreachable palace never loses it.\n\nCaptures are idempotent: deterministic drawer IDs + `check_duplicate` mean re-running the wave re-files the same content without duplication. On any error, skip and let the wave complete normally.\n" + "inline": "\n### Capture problems → fixes (MemPalace)\n\n**Gate first.** Read `.planning/config.json`. If `mempalace.enabled` is not `true`, or `mempalace.capture_artifacts` is `false`, **skip this entire section** and let the wave complete unchanged. (This contribution is only injected when the capability is enabled; the `capture_artifacts` check lets you turn capture off without disabling the rest of the capability.)\n\nOtherwise — after verifying this wave, persist any *confirmed* problem→fix pairs into the palace so they are recalled in future phases. This is best-effort; if MemPalace is unreachable, skip silently — capture never fails a wave.\n\nFor each confirmed bug/issue resolved in this wave:\n\n1. **Resolve the wing** (`mempalace.wing`, else `project_code`, else project dir) and target `room: problems`.\n2. **Dedupe first.** Call `mempalace_check_duplicate` (interactive) before filing so re-runs don't create duplicate drawers.\n3. **File the drawer verbatim.** Store the problem statement and its fix as a drawer in `room: problems` — interactive: `mempalace_add_drawer`; headless: stage the artifact under the `problems/` folder and run `mempalace mine --daemon --background` (no `--room` flag; the background queue defers the write when another writer holds the palace lock, #4700 — see [CLI reference](https://mempalaceofficial.com/reference/cli.html); room assignment is via `detect_room()` folder-path match per the [mining guide](https://mempalaceofficial.com/guide/mining.html); use the same staging pattern documented in `gsd-mempalace-capture` Step 3). Include provenance (`source_file`, phase id).\n4. **Mirror the KG fact** when `mempalace.mirror_kg` is on: add `(, fixed_by, )` with `valid_from` = the phase date via `mempalace_kg_add`.\n5. **Mode awareness** (`mempalace.memory_mode`). Under `augment` the fact is an *additive* mirror alongside `.planning/graphs/`. Under `kg_backend`/`replace` the palace is the *authoritative* store for the fact; GSD still writes `.planning/graphs/` through its normal graphify, so an unreachable palace never loses it.\n\nCaptures are idempotent: deterministic drawer IDs + `check_duplicate` mean re-running the wave re-files the same content without duplication. On any error, skip and let the wave complete normally.\n" }, "produces": [], "consumes": [], diff --git a/skills/gsd-mempalace-capture/SKILL.md b/skills/gsd-mempalace-capture/SKILL.md index 337209f92..b9fb80b74 100644 --- a/skills/gsd-mempalace-capture/SKILL.md +++ b/skills/gsd-mempalace-capture/SKILL.md @@ -84,15 +84,19 @@ On any error or timeout, stop and let the phase continue -- capture is best-effo ROOM_DIR="$STAGE//" mkdir -p "$ROOM_DIR" cp "" "$ROOM_DIR/" - # Mine with --wing only — no --room flag; detect_room() assigns from the folder path - mempalace mine "$STAGE" --wing + # Mine with --wing only — no --room flag; detect_room() assigns from the folder path. + # --daemon --background (#4700): MemPalace wraps every mine in a per-palace + # lock; a concurrent writer would make a foreground mine exit 1 + # (MineAlreadyRunning) and this onError: skip step would silently drop the + # capture. The background queue defers the job until the lock frees. + mempalace mine "$STAGE" --wing --daemon --background ``` 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 -Print a one-line summary: `Filed → / ( KG facts)` or `MemPalace unavailable — capture skipped`. +Print a one-line summary: `Filed → / ( KG facts)`, `Capture queued — palace busy; MemPalace will file it when the lock frees` (the background mine was deferred behind another writer), or `MemPalace capture skipped — ` (#4700: a skipped capture is never silent). ## Anti-Patterns diff --git a/tests/mempalace-capture-headless-invocation.test.cjs b/tests/mempalace-capture-headless-invocation.test.cjs index 66b6efa9d..c8891b3ee 100644 --- a/tests/mempalace-capture-headless-invocation.test.cjs +++ b/tests/mempalace-capture-headless-invocation.test.cjs @@ -22,6 +22,8 @@ const fs = require('fs'); const path = require('path'); const yaml = require('js-yaml'); +const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); + const ROOT = path.resolve(__dirname, '..'); const SKILL_FILES = [ @@ -148,3 +150,53 @@ describe('#2414 — rooms: entries are dicts with a name key (not bare strings)' }); } }); + +// ── #4700 — the headless mine must queue on the palace lock, and a skipped +// capture must be visible in the report. MemPalace wraps every mine in a +// per-palace lock; a concurrent writer makes a plain foreground `mine` exit 1 +// (MineAlreadyRunning) and the capability's onError: skip silently dropped the +// capture. The shipped command queues via --daemon --background (MemPalace +// #2029), and the report step names the queued/skipped outcome. + +describe('#4700 — headless mine queues on the palace lock', () => { + const SURFACES = [ + 'commands/gsd/mempalace-capture.md', + 'skills/gsd-mempalace-capture/SKILL.md', + ]; + + test('every mempalace mine command line queues via --daemon --background (#4700)', () => { + for (const rel of SURFACES) { + const lines = splitLines(fs.readFileSync(path.join(ROOT, rel), 'utf8')); + let sawMine = false; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (!MINE_CMD_RE.test(line)) continue; + sawMine = true; + assert.match( + line, /--daemon\s+--background/, + `${rel}:${i + 1}: "mempalace mine" must queue via --daemon --background so a held palace lock defers the write instead of dropping it (#4700). Offending line: ${line.trim()}`, + ); + } + assert.ok(sawMine, `${rel}: expected at least one mempalace mine command line`); + } + }); + + test('the wave:post problems fragment queues its headless mine too (#4700)', () => { + const frag = 'capabilities/mempalace/fragments/capture-problems.md'; + const content = fs.readFileSync(path.join(ROOT, frag), 'utf8'); + assert.match( + content, /mempalace mine --daemon --background/, + `${frag}: the headless mine must queue via --daemon --background — the issue names the execute:wave:post problem-fix pair as the unrecoverable loss (#4700)`, + ); + }); + + test('the report step names queued and skipped captures (#4700)', () => { + for (const rel of SURFACES) { + const content = fs.readFileSync(path.join(ROOT, rel), 'utf8'); + assert.match(content, /queued/, + `${rel}: the report step must surface the queued outcome (#4700)`); + assert.match(content, /skipped/i, + `${rel}: the report step must name skipped captures rather than staying silent (#4700)`); + } + }); +});