mirror of https://github.com/garrytan/gstack.git
fix(memory-ingest): stop silently ingesting 0 pages — include gitignored staging, reconcile counts
Pages stage into ~/.gstack/.staging-ingest-*/ inside a repo whose .gitignore is `*`, and gbrain import honours .gitignore — so it collected 0 files, imported nothing, and the ingest still reported "written: N" from the STAGED count while advancing state, meaning no future run ever retried. Three layers now: (1) pass --include-gitignored (root cause); (2) if the installed gbrain predates the flag, retry without it (subcommand --help is generic, so the attempt is the only probe) with an upgrade pointer; (3) reconcile gbrain's imported+unchanged accounting against the staged count and REFUSE to advance state on a shortfall, naming the gitignore collision. Fixes #2144, #2104. Contributed by @gawievanblerk (PR #2560) and @Charles-Grant (PR #2486). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
75cd221850
commit
7847fcf9ba
|
|
@ -1407,9 +1407,43 @@ export function resolveImportTimeoutMs(
|
|||
return n;
|
||||
}
|
||||
|
||||
function runGbrainImport(
|
||||
/**
|
||||
* True when the import failed because the installed gbrain predates
|
||||
* --include-gitignored. gbrain's subcommand --help is generic (no flag list),
|
||||
* so the only reliable probe is the attempt itself.
|
||||
*/
|
||||
function failedOnUnknownIncludeGitignored(status: number | null, stderr: string): boolean {
|
||||
if (status === 0 || status === null) return false;
|
||||
return /(unknown|unexpected|unrecognized|invalid)[^\n]*--include-gitignored|--include-gitignored[^\n]*(unknown|unexpected|unrecognized|invalid)/i.test(
|
||||
stderr,
|
||||
);
|
||||
}
|
||||
|
||||
async function runGbrainImport(
|
||||
stagingDir: string,
|
||||
timeoutMs: number,
|
||||
): Promise<{ status: number | null; stdout: string; stderr: string; timedOut: boolean }> {
|
||||
const first = await runGbrainImportOnce(stagingDir, timeoutMs, true);
|
||||
if (failedOnUnknownIncludeGitignored(first.status, first.stderr)) {
|
||||
// Older gbrain: retry without the flag. If .gitignore then hides the
|
||||
// staged pages, the imported<staged reconciliation guard below refuses
|
||||
// to advance state and names the remedy — loud failure, never silent
|
||||
// loss, and never a hard-block for gbrain versions that don't need the
|
||||
// flag's semantics.
|
||||
console.error(
|
||||
"[memory-ingest] installed gbrain does not support --include-gitignored — " +
|
||||
"retrying without it. If the import then collects 0 files, upgrade gbrain " +
|
||||
"(gstack-gbrain-install) so staged pages inside gitignored dirs are visible.",
|
||||
);
|
||||
return runGbrainImportOnce(stagingDir, timeoutMs, false);
|
||||
}
|
||||
return first;
|
||||
}
|
||||
|
||||
function runGbrainImportOnce(
|
||||
stagingDir: string,
|
||||
timeoutMs: number,
|
||||
includeGitignored: boolean,
|
||||
): Promise<{ status: number | null; stdout: string; stderr: string; timedOut: boolean }> {
|
||||
installSignalForwarder();
|
||||
return new Promise((resolve) => {
|
||||
|
|
@ -1417,7 +1451,20 @@ function runGbrainImport(
|
|||
// inside Next.js / Prisma / Rails projects with their own
|
||||
// .env.local (codex review #7 — defense in depth on top of the
|
||||
// parent gstack-gbrain-sync seeding the bun grandchild's env).
|
||||
const child = spawnGbrainAsync(["import", stagingDir, "--no-embed", "--json"]);
|
||||
// --include-gitignored is load-bearing, not a convenience. Pages are
|
||||
// staged into ~/.gstack/.staging-ingest-<pid>-<ts>/, and ~/.gstack is a
|
||||
// git repo whose .gitignore is `*`. `gbrain import` honours .gitignore,
|
||||
// so without this flag it collects files=0 and imports NOTHING, while
|
||||
// still reporting `written: N` from the staged count. Silent data loss
|
||||
// on every run. A working run logs `import.collect_files done ... files=N`
|
||||
// with N > 0 and takes minutes, not seconds.
|
||||
const child = spawnGbrainAsync([
|
||||
"import",
|
||||
stagingDir,
|
||||
"--no-embed",
|
||||
...(includeGitignored ? ["--include-gitignored"] : []),
|
||||
"--json",
|
||||
]);
|
||||
_activeImportChild = child;
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
|
|
@ -1813,6 +1860,49 @@ async function ingestPass(args: CliArgs): Promise<BulkResult> {
|
|||
);
|
||||
failed += failedSources.size;
|
||||
|
||||
// Reconcile gbrain's own accounting against what we staged. Without this,
|
||||
// a batch that gbrain never SAW is indistinguishable from a batch that
|
||||
// succeeded: readNewFailures() only reports PER-FILE failures, so when
|
||||
// `gbrain import` collects zero files it writes nothing to
|
||||
// sync-failures.jsonl, failedSources is empty, and every prepared file
|
||||
// gets state-recorded as ingested. The pass then reports "N written"
|
||||
// while the brain gained nothing — and because state now says "done",
|
||||
// no future run retries. Silent, permanent data loss.
|
||||
//
|
||||
// Observed cause: `gbrain import` honours .gitignore, and
|
||||
// `gstack-artifacts-init` writes `.gitignore = "*"` into $GSTACK_HOME.
|
||||
// makeStagingDir() stages under $GSTACK_HOME, so on any machine that has
|
||||
// run artifacts-init, collect_files returns 0 for every batch.
|
||||
//
|
||||
// `skipped` counts content_hash no-ops, which ARE successful landings.
|
||||
const expectedLandings = prep.prepared.length - failedSources.size;
|
||||
const accountedLandings =
|
||||
(importJson.imported ?? 0) + (importJson.skipped ?? 0);
|
||||
if (accountedLandings < expectedLandings) {
|
||||
const collected =
|
||||
importJson.total_files !== undefined
|
||||
? ` gbrain collected ${importJson.total_files} file(s) from the staging dir.`
|
||||
: "";
|
||||
const msg =
|
||||
`gbrain import accounted for ${accountedLandings} of ${expectedLandings} staged page(s) ` +
|
||||
`(imported=${importJson.imported ?? 0}, unchanged=${importJson.skipped ?? 0}).${collected} ` +
|
||||
`Refusing to advance state — the unaccounted pages would be marked ingested without ` +
|
||||
`landing in the brain. If the count is 0, check whether ${stagingDir} is inside a git ` +
|
||||
`repo that ignores it (gbrain import honours .gitignore).`;
|
||||
console.error(`[memory-ingest] ERR: ${msg}`);
|
||||
failed += prep.prepared.length;
|
||||
return {
|
||||
written: 0,
|
||||
skipped_secret: prep.skippedSecret,
|
||||
skipped_dedup: prep.skippedDedup,
|
||||
skipped_unattributed: prep.skippedUnattributed,
|
||||
failed,
|
||||
duration_ms: Date.now() - t0,
|
||||
partial_pages: prep.partialPages,
|
||||
system_error: msg,
|
||||
};
|
||||
}
|
||||
|
||||
// Phase 3: state recording. Only files that landed in gbrain get
|
||||
// their mtime+sha256 stamped. Failed source paths are deliberately
|
||||
// left un-state'd so the next run re-prepares them and gbrain's
|
||||
|
|
|
|||
|
|
@ -330,7 +330,7 @@ describe("gstack-memory-ingest --limit", () => {
|
|||
*/
|
||||
function installFakeGbrain(
|
||||
home: string,
|
||||
opts: { failingPaths?: string[] } = {},
|
||||
opts: { failingPaths?: string[]; collectNothing?: boolean } = {},
|
||||
): { binDir: string; logFile: string; argsFile: string; stagingListFile: string } {
|
||||
const binDir = join(home, "fake-bin");
|
||||
mkdirSync(binDir, { recursive: true });
|
||||
|
|
@ -392,6 +392,13 @@ EOF
|
|||
else
|
||||
TOTAL=0
|
||||
fi
|
||||
# collectNothing: simulate gbrain walking the staging dir and finding
|
||||
# nothing — the real-world shape when .gitignore hides every staged file
|
||||
# from collect_files. Crucially this writes NO sync-failures.jsonl entry,
|
||||
# because there is no per-file failure: gbrain never saw the files.
|
||||
if [ "${opts.collectNothing ? "1" : "0"}" = "1" ]; then
|
||||
TOTAL=0
|
||||
fi
|
||||
ERRORS=0
|
||||
if [ -n "\$FAILING_LIST" ]; then
|
||||
ERRORS=\$(echo "\$FAILING_LIST" | tr '|' '\\n' | wc -l | tr -d ' ')
|
||||
|
|
@ -470,6 +477,51 @@ describe("gstack-memory-ingest writer (gbrain v0.20+ batch `import` interface)",
|
|||
expect(stagedList).toMatch(/^\.\/transcripts\/claude-code\/.+\.md$/m);
|
||||
});
|
||||
|
||||
// Silent-data-loss regression: gbrain accepts the import call, exits 0, and
|
||||
// reports imported=0 because collect_files found nothing in the staging dir
|
||||
// (real-world cause: gstack-artifacts-init writes `.gitignore = "*"` into
|
||||
// $GSTACK_HOME, and `gbrain import` honours .gitignore, so every file staged
|
||||
// under $GSTACK_HOME is invisible to it).
|
||||
//
|
||||
// No per-file failure is written to sync-failures.jsonl — gbrain never SAW
|
||||
// the files — so readNewFailures returns empty. Before the reconciliation
|
||||
// check, that made a total loss indistinguishable from success: every
|
||||
// prepared file got state-recorded as ingested and the pass reported
|
||||
// "N written". State then said "done", so no later run ever retried.
|
||||
it("refuses to advance state when gbrain imports fewer pages than were staged", () => {
|
||||
const home = makeTestHome();
|
||||
const gstackHome = join(home, ".gstack");
|
||||
mkdirSync(gstackHome, { recursive: true });
|
||||
const { binDir, logFile } = installFakeGbrain(home, { collectNothing: true });
|
||||
|
||||
const session =
|
||||
`{"type":"user","message":{"role":"user","content":"hi"},"timestamp":"2026-05-01T00:00:00Z","cwd":"/tmp/foo"}\n` +
|
||||
`{"type":"assistant","message":{"role":"assistant","content":"hello"},"timestamp":"2026-05-01T00:00:01Z"}\n`;
|
||||
writeClaudeCodeSession(home, "tmp-foo", "abc123", session);
|
||||
|
||||
const r = runScript(["--bulk", "--include-unattributed", "--quiet"], {
|
||||
HOME: home,
|
||||
GSTACK_HOME: gstackHome,
|
||||
PATH: `${binDir}:${process.env.PATH || ""}`,
|
||||
});
|
||||
|
||||
// gbrain WAS called — this is not a "gbrain missing" path.
|
||||
expect(existsSync(logFile)).toBe(true);
|
||||
|
||||
// The pass must not claim success.
|
||||
expect(r.stderr).toMatch(/\[memory-ingest\] ERR:.*accounted for 0 of 1 staged page/);
|
||||
expect(r.stderr).toMatch(/Refusing to advance state/);
|
||||
expect(r.stdout).not.toMatch(/written:\s+1/);
|
||||
|
||||
// The critical assertion: state must NOT mark the session ingested, or the
|
||||
// next run skips it forever and the transcript is lost silently.
|
||||
const statePath = join(gstackHome, ".transcript-ingest-state.json");
|
||||
if (existsSync(statePath)) {
|
||||
const state = JSON.parse(readFileSync(statePath, "utf-8"));
|
||||
expect(Object.keys(state.sessions || {}).length).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
// Originally landed in v1.32.0.0 (PR #1411) on the per-file `gbrain put`
|
||||
// path. Postgres rejects 0x00 in UTF-8 text columns. Some Claude Code
|
||||
// transcripts contain NUL inside user-pasted content or tool output. The
|
||||
|
|
|
|||
|
|
@ -0,0 +1,79 @@
|
|||
/**
|
||||
* Regression pin: `gstack-memory-ingest` must pass `--include-gitignored` to
|
||||
* `gbrain import`.
|
||||
*
|
||||
* gstack-artifacts-init writes an ignore-everything `.gitignore` (a bare `*`,
|
||||
* headed "Do not edit") at the root of `~/.gstack`. The memory ingest stages
|
||||
* pages into `~/.gstack/.staging-ingest-<pid>-<ts>/`, which is INSIDE that
|
||||
* repo, and gbrain's markdown collector honours .gitignore. So the collector
|
||||
* walks the staging dir, matches every file against `*`, and collects zero.
|
||||
*
|
||||
* The failure is silent: `gbrain import` exits 0 having imported nothing,
|
||||
* while the ingest still prints `written: N` from the STAGED count rather
|
||||
* than the imported count. A run that indexes nothing is indistinguishable
|
||||
* from a healthy one, and the memory corpus quietly stops growing.
|
||||
*
|
||||
* Two tests here:
|
||||
* 1. Source pin (same shape as memory-ingest-no-put_page.test.ts): the flag
|
||||
* is present in active code, so removing it trips the build.
|
||||
* 2. Behavioural proof of the underlying collision, using git's own ignore
|
||||
* machinery. No gbrain and no network required.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { execFileSync } from "child_process";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { readFileSync } from "fs";
|
||||
|
||||
const SOURCE_PATH = join(import.meta.dir, "..", "bin", "gstack-memory-ingest.ts");
|
||||
|
||||
/** Strip comments so the pin only inspects executable code. */
|
||||
function stripComments(src: string): string {
|
||||
const noBlock = src.replace(/\/\*[\s\S]*?\*\//g, "");
|
||||
return noBlock.replace(/\/\/[^\n]*/g, "");
|
||||
}
|
||||
|
||||
describe("gstack-memory-ingest: gbrain import must not be filtered by .gitignore", () => {
|
||||
it("passes --include-gitignored in active code", () => {
|
||||
const stripped = stripComments(readFileSync(SOURCE_PATH, "utf-8"));
|
||||
expect(stripped).toContain("--include-gitignored");
|
||||
});
|
||||
|
||||
it("keeps the flag on the same import invocation as the staging dir", () => {
|
||||
const stripped = stripComments(readFileSync(SOURCE_PATH, "utf-8"));
|
||||
// Match the spawn call's argument array and assert both the subcommand
|
||||
// and the flag live in it, so the flag can't drift onto another call.
|
||||
const call = stripped.match(/spawnGbrainAsync\(\s*\[[^\]]*"import"[^\]]*\]/s);
|
||||
expect(call).not.toBeNull();
|
||||
expect(call![0]).toContain("--include-gitignored");
|
||||
});
|
||||
|
||||
it("demonstrates the collision: an ignore-everything root hides staged pages", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "gstack-ingest-gitignore-"));
|
||||
try {
|
||||
const git = (...args: string[]) =>
|
||||
execFileSync("git", args, { cwd: dir, encoding: "utf-8" });
|
||||
git("init", "-q", ".");
|
||||
|
||||
const staging = join(dir, ".staging-ingest-12345-1700000000000", "learnings");
|
||||
mkdirSync(staging, { recursive: true });
|
||||
writeFileSync(join(staging, "page.md"), "# a staged page\n", "utf-8");
|
||||
|
||||
// Exactly what gstack-artifacts-init writes at the root of ~/.gstack.
|
||||
writeFileSync(join(dir, ".gitignore"), "*\n", "utf-8");
|
||||
|
||||
// `git ls-files --others --exclude-standard` is the same view a
|
||||
// gitignore-honouring collector takes: untracked and not ignored.
|
||||
const collectable = git("ls-files", "--others", "--exclude-standard")
|
||||
.split("\n")
|
||||
.filter(Boolean);
|
||||
|
||||
// The staged page is invisible. This is the silent data loss.
|
||||
expect(collectable).toHaveLength(0);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue