fix(memory-helpers): handle gbrain >=0.25 doctor non-zero exit and schema_version 2#1418
Open
mvanhorn wants to merge 1 commit into
Open
fix(memory-helpers): handle gbrain >=0.25 doctor non-zero exit and schema_version 2#1418mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
…hema_version 2
gbrain doctor exits 1 whenever health_score < 100 and schema_version 2
dropped the top-level engine field. The previous execSync wrapper threw
on non-zero exit and the parser only knew schema_version 1, so any
unhealthy Supabase brain on gbrain >=0.25 collapsed to engine=unknown
and silently skipped /sync-gbrain stages.
- Switch the gbrain doctor call to spawnSync with stdio ['ignore',
'pipe', 'ignore'] to keep the original 2>/dev/null stderr suppression
while reading stdout regardless of exit code.
- Add a defensive engine lookup that handles schema_version 1's
parsed.engine, schema_version 2 top-level renames (engine_kind,
backend, engine_type), and a checks[] fallback whose name matches
/engine|backend|kind/i.
- When JSON parses but no engine resolves, emit a GSTACK_DEBUG-gated
stderr warning with the payload so the lookup ladder can be tightened
against real-world output. Default behavior is unchanged.
Tests: extend describe('detectEngineTier') with a PATH-shim that stubs
gbrain via tmpdir and afterEach restoration so the existing cache
tests at lines 296 and 305 are not affected. Three new cases cover
schema_v1, schema_v2 healthy, and schema_v2 unhealthy/non-zero exit.
bun test test/gstack-memory-helpers.test.ts: 25 pass, 0 fail.
Fixes garrytan#1415
Contributor
Author
|
Heads up @garrytan: the to the report job (or the workflow) should fix it. PR body itself is fine, CI evals all pass otherwise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/sync-gbrainand every caller ofdetectEngineTier()now resolve the engine correctly whengbrain doctorexits non-zero (every Supabase brain withhealth_score < 100) or returns schema_version 2 output (any gbrain >=0.25). Previously these paths collapsed toengine: "unknown"and the orchestrator silently skipped all sync stages.Why this matters
Two layered bugs both fired for the same users:
execSyncthrew on non-zero exit.gbrain doctorexits 1 wheneverhealth_score < 100(a fresh Supabase install always has at least one warning, so this is essentially everyone), so the JSON written to stdout never reached the parser. Seelib/gstack-memory-helpers.ts:250(pre-fix).schema_version: 2and dropped the top-levelenginefield.parsed?.enginebecameundefined, falling straight to"unknown". The project recognizes the new schema elsewhere (test/skill-e2e-brain-privacy-gate.test.ts:52already stubs{"status":"ok","schema_version":2}) butfreshDetectEngineTierwas missed during the gbrain >=0.25 update wave.The v1.29.0.0 release note explicitly bumped the required gbrain version to v0.30.0+, so schema_version 2 support is a project requirement, not a nice-to-have.
Changes
freshDetectEngineTiercallsspawnSync('gbrain', ['doctor', '--json', '--fast'], { stdio: ['ignore', 'pipe', 'ignore'], timeout: 5000 }). The array form drops the shell wrapper while preserving the original2>/dev/nullstderr suppression. Other gstack call sites use the same shape.result.stdoutis parsed even whenresult.statusis non-zero. Only spawn errors and unparseable JSON fall to"unknown".detectEngineFromDoctorJsonhelper implements a defensive lookup ladder: schema_v1 (parsed.engine) first, then schema_v2 top-level renames (engine_kind,backend,engine_type), then achecks[]fallback whosenamematches/engine|backend|kind/ireadingvalue/engine/kind. The ladder errs toward "engine resolves" rather than dropping to unknown when one path exists.GSTACK_DEBUG-gated stderr warning fires when JSON parses but no engine resolves, so the lookup ladder can be tightened against real-world schema_v2 output. Default behavior is unchanged.EngineDetectschema is unchanged (schema_version: 1from gstack's side); the on-disk cache contract is preserved.Testing
Three new cases under
describe("detectEngineTier")cover schema_v1 (existing behavior), schema_v2 healthy, and schema_v2 unhealthy with non-zero exit. The fixtures use a PATH-shim that writes agbrainbash stub to a tmpdir, prepended inbeforeEachand restored inafterEachso the shim does not leak into the cache tests at lines 296 and 305.Fixes #1415
Need help on this PR? Tag
@codesmithwith what you need.