feat: self-contained API keys (read from gbrain's own config, not just env)#121
feat: self-contained API keys (read from gbrain's own config, not just env)#121vinsew wants to merge 3 commits intogarrytan:masterfrom
Conversation
98cd225 to
83d3851
Compare
GBrain stores internal cross-page references in slug form (e.g. `[Alice](./alice)`) because the slug is the canonical identifier in the DB. That works inside GBrain's own resolution layer. But when those pages are exported as `.md` files on disk and opened in standard markdown viewers (Obsidian, VS Code preview, GitHub web view, typical mkdocs/jekyll renderers), the viewers look for a literal file at `./alice` — which doesn't exist. The actual file is `./alice.md`. Result: every internal link in an exported brain is silently broken on disk. The user clicks `[小龙]` in `龙虾群.md`, sees a 404 / empty page, and cannot navigate the brain outside of GBrain itself. This defeats half the value of having the brain stored as portable markdown. Fix: Add `normalizeInternalLinks(content)` that runs over each page's serialized markdown right before `writeFileSync` and rewrites slug-form internal links to filename-form by appending `.md`: [Alice](./alice) -> [Alice](./alice.md) [Alice](alice) -> [Alice](alice.md) [Alice](../people/alice) -> [Alice](../people/alice.md) [小龙](../people/小龙) -> [小龙](../people/小龙.md) Conservative: leaves untouched anything that looks external or already extended: - URL schemes (http:, https:, mailto:, ftp:, file:, tel:, ...) — skip - Anchors (#section) — skip - Empty targets — skip - Trailing slash (directory references) — skip - Already has any extension (.md, .png, .pdf, .MD, ...) — skip - Preserves query strings and anchors when appending: [Section](./alice#bio) -> [Section](./alice.md#bio) [Search](./alice?q=t) -> [Search](./alice.md?q=t) The DB content stays slug-form (GBrain's internal convention is unchanged). Only the on-disk export gets the `.md` annotation, so the exported markdown is viewable as-is by any standard renderer. Real-world reproduction this fix addresses: $ gbrain put 龙虾群 < <(echo '[小龙](./小龙)') $ gbrain export --dir /tmp/out $ cat /tmp/out/龙虾群.md # before this PR: contains [小龙](./小龙) — clicking 404s # after this PR: contains [小龙](./小龙.md) — clicking opens the file Impact: - 2 files changed, +149 / -1 lines (1 line of helper invocation + ~40 lines of helper + comment + 26 tests) - Zero behavior change for external URLs, anchors, or already-extended links - DB content unchanged — only the on-disk export representation gains the `.md` annotation - Existing exports remain valid (re-running export on an already-exported brain is idempotent because already-extended links are skipped) Tests: - 26 new tests covering: same-dir slug, parent-dir slug, deep nesting, CJK slugs, multiple links per line, multi-line markdown, all 6 external schemes (http/https/mailto/file/ftp/tel), all 4 extension cases (md/png/pdf/uppercase), anchor preservation, query preservation, empty/trailing-slash/no-link edge cases. - All 26 tests pass. - Full suite: 612 pass / no new regressions (4 pre-existing PGLiteEngine failures are unrelated and exist on master). Fifth in a series of practical PRs from a real Chinese-speaking deploy. Companion to: - garrytan#114 (chunker CJK) - garrytan#115 (slugify CJK) - garrytan#119 (sync git quotepath CJK) - garrytan#121 (self-contained API keys) Same theme: GBrain is meaningfully more useful when the markdown export is a first-class deliverable, not a half-broken side-effect.
83d3851 to
b8410f9
Compare
The "no keys when neither env nor file provides them" case became impossible once garrytan#121 (self-contained API keys) ships — loadConfig now reads keys from config.json if env is empty. Test now asserts the stronger invariant: after env deletion, the previous env sentinel value must not leak back via the returned config. File-level keys may legitimately persist and are no longer asserted undefined. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GBrain stores internal cross-page references in slug form (e.g. `[Alice](./alice)`) because the slug is the canonical identifier in the DB. That works inside GBrain's own resolution layer. But when those pages are exported as `.md` files on disk and opened in standard markdown viewers (Obsidian, VS Code preview, GitHub web view, typical mkdocs/jekyll renderers), the viewers look for a literal file at `./alice` — which doesn't exist. The actual file is `./alice.md`. Result: every internal link in an exported brain is silently broken on disk. The user clicks `[小龙]` in `龙虾群.md`, sees a 404 / empty page, and cannot navigate the brain outside of GBrain itself. This defeats half the value of having the brain stored as portable markdown. Fix: Add `normalizeInternalLinks(content)` that runs over each page's serialized markdown right before `writeFileSync` and rewrites slug-form internal links to filename-form by appending `.md`: [Alice](./alice) -> [Alice](./alice.md) [Alice](alice) -> [Alice](alice.md) [Alice](../people/alice) -> [Alice](../people/alice.md) [小龙](../people/小龙) -> [小龙](../people/小龙.md) Conservative: leaves untouched anything that looks external or already extended: - URL schemes (http:, https:, mailto:, ftp:, file:, tel:, ...) — skip - Anchors (#section) — skip - Empty targets — skip - Trailing slash (directory references) — skip - Already has any extension (.md, .png, .pdf, .MD, ...) — skip - Preserves query strings and anchors when appending: [Section](./alice#bio) -> [Section](./alice.md#bio) [Search](./alice?q=t) -> [Search](./alice.md?q=t) The DB content stays slug-form (GBrain's internal convention is unchanged). Only the on-disk export gets the `.md` annotation, so the exported markdown is viewable as-is by any standard renderer. Real-world reproduction this fix addresses: $ gbrain put 龙虾群 < <(echo '[小龙](./小龙)') $ gbrain export --dir /tmp/out $ cat /tmp/out/龙虾群.md # before this PR: contains [小龙](./小龙) — clicking 404s # after this PR: contains [小龙](./小龙.md) — clicking opens the file Impact: - 2 files changed, +149 / -1 lines (1 line of helper invocation + ~40 lines of helper + comment + 26 tests) - Zero behavior change for external URLs, anchors, or already-extended links - DB content unchanged — only the on-disk export representation gains the `.md` annotation - Existing exports remain valid (re-running export on an already-exported brain is idempotent because already-extended links are skipped) Tests: - 26 new tests covering: same-dir slug, parent-dir slug, deep nesting, CJK slugs, multiple links per line, multi-line markdown, all 6 external schemes (http/https/mailto/file/ftp/tel), all 4 extension cases (md/png/pdf/uppercase), anchor preservation, query preservation, empty/trailing-slash/no-link edge cases. - All 26 tests pass. - Full suite: 612 pass / no new regressions (4 pre-existing PGLiteEngine failures are unrelated and exist on master). Fifth in a series of practical PRs from a real Chinese-speaking deploy. Companion to: - garrytan#114 (chunker CJK) - garrytan#115 (slugify CJK) - garrytan#119 (sync git quotepath CJK) - garrytan#121 (self-contained API keys) Same theme: GBrain is meaningfully more useful when the markdown export is a first-class deliverable, not a half-broken side-effect.
Today embedding.ts and expansion.ts call `new OpenAI()` / `new Anthropic()`
with no arguments, which makes the SDKs read OPENAI_API_KEY /
ANTHROPIC_API_KEY from the process's env. That puts the burden on every
caller — shells, cron jobs, agent subprocesses, daemons — to propagate
those env vars correctly. When a caller's env doesn't have them (e.g.
launchd-spawned daemons, agent terminal tools with sanitized env), the
caller silently gets empty results from `gbrain query` / `gbrain embed`
because the SDK falls back to anonymous API calls that fail.
GBrain already has `openai_api_key` and `anthropic_api_key` fields in
its GBrainConfig schema (src/core/config.ts) and stores them in
~/.gbrain/config.json, but none of the runtime code actually reads
those fields — the config is populated but never consulted. This PR
connects that wiring so gbrain becomes self-contained: callers just
run `gbrain ...` and gbrain finds its own keys.
Changes:
- config.ts: merge ANTHROPIC_API_KEY from env into loaded config
(was silently dropped — only OPENAI_API_KEY was being merged)
- embedding.ts: read openai_api_key from loadConfig() and pass to
`new OpenAI({ apiKey })`. Falls back to SDK's env-default behavior
when config has no key (preserves current behavior for users who
rely on env vars).
- expansion.ts: same pattern for Anthropic.
Usage for callers:
# One-time setup (put keys in gbrain's own config file)
$ cat >> ~/.gbrain/config.json.fragment <<EOF
{"openai_api_key": "sk-...", "anthropic_api_key": "sk-ant-..."}
EOF
$ chmod 600 ~/.gbrain/config.json
# (or edit config.json directly)
# Then from ANY caller, no env vars needed:
$ gbrain query "..." # just works
$ gbrain embed --stale # just works
This is especially valuable for:
- Cron jobs run under launchd/systemd (which don't inherit shell env)
- Agent terminal tools with env sanitization
- Subprocess calls from Python/Node agents without env passthrough
- Docker containers without explicit env forwarding
Precedence (preserved from existing code):
env var > config file
So users who want to override per-process still can.
Impact:
- 4 files changed, +67 / -5 lines
- Zero behavior change for users who already have env vars set
- Callers without env vars in their subprocess context now work IF
the keys are written to ~/.gbrain/config.json
Tests:
- 4 new tests in test/config.test.ts cover: OPENAI env merge,
ANTHROPIC env merge (regression — was missing), both together,
and absence-when-neither.
- All 12 config tests pass; no pre-existing regressions.
The "no keys when neither env nor file provides them" case became impossible once garrytan#121 (self-contained API keys) ships — loadConfig now reads keys from config.json if env is empty. Test now asserts the stronger invariant: after env deletion, the previous env sentinel value must not leak back via the returned config. File-level keys may legitimately persist and are no longer asserted undefined. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b8410f9 to
18b0ebd
Compare
|
Hi, embedding.ts/expansion.ts now depend on loadConfig() to supply API keys, but loadConfig() always reads ~/.gbrain/config.json via os.homedir() and ignores the GBRAIN_HOME override used elsewhere. In environments that set GBRAIN_HOME (tests, Docker, multi-tenant), config-file keys won’t be found and SDKs will be instantiated without apiKey. Severity: action required | Category: correctness How to fix: Make loadConfig honor GBRAIN_HOME Agent prompt to fix - you can give this to your LLM of choice:
We noticed a couple of other issues in this PR as well - happy to share if helpful. Qodo code review - free for open-source. |
Per qodo-ai review on PR garrytan#121: loadConfig() / saveConfig() were going through a private getConfigDir/getConfigPath that called homedir() directly and ignored GBRAIN_HOME, while configDir/configPath (the public API used by getDbUrlSource and the docs) honored it. That split meant config-file API keys were invisible in tests, Docker containers, and multi-tenant deployments — exactly the contexts that motivated GBRAIN_HOME existing in the first place. The new self-contained API keys feature this PR adds depends on loadConfig() finding the keys, so the split also broke the feature under any GBRAIN_HOME-rooted setup. Fix collapses to one set: delete getConfigDir/getConfigPath, route loadConfig/saveConfig through configDir()/configPath(). Function declarations hoist so the forward reference is fine. homedir() import stays — configDir() still falls back to it when GBRAIN_HOME is unset. Tests: 5 new cases in test/config.test.ts covering configDir/configPath honoring GBRAIN_HOME, saveConfig writing under override, loadConfig reading from override, two-home isolation invariant (write A, read B sees null), and full round-trip. All 17 config tests pass; the 20 check-update tests stay green; the e2e/PGLite failures observed are pre-existing and unrelated (garrytan#223 macOS WASM bug per CLAUDE.md memory).
…#121) Two small ergonomics fixes folded together (#765 deferred — see TODOS.md follow-up; the CJK PGLite extraction was bigger than the plan estimated). #779 reworked (alexandreroumieu-codeapprentice): silence the missing-max_batch_tokens startup warning for recipes with genuinely dynamic batch capacity. New `EmbeddingTouchpoint.no_batch_cap?: true` field. Set on ollama (capacity depends on locally loaded model + OLLAMA_NUM_PARALLEL), litellm-proxy (depends on backend), llama-server (set by --ctx-size at server launch). Three less stderr warnings on every gateway configure; google still warns (it's a real fixed-cap provider that ought to ship a max_batch_tokens declaration). Bonus: litellm-proxy now declares `user_provided_models: true`, removing the last consumer of the legacy `recipe.id === 'litellm'` hardcode in gateway.ts:223 (D8=A wire-through completion). #121 reworked (vinsew): self-contained API keys. Two parts: 1. config.ts: ANTHROPIC_API_KEY env merge was silently missing. loadConfig() merged OPENAI_API_KEY but not ANTHROPIC_API_KEY into the file-config-shape result. One-line addition. 2. cli.ts:buildGatewayConfig: when ~/.gbrain/config.json declares openai_api_key / anthropic_api_key but the process env doesn't have those env vars set (common for launchd-spawned daemons, agent subprocess tools, containers that don't propagate ~/.zshrc), fold the config-file values into the gateway env snapshot. Process env still wins (loaded last) so per-process overrides keep working. Tests (4 cases in test/ai/no-batch-cap-suppression.test.ts): - Ollama / LiteLLM / llama-server all declare no_batch_cap: true - configureGateway does NOT warn for those three - configureGateway STILL warns for google (regression guard) - Cross-cutting invariant: empty-models recipes declare user_provided_models Tests: bun test test/ai/ — 128/128 (4 new + 124 prior). Plan: ~/.claude/plans/ok-lets-turn-this-enumerated-sonnet.md (commit 9 of 11). #765 (Hunyuan PGLite + CJK keyword fallback) deferred to TODOS.md follow-up; the CJK extraction (~150 lines + scoring logic + tests) is larger than the wave's adjacent-fix lane should carry. Closes that PR with a deferral note. Co-Authored-By: alexandreroumieu-codeapprentice <noreply@github.com> Co-Authored-By: vinsew <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Today
src/core/embedding.tsandsrc/core/search/expansion.tscallnew OpenAI()/new Anthropic()with no arguments, which makes the SDKs readOPENAI_API_KEY/ANTHROPIC_API_KEYfrom the process's env. That puts the burden on every caller — shells, cron jobs, agent subprocess tools, daemons, containers — to propagate those env vars correctly.When a caller's subprocess env doesn't have them (common for launchd-spawned daemons, agent terminal tools with env sanitization, or any subprocess without explicit env forwarding), the caller silently gets empty results from
gbrain query/gbrain embedbecause the SDK falls back to anonymous API calls that fail with 401 (then throw, then get caught by gbrain's retry/fallback logic, then return "No results").The debugging experience is painful: the user sees "No results" in their agent's output, assumes the brain is empty, doesn't realize the subprocess env is missing keys. I hit this personally running
gbrain queryfrom a launchd-managed agent's terminal tool — the .zshrc-sourced keys were in my interactive shell but not in the daemon's env, and there's no surface-level error telling you that.The existing schema is almost there — just unconnected
GBrainConfig(src/core/config.ts:10-16) already definesopenai_api_keyandanthropic_api_keyfields, andsaveConfig()writes them to~/.gbrain/config.jsonwith 0600 perms. But none of the runtime code actually reads those fields — the config is populated but never consulted. This PR just connects the wiring.Also fixed:
loadConfig()was silently droppingANTHROPIC_API_KEYfrom the env merge (onlyOPENAI_API_KEYwas being merged on line 43). Minor but present bug.Fix
Three small changes:
config.ts: mergeANTHROPIC_API_KEYenv var into loaded config alongsideOPENAI_API_KEY(was silently dropped).embedding.ts: readopenai_api_keyfromloadConfig()and pass tonew OpenAI({ apiKey }). Falls back tonew OpenAI()(SDK env-default) when config has no key — preserves current behavior for users who rely on env vars.expansion.ts: same pattern for Anthropic.Precedence preserved from existing code:
env var > config file(becauseloadConfig()merges env over file). Users who want to override per-process still can.Usage for callers (new capability)
One-time setup:
Then from any caller — no env vars needed:
Especially valuable for:
-e OPENAI_API_KEYpassthroughImpact
~/.gbrain/config.jsonsaveConfig)Test plan
test/config.test.ts:OPENAI_API_KEYenv merges into configANTHROPIC_API_KEYenv merges into config (regression — was missing)bun testsuite: no new regressions (the 4 pre-existingPGLiteEnginefailures are unrelated and exist onmaster)Context
Fourth in a series of PRs from real-user setup on a Chinese-speaking deployment: #114 (chunker CJK), #115 (slugify CJK), #119 (sync CJK via core.quotepath), and now this one (key portability). Each addresses a distinct anti-pattern surfaced by running gbrain outside the "English-speaker, interactive-shell, env-vars-in-.zshrc" default assumption.
This PR is the only one that's a feature, not a bug fix — but it makes gbrain meaningfully more embeddable as the knowledge backbone for cron-driven agents and daemons, which is the direction the SKILLPACK itself advocates.