mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix/backlog-c30
458
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Garry TanandClaude Fable 5
|
bcb9d298f2 |
fix(pricing): add openai:gpt-5.2 canonical entry for the new default slot A
DEFAULT_SLOTS slot A moved to openai:gpt-5.2, which had no CANONICAL_PRICING entry — estimateCost silently dropped slot A from the --max-usd pre-flight and est_cost_usd audit rows (~1/3 under-count on the default panel). Rates from the OpenAI recipe chat touchpoint (verified 2026-04-20). Also refresh the --slot-a-model help text default and pin a pricing-presence assertion in the DEFAULT_SLOTS consistency test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d76bb7fd68 |
fix(autopilot,eval): nightly quality probe enable path works end-to-end + wire conversation-parser probe
Takeover of #2629 and #2630 (rebased onto master; dropped the test/engine-find-trajectory.test.ts hunk both PRs carried — master already ships the equivalent gateway-dims fix). #2629 — nightly quality probe enable path: - autopilot + doctor read the probe flag dual-plane (DB config row from 'gbrain config set' wins, ~/.gbrain/config.json fallback) via new resolveProbeEnabled/resolveProbeMaxUsd helpers - resolveRepoRoot prefers the gbrain package root where the committed fixture lives, not the brain repoPath - rate_limited skips no longer write an audit row every autopilot cycle - eval-longmemeval strips 'provider:' recipe ids before raw Anthropic SDK calls and emits the gold answer for downstream judges - cross-modal batch folds the gold answer into the judge task; probe passes QA-shaped dimensions instead of the agent-response rubric - DEFAULT_SLOTS slot A moves to openai:gpt-5.2 (gpt-4o left the recipe); new consistency test pins every default slot to its recipe #2630 — conversation-parser nightly probe wire-up: - autopilot step 4.6 invokes runConversationParserNightlyProbe (dual-plane flag + D10 tokenmax mode-gate, package-root fixtures, 24h gate, audit trail via new src/core/audit-parser-probe.ts) - doctor's conversation_parser_probe_health replaces the hardcoded 'Skipped' stub with a real pure-function check over the audit trail Co-authored-by: p3ob7o <p3ob7o@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
MasaandGitHub
|
0612b0daa8 |
fix(dream): require self-contained opening summary in synthesized pages (#2770)
Synth pages written by dream synthesize currently open straight into detail (quotes, cross-references) with no framing, so a reader who lands on the page later — without the source transcript in front of them — has no way to tell what it's about without reading the whole thing. Add OUTPUT POLICY item 5: every new page's body must open with a 2-3 sentence self-contained summary a reader unfamiliar with the source conversation could understand on its own, before any quotes or detail. |
||
|
|
f529eaa231 |
fix(jobs): refresh gateway config for queued AI work (#2125)
Long-lived minion workers can outlive DB-backed model config changes. Refresh the AI gateway before gateway-backed handlers run so queued cycle/propose_takes work does not fall back to a stale Anthropic default when the operator configured another provider. Also record the active gateway chat model in propose_takes budget/proposal metadata instead of hardcoding claude-sonnet-4-6, and keep provider:model IDs intact for budget pricing. Regression coverage verifies queued worker refresh, propose_takes model metadata, nested provider IDs, skipFence threading, and the updated autopilot signal source guard. Co-authored-by: maxpetrusenkoagent <[REDACTED EMAIL]> |
||
|
|
447e57ec41 |
fix(ai): tier-configured models reach the recipe allowlist — refresh Anthropic models, register tier resolutions, honest probe labels (#2800)
Setting `models.tier.deep anthropic:claude-opus-4-8` silently disabled think and auto_think: the Anthropic recipe's chat allowlist stopped at Opus 4.7, the tier-resolved model never joined the extended set that assertTouchpoint's contract promises for config-chosen models, and the resulting probe failure was stamped NO_ANTHROPIC_API_KEY — sending the operator to debug env/keychain when the fix was the model id. Three fixes, one per layer: - recipes/anthropic.ts: add claude-fable-5, claude-opus-4-8, and claude-sonnet-5 to chat models; claude-sonnet-5 to expansion models. - gateway.ts reconfigureGatewayWithEngine: resolve all four tiers and register the results as extended models, honoring the documented contract for models.default / models.tier.* (model-resolver.ts docstring). A tier-only model now validates like a chat/expansion one. - think/index.ts: when the gateway client can't be built, re-probe and label honestly — MODEL_NOT_USABLE:<reason> for unknown_model / unknown_provider, NO_ANTHROPIC_API_KEY only for the actual missing-key case; the stub answer carries the probe detail and fix hint. Tests: recipe-list presence pins; a new gateway-tier-extended-models suite proving a fictional tier model validates post-reconfigure (and an unconfigured one still doesn't); think-pipeline coverage for the honest label (unknown_model beats missing-key even keyless); the existing non-explicit bogus-provider test updated from the old catch-all label to the honest one (no-throw contract unchanged). Verified live: a brain with tier.deep=claude-opus-4-8 had think degrade to gather-only with the misleading key warning; with this change the probe passes and synthesis runs. Co-authored-by: Paolo Belcastro <p3ob7o@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d61808d806 |
v0.42.64.0 fix: harden confidential OAuth token revocation (takeover of #3017) (#3032)
* fix(oauth): validate confidential revoke secrets * fix(oauth): harden confidential token revocation * chore: bump version and changelog (v0.42.64.0) Co-Authored-By: OpenAI Codex <noreply@openai.com> --------- Co-authored-by: Robin <rayme@boltdsolutions.com> Co-authored-by: OpenAI Codex <noreply@openai.com> Co-authored-by: Garry Tan <garrytan@gmail.com> |
||
|
|
60125ee626 |
feat(dream): --once for one-shot phase runs without toggling config gates (takeover of #2983) (#3031)
* feat(dream): --once for one-shot phase runs without toggling config gates Fixes the "toggle enabled true, run, toggle back to false" workaround that gbrain doctor's extract_atoms_backlog message implicitly recommends and that #2860's reporter had to script around: with an external orchestrator running `gbrain dream --phase patterns` on a cadence outside the autopilot, the only way to run patterns once was `config set dream.patterns.enabled true` -> run -> `config set ... false`. A crash between steps left the flag stuck true, and the autopilot (which polls the same flag) re-enqueued patterns every cycle -- 119 LLM jobs / ~$400 over 24h before it was caught. Root cause: `--phase X` only controls which phase FUNCTION cycle.ts calls; it does not bypass that phase's own `dream.<phase>.enabled` / `cycle.<phase>.enabled` config read. Each gated phase (patterns, synthesize, conversation_facts_backfill, enrich_thin, skillopt) reads its enabled flag internally and skips regardless of how the phase was selected -- confirmed by reading each phase module, not assumed. extract_atoms/synthesize_concepts are a DIFFERENT mechanism entirely (pack-declaration via packDeclaresPhase, not a config .enabled read) and already have a working one-shot escape hatch: `--drain`. The existing doctor message for extract_atoms already says `--phase extract_atoms --drain --window 120`, so no doctor text needed updating there -- verified by reading src/commands/doctor.ts directly rather than assuming the paraphrase in the issue was literal. Design: `gbrain dream --phase <name> --once`. Requires an explicit --phase (bare --once is a usage error, exit 2) so it can never force-enable every disabled phase at once in a full/default cycle -- that would recreate the same unbounded-spend risk the flag exists to prevent. Threaded through CycleOpts as `onceForPhase?: CyclePhase` (the literal phase name, not a boolean) so the bypass can never leak to a phase other than the one named, even if a future programmatic caller passes a wider `phases` array than the CLI does. Never reads or writes config -- the phase still evaluates its .enabled gate every call; --once only overrides the boolean OUTCOME for that one invocation, mirroring the existing --unsafe-bypass-dream-guard / --input precedents (stderr warning at the bypass point, no new config-touching code path). Rejected alternatives (documented per task instructions): - Making explicit --phase X always bypass .enabled: breaking change for existing crons that rely on the disabled flag as a cheap no-op; an upgrade would silently start running LLM/write phases. - A new subcommand: adds a whole dispatch/help/arg surface that internally routes through the same override anyway. - Extending --once to also bypass packDeclaresPhase for extract_atoms/synthesize_concepts: conflates two different gating mechanisms (config toggle vs. pack membership) under one flag; extract_atoms already has --drain, which is purpose-built for its batched/windowed execution model. Design was cross-validated by an independent second-model review (external design consultation) before implementation; its recommendation to also update the extract_atoms doctor message to `--once` was NOT adopted because that phase has no .enabled gate to bypass -- doing so would be a documented no-op, contradicted by reading src/commands/doctor.ts:3264 directly. Tests: 9 new (structural CLI-flag wiring in dream-cli-flags.test.ts; a real PGLite E2E test in dream-patterns-pglite.test.ts proving the bypass fires AND that dream.patterns.enabled is never written; 4 runCycle-level tests in cycle.serial.test.ts proving onceForPhase does not leak across phases). Verified 6 of 9 fail against the pre-fix source (via git stash of source-only changes) to confirm they're meaningful regressions, not tautologies. Closes #2860 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(dream): --help short-circuits before --once usage validation Codex review finding (P2): `gbrain dream --help --once` (no --phase) called process.exit(2) from the new --once usage-error check inside parseArgs before runDream's documented IRON RULE ("--help short-circuits BEFORE any engine-bearing work") ever got a chance to run -- parseArgs computes ALL its validations unconditionally before runDream checks opts.help. Repo precedent for this ordering already exists as a pinned regression test (test/dream.test.ts's "--help --source whatever prints help and exits 0"). Fix: compute wantsHelp once in parseArgs and exempt the --once validation when it's set, mirroring that precedent. Added the same class of pinned tests here: bare `--once` still exits 2 with the usage hint, `--help --once` prints help and exits 0, and a real --phase patterns --once run against a PGLite engine proves the bypass actually fires (falls through to insufficient_evidence instead of disabled) without writing dream.patterns.enabled. Also fixed the structural test in dream-cli-flags.test.ts that asserted the exact pre-fix guard-condition source text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(dream): --once must require an EXPLICIT --phase, not a derived one Codex review finding (P3): the --once validation checked the derived `phase` value, but `phase` gets defaulted implicitly by --input (implies --phase synthesize) and --drain (implies --phase extract_atoms) BEFORE that check ran. So `gbrain dream --input <f> --once` and `gbrain dream --drain --once` both slipped past the "explicit --phase required" contract silently -- and --once became a true no-op in both cases: --drain returns from runDream before onceForPhase is ever read (the drain path doesn't call runCycle at all), and --input already bypasses the synthesize enabled-gate on its own via the existing opts.inputFile check, so onceForPhase would never even be consulted. Fix: capture `phaseWasExplicit = phaseIdx !== -1` at the very top of parseArgs, before the --input/--drain defaulting blocks run, and validate --once against that instead of the derived `phase`. Updated the usage-error message and --help text to say "an explicit --phase" so a user hitting this understands why `--input ... --once` doesn't count. Tests: 2 new pins in test/dream.test.ts exercising runDream directly (--input <file> --once exits 2; --drain --once exits 2), plus a structural test in dream-cli-flags.test.ts pinning that phaseWasExplicit is captured before both implicit-defaulting blocks. Updated the two existing structural/behavioral tests whose literal guard-condition / error-message assertions changed shape. Verified: dream-cli-flags.test.ts 27/27, dream.test.ts 31/31, typecheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: masashiono0611 <masashi.ono.0611@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Garry Tan <garrytan@gmail.com> |
||
|
|
e320ad71b3 |
fix(providers): reuse buildGatewayConfig for --model test override (takeover of #2980) (#3029)
* fix(providers): reuse buildGatewayConfig for --model test override (#2863)
`gbrain providers test --model <id>` overrode the gateway with only
embedding_model/chat_model + env, dropping config.provider_base_urls
entirely. A brain configured with a custom endpoint (e.g. a China-region
DashScope base URL) would pass the bare `providers test` (which goes
through configureFromEnv() and does forward base_urls) but fail the
`--model`-scoped probe with a misleading "Incorrect API key" error, even
though the key was valid for the configured endpoint — the probe silently
fell back to the recipe's hardcoded default endpoint instead.
Root cause: two independent, drifted resolvers. The production path
(src/cli.ts#connectEngine, src/core/init-embed-check.ts) builds its
AIGatewayConfig via buildGatewayConfig(), which folds provider_base_urls,
env-sourced local-server base URLs, provider_chat_options, and file-plane
API keys. The --model override branch in runTest() hand-rolled a second,
narrower config object that only carried the overridden model + raw env.
Fix: lift `cfg` out of the existing try/catch (it was already loaded there
for the isolation-warning message) and spread `buildGatewayConfig(cfg)`
into both configureGateway() calls before overriding embedding_model/
chat_model. The isolated --model probe now resolves its endpoint exactly
the way the brain's real import/query path would; only the requested
model is overridden, so the probe still targets exactly the model the
user asked for. Falls back to bare env when no brain is configured yet
(cfg is null), matching prior first-time-install behavior.
Confirmed chat_fallback_chain (also threaded through by buildGatewayConfig)
has no runtime retry effect — it's only consumed to pre-register extended
model ids — so spreading the full production config does not mask an
isolated model's own failures behind a silent fallback.
Other diagnostic surfaces (providers list/env/explain) were checked and
are unaffected: `runProviders()` already calls configureFromEnv() (which
forwards base_urls correctly) before dispatch, and none of them accept
--model, so they never hit the broken override branch.
Adds test/providers-test-model-base-url.test.ts: drives runProviders('test',
...) end-to-end against a mocked fetch + temp GBRAIN_HOME/config.json with
provider_base_urls set for the dashscope recipe (the exact recipe named in
the bug report), asserting the outbound request hits the configured base
URL rather than the recipe default. Verified red on pre-fix code via
git stash, green after.
Closes #2863
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: drop duplicate buildGatewayConfig import after master merge
Master's
|
||
|
|
b6dd3e1121 |
fix(test): reset AI gateway after adaptive-embed-batch suite — cross-file config leak (#3065)
The file's final test configures the gateway with a remote provider and a fake key, and its afterEach only clears the mock transport. With no afterAll, the poisoned global config survives the file boundary; the next test file in the shard that triggers an embed makes a real HTTP call and fails. Surfaced on master when #3022's new test file reshuffled shard composition (shard 6: synthesize-concepts-progress failed twice with a live Google embed rejection). The legacy-embedding preload can't catch this: it only re-applies defaults when the gateway slot is empty. One-line root-cause fix at the leaker. A repo-wide guard for the class (~70 files call configureGateway without a final reset) is filed as a follow-up. Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3cc34c92ee |
feat(ai): add NVIDIA NIM provider recipe (#2965) (#3022)
Adds NVIDIA NIM / API Catalog as a first-class OpenAI-compatible AI recipe: - chat via nvidia/nemotron-3-super-120b-a12b (conservative capability claims: no tools, no subagent loop until proven) - hosted embedding models incl. nvidia/llama-nemotron-embed-1b-v2 with Matryoshka-style dimension overrides (1024/1280/1536/2048) and fixed natural dims for the other catalog models - asymmetric input_type mapping (document -> passage, query -> query) via a gateway compat fetch shim, since the generic openai-compatible recipe cannot infer that provider-specific requirement - base URL https://integrate.api.nvidia.com/v1 verified live (OpenAI-shaped /v1/models, all five recipe model ids present in the catalog) Changed from the original PR: dropped the recipe's custom resolveAuth — it duplicated defaultResolveAuth's Authorization-Bearer behavior exactly and violated the IRON RULE that only Azure overrides resolveAuth (test/ai/recipes-existing-regression.test.ts). NVIDIA_API_KEY now flows through defaultResolveAuth via auth_env.required, and the recipe test pins resolveAuth === undefined + the default Bearer resolution + the missing-key AIConfigError. Also scrubbed a private downstream-agent name from ported comments per the repo privacy rule. Takeover of #2965 by @ravehorn. Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: SAGE Codex <codex@sage.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Anton SenkovskiyandGitHub
|
a93fcf504f |
fix(chronicle): bound last-seen to <= asof/today so future events do not read as "seen today" (#2993)
getLastSeen had no upper date bound, so a future-dated chronicle event (a scheduled calendar-event, a planned milestone) became the entity's "last seen" date; finalizeLastSeen's Math.max(0, ...) then clamped the negative delta, reporting days_ago: 0 -- the entity reads as seen-today. Recording future events is intended (eligibility ELIGIBLE_TYPES includes calendar-event); the reader just needs to stop counting them as "seen". Bound the query to te.date <= COALESCE(asof, current_date) in both engines, mirroring getOnThisDay's existing te.date < target bound. asof now reaches the WHERE clause (previously it only reached finalizeLastSeen), so as-of time-travel is honored for the date filter too. Regression test added: an entity with past events plus a future event -> last seen returns the most recent PAST event, not the future one; and as-of after the future date lets it through. Fails before, passes after. |
||
|
Hanchen QiuandGitHub
|
84fad4738d |
fix(pages): default chunker_version to MARKDOWN_CHUNKER_VERSION on INSERT (#2807) (#2988)
putPage's INSERT used COALESCE(<chunkerVersion>, 1), so callers that don't supply chunker_version (no MCP/subagent caller does — it's internal metadata) landed new pages at version 1. Dream subagents write through putPage directly, so their pages got v1 and doctor's contextual_retrieval_coverage check flagged them as "older chunker_version" forever, even though they were chunked and embedded with the current chunker. Default the INSERT to MARKDOWN_CHUNKER_VERSION on both engines. The ON CONFLICT UPDATE still COALESCE-preserves an explicitly supplied version. Add an engine-level regression test. |
||
|
Hanchen QiuandGitHub
|
f815246eef |
fix(cli): register reconcile-links in CLI_ONLY so dispatch reaches its handler (#2900) (#2987)
reconcile-links is advertised in `gbrain --help` and implemented with a `case 'reconcile-links'` block in handleCliOnly, but it was missing from the CLI_ONLY Set. Dispatch only enters handleCliOnly when the command is in CLI_ONLY, so every invocation fell through to the shared-operations lookup and hit the generic "Unknown command" branch — leaving the documented doc↔impl edge-rebuild tool silently unreachable via the CLI. Add 'reconcile-links' to CLI_ONLY, plus a reachability regression test mirroring the #2035 (`calibration`) guard. |
||
|
|
42c4ea929f |
fix(test): isolate audit writes to a per-run scratch dir in the shared bootstrap (#2823) (#2966)
The content-sanity gate's audit logger (logContentSanityAssessment)
defaults, via audit-writer.ts::resolveAuditDir(), to writing
~/.gbrain/audit/content-sanity-YYYY-Www.jsonl on disk. A GBRAIN_AUDIT_DIR
env override exists, but nothing in the shared test bootstrap ever set
it, so any test that exercised an audit-emitting code path without
wrapping the call in its own withEnv() fell through to the operator's
real audit trail. test/import-file.test.ts's oversize-boundary fixture
('borderline-slug', content just under MAX_FILE_SIZE but over
DEFAULT_BYTES_BLOCK) fired a real soft_block event into the developer's
live ~/.gbrain/audit on every run — which doctor's
content_sanity_audit_recent check then reported as production signal.
Fix: add a bootstrap preload (test/helpers/audit-dir-preload.ts, wired
via bunfig.toml) that points GBRAIN_AUDIT_DIR at a fresh per-process
mkdtemp dir before any test file loads. Each run-unit-shard.sh shard is
its own bun process, so each shard gets its own scratch dir with no
cross-shard collision. This closes the leak for every audit-emitting
test, not just this fixture. It respects a developer-exported override
(only sets the var when unset), and files that manage their own
per-test GBRAIN_AUDIT_DIR via withEnv() are unaffected.
Also fix a latent isolation bug this surfaced: gbrain-home-isolation.test.ts
unconditionally deleted GBRAIN_AUDIT_DIR in a finally block instead of
restoring the prior value, which clobbered the bootstrap's scratch dir
for every test file that ran after it in the same shard process.
Adds test/audit/audit-dir-preload.test.ts to pin the behavior: it
reproduces the exact soft_block event shape and asserts it lands in the
scratch dir, never in ~/.gbrain/audit.
Reported by @paul-0320.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Benjamin D. SmithandGitHub
|
6370ce3d7e |
fix(infrastructure): chmod 644 autopilot supervisor files on install (#2963)
📝 Summary: • launchd rejects group/world-writable agent plists — when the installer runs under a umask-0 parent shell, `writeFileSync(plistPath(), plist)` produces a 0666 plist that makes `launchctl load`/`bootstrap` fail with the opaque `Bootstrap failed: 5: Input/output error` and the login-time LaunchAgents scan skip the file silently • on an affected machine the daemon never registers while everything looks installed — the plist exists, launchd's disabled-table says enabled, and no log file is ever created 🔧 Technical Improvements: • `installLaunchd`: write plist with `{ mode: 0o644 }` AND `chmodSync(0o644)` — writeFileSync mode applies only on create, so a reinstall over an existing 0666 plist must normalize explicitly • `installSystemd`: same hardening on the unit file (systemd warns on world-writable units); symmetric with the launchd path • Restart-policy rewrite path (`generateSystemdUnit` rewrite of an existing unit): chmod is load-bearing here — the file always exists, so the write mode never applies • `chmodSync` added to the fs import 📊 Code Changes: 18 insertions, 4 deletions (net +14) 📦 Files Modified: • src/commands/autopilot.ts (minor updates) — mode + chmod on the three supervisor-file writers; comments carry the launchd failure signature so the next EIO hunt greps straight to it |
||
|
levineamandGitHub
|
c21d7b253a | fix(doctor): derive host skill manifests (SUP-3488) (#2961) | ||
|
|
4df7796061 |
Preserve modality and symbol metadata in embed-stale merge (#2969)
embedStaleForSource rebuilds each page's chunks as a merged ChunkInput[] carrying only five fields (chunk_index, chunk_text, chunk_source, embedding, token_count), while upsertChunks writes the metadata columns as EXCLUDED.<col>. Any page containing at least one stale chunk therefore has ALL its chunks' metadata reset on the next embed-stale pass: - image chunks flip modality 'image' -> 'text' and disappear from the cross-modal image search arm permanently (it filters modality='image'), while keeping their embedding_image vector — the data looks intact but is unreachable; - code chunks lose language, symbol_name, symbol_type, symbol_name_qualified. The read side compounds this: rowToChunk never returned modality, so a correct merge was impossible without also extending the Chunk shape. Fix: expose modality on Chunk/rowToChunk and carry modality, language, and the symbol fields through the merge. embedding_image is deliberately not carried — upsertChunks already COALESCEs it server-side. Repair for affected brains: UPDATE content_chunks SET modality='image' WHERE chunk_source='image_asset' AND embedding_image IS NOT NULL. The new test seeds a mixed page (settled image chunk + stale text chunk with symbol metadata) and asserts both survive an embedStaleForSource pass; it fails on master. Co-authored-by: Forge (Ron) <forge@zsimovan.dev> |
||
|
|
6db4cea2e4 |
Resolve relative storage paths in doctor image_assets check (#2971)
The image_assets check statSyncs files.storage_path directly, but sync-ingested assets store repo-relative paths. Run doctor from any directory other than the brain repo and every image is reported 'missing from disk' — a persistent false WARN with a suggested fix (gbrain sync --skip-failed) that does nothing. Resolve relative paths against sync.repo_path before statting; absolute paths are untouched. Falls back to cwd when the config key is unset, preserving the old behavior for brains without a configured repo. Co-authored-by: Forge (Ron) <forge@zsimovan.dev> |
||
|
|
11eebc3605 |
fix(conversation): extract iMessage facts with real timestamps (#2756) (#2958)
Co-authored-by: Sameer Bopardikar <203024074+sameerbopardikar@users.noreply.github.com> |
||
|
|
ee45653a02 |
fix: initialize foreground chat gateway (#2590) (#3003)
Co-authored-by: Eddie Ash <119880+cazador481@users.noreply.github.com> |
||
|
|
706d3cea3d |
Scope maxWaiting backpressure by data.sourceId (#2970)
The submission-time backpressure cap counts all waiting (name, queue) rows regardless of which source a job targets. On a multi-source brain this makes per-source submissions with maxWaiting: 1 mutually exclusive: while one source's sync sits waiting, every other source's freshness sync coalesces into that row and never runs. The dispatch log shows the starved source 'dispatched' each interval (queue.add returns the other source's waiting row), so the starvation is invisible unless you notice sources.last_sync_at falling behind — we found a secondary source 29 hours stale on a 5-minute freshness interval. Fix: when the submitted data carries a string sourceId, key the advisory lock, the waiting count, and the coalesce target on it. Submissions without sourceId keep the existing single-scope behavior, so single-source brains and non-sync jobs are unchanged. The new test asserts same-source submissions still coalesce while a different source gets its own row and its own cap; it fails on master. Co-authored-by: Forge (Ron) <forge@zsimovan.dev> |
||
|
|
2934c53c1d |
fix(sources): validate --path is a git repo with committed content at registration (#2707) (#2975)
* fix(sources): validate --path is a git repo at registration time (#2707) `sources add --path <dir>` accepted any existing non-git directory with zero validation, deferring the failure to the first `gbrain sync` ("Not inside a git repository: ..."). By the time that surfaces, the source has been silently stale for however long nobody read the sync logs. Add a registration-time check (git-remote.ts:isInsideGitRepo, mirroring sync.ts's discoverGitRoot walk-up so subdir-of-git-repo sources still pass) that rejects an existing-but-non-git --path directory with an actionable error pointing at `git init && git add -A && git commit`. Non-existent paths are unaffected (out of scope — different, pre-existing failure mode) and `--force` opts out for callers who want to register before git-init exists. This is registration-time validation ONLY — it never auto-`git init`s the directory, preserving the consent boundary #2967 established for sync-time self-heal (a --path source is the user's own external directory; gbrain must not mutate it without explicit ask). Also documents the git requirement (docs/guides/multi-source-brains.md), including the "files must be committed, not just present" gotcha and that a stale/unreachable sync anchor already self-heals on plain `gbrain sync` (verified manually against HEAD — no reset-anchor command needed). * fix(sources): require a committed HEAD + shell-quote remediation cmd (codex round 1) Codex review round 1 on #2707 found two real gaps: 1. isInsideGitRepo alone accepts a `git init`ed-but-never-committed directory (rev-parse --show-toplevel succeeds with no HEAD), so registration would still pass a source that fails sync's own "No commits in repo ... Make at least one commit before syncing." Add hasGitCommits (git rev-parse HEAD) as a second required check. 2. The remediation command in the error message interpolated the raw path unquoted — spaces, $(), backticks, etc. would break or, worse, execute unintended shell syntax if pasted. POSIX single-quote it (mirrors src/commands/connect.ts:shellQuote; duplicated locally rather than imported, since commands/ depends on core/ not the reverse). * fix(sources): require tracked content in HEAD, not just a resolvable HEAD (codex round 2) Codex review round 2 P1: hasGitCommits (rev-parse HEAD) accepted a repo with an empty commit (git commit --allow-empty) followed by untracked files — HEAD resolves fine (to git's well-known empty-tree object), so registration passed, but the first sync would "succeed" importing nothing and then silently never notice the untracked files change. The exact same gap applied to an untracked subdirectory of an otherwise- real git repo (monorepo case). Replace hasGitCommits with hasTrackedContent (`git ls-tree HEAD -- .`, non-recursive — one entry is enough, no need to walk the whole subtree). `-C path` + pathspec `.` scopes correctly to both a repo toplevel and a subdirectory-of-a-repo source, and an empty tree lists zero entries where a bare `rev-parse HEAD` would still succeed. Also subsumes the "no commits at all" case hasGitCommits covered (ls-tree on an unborn repo fails the same way), so this is one check instead of two. Updated the error copy and docs/guides/multi-source-brains.md to match what's actually verified now. * fix(sources): O(1)-output tree-emptiness probe, avoid maxBuffer overflow (codex round 3) Codex review round 3 found the round-2 `git ls-tree HEAD -- .` listing buffers the whole (non-recursive) tree — a real repo with ~17-20K directly-tracked entries exceeds execFileSync's default 1 MiB maxBuffer, throws ENOBUFS, and the catch-all incorrectly rejects a perfectly valid registration. Replace the listing with `git rev-parse --verify HEAD:./` (resolves the tree object for `path` specifically, correct for both toplevel and subdirectory sources same as before) compared against git's canonical empty-tree SHA-1 (4b825dc6...) — a fixed ~40-byte read regardless of how many entries the tree has, structurally immune to this class of bug rather than just raising the threshold. Added a 300-file regression test locking this in. Declined a second round-3 finding (P1: reject a tree if ANY untracked file exists anywhere under the path, not just when the tree is entirely empty) — untracked files never being synced is standard, existing git-source behavior throughout this codebase (identical for --url managed clones), not a bug specific to this validation. Enforcing zero-untracked-files at registration would reject ordinary repos with gitignored build output, .DS_Store, editor swapfiles, etc. Out of scope relative to what #2707 actually asks for (a directory with real, committed content that will sync) and how every other git source in this system already behaves. * fix(sources): derive empty-tree OID per repo instead of hardcoding SHA-1 (codex round 4) Codex review round 4 P2, confirmed by directly testing against a `git init --object-format=sha256` repo: the hardcoded SHA-1 empty-tree constant only matches SHA-1 repositories. An empty SHA-256 repo's real empty-tree OID is a different (64-char) hash, so the SHA-1 comparison silently mismatched and let an empty/untracked SHA-256 source through — exactly the case this validation exists to catch. Replace the constant with `emptyTreeOid()`: `git hash-object -t tree --stdin < /dev/null` computed in the target repo's own context, so it returns the correct empty-tree OID for whichever object format that repo actually uses, without gbrain needing to know or care which one. Added gated regression tests (git 2.29+ / --object-format=sha256, test.skipIf on older git) for both the empty-repo-rejected and real-content-registers-fine cases. Converging here (4 review rounds; this is the last outstanding finding from round 4, and round 4 raised only this one issue). * fix(test): --force the incidental non-git second source in #1434 routing test (#2707) CI caught a real regression from this PR's registration-time git validation: test/sync-sole-non-default-routing.test.ts's "2+ non-default sources" case registers a bare mkdtempSync temp dir (no git init) as a second source purely to have 2 sources present — the directory's content was never meant to be exercised, only its existence as a distinct local_path. #2707's new validation correctly rejects that dir at registration time, since nothing else in the test suite told it otherwise. --force is the right fix, not adding unnecessary git-init/commit boilerplate to secondRepo: it documents that this specific registration intentionally doesn't care about git-validity, matching what a real caller opting into the legacy lenient behavior would do. Verified: the specific test (3/3 pass), plus every other test file in the repo using `sources add --path` (sources.test.ts, sources-ops.test.ts already covered by the PR's own commits; repos-alias.test.ts, sync-cost-gate.serial.test.ts — 11/11 pass, no similar fixture gap). typecheck clean, verify 31/31. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
b60656245f |
fix(migrate): v124 notice to stderr — the stdout-cleanliness guard caught it on master (#3035)
Same class as #3019's v123 fix; the guard test added there flagged this within one push. Route the notice through process.stderr.write. Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d698b44438 |
fix(search): drop compiled_truth from pages.search_vector — was overflowing tsvector on large pages (#2704) (#2977)
A single markdown page whose compiled_truth exceeds Postgres's hard 1,048,575-byte tsvector cap made update_page_search_vector() throw "string is too long for tsvector" INSIDE the pages UPSERT transaction. Not a per-file ledger entry — a transaction abort. The whole source's sync checkpoint stayed pinned (Sync BLOCKED) until the oversized file was fixed or manually skipped, even though every other file in the run imported fine. --retry-failed re-failed the same files every run; the 3-consecutive-failure auto-skip eventually moved past them, but for a scheduled collector that meant hours of blocked cycles per oversized file, per source. Root cause: pages.search_vector indexed compiled_truth — the unbounded whole-page body — even though it's write-only dead weight for actual search. searchKeyword() (postgres-engine.ts / pglite-engine.ts) ranks and queries content_chunks.search_vector exclusively (Cathedral II Layer 3, chunk-grain, already populated separately from compiled_truth via chunking at import time, and well under the tsvector cap since chunkText() targets embedding-sized pieces). Verified directly: no pages.search_vector / bare search_vector read appears anywhere outside this trigger's own definition and the reindex/backfill machinery that maintains it. Fix: v124 migration recreates update_page_search_vector() without compiled_truth — title + timeline stay (both naturally small), so the column keeps carrying some signal rather than going fully inert. Updated in lockstep (documented contract, see reindex-search-vector.ts's own comment): migrate.ts's new v124, reindex-search-vector.ts's recreatePagesFn, and the fresh-install baselines in pglite-schema.ts + src/schema.sql (regenerates schema-embedded.ts via `bun run build:schema`). No backfill: existing rows keep whatever search_vector they already computed until their next UPDATE — harmless, since nothing reads this column, and the brains that actually hit this bug never successfully wrote a value for the oversized page in the first place. Considered (from the issue) and rejected: truncating compiled_truth to fit under the cap. Silent, position-dependent recall loss, and the byte cap doesn't line up cleanly with any natural character/token boundary for UTF-8 content. content_chunks.search_vector already gives full, untruncated chunk-grain coverage for large pages — truncating a now-redundant whole-page vector would trade a real bug for a subtler one. ## Test plan - New test/page-search-vector-overflow.test.ts: a >1MB page (genuinely diverse tokens — a repetitive lorem-ipsum-style fixture does NOT reproduce this bug, since to_tsvector's cap is on its DEDUPLICATED output size, not raw input length) now imports successfully instead of throwing; remains keyword-searchable via the chunk-grain path; a normal page's search_vector still carries title signal (not fully inert). Verified the test is meaningful both directions: fails with the exact reported error on the pre-fix trigger (git-stashed the fix, reran, confirmed byte-for-byte match: "string is too long for tsvector (2684620 bytes, max 1048575 bytes)"), passes with the fix restored. - Updated fts-language-migration.serial.test.ts: removed an assertion that configurable_fts_language (v123) is LATEST_VERSION — that was only ever true until the next migration landed; the codebase's own pattern elsewhere for this (migrate.test.ts) uses toBeGreaterThanOrEqual, not exact-match, for exactly this reason. - bun run typecheck clean, bun run verify 31/31 green. - test/page-search-vector-overflow.test.ts (3/3), test/reindex-search-vector.serial.test.ts, test/fts-language-migration.serial.test.ts, test/migration-v120.test.ts, test/sync.test.ts, test/bootstrap.test.ts, test/migrate.test.ts — 254 total, 0 fail, no regressions. ## Design consultation Investigated jointly with masa-codex (async design review) before implementing — their read of the codebase (content_chunks.search_vector already covers keyword search; pages.search_vector's compiled_truth feed is the only overflow-prone, effectively-dead write) matched independent verification and shaped the "remove from trigger" fix over the issue's alternative options (chunk-grain rebuild — largely already exists; input truncation — rejected above; ledger-entry-only — insufficient alone, since the page upsert failing in the same transaction also loses the chunk write, so checkpoint advancing without this fix would make the content permanently unsearchable, not just delayed). Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
1d0b5ed816 |
fix(autopilot): forward process.env to execSync('which gbrain') under Bun (#2747) (#2976)
resolveGbrainCliPath() (both copies — src/commands/autopilot.ts and the
inlined duplicate in src/core/brain-repo-durability.ts) called `execSync`
without an explicit `env`, relying on default inheritance. Under Bun,
execSync/execFileSync snapshot process.env at BUN'S OWN STARTUP, not at
call time — a runtime PATH mutation (dotenv/config loading, wrapper-script
env sourcing, etc.) happening after Bun boots but before this call is
invisible to `which gbrain` unless the current env is forwarded
explicitly.
This is a known, already-precedented Bun quirk in this exact codebase:
spawn-helpers.ts's detectTini() was already fixed for the identical
symptom with the identical one-line fix (`env: process.env`), with a
comment explaining the mechanism — this call site was simply missed.
Matches the reported symptom precisely: "which gbrain" resolves fine when
run standalone (a fresh Bun process, no prior env mutation to hide), but
throws specifically from inside autopilot's managed-worker spawn path
(src/commands/autopilot.ts:416, guarded by `spawnManagedWorker` — Postgres
engine + minion_mode enabled), which fires after config/dotenv loading has
already run in that process. Impact per the report: this silently
degrades to no worker ever picking up queued jobs (including embed jobs),
with `gbrain doctor` only showing a growing "N stale chunks" warning that
reads like an ordinary backlog rather than a broken worker.
Also improved the throw-path error message to include the actual
PATH/execPath/argv[1] values observed at failure time, so a future report
doesn't require guessing at what the process actually saw.
Not fixed here (documented as a separate, smaller finding): a third call
site with the identical missing-env pattern exists in
src/core/claw-test/runners/openclaw.ts ('which openclaw'). Left out of
scope for this PR, which is specifically about #2747's reported symptom;
worth a small follow-up.
Verification: bun run typecheck clean, bun run verify 31/31 green,
test/autopilot-resolve-cli.test.ts 4/4 pass (existing coverage, no
regressions — a genuinely-simulated Bun-env-snapshot race isn't
reproducible in a same-process unit test, and this codebase's own
convention explicitly avoids mock.module for child_process per
doctor-orphan-ratio.test.ts's stated test-isolation rule, so this PR
relies on the fix's precedent-match + existing coverage rather than a new
mock-based regression test).
Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
|
4c71a76c0a |
fix(jobs): retry resets started_at/attempts/stalled_counter (#2783) (#2974)
* fix(jobs): retry resets started_at/attempts_made/attempts_started (#2783) `gbrain jobs retry` re-queued a dead job by resetting status/error_text/ locks/delay/finished_at, but left started_at, attempts_made, and attempts_started untouched. On re-claim, claim()'s `started_at = COALESCE(started_at, now())` preserved the ORIGINAL first-claim timestamp instead of re-stamping it. handleWallClockTimeouts() anchors on `now() - started_at`: a retry issued more than timeout_ms * 2 after the original claim was immediately dead-lettered again in under a second, with attempts_made already past max_attempts — making retry useless for exactly the case it exists for (recovering work after an outage that outlasted the job's timeout). An explicit `jobs retry` is an operator asserting "run this fresh", so retryJob now also clears started_at (NULL, re-stamped on next claim) and resets attempts_made/attempts_started to 0. Two new tests: direct assertion that retry resets all three columns, and a full repro of the reported bug (wall-clock-killed job retried long after the original claim now survives re-claim instead of being immediately re-killed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(jobs): also reset stalled_counter on retry (#2783) Codex review round 1 found the fix was incomplete: a job dead-lettered by stall exhaustion (handleStalled() at stalled_counter + 1 >= max_stalled) retained its exhausted stalled_counter across retry. The retried job's very first lock expiry after re-claim would immediately re-satisfy the dead-letter threshold, contradicting the same "run this fresh" intent the started_at/attempts reset already established. New test mirrors the existing wall-clock repro: exhaust the stall budget via two real handleStalled() calls, retry, confirm one more stall now requeues instead of dead-lettering again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
MasaandGitHub
|
354c8c36a9 |
fix(takes): fail closed when takes-write source resolution errors (#2698 follow-up) (#2973)
resolveTakesSourceId() caught every error from resolveSourceId() and fell back to undefined, which restores the pre-#2698 unscoped (cross-source) slug lookup for takes add/update/supersede/resolve. resolveSourceId() only ever throws when a source was explicitly in play (an invalid or unregistered GBRAIN_SOURCE, a .gbrain-source dotfile pointing at a source that doesn't exist, or a genuine DB error) — it never throws for "nothing configured," which resolves cleanly to the seeded 'default' source. So swallowing the error had no legitimate case to protect and only reintroduced the cross-source write bug on any resolution failure. Let it propagate so the write is blocked instead. Adds regression coverage for both the unchanged happy path (no source configured resolves cleanly) and the newly fail-closed path (an unregistered GBRAIN_SOURCE blocks the write instead of falling back to an unscoped lookup). |
||
|
|
dbf2b3f562 |
fix(takes): default takes extraction to the configured chat_model instead of hardcoded cloud Haiku (#2997) (#3021)
extractTakesFromPages hardcoded anthropic:claude-haiku-4-5 as the classifier
model. On an OAuth/local-only install (no ANTHROPIC_API_KEY; chat routed
through a gateway model) every takes extraction died with llm_unavailable —
the takes layer silently never populated and the takes_count health check
stayed red despite a working configured chat_model.
Resolution is now `opts.model || getChatModel()` — the same file-plane
gateway-config idiom enrich.ts uses — NOT engine.getConfig('chat_model')
(the DB config plane), keeping model routing on the single config plane the
rest of the codebase reads. Explicit opts.model still wins; unconfigured
installs fall through to the gateway's DEFAULT_CHAT_MODEL.
Adds a regression test that pins the file-plane read: a conflicting DB-plane
config.chat_model row is ignored, the gateway-configured chat_model is used
when opts.model is unset, and explicit opts.model wins. Verified the
file-plane test fails against the pre-fix code.
Takeover of #2997 by @Nazim22 with the model read moved from the DB config
plane to the file-plane gateway idiom.
Co-authored-by: Garry Tan <garrytan@gmail.com>
Co-authored-by: Nazz <nazim.mj@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
9ed53e4e1c |
fix(code-edges): per-row $n::text::jsonb binds in addCodeEdges — Bun SQL mis-encodes jsonb[] arrays (#2968) (#3020)
Bun SQL double-encodes ::jsonb[] array binds on the Postgres engine: every edge_metadata element landed as a jsonb string scalar instead of an object, so the resolver's `edge_metadata || jsonb_build_object(...)` UPDATE produced a jsonb array and resolved_chunk_id was never readable — code_callers / code_callees / code_blast returned nothing on Postgres-engine brains while the resolver logged edges_resolved > 0. PGLite was unaffected (per-row placeholders already). Rewrites both inserts (code_edges_chunk, code_edges_symbol) to per-row $n::text::jsonb placeholders via sql.unsafe — the same shape executeRawJsonb and the PGLite engine use. Adds the DATABASE_URL-gated Postgres regression test this class requires (PGLite cannot reproduce it): asserts jsonb_typeof(edge_metadata) = 'object' for resolved + unresolved inserts and that the resolver-style || UPDATE keeps object shape. Verified the test fails 3/3 against the pre-fix code and passes with the fix. Takeover of #2968 by @zsimovanforgeops with the missing regression test added. Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Forge (Ron) <forge@zsimovan.dev> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
MasaandGitHub
|
9f7244a77f |
fix(cli): remove sync --install-cron help text — no handler ever existed (#2795) (#2972)
The top-level `gbrain --help` advertised `sync --install-cron` since the line was first added, but `src/commands/sync.ts` never parsed or handled the flag — `gbrain sync --install-cron` silently ran an ordinary one-off sync instead of installing anything, manufacturing false confidence in the exact durability layer operators reach for it to secure. git blame shows the line was introduced once (v0.42.29.0 help-text scaffold) and never touched again — no design intent to recover. Implementing it would also compete with autopilot, which already owns this job: `gbrain autopilot --install` runs a self-maintaining daemon (sync+extract+embed) on a schedule, including a per-source freshness check that submits `sync` jobs on its own interval. A second, separate sync-only cron would be a competing scheduler outside the D10 cycle-lock invariant that already keeps autopilot's own targeted-submit and full-cycle paths from double-processing. Removed the misleading line and pointed sync's --watch entry at `autopilot --install`, mirroring the existing `dream` command's "See also: autopilot --install (continuous daemon)." pattern one section below. Added regression coverage to test/cli-help-discoverability.test.ts asserting the help text no longer promises install-cron and does point at autopilot. |
||
|
|
6ec3dd410e |
fix(gateway): land Anthropic cache_control breakpoints on the system block, not just the call-level auto marker (#2490) (#2981)
gateway.chat() requested cacheSystem:true but never got a system-prompt cache hit on single-turn callers (page-summary, skillopt, enrich): the call-level providerOptions.anthropic.cacheControl is real (it becomes Anthropic's documented top-level "auto-cache the last cacheable block" shorthand via @ai-sdk/anthropic 3.0.47+), but for a stable system prompt paired with a different user message every call, "the last cacheable block" is that ever-varying tail -- every call writes a fresh cache entry there and never reads a prior one. Fix: pass system as a SystemModelMessage object (ai's documented shape for attaching provider options to the system block) carrying its own providerOptions.anthropic.cacheControl when cacheSystem is requested, and mirror the same marker onto the last tool def (Anthropic caches everything up to and including the last cache_control block it sees). The call-level marker is kept, not removed -- it still gives toolLoop()'s growing multi-turn conversation a rolling cache breakpoint on each turn's tail. All three markers now derive from one canonical cacheControlValue computed after provider_chat_options config merging, so a configured TTL override (e.g. ttl: '1h') applies consistently instead of only reaching the call-level marker. Verified red-before-fix by stashing the gateway.ts diff and confirming the new assertions fail on unfixed code, then restoring. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
bcf3b73dcf |
fix(sync): self-heal a never-git-initialized default brain dir (#2964) (#2967)
* fix(sync): self-heal a never-git-initialized default brain dir (#2964) The dream cycle's sync phase throws unconditionally on a legacy sync.repo_path-anchored default brain dir that was never git init-ed (predates git-backed sync, or was rsync'd without its .git), failing every nightly run with no recovery. doctor's sync_freshness/ sync_consolidation checks report "ok" for this exact brain, but only because they query the sources table (0 rows for a legacy default brain) — a coincidental false-negative, not a real diagnosis. Self-heal by git-initializing the dir and capturing the current on-disk state as the sync baseline, scoped to !opts.sourceId only — gbrain owns this directory outright, unlike a registered local source (sources add --path, no --url) which is the user's own external directory and should keep failing loudly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NCVEvUVm15bFuKqskWgfNS * fix(sync): dry-run no-write contract, unborn-HEAD recovery, no-gpg-sign (#2964) Codex review on the initial self-heal patch ( |
||
|
|
23e0541d9b |
fix(cycle): budget the patterns subagent from remaining job time — one phase's fixed 35-min worst case defeats any interval-derived cycle budget (#2781) (#2959)
* fix(cycle): budget the patterns subagent from remaining job time, not a fixed constant (#2781) The autopilot-cycle job gets an interval-derived timeout stamped at submit, but the patterns phase submits its subagent with a fixed 30-min job timeout and waits up to 35 min — one phase's worst case exceeds ANY interval-derived budget <= 35 min, so the parent job dead-letters mid-patterns and the tail phases (consolidate -> schema-suggest) starve for days (#2781, the deeper half left open by the #2852 dispatch-floor fix). - MinionJobContext.deadlineAtMs: absolute deadline from the claim-time timeout_at stamp (the DB ground truth handleTimeouts() sweeps against; re-stamped on every claim so retries get a fresh budget). Null when the job has no per-job timeout. - worker: the per-job abort timer now derives its delay from timeout_at when present, so the in-process timer, the DB sweeper, and the handler-visible deadline agree on ONE absolute instant. - autopilot-cycle + autopilot-global-maintenance handlers thread deadlineAtMs into runCycle; CycleOpts carries it to the patterns phase. - patterns: clampSubagentBudgets() derives BOTH the child job timeout and the wait timeout from the same child deadline (parent deadline minus a 60s stop-margin reserve — enough for the wait poll + force-evict grace + cleanup, deliberately NOT a promise that tail phases complete). Under a 2-min minimum the phase skips honestly (insufficient_cycle_budget) instead of submitting a guaranteed-kill LLM call; the next cycle retries with a fresh budget. - Direct callers (gbrain dream) pass no deadline and keep the configured timeouts unchanged. Follow-up (separate PR): synthesize has the same shape plus sequential per-child waits that accumulate N x subagent_wait_timeout_ms past any parent budget; it needs per-wait remaining-time recomputation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaNDjecPPvhC7LRqkxkhGF * fix(cycle): address review — cancel timed-out patterns child; thread deadline through phase-wrapper handlers - P1: the child's timeout_ms clock starts at ITS claim, so a queued child could outlive the parent deadline the wait was clamped to. On wait timeout, cancelJob strips it (waiting -> cancelled; active -> lock stripped, worker abort fires next renew tick). - P2: makePhaseHandler (standalone patterns/synthesize/... minion jobs) now threads job.deadlineAtMs into runCycle like the autopilot handlers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaNDjecPPvhC7LRqkxkhGF --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
c873ce3014 |
feat(ai): add Mistral provider recipe (#3001)
Adds an EU-hosted provider covering embedding, expansion and chat on one OpenAI-compatible endpoint (https://api.mistral.ai/v1), so a brain that must stay inside EU jurisdiction does not need a US hop for any AI touchpoint. Every field is measured against the live API, not copied from docs: - mistral-embed is fixed 1024 dims and accepts no dimension parameter. Both spellings are rejected: {"dimensions": N} returns 400 extra_forbidden, {"output_dimension": N} returns 400 "does not support output_dimension". The generic openai-compatible branch of dimsProviderOptions() already falls through to `return undefined` for these model ids, so nothing is emitted. Same contract as voyage-4-nano, pinned by a negative assertion in the test. - max_batch_tokens 65536: a 65,286-token batch is accepted, 66,960 returns 400 code 3210 "Too many tokens overall, split into more batches." - chars_per_token 2: the value is a DIVISOR in splitByTokenBudget() (estTokens = text.length / charsPerToken), so lower is the conservative direction. The module default of 4 assumes English prose; a German-language corpus measured 3.58 chars/token, which the default overshoots toward overflow. codestral-embed is deliberately left out: it returns 1536 dims, and a touchpoint carries a single default_dims. Listing it under a 1024 declaration is the mixed-dim case embedding-dim-check.ts exists to catch. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
hhamilton-fvandGitHub
|
f3e78fd2fb |
fix(providers): route diagnostics through buildGatewayConfig so file-plane keys are visible (#3000)
configureFromEnv() hand-assembled its own AIGatewayConfig instead of calling buildGatewayConfig(), the single seam that folds file-plane API keys (openrouter_api_key, zeroentropy_api_key, ...) into the gateway env. That let `gbrain providers list`/`test` report a provider as missing env even when it was correctly set in ~/.gbrain/config.json and the real gateway path resolved it fine. Both configureFromEnv() and runList() now build their env through buildGatewayConfig() (falling back to a bare process.env passthrough pre-init), matching what init-provider-picker.ts already does. |
||
|
|
4528bfa79c |
feat(cli): GBRAIN_DRAIN_TIMEOUT_MS env override for the per-sink teardown drain budget (#2996)
The one-shot CLI teardown drains fire-and-forget background sinks with a hardcoded 2s per-sink budget (DEFAULT_DRAIN_TIMEOUT_MS). That budget assumes a sub-second cloud chat provider; on a self-hosted provider (e.g. an ollama model at 10-20s per completion) a facts:absorb extraction can never finish inside it, so every one-shot CLI exit — sync timers especially — aborts the in-flight chat with 'pipeline_error: The operation was aborted', and the same touched pages retry-and-abort on every subsequent sync. Facts from those pages silently never land, and doctor's facts_extraction_health warns permanently. Fix: resolveDrainTimeoutMs() — GBRAIN_DRAIN_TIMEOUT_MS env override (same env-only escape-hatch pattern as GBRAIN_TEARDOWN_DEADLINE_MS and GBRAIN_FLUSH_GRACE_MS) over the 2000ms default. Explicit drainTimeoutMs from a call site still wins; computeTeardownDeadlineMs already computes the backstop from the resolved value, so the deadline scales with it. Garbage/zero/negative env values fall back to the default. Tests: default, env override, garbage/zero/negative fallback, finishCliTeardown drains with the env-resolved budget, explicit opts still win over env. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Anton SenkovskiyandGitHub
|
6498b872ea |
fix(extract): --dry-run must not write extract_rollup_7d (#2994)
`extract-conversation-facts --dry-run` promises "no DB writes, no
checkpoint advance" (help text) and correctly skips the fact INSERTs,
orphan delete, checkpoint advance, and receipt page. But
writeRunReceiptAndRollup was called unconditionally at both exit paths,
and its upsertExtractRollup always UPSERTs a row into extract_rollup_7d
("ALWAYS fire so doctor's extract_health sees the cycle ran") — so a
dry run mutates the DB.
Gate both writeRunReceiptAndRollup call sites on !dryRun. The writer
returns void and its only non-rollup action (the receipt page) is
already suppressed in dry-run via facts_inserted > 0, so gating at the
call site skips nothing else. Mirrors the existing !dryRun guards on
the fact-insert / checkpoint / audit paths.
Regression test: a dry run leaves extract_rollup_7d empty. Fails before
(row count 1), passes after. Hermetic PGLite + stubbed transports, no
live LLM.
Note: --dry-run still calls the extractor (LLM) by design — facts_extracted
is the reported "would extract N" preview count; left unchanged.
|
||
|
|
324c355318 |
test(e2e): production guard — setupDB refuses non-test databases (#2957)
setupDB() TRUNCATEs every data table on whatever DATABASE_URL points at, and run-e2e.sh deliberately preserves an exported DATABASE_URL — one stray environment variable away from wiping a production brain, with no guard of any kind (found during an independent review, 2026-07-18). assertSafeE2eDatabaseUrl (pure, unit-tested) now runs before any connection: allowed when the database name carries "test" as a word segment (the gbrain_test convention used by CI and .env.testing.example), or when GBRAIN_E2E_ALLOW_DB names the exact database intentionally. Refusal is loud and actionable. 7 unit tests, no DB required. Co-authored-by: Aleksei Razsadin <razsoc.01@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
184b6cb8a1 |
fix search: title candidate arm + gated OR fallback for lexical recall (#2956)
Pages were unreachable by their own exact titles: FTS indexed only chunk body text while the title-weighted pages.search_vector (GIN-indexed since its introduction) was never queried by any search path, and websearch_to_tsquery AND-at-chunk-grain semantics meant one non-matching token zeroed keyword recall with no fallback — long or acronym-bearing titles (e.g. "IAWG ... AAR-LL deck") fell through to the vector arm alone and missed. - searchTitles (both engines): page-grain candidate arm over pages.search_vector (title 'A' + compiled_truth 'B' + timeline 'C'), ts_rank_cd ranked, representative-chunk LATERAL join, full filter parity with searchKeyword (visibility, soft-delete, source grants, hard-excludes, dates, types); fused as a weighted RRF list at the keyword arm's intent-effective k on all three hybrid return paths; fail-open with warnOncePerProcess. No schema changes — the index already existed, dark. - AND->OR one-retry fallback for the keyword arm, gated behind SearchOpts.orFallback (only hybridSearch opts in; countMentions, link resolution, eval, and keyword-only MCP callers keep the strict-AND contract). Refused for queries carrying websearch operators (negation, quoted phrases). searchTitles carries its own page-grain fallback. - Lexical arms parallelized (Promise.all) on the main path. Verified: typecheck clean; 18 hermetic PGLite tests + 2 engine-parity e2e cases (CI Postgres); consumer regression enrichment 18/0 + link-extraction 127/0; independent live QA on a 10,664-page brain — exact-title target miss -> rank 1 (exact_title_match), controls held, negation/quoted guards proven, strict-consumer contract pinned. Diagnosed from a 3-lane read-only diagnostic; adversarial review round closed findings on fallback scope, Postgres test coverage, and operator handling before this commit. Co-authored-by: Aleksei Razsadin <razsoc.01@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
912407bef1 |
fix(search): per-call token-budget meta masked the real cut on both cache paths; restore vacuous search-lite coverage (#2954)
* fix(search): per-call token-budget meta no longer masks the real cut; restore vacuous search-lite coverage
The search-lite integration tests were structurally vacuous: putPage never
creates chunks and searchKeyword joins content_chunks, so every fixture
query returned zero rows on every machine. The tight-budget cut test's
defensive skip ('keyword search may dedupe by page') silently returned
before its assertions had ever executed anywhere, and the two budget-meta
tests ran against empty result sets.
Restoring the fixture (upsertChunks per page) and hardening the
assertions immediately exposed a real meta bug: with a per-call
tokenBudget, the inner hybridSearch enforces the same resolved budget
(per-call wins in resolveSearchMode) and its meta carries the true
dropped count — but hybridSearchCached re-applies the budget to the
already-cut set and published THAT pass's meta, which always reads
dropped=0. onMeta consumers saw a budget record claiming nothing was
dropped while rows were; telemetry (recorded from the inner meta)
disagreed with the caller-visible meta.
- hybrid.ts finalMeta: prefer innerMeta.token_budget when a per-call
budget is set (outer budgetMeta stays as the fallback and remains the
enforcement for the cache-HIT path, where no inner run exists)
- test fixture: chunk each page (pattern: chunk-grain-fts.test.ts)
- cut test: defensive skip replaced with a hard >=2 precondition;
results non-empty + strictly-fewer-than-unbounded + dropped>0 now
actually execute (revert-checked red on the unfixed meta)
- budget-meta tests: assert non-empty result sets so kept=results.length
can no longer pass vacuously at 0=0
The unbounded 'builder' query returns 2 of 3 fixture pages by design —
dedup Layer 3 caps any single page type at 60% of results and the
fixture is all-person — which the >=2 precondition accommodates.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFWV7zBZFcmD94Vek6xRDT
* fix(search): cache-HIT budget meta prefers the stored cut record per review; exact-count assertions, mixed-type fixture
- hit path had the symmetric masking (codex P2): the re-application runs
on the already-trimmed stored set and read dropped=0 while the miss
that produced the same result set reported the real cut. Prefer
hit.meta.token_budget unconditionally — tokenBudget is folded into
knobsHash ('tb='), so a hit only ever serves a lookup with the
identical resolved budget as the write and the outer pass can never
cut further (verified against mode.ts; this is why the reviewer's
'outer wins when it drops' branch is unreachable). budgetMeta remains
the fallback for legacy rows stored without a budget record
- new serial test drives a real store-then-hit roundtrip (mocked
embedQuery, real PGLite cache) and pins hit token_budget == miss
token_budget; revert-checked red (dropped=0) on the unfixed path
- lite test: dropped asserted as the exact unbounded-minus-kept count
and used>0 (dropped>0 alone accepts any wrong positive; used<=250
alone accepts a bogus zero), fixture types mixed (person/company/note)
so dedup Layer-3 type-diversity policy no longer shapes the test
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFWV7zBZFcmD94Vek6xRDT
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
89f226eb38 |
fix(search): classify cache hit/miss in telemetry — hits were invisible, misses unclassified (#2953)
* fix(search): classify cache hit/miss in telemetry — hits were invisible, misses unclassified (#2952) search stats reported 0 hit / 0 miss forever: recordSearchTelemetry fired only from bare hybridSearch, whose meta never carries a cache field, and a cache HIT returned from hybridSearchCached before any record at all — so hit searches also vanished from count/results/tokens/rank-1. - HybridSearchOpts: internal _telemetryCacheStatus ('miss' | 'disabled') threaded from hybridSearchCached into the inner hybridSearch (same pattern as _queryEmbedDeadline), folded into the RECORDED meta only — onMeta payloads unchanged, count/sum_tokens/budget_dropped/rank-1 behavior byte-identical for the miss/disabled paths - hit path: record once from hybridSearchCached with the already-built cachedMeta (cache.status='hit'), post-slice/budget result count, tokens from the budget pass, and the same rank-1 rule as the inner paths - bare hybridSearch direct callers (think/gather, brainstorm, enrich, evals, ...) keep recording exactly as before, with no cache field - test: serial wiring test drives a real store-then-hit roundtrip through hybridSearchCached (mocked embedQuery, real PGLite SemanticQueryCache) and pins the decision matrix (miss / hit / consult-skipped / bare); revert-checked red on pre-fix source at the miss classification Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFWV7zBZFcmD94Vek6xRDT * fix(search): harden cache-hit telemetry per review — mode-gated tokens, embed-failure coverage - hit-path tokens_estimate now gated on the MODE-resolved budget, mirroring the inner paths' resolvedMode.tokenBudget > 0 meta condition (a tokenmax budget-off brain would otherwise record real tokens on hits but 0 on misses, inflating avg-tokens as the hit rate rises) - test: exact token-delta parity assertion (hit contributes the same tokens as the miss that stored the served set) — catches the class of hit/miss accounting asymmetry the increase-only check accepted - test: embed-provider-failure flavor of the disabled path (consult degrades via catch, keyword fallback serves, neither counter bumps) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFWV7zBZFcmD94Vek6xRDT --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
f1031d5a0b |
fix(cli): stop thin-client jobs/config from fabricating a scratch PGLite (#2951)
`jobs list|get` have had remote MCP routing since v0.32, but the CLI shell still ran connectEngine() before dispatch — on a thin-client install that fabricates an empty scratch PGLite in the thin-client GBRAIN_HOME and replays the entire migration chain on every invocation, before the remote call even runs. Host-only jobs subcommands (work, supervisor, submit, ...) and `config` did the same instead of refusing. - cli.ts: dispatch thin-client `jobs list|get` engine-free (runJobs(null, ...)); refuse the other jobs subcommands with a pinpoint hint; add `config` to THIN_CLIENT_REFUSED_COMMANDS with a hint (it reads/writes the host brain's config plane). - jobs.ts: widen runJobs to accept a null engine, guarded so null can only reach the MCP-routed list/get branches. - tests: behavioral (no scratch store created, no migration replay, refusals carry hints) + source-audit pins in the existing idioms. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3a5c4c194c |
fix(remote): poll MinionJob.status, not .state — ping now sees completion (#2950)
submit_job/get_job return the MinionJob row verbatim; its lifecycle field is `status` (src/core/minions/types.ts), not `state`. remote ping typed and read `state`, so every poll saw undefined, the terminal check never matched, and ping always burned its full --timeout and exited 1 even when the autopilot-cycle had completed — printing "Job #N is still undefined." on the way out. Reads fixed to `status`; the ping's own JSON output keys (`state`, `last_state`) are unchanged for consumers. Source-audit regression test pins the field reads. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d165e99f0b |
fix(brain-writer): deadline race verdict from the sentinel, not the clock the timer raced (#2947)
Part of #2946. The hung-COUNT deadline race derived its verdict from a post-await wall-clock re-check, which races the timer's own drift: on loaded CI runners setTimeout callbacks fire measurably EARLY relative to Date.now(), so each padding/boundary adjustment (>=, +1ms) only moved which wrong status the partial-scan test received ('scanned', then 'partial'). The race's timeout arm now resolves a module-private sentinel; the sentinel winning IS the deadline verdict (deadlineHit), consulted by the post-await check without re-reading the clock. A COUNT that resolves null (failed/absent count) stays distinguishable and does not skip the scan; a COUNT that resolves slowly without the timer winning is still caught by the retained wall-clock re-check. The +1ms pad is gone — the sentinel makes timer drift irrelevant for the hung path. Verified: partial-scan suite green 8 consecutive runs incl. the new null-vs-sentinel distinction test. Claude-Session: https://claude.ai/code/session_01KFWV7zBZFcmD94Vek6xRDT Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
8b325041ee |
v0.42.63.0 fix: preserve configured PGLite schema database path (#3016)
* fix(schema): preserve configured PGLite database path * chore: bump version and changelog (v0.42.63.0) Co-Authored-By: OpenAI Codex <noreply@openai.com> --------- Co-authored-by: OpenAI Codex <noreply@openai.com> |
||
|
|
a46f28a63e |
fix(cli): keep doctor --json stdout clean — v123 migration handler printed to stdout (#3019)
The v123 configurable-FTS migration (#2941) logged its completion notice via console.log. Migrations run lazily inside any command's first DB connect, so on the nightly heavy run (fresh Postgres service DB) the line landed as the first line of `gbrain doctor --json` stdout and broke the fm_wallclock jq parse ("Invalid numeric literal at line 1, column 7", run 29731426470). runMigrations' contract routes all migration noise to stderr; move the v123 prints (and the pre-existing v2 slug-rename print) there. Also un-vacuous the fm_wallclock harness: its register-source step used `bun run -e` (bun dumps usage with exit 0 instead of running the code) and `connect({})` (in-memory), so the source was never registered and doctor scanned nothing. It now resolves the engine the way the CLI does and registers the source in the DB doctor actually reads. Regression test: test/migrate-stdout-clean.test.ts re-runs migrations from v122 asserting zero stdout writes, plus a source-level guard that migrate.ts contains no console.log. Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
f72de97943 |
feat(sync): --src-subpath + --exclude for monorepo subdir-source support (#753, supersedes #774) (#2942)
* feat(sync): --src-subpath + --exclude for monorepo subdir-source support (#753) Rebased port of PR #774 onto current master. A single git repo can hold N logical sources at subdirectories: git operations run at the discovered repo root (git rev-parse --show-toplevel — worktrees and submodules resolve natively) while file walking, imports, deletes and renames are scoped to the subpath. Passing the subdirectory directly as the repo path (auto-discovery) works through the same code path. Path-containment guards (the point of the feature): - NAV-1/NAV-2: the realpath-resolved scope must live inside the realpath-resolved git root — ../-traversal and symlinked subdirs pointing outside the repo are rejected before any git op. - NAV-1 TOCTOU: per-file realpath re-validation during the incremental import drain and rename reimport; symlink-escape files are recorded as failures (fail-closed — the bookmark cannot advance past an escape). - NAV-4: an --exclude set that filters out every candidate warns loudly. Scoped syncs use git-root-relative slugs + source_path in BOTH the full and incremental paths (runImport gains slugRoot), fixing the original PR's full/incremental slug divergence in the auto-discovery flow. --exclude matches scope-relative paths in both paths; exclusion never deletes previously-imported pages. The full-sync delete reconcile is scope-restricted and relativizes against the slug base so a healthy scoped source can't trip the #2828 mass-delete valve. .gitignore management resolves to the git root at every call site. Preserves all master-side sync work since the original branch: the #2828 mass-delete safety valve, #1794 resumable checkpoints + pinned targets, #1950 stall watchdog, #2335 heartbeat bump, #1970 bookmark reachability, and the git-ls-files walker (#2315/#2462/#2678, whose symlink/cycle hardening is untouched). Co-authored-by: Jeremy Knows <jeremy@veefriends.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: listEverCommittedPaths uses gitContextRoot after #753 root-triple refactor Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Garry Tan <garrytan@gmail.com> Co-authored-by: Jeremy Knows <jeremy@veefriends.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
f8d11f67a3 |
feat(search): configurable FTS language + reindex command (lands #580/#581/#582) (#2941)
Squashed superset takeover of the FTS-language trilogy by @rafaelreis-r (#580 env-var language for query+write side, #581 migration backfill, #582 gbrain reindex-search-vector), rebased onto current master with the security review's required fixes applied: - Migration renumbered v116 -> v123 (master's v116 was already claimed by code_edges_source_backfill; master is at v122). - Restored the v120/#1647 search_path hardening: all four CREATE OR REPLACE trigger-function bodies (migration handler + reindex command) now carry SET search_path = pg_catalog, public, since CREATE OR REPLACE resets proconfig and would otherwise strip the hardening on upgrade. - reindex-search-vector: shared progress reporter (stderr phases reindex_search_vector.pages/.chunks), id-keyset batched backfill (5000 rows/UPDATE) instead of single whole-table statements, and --json no longer bypasses the --yes/TTY confirmation gate. - Allowlist validation regex + injection tests kept exactly as authored. - Stale v33/v116 comments swept; docs/guides/multi-language-fts.md written (README referenced it but no PR added it); llms bundles regenerated via bun run build:llms. - Trilogy tests quarantined as *.serial.test.ts (env mutation, per check-test-isolation). Verified live on PGLite: fresh init with GBRAIN_FTS_LANGUAGE=portuguese produces portuguese-stemmed vectors with search_path pinned; the reindex command retokenizes an english brain to portuguese in place. Co-authored-by: Sinabina <sinabina@Sinabinas-MacBook-Pro-4.local> Co-authored-by: Rafael Reis <rafael.reis@contabilizei.com.br> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
93cfb37540 |
feat(engine): opt-in Postgres RLS source-scope binding (lands #2387) (#2940)
Takeover of community PR #2387 with the security review's required fixes applied. Original work by @harrisali0101. With GBRAIN_RLS_SCOPE_BINDING=1, source-scoped Postgres read methods wrap their queries in a transaction that binds set_config('app.scopes', $1, true) (federated sourceIds CSV > scalar sourceId > '*') so operator-managed RLS policies can filter rows at the SQL layer — defense-in-depth layer 2 under the mandatory app-layer source filters. Review fixes on top of the original PR: - Flag-off is now a TRUE pass-through: no new per-read transaction wrap (the #1794 PgBouncer pool-exhaustion class). Only the three search methods keep a transaction when off — exactly the sql.begin() + SET LOCAL statement_timeout wrap they already had on master — via the helper's alwaysTransaction option. - Preserved the PR's latent setseed fix: listCorpusSample pins setseed() + SELECT to one connection when seeded (alwaysTransaction gated on opts.seed), so the deterministic path can't split across pooled connections. - Updated the two postgres-engine shape tests to pin the new invariant (search methods route through withScopedReadTransaction with alwaysTransaction; helper owns the sql.begin(); flag-off path is callback(this.sql)). - New behavioral tests (test/postgres-engine-rls-scope.test.ts): flag-off pass-through, flag-off alwaysTransaction, flag-on set_config emission, federated > scalar > '*' precedence, and the CSV as a bound parameter (never interpolated). - Fixed the helper header comment to match the actual branching behavior. - Operator docs in docs/ENGINES.md: env var, policy SQL, the ALTER ROLE ... SET app.scopes='*' default requirement, FORCE ROW LEVEL SECURITY for owner roles, and the honest caveat about unwrapped paths. Co-authored-by: Sinabina <sinabina@Sinabinas-MacBook-Pro-4.local> Co-authored-by: Harris <79081645+harrisali0101@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
9fe4628d02 |
feat(engine): opt-in Postgres RLS source-scope binding (lands #2387) (#2940)
Takeover of community PR #2387 with the security review's required fixes applied. Original work by @harrisali0101. With GBRAIN_RLS_SCOPE_BINDING=1, source-scoped Postgres read methods wrap their queries in a transaction that binds set_config('app.scopes', $1, true) (federated sourceIds CSV > scalar sourceId > '*') so operator-managed RLS policies can filter rows at the SQL layer — defense-in-depth layer 2 under the mandatory app-layer source filters. Review fixes on top of the original PR: - Flag-off is now a TRUE pass-through: no new per-read transaction wrap (the #1794 PgBouncer pool-exhaustion class). Only the three search methods keep a transaction when off — exactly the sql.begin() + SET LOCAL statement_timeout wrap they already had on master — via the helper's alwaysTransaction option. - Preserved the PR's latent setseed fix: listCorpusSample pins setseed() + SELECT to one connection when seeded (alwaysTransaction gated on opts.seed), so the deterministic path can't split across pooled connections. - Updated the two postgres-engine shape tests to pin the new invariant (search methods route through withScopedReadTransaction with alwaysTransaction; helper owns the sql.begin(); flag-off path is callback(this.sql)). - New behavioral tests (test/postgres-engine-rls-scope.test.ts): flag-off pass-through, flag-off alwaysTransaction, flag-on set_config emission, federated > scalar > '*' precedence, and the CSV as a bound parameter (never interpolated). - Fixed the helper header comment to match the actual branching behavior. - Operator docs in docs/ENGINES.md: env var, policy SQL, the ALTER ROLE ... SET app.scopes='*' default requirement, FORCE ROW LEVEL SECURITY for owner roles, and the honest caveat about unwrapped paths. Co-authored-by: Sinabina <sinabina@Sinabinas-MacBook-Pro-4.local> Co-authored-by: Harris <79081645+harrisali0101@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |