Skip to content

fix(memory-ingest): refuse to advance state when gbrain imports fewer pages than staged - #2486

Open
Charles-Grant wants to merge 1 commit into
garrytan:mainfrom
Charles-Grant:fix/memory-ingest-verify-import-landed
Open

fix(memory-ingest): refuse to advance state when gbrain imports fewer pages than staged#2486
Charles-Grant wants to merge 1 commit into
garrytan:mainfrom
Charles-Grant:fix/memory-ingest-verify-import-landed

Conversation

@Charles-Grant

Copy link
Copy Markdown

Summary

gstack-memory-ingest can report N written while writing nothing to the brain, and mark every one of those sessions as ingested in state — so no later run ever retries them. On the install where I found this, state claimed 1013 sessions ingested while the brain held 391 pages. Transcripts from ~2 days were gone with no error anywhere.

This PR makes that failure loud instead of silent. It does not change the happy path.

The bug

written is counted from the staged set, never reconciled against gbrain's own totals:

for (const p of prep.prepared) {
  if (failedSources.has(p.source_path)) continue;
  state.sessions[p.source_path] = {...};   // marked ingested
  written++;                                // counted as written
}

failedSources comes from readNewFailures(), which reads per-file failures out of sync-failures.jsonl. When gbrain import collects zero files, there are no per-file failures to report — gbrain never saw them — so that set is empty, every prepared file is state-recorded, and the pass reports success.

importJson.imported holds the truth. It's used in exactly one place: the console.error summary a few lines below. It is never compared to anything.

Why it triggers in practice

gbrain import honours .gitignore. bin/gstack-artifacts-init writes .gitignore = "*" into $GSTACK_HOME:

bin/gstack-artifacts-init:21   #   2. Write .gitignore = "*"  (ignore everything; allowlist is explicit)
bin/gstack-artifacts-init:213  cat > "$GSTACK_HOME/.gitignore" <<'EOF'

makeStagingDir() stages into $GSTACK_HOME/.staging-ingest-<pid>-<ts>/. So on any machine that has run artifacts-init, every staged file is invisible to collect_files, forever.

Only affects the local-gbrain path. In remote-http MCP mode the code takes makePersistentTranscriptDir() and never calls gbrain import, so those users don't hit it.

Clean-room reproduction

No reference to my setup — reproducible on a fresh checkout in about two minutes:

export H=$(mktemp -d)
bin/gstack-artifacts-init --remote "file://$(mktemp -d)/remote.git"   # GSTACK_HOME=$H
cat "$H/.gitignore"        # => *

D="$H/.staging-ingest-1-1"; mkdir -p "$D/transcripts/x"
printf -- '---\ntitle: probe\n---\nbody\n' > "$D/transcripts/x/probe.md"

gbrain import "$D" --no-embed        # Found 0 markdown files   <-- silent loss
cp -R "$D" /tmp/control
gbrain import /tmp/control --no-embed # Found 1 markdown files  <-- control

Same bytes, same filenames, same directory shape. The only variable is whether the path sits inside an artifacts-init home.

The fix

Reconcile imported + skipped against prepared - failed (skipped is a content-hash no-op, which is a successful landing). On a shortfall: error, return written: 0, and leave state untouched so the next run re-prepares and gbrain's content_hash dedup short-circuits the re-import.

This mirrors the existing "exited 0 but emitted no parseable --json payload. Refusing to advance state." branch directly above — same intent, one failure mode further along.

The error names the likely cause and the staging path, so the next person sees it in one line rather than bisecting a 70KB file.

Test

test/gstack-memory-ingest.test.ts gains a collectNothing option on the existing installFakeGbrain shim: gbrain exits 0, reports imported=0/total_files=0, and writes no sync-failures.jsonl entry — the exact silent-loss shape.

The test asserts the state file stays empty. It fails without the source change (verified by reverting the fix and re-running).

bun test test/gstack-memory-ingest.test.ts   # 24 pass, 0 fail
bun test                                      # full free suite, exit 0

Notes for the maintainer

  • No VERSION bump or CHANGELOG entry included — those collide with whatever lands first, and your CLAUDE.md scopes them to /ship. Happy to add both at whatever version you want this to land at, just say the word.
  • The deeper fix is arguably to stop staging inside $GSTACK_HOME — ephemeral scratch in a directory that syncs to a GitHub artifacts repo is uncomfortable regardless, since transcripts can carry PII. I left that alone: checkOwnedStagingDir() hard-requires "direct child of $GSTACK_HOME", so moving it means changing that contract and its tests. Reconciling the counts makes the failure visible either way, and seemed like the right smallest first step. Glad to follow up if you'd prefer the relocation.
  • No test asserted that staged pages actually land after import, which is how this shipped. The new one closes that gap narrowly.
  • This comes from a fork, so the eval/E2E CI jobs won't receive base-repo secrets and will fail on empty-env auth. The free suite (bun test) is unaffected and passes. Push the branch to the base repo if you want a full green run.

… pages than staged

`written` was counted from the staged set, never reconciled against gbrain's
own `imported`/`skipped` totals. readNewFailures() only surfaces PER-FILE
failures, so when `gbrain import` collects zero files it writes nothing to
sync-failures.jsonl, failedSources comes back empty, and every prepared file
is state-recorded as ingested. The pass reports "N written" while the brain
gained nothing, and because state now says "done", no later run retries.
Silent, permanent data loss.

This reproduces on any machine that has run `gstack-artifacts-init`: it
writes `.gitignore = "*"` into $GSTACK_HOME, `gbrain import` honours
.gitignore, and makeStagingDir() stages under $GSTACK_HOME — so collect_files
returns 0 for every batch. Observed on a real install: 1013 sessions marked
ingested in state, 391 pages actually in the brain.

Reconcile imported+skipped against prepared-minus-failed. On a shortfall,
error out and return written:0 without touching state, so the next run
re-prepares and gbrain's content_hash dedup short-circuits the re-import.
Mirrors the existing "exited 0 but emitted no parseable --json payload"
branch directly above.

Regression test asserts the state file stays empty when gbrain reports
collecting nothing — it fails without this change.
@trunk-io

trunk-io Bot commented Aug 8, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant