diff --git a/dnt.ts b/dnt.ts index 502a1f9..23d02cf 100644 --- a/dnt.ts +++ b/dnt.ts @@ -13,9 +13,7 @@ await Deno.remove(outDir, { recursive: true }).catch(() => undefined); await build({ entryPoints: ["./mod.ts"], outDir, - shims: { - deno: true, - }, + shims: {}, typeCheck: "single", test: false, package: { @@ -45,6 +43,12 @@ await build({ engines: { node: ">=20", }, + dependencies: { + cosmiconfig: "^9.0.0", + }, + devDependencies: { + "@types/node": "^20.0.0", + }, main: "./esm/mod.js", types: "./esm/mod.d.ts", opencode: { diff --git a/docs/superpowers/plans/2026-03-27-config-default-fallback-implementation.md b/docs/superpowers/plans/2026-03-27-config-default-fallback-implementation.md new file mode 100644 index 0000000..27b9590 --- /dev/null +++ b/docs/superpowers/plans/2026-03-27-config-default-fallback-implementation.md @@ -0,0 +1,609 @@ +# Config Default Fallback Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make config loading fail open with localhost defaults, OpenCode +warning notifications for malformed or legacy-file problems, and sibling +endpoint inference when only one endpoint is configured. + +**Architecture:** Keep `src/config.ts` as the single source of truth for config +discovery, normalization, recovery, and final resolution. Convert recoverable +config failures into warning-and-fallback behavior, add explicit sibling +endpoint inference after canonical source selection, and verify the new +semantics through focused config and warning-service tests before broader Deno +validation. + +**Tech Stack:** Deno, TypeScript, `cosmiconfig`, existing config loader in +`src/config.ts`, OpenCode warning service in `src/services/opencode-warning.ts`, +Deno test/lint/fmt tasks. + +--- + +## File Structure And Responsibility Lock-In + +- Modify: `src/config.ts` + - Owns config discovery, legacy fallback handling, endpoint normalization, + recoverable error policy, sibling endpoint inference, and final resolved + config assembly. +- Modify: `src/config.test.ts` + - Owns behavior-level regression coverage for config discovery, malformed + config recovery, legacy fallback semantics, and endpoint inference. +- Modify: `src/services/opencode-warning.ts` + - Provides the neutral warning helper used by config loading and existing + Graphiti availability warnings. +- Modify: `src/services/opencode-warning.test.ts` + - Verifies the neutral warning helper still routes structured logs, toasts, + and console fallback safely. +- Reference: + `docs/superpowers/specs/2026-03-27-config-default-fallback-design.md` + - The approved design and source of truth for behavior. + +Do not add new config modules. Keep the recovery logic in `src/config.ts` and +reuse the existing warning infrastructure instead of introducing a parallel +notification path. + +### Task 1: Add Neutral Warning Plumbing For Config Recovery + +**Files:** + +- Modify: `src/services/opencode-warning.ts` +- Test: `src/services/opencode-warning.test.ts` + +- [ ] **Step 1: Read the warning service and approved spec together** + +Read: + +- `src/services/opencode-warning.ts` +- `src/services/opencode-warning.test.ts` +- `docs/superpowers/specs/2026-03-27-config-default-fallback-design.md` + +Expected: confirm the current composed helper is availability-specific and the +plan needs a neutral wrapper or rename for config warnings. + +- [ ] **Step 2: Write a failing warning-service test for neutral plugin + warnings** + +Add one focused test in `src/services/opencode-warning.test.ts` that calls the +new neutral helper and asserts it schedules the same structured warning/toast +behavior as the current availability helper. + +Suggested shape: + +```ts +it("delivers neutral plugin warnings through the shared warning path", async () => { + const logCalls: unknown[] = []; + const toastCalls: unknown[] = []; + const scheduledTasks: Array<() => void> = []; + + setWarningTaskScheduler((callback) => { + scheduledTasks.push(callback); + }); + setOpenCodeClient({ + app: { + log: (input: unknown) => { + logCalls.push(input); + return Promise.resolve(); + }, + }, + tui: { + showToast: (input: unknown) => { + toastCalls.push(input); + return Promise.resolve(); + }, + }, + }); + + notifyPluginWarning("config warning", { source: "legacy" }); + + assertEquals(logCalls.length, 0); + assertEquals(toastCalls.length, 0); + assertEquals(scheduledTasks.length, 2); + + for (const task of scheduledTasks) task(); + await Promise.resolve(); + + assertEquals(logCalls.length, 1); + assertEquals(toastCalls.length, 1); +}); +``` + +- [ ] **Step 3: Run the new warning-service test to verify RED** + +Run: +`deno test src/services/opencode-warning.test.ts --filter "neutral plugin warnings"` + +Expected: FAIL because the neutral helper does not exist yet. + +- [ ] **Step 4: Implement the minimal neutral warning helper** + +In `src/services/opencode-warning.ts`, add a neutral exported helper such as +`notifyPluginWarning()` that composes the existing structured log + warning +toast path. Keep `notifyGraphitiAvailabilityIssue()` as a thin compatibility +wrapper that delegates to the neutral helper. + +Target shape: + +```ts +export const notifyPluginWarning = ( + message: string, + extra?: unknown, +): void => { + const logged = scheduleStructuredWarning(message, extra); + const toasted = scheduleWarningToast(message, extra); + if (!logged && !toasted) { + warnToConsole(message, extra); + } +}; + +export const notifyGraphitiAvailabilityIssue = ( + message: string, + extra?: unknown, +): void => { + notifyPluginWarning(message, extra); +}; +``` + +- [ ] **Step 5: Re-run the warning-service test to verify GREEN** + +Run: +`deno test src/services/opencode-warning.test.ts --filter "neutral plugin warnings"` + +Expected: PASS. + +- [ ] **Step 6: Run the full warning-service test file** + +Run: `deno test src/services/opencode-warning.test.ts` + +Expected: PASS with existing warning behavior unchanged. + +- [ ] **Step 7: Add a direct compatibility-wrapper regression test** + +In `src/services/opencode-warning.test.ts`, add one focused test that calls +`notifyGraphitiAvailabilityIssue()` and asserts it produces the same scheduled +warning side effects as `notifyPluginWarning()`. + +Expected: this directly proves the compatibility wrapper delegates correctly. +Treat this as a verification assertion, not a RED-first step. + +### Task 2: Make Invalid Config Recoverable And Silent On Discovery Failure + +**Files:** + +- Modify: `src/config.ts` +- Test: `src/config.test.ts` + +- [ ] **Step 1: Add the warning-capture test seam first** + +Before writing config tests, add the minimal warning-capture seam they depend +on. Export a tiny test-only setter such as +`setConfigWarningNotifierForTesting()` from `src/config.ts` or reuse a similarly +small seam in `src/services/opencode-warning.ts`, then reset it in `afterEach`. + +The seam is required for these tests; it is not optional. + +Preferred wiring: keep a module-level notifier in `src/config.ts`, for example +`let notifyConfigWarning = notifyPluginWarning`, export +`setConfigWarningNotifierForTesting()` to override it during tests, and have the +config loader call `notifyConfigWarning(...)` wherever it reports recoverable +config problems. + +Add the matching reset call to the existing `afterEach` block in +`src/config.test.ts` so the notifier seam does not leak across tests. + +- [ ] **Step 2: Add a failing test for malformed discovered config fallback** + +In `src/config.test.ts`, replace the current throwing expectation for malformed +discovered config with a new test that asserts: + +- `loadConfig()` does not throw +- the result falls back to defaults when the discovered source is unreadable +- a plugin warning is emitted +- the warning message identifies the source as discovered config + +Suggested shape: + +```ts +it("warns and falls back when discovered config is malformed", () => { + const warnings: Array<{ message: string; extra: unknown }> = []; + setConfigWarningNotifierForTesting((message, extra) => { + warnings.push({ message, extra }); + }); + setConfigExplorerAdapterForTesting(() => + makeAdapter({ + searchResult: { + graphiti: { endpoint: "not a valid url" }, + }, + }) + ); + + const config = loadConfig(); + + assertEquals(config.graphiti.endpoint, "http://localhost:8000/mcp"); + assertEquals(config.redis.endpoint, "redis://localhost:6379"); + assertEquals(warnings.length, 1); +}); +``` + +- [ ] **Step 3: Add a failing test for malformed legacy config fallback** + +Add a test where discovery returns `null`, legacy `load()` returns malformed +data, and the loader falls back to defaults with one warning. + +Assert that the warning message identifies the source as legacy config. + +At least one malformed-config warning test in this task should use an endpoint +containing credentials such as `http://user:secret@bad host` and assert the +captured warning message redacts the sensitive user info. + +- [ ] **Step 4: Add a failing test for legacy load failure warning** + +Add a test where discovery returns `null`, legacy `load()` throws, and the +loader returns defaults with one warning instead of throwing. + +Assert that the warning message identifies the source as legacy config. + +- [ ] **Step 5: Augment the existing discovery search failure test with silence + assertions** + +Update the existing discovery-search failure test so it also asserts: + +- `loadConfig()` returns localhost defaults +- no plugin warning is emitted +- legacy load is not attempted +- `logger.warn(...)` is not called + +Use a counter in the adapter stub to prove `load()` was not called. This should +remain GREEN for the existing default-resolution and no-legacy-attempt +assertions; only the new warning-path assertions should drive RED if any warning +reporting still leaks through the wrong path. + +Add a `logger.warn` spy so the test fails until the old logging path is removed. + +Implementation hint: import `logger` from `src/services/logger.ts` and use +`stub(logger, "warn", () => {})` so the test can assert no logger warning was +emitted. + +- [ ] **Step 6: Augment the existing discovery init failure test with silence + assertions** + +Update the existing discovery-init failure test so it also asserts +`loadConfig()` returns defaults without emitting a plugin warning and without +calling `logger.warn`, and that legacy `load()` was not attempted. The defaults +assertion may remain GREEN; the warning assertions are the intended RED signal +until the catch path is corrected. + +Use the same `logger.warn` spy pattern here so the test cannot pass for the +wrong reason. + +- [ ] **Step 7: Run malformed-config RED checks** + +Run: `deno test src/config.test.ts --filter "malformed|legacy load failure"` + +Expected: FAIL because the loader still throws or warns incorrectly. + +Note: some pre-existing throw-based tests in `src/config.test.ts` will conflict +with the new fail-open behavior after Step 8 lands. That temporary conflict is +expected and is resolved in Task 4 Step 1. + +- [ ] **Step 8: Run discovery-failure RED checks** + +Run: `deno test src/config.test.ts --filter "discovery"` + +Expected: FAIL until discovery failures stop calling `logger.warn(...)`. + +- [ ] **Step 9: Implement recoverable config warning injection in + `src/config.ts`** + +Update `src/config.ts` so: + +- discovery init/search failures return `resolveConfig(null)` silently, without + calling `logger.warn` or `notifyConfigWarning` +- `config-invalid` joins the recoverable error set for file-based config loading +- malformed discovered config and malformed legacy config are treated as an + unreadable source and resolved by falling back to the next source/defaults +- legacy load failures trigger the neutral plugin warning path instead of throw + +Preserve source provenance while doing this so warning messages can identify +whether the ignored source was discovered config or legacy config. + +Keep the file-source discard behavior simple: if one source throws +`ConfigLoadError` with a recoverable code during normalization/loading, discard +that entire source and continue. + +Call out the existing implementation points explicitly while editing: + +- update `isRecoverableConfigLoadFailure()` so it includes `config-invalid` +- branch the `loadConfig()` catch block explicitly: + - `config-discovery-init` and `config-discovery-search` -> silent fallback + - `config-file-load` and `config-invalid` -> `notifyConfigWarning(...)` +- keep discovery init/search failures silent +- route legacy/malformed-file warnings through `notifyConfigWarning(...)` + +If needed, use separate try/catch handling for discovered config and legacy +fallback loading so `config-invalid` warnings can name the correct source. + +- [ ] **Step 10: Re-run malformed-config checks to verify GREEN** + +Run: `deno test src/config.test.ts --filter "malformed|legacy load failure"` + +Expected: PASS. + +- [ ] **Step 11: Re-run discovery-failure checks to verify GREEN** + +Run: `deno test src/config.test.ts --filter "discovery"` + +Expected: PASS. + +- [ ] **Step 12: Run the legacy fallback regression test** + +Run: +`deno test src/config.test.ts --filter "legacy fallback file when discovery finds nothing"` + +Expected: PASS, proving successful legacy fallback still wins when discovery +returns no config. + +### Task 3: Add Sibling Endpoint Inference + +**Files:** + +- Modify: `src/config.ts` +- Test: `src/config.test.ts` + +- [ ] **Step 1: Add a failing test for graphiti-only endpoint inference** + +Add a test where only `graphiti.endpoint` is configured and assert the resolved +Redis endpoint uses the same hostname with `redis://` and port `6379`. + +Suggested expectation: + +```ts +assertEquals(config.graphiti.endpoint, "http://graphiti.internal:9000/custom"); +assertEquals(config.redis.endpoint, "redis://graphiti.internal:6379"); +``` + +- [ ] **Step 2: Add a failing test for redis-only endpoint inference** + +Add a test where only `redis.endpoint` is configured and assert the resolved +Graphiti endpoint uses the same hostname with `http://`, port `8000`, and path +`/mcp`. + +- [ ] **Step 3: Add a failing test for explicit-both-endpoints no-inference + behavior** + +Add a test where both endpoints are provided and assert both are preserved as +configured. + +- [ ] **Step 4: Add a both-endpoints-absent defaults verification test** + +Add a test where the config source provides only non-endpoint fields, such as +`redis.batchSize`, and assert both resolved endpoints remain the localhost +defaults while the non-endpoint value is preserved. + +Suggested assertion shape: + +```ts +setConfigExplorerAdapterForTesting(() => + makeAdapter({ + searchResult: { + redis: { batchSize: 10 }, + }, + }) +); + +assertEquals(config.graphiti.endpoint, "http://localhost:8000/mcp"); +assertEquals(config.redis.endpoint, "redis://localhost:6379"); +assertEquals(config.redis.batchSize, 10); +``` + +Treat this as a verification assertion rather than a RED-first step. + +- [ ] **Step 5: Add a whole-source discard regression test on mixed + valid/invalid endpoints** + +Add a test where one source contains one valid endpoint and one malformed +endpoint, then assert the loader discards the entire source and returns defaults +rather than partially recovering the valid endpoint. + +This may already pass once Task 2 makes `config-invalid` recoverable. If so, +treat it as a verification assertion instead of forcing an artificial RED step. + +- [ ] **Step 6: Add a failing test for IPv6 hostname transfer** + +Add a test using `http://[::1]:9000/custom` or `redis://[::1]:6380` and assert +the inferred sibling endpoint uses the same IPv6 hostname with the target +service's canonical port and remains a valid URL string, for example +`redis://[::1]:6379`. + +The assertion should prove the inferred value parses as a URL, not just that it +contains the host text. + +- [ ] **Step 7: Add a failing test for `rediss://` canonical Graphiti + inference** + +Add a test where the Redis source uses `rediss://cache.internal:6380` and assert +the inferred Graphiti endpoint remains `http://cache.internal:8000/mcp` per the +approved non-TLS-propagation rule. + +- [ ] **Step 8: Add a failing test for nested endpoint precedence before + inference** + +Add a test where the source provides both top-level legacy `endpoint` and nested +`graphiti.endpoint`, omits `redis.endpoint`, and assert the inferred Redis host +comes from the nested Graphiti endpoint rather than the legacy alias. + +Suggested assertion shape: + +```ts +assertEquals( + config.graphiti.endpoint, + "http://nested-host.example:9000/custom", +); +assertEquals(config.redis.endpoint, "redis://nested-host.example:6379"); +``` + +- [ ] **Step 9: Add a failing test for invalid explicit scheme fallback** + +Add a test where a configured endpoint uses a disallowed explicit scheme such as +`ftp://bad.example/mcp`, then assert the loader emits one warning, does not +throw, and falls back to defaults. + +Also assert the warning message identifies the source type and redacts any +sensitive endpoint user info. + +- [ ] **Step 10: Run the expanded focused inference tests to verify RED** + +Run: +`deno test src/config.test.ts --filter "infers|IPv6|both endpoints|whole source|scheme|nested|non-endpoint"` + +Expected: FAIL because sibling inference and invalid-scheme recovery are not +implemented yet. + +- [ ] **Step 11: Implement endpoint sibling inference in `src/config.ts`** + +Add a small helper in `src/config.ts` that: + +- inspects the canonical configured endpoints after source selection +- parses the present endpoint with `new URL(...)` +- copies only `hostname` +- builds the missing sibling endpoint with the target service's canonical + scheme/port/path +- leaves the source endpoint untouched +- skips inference when both endpoints are already present or both are absent +- uses the same precedence ordering already established elsewhere in + `src/config.ts`, so nested Graphiti values remain authoritative over the + top-level legacy alias +- re-brackets IPv6 hostnames before interpolating them into inferred URLs + +Suggested helper shape: + +```ts +const formatHostForUrl = (hostname: string): string => { + return hostname.includes(":") && !hostname.startsWith("[") + ? `[${hostname}]` + : hostname; +}; + +const getCanonicalGraphitiEndpoint = ( + value: RawGraphitiConfig | null, +): string | undefined => { + return value?.graphiti?.endpoint ?? value?.endpoint; +}; + +const inferSiblingEndpoints = ( + value: RawGraphitiConfig | null, +): RawGraphitiConfig | null => { + if (!value) return value; + + const graphitiEndpoint = getCanonicalGraphitiEndpoint(value); + const redisEndpoint = value.redis?.endpoint; + + if (graphitiEndpoint && !redisEndpoint) { + // Safe because inference runs after normalizeConfiguredEndpoints(...). + const host = formatHostForUrl(new URL(graphitiEndpoint).hostname); + return { + ...value, + redis: { + ...value.redis, + endpoint: `redis://${host}:6379`, + }, + }; + } + + if (redisEndpoint && !graphitiEndpoint) { + // Safe because inference runs after normalizeConfiguredEndpoints(...). + const host = formatHostForUrl(new URL(redisEndpoint).hostname); + return { + ...value, + graphiti: { + ...value.graphiti, + endpoint: `http://${host}:8000/mcp`, + }, + }; + } + + return value; +}; +``` + +Wire the helper into resolution after `normalizeConfiguredEndpoints(...)` and +before `resolveConfig()` applies numeric defaults. Extract the Graphiti endpoint +precedence expression into one small helper that both `resolveConfig()` and +inference can share, rather than duplicating it in multiple places. + +Update `resolveConfig()` itself to call the shared helper, then verify the +existing nested-over-legacy precedence test still passes. + +The existing +`prefers nested graphiti and redis values over legacy top-level +graphiti keys` +test in `src/config.test.ts` is the regression guard for this shared-precedence +refactor. + +- [ ] **Step 12: Re-run the expanded focused inference tests to verify GREEN** + +Run: +`deno test src/config.test.ts --filter "infers|IPv6|both endpoints|whole source|scheme|nested|non-endpoint"` + +Expected: PASS. + +### Task 4: Clean Up Test Coverage And Run Full Verification + +**Files:** + +- Modify: `src/config.test.ts` +- Modify: `src/services/opencode-warning.test.ts` + +- [ ] **Step 1: Reconcile old throwing tests with the new fail-open behavior** + +Remove or rewrite any remaining tests in `src/config.test.ts` that still expect +`ConfigLoadError` for malformed endpoint values. Keep direct unit coverage for +`ConfigLoadError` only where constructor semantics are still relevant. + +Preserve the existing passing tests that already match the approved behavior, +including: + +- `returns defaults when no config is found` +- `uses legacy fallback file when discovery finds nothing` + +That second test is the regression guard for the spec requirement that a +successful legacy fallback still wins when discovery returns no config. + +- [ ] **Step 2: Run the full config test file** + +Run: `deno test src/config.test.ts` + +Expected: PASS. + +- [ ] **Step 3: Run the combined focused test files** + +Run: `deno test src/config.test.ts src/services/opencode-warning.test.ts` + +Expected: PASS. + +- [ ] **Step 4: Run project type-check validation** + +Run: `deno task check` + +Expected: PASS. + +- [ ] **Step 5: Run project lint validation** + +Run: `deno task lint` + +Expected: PASS. + +- [ ] **Step 6: Run project formatting validation** + +Run: `deno fmt --check` + +Expected: PASS. + +- [ ] **Step 7: Update the plan checklist with any deviations discovered during + validation** + +If any command requires a narrower or alternate invocation in practice, record +that directly in the execution notes when implementing so the final artifact +stays reproducible. diff --git a/docs/superpowers/specs/2026-03-27-config-default-fallback-design.md b/docs/superpowers/specs/2026-03-27-config-default-fallback-design.md new file mode 100644 index 0000000..23fbce2 --- /dev/null +++ b/docs/superpowers/specs/2026-03-27-config-default-fallback-design.md @@ -0,0 +1,179 @@ +# Config Default Fallback Design + +## Goal + +Make config loading fail open when no valid config is available, so the plugin +always has a usable minimum runtime configuration without throwing during +startup. + +The minimum effective config is: + +```json +{ + "graphiti": { + "endpoint": "http://localhost:8000/mcp" + }, + "redis": { + "endpoint": "redis://localhost:6379" + } +} +``` + +## Why This Change + +Current behavior treats some config discovery and validation problems as fatal. +That is too strict for the plugin's architecture because both backends already +have canonical localhost defaults and the plugin is expected to degrade +gracefully when optional configuration is absent or incomplete. + +The new behavior should make startup resilient while still surfacing operator +actionability through OpenCode's warning channel instead of hard failures or raw +logger warnings. + +## Required Behavior + +### No Config Found + +- If `cosmiconfig` discovery finds no project or home config, treat that as a + normal empty config. +- Resolve the final config from built-in defaults without throwing or warning. + +### Discovery Failures + +- If `cosmiconfig` initialization fails, treat it the same as config-not-found. +- If `cosmiconfig` search fails, treat it the same as config-not-found. +- Discovery init/search failures short-circuit directly to default resolution; + they do not continue into legacy fallback loading. This preserves the current + exception-based control flow rather than adding a new fallback branch. +- Do not throw and do not emit the current logger warning in these cases. + +### Legacy File Failures + +- If discovery returns no config and the legacy fallback file cannot be loaded, + do not throw. +- Emit an OpenCode warning notification describing that the legacy config was + ignored and defaults were used. +- Continue with default/inferred config resolution. + +### Malformed Config Files + +- If a discovered config file or legacy config file contains malformed endpoint + values or otherwise invalid config values, do not throw. +- Errors currently surfaced as `config-invalid` become recoverable for config + loading and should follow this warning-and-fallback path. +- When one config source produces a `config-invalid` error, treat that entire + source as unreadable rather than partially recovering individual fields from + it. In practice, the existing exception flow already supports this model by + discarding the entire source on the first invalid field. +- This includes the top-level legacy `endpoint` alias as well as nested endpoint + fields. +- Emit an OpenCode warning notification describing that invalid config was + ignored. +- Continue with default/inferred config resolution. + +### Endpoint Inference + +- If both endpoints are absent, use the localhost defaults exactly. +- If only `graphiti.endpoint` is defined, infer `redis.endpoint` from the same + host using the Redis scheme and default Redis port. +- If only `redis.endpoint` is defined, infer `graphiti.endpoint` from the same + host using the HTTP scheme, default Graphiti port, and `/mcp` path. +- Inference should preserve the configured host while normalizing the target + service's canonical scheme/port/path. +- The inferred Graphiti path is always `/mcp`; non-default Graphiti paths + require an explicit `graphiti.endpoint`. +- Scheme inference always uses the target service's canonical default scheme: + `http` for Graphiti and `redis` for Redis, regardless of whether the source + endpoint used `https` or `rediss`. Operators that need TLS on both services + must configure both endpoints explicitly. +- Host transfer uses the parsed URL's `hostname` value, so IPv6 literals are + supported. Only the hostname is copied; the inferred sibling always uses the + target service's canonical default port. + +Examples: + +- `graphiti.endpoint = "http://graphiti.internal:9000/custom"` implies + `redis.endpoint = "redis://graphiti.internal:6379"` +- `redis.endpoint = "rediss://cache.internal:6380"` implies + `graphiti.endpoint = "http://cache.internal:8000/mcp"` + +The source endpoint's own explicit scheme and port remain valid for that source +field; only the inferred sibling endpoint is normalized to its service default. + +### Partial Nested Config + +- Nested `graphiti` and `redis` config remain canonical. +- Existing precedence rules stay intact: nested values win over legacy top-level + graphiti aliases. +- Inference runs after normalization and precedence resolution, so a single + canonical endpoint can seed the missing sibling endpoint. + +## Warning Delivery + +- Replace direct throw-or-log behavior for recoverable malformed/legacy config + problems with OpenCode warning notifications via the existing warning service. +- Warning delivery should use a neutral plugin warning helper that shares the + same structured notification path already used for Graphiti availability + issues. +- Discovery-not-found and discovery-init/search failures should remain silent. +- Legacy load failures and malformed config files warn; silent fallback is only + for config-not-found and discovery init/search failures themselves. + +## Implementation Shape + +### `src/config.ts` + +- Keep `DEFAULT_CONFIG` as the canonical baseline for all numeric and endpoint + defaults. +- Separate "recoverable config problem" handling from truly unrecoverable + programmer/runtime failures. +- Change config loading so discovery init/search failures return `null` config + instead of raising warnings. +- Add `config-invalid` to the recoverable config error path. +- Change config file normalization so malformed file contents can be reported + and ignored without aborting `loadConfig()`. +- Add endpoint sibling inference after canonical raw config selection and before + final resolved config output. + +### Warning Integration + +- Reuse the existing structured warning path, but rename or wrap + `notifyGraphitiAvailabilityIssue()` behind a neutral helper before using it + for config warnings. +- Warning messages should be specific enough to identify whether the ignored + source was discovered config or legacy config. +- Warning payloads should redact sensitive endpoint user info consistently with + existing config validation behavior. + +## Validation Plan + +Add or update tests in `src/config.test.ts` to cover: + +- no config found returns localhost defaults +- discovery init failure returns defaults without throwing +- discovery search failure returns defaults without throwing +- malformed discovered config triggers warning and falls back instead of + throwing +- malformed legacy config triggers warning and falls back instead of throwing +- legacy file load failure triggers warning and falls back instead of throwing +- config with one valid and one invalid endpoint in the same source discards the + entire source rather than partially recovering fields +- graphiti-only config infers redis host/scheme/port +- redis-only config infers graphiti host/scheme/port/path +- both endpoints explicitly provided bypass inference and remain as configured +- discovery init/search failure skips legacy fallback and goes straight to + defaults without warning +- discovery returns no config and successful legacy fallback still uses the + legacy-derived config +- nested endpoint precedence still wins before inference +- invalid explicit endpoint schemes are treated as ignored malformed config with + warning, not fatal exceptions +- `rediss://` source config still infers the documented canonical Graphiti + scheme behavior +- IPv6 endpoint hosts infer the sibling endpoint from the same hostname + +## Non-Goals + +- Do not change the documented canonical config shape. +- Do not reintroduce removed top-level Redis aliases. +- Do not make Graphiti or Redis availability a startup-time hard requirement. diff --git a/packaging.test.ts b/packaging.test.ts new file mode 100644 index 0000000..1363341 --- /dev/null +++ b/packaging.test.ts @@ -0,0 +1,130 @@ +import { assertEquals } from "jsr:@std/assert@^1.0.0"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +const workspaceRoot = new URL(".", import.meta.url); +const workspacePath = workspaceRoot.pathname; + +const decodeText = (value: Uint8Array): string => + new TextDecoder().decode(value); + +const commandExists = async (command: string): Promise => { + const whichCommand = Deno.build.os === "windows" ? "where" : "which"; + try { + const output = await new Deno.Command(whichCommand, { + args: [command], + stdout: "null", + stderr: "null", + }).output(); + return output.code === 0; + } catch { + return false; + } +}; + +const run = async ( + command: string, + args: string[], + cwd = workspacePath, +): Promise<{ code: number; stdout: string; stderr: string }> => { + const output = await new Deno.Command(command, { + args, + cwd, + stdout: "piped", + stderr: "piped", + }).output(); + return { + code: output.code, + stdout: decodeText(output.stdout), + stderr: decodeText(output.stderr), + }; +}; + +Deno.test("built npm package loads in node through the published ESM entrypoint", async () => { + const build = await run("deno", ["task", "build"]); + assertEquals(build.code, 0, build.stderr || build.stdout); + + const builtPackage = JSON.parse( + await Deno.readTextFile(join(workspacePath, "dist/package.json")), + ) as { + dependencies?: Record; + devDependencies?: Record; + }; + assertEquals( + builtPackage.dependencies?.cosmiconfig, + "^9.0.0", + "generated npm package must declare cosmiconfig for runtime config loading", + ); + assertEquals( + typeof builtPackage.devDependencies?.["@types/node"], + "string", + "generated npm package must declare Node typings for dnt typecheck", + ); + + const tempDir = await Deno.makeTempDir(); + try { + const esmRunnerPath = join(tempDir, "load-esm.mjs"); + const bunRunnerPath = join(tempDir, "load-bun.mjs"); + const esmEntrypoint = + pathToFileURL(join(workspacePath, "dist/esm/mod.js")).href; + const packageDir = join(tempDir, "node_modules", "opencode-graphiti"); + const isolatedHome = join(tempDir, "home"); + const isolatedConfig = join(isolatedHome, ".config", "opencode"); + + await Deno.mkdir(join(tempDir, "node_modules"), { recursive: true }); + await Deno.mkdir(isolatedConfig, { recursive: true }); + await Deno.symlink(join(workspacePath, "dist"), packageDir, { + type: "dir", + }); + + await Deno.writeTextFile( + esmRunnerPath, + `import * as plugin from ${ + JSON.stringify(esmEntrypoint) + };\nconsole.log(JSON.stringify(Object.keys(plugin).sort()));\n`, + ); + await Deno.writeTextFile( + bunRunnerPath, + 'import * as plugin from "opencode-graphiti";\n' + + "console.log(JSON.stringify(Object.keys(plugin).sort()));\n", + ); + + const esmLoad = await run("node", [esmRunnerPath]); + assertEquals(esmLoad.code, 0, esmLoad.stderr || esmLoad.stdout); + assertEquals(esmLoad.stdout.trim(), '["graphiti"]'); + + if (await commandExists("bun")) { + const bunLoad = await run("bun", [bunRunnerPath], tempDir); + assertEquals(bunLoad.code, 0, bunLoad.stderr || bunLoad.stdout); + assertEquals(bunLoad.stdout.trim(), '["graphiti"]'); + } + + const localOpenCodePath = "/Users/vicary/.opencode/bin/opencode"; + try { + const opencodeInfo = await Deno.stat(localOpenCodePath); + if (opencodeInfo.isFile) { + const isolatedOpenCode = await new Deno.Command(localOpenCodePath, { + args: ["--print-logs", "stats"], + cwd: workspacePath, + env: { + HOME: isolatedHome, + XDG_CONFIG_HOME: join(isolatedHome, ".config"), + }, + stdout: "piped", + stderr: "piped", + }).output(); + const isolatedOpenCodeOutput = decodeText(isolatedOpenCode.stdout) + + decodeText(isolatedOpenCode.stderr); + assertEquals( + isolatedOpenCodeOutput.includes("Missing 'default' export"), + false, + isolatedOpenCodeOutput, + ); + } + } catch { + // OpenCode is not available in CI; keep the portable package checks above. + } + } finally { + await Deno.remove(tempDir, { recursive: true }).catch(() => undefined); + } +}); diff --git a/src/config.ts b/src/config.ts index cef20be..86c1fa7 100644 --- a/src/config.ts +++ b/src/config.ts @@ -60,7 +60,7 @@ export interface ConfigExplorerAdapter { type ConfigExplorerFactory = () => ConfigExplorerAdapter; -const require = createRequire(import.meta.url); +const nodeRequire = createRequire(import.meta.url); const isRecord = (value: unknown): value is Record => !!value && typeof value === "object" && !Array.isArray(value); @@ -326,7 +326,7 @@ const resolveConfig = (value: RawGraphitiConfig | null): GraphitiConfig => { }; const createCosmiconfigAdapter = (): ConfigExplorerAdapter => { - const { cosmiconfigSync } = require("cosmiconfig") as { + const { cosmiconfigSync } = nodeRequire("cosmiconfig") as { cosmiconfigSync: ( moduleName: string, options?: { searchStrategy?: string }, diff --git a/src/services/connection-manager.ts b/src/services/connection-manager.ts index 357e4c8..87aae36 100644 --- a/src/services/connection-manager.ts +++ b/src/services/connection-manager.ts @@ -1,10 +1,60 @@ -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { createRequire } from "node:module"; +import { pathToFileURL } from "node:url"; import manifest from "../../deno.json" with { type: "json" }; import { isAbortError } from "../utils.ts"; import { redactEndpointUserInfo } from "./endpoint-redaction.ts"; import { logger } from "./logger.ts"; +type McpClientInstance = { + connect(transport: unknown): Promise; + close(): Promise; + callTool( + request: GraphitiToolRequest, + metadata?: unknown, + options?: GraphitiRequestOptions, + ): Promise; +}; + +type McpClientConstructor = new ( + clientInfo: { name: string; version: string }, +) => McpClientInstance; + +type McpTransportConstructor = new (url: URL) => unknown; + +type McpRuntimeModules = { + Client: McpClientConstructor; + StreamableHTTPClientTransport: McpTransportConstructor; +}; + +const nodeRequire = createRequire(import.meta.url); +let mcpRuntimeModulesPromise: Promise | null = null; + +const importResolvedModule = async (specifier: string): Promise => { + const resolvedPath = nodeRequire.resolve(specifier); + return await import(pathToFileURL(resolvedPath).href) as T; +}; + +const loadMcpRuntimeModules = async (): Promise => { + if (mcpRuntimeModulesPromise) { + return await mcpRuntimeModulesPromise; + } + + mcpRuntimeModulesPromise = Promise.all([ + importResolvedModule<{ Client: McpClientConstructor }>( + "@modelcontextprotocol/sdk/client/index.js", + ), + importResolvedModule<{ + StreamableHTTPClientTransport: McpTransportConstructor; + }>("@modelcontextprotocol/sdk/client/streamableHttp.js"), + ]).then(([clientModule, transportModule]) => ({ + Client: clientModule.Client, + StreamableHTTPClientTransport: + transportModule.StreamableHTTPClientTransport, + })); + + return await mcpRuntimeModulesPromise; +}; + export type GraphitiConnectionState = | "connecting" | "connected" @@ -167,17 +217,44 @@ type GraphitiConnectionManagerOptions = { }; function createMcpConnection(endpoint: string): GraphitiConnection { - const client = new Client({ - name: manifest.name, - version: manifest.version, - }); - const transport = new StreamableHTTPClientTransport(new URL(endpoint)); + let runtimePromise: + | Promise<{ client: McpClientInstance; transport: unknown }> + | null = null; + + const getRuntime = async (): Promise<{ + client: McpClientInstance; + transport: unknown; + }> => { + if (runtimePromise) { + return await runtimePromise; + } + + runtimePromise = loadMcpRuntimeModules().then( + ({ Client, StreamableHTTPClientTransport }) => ({ + client: new Client({ + name: manifest.name, + version: manifest.version, + }), + transport: new StreamableHTTPClientTransport(new URL(endpoint)), + }), + ); + + return await runtimePromise; + }; return { - connect: () => client.connect(transport), - close: () => client.close(), - callTool: (request, options) => - client.callTool(request, undefined, options), + connect: async () => { + const { client, transport } = await getRuntime(); + await client.connect(transport); + }, + close: async () => { + const { client } = await getRuntime(); + await client.close(); + }, + callTool: async (request, options) => { + const { client } = await getRuntime(); + return await client.callTool(request, undefined, options); + }, }; } diff --git a/src/services/logger.ts b/src/services/logger.ts index a9ece95..f1764c6 100644 --- a/src/services/logger.ts +++ b/src/services/logger.ts @@ -1,3 +1,4 @@ +import process from "node:process"; import { logStructuredWarning, shouldSuppressConsoleWarningsDuringTests, @@ -66,7 +67,7 @@ const toWarningPayload = ( const isDebugEnabled = (): boolean => { if (debugOverride !== undefined) return debugOverride; try { - return !!Deno.env.get("GRAPHITI_DEBUG"); + return !!process.env.GRAPHITI_DEBUG; } catch { return false; } diff --git a/src/services/redis-events.ts b/src/services/redis-events.ts index d947164..a73e4b7 100644 --- a/src/services/redis-events.ts +++ b/src/services/redis-events.ts @@ -653,7 +653,7 @@ export class RedisEventsService { await this.redis.deleteKeyIfValue(drainClaimLockKey(groupId), claimToken); await this.redis.setString( drainCursorKey(groupId), - entries.at(-1)?.event.id ?? "", + entries[entries.length - 1]?.event.id ?? "", DRAIN_TTL_SECONDS, ); } diff --git a/src/services/runtime-teardown.test.ts b/src/services/runtime-teardown.test.ts index f92046d..9ff4a20 100644 --- a/src/services/runtime-teardown.test.ts +++ b/src/services/runtime-teardown.test.ts @@ -219,6 +219,37 @@ describe("runtime teardown", () => { ); }); + it("registers process signal handlers for node-style runtimes", () => { + const processHandlers = new Map void>>(); + + const runtime = { + process: { + on(event: string, handler: () => void) { + const handlers = processHandlers.get(event) ?? new Set<() => void>(); + handlers.add(handler); + processHandlers.set(event, handlers); + }, + off(event: string, handler: () => void) { + processHandlers.get(event)?.delete(handler); + }, + }, + }; + + const registration = registerRuntimeTeardown([], runtime); + + assertEquals( + [...processHandlers.keys()].sort(), + ["SIGINT", "SIGTERM", "beforeExit", "exit"], + ); + + registration.dispose(); + + assertEquals( + [...processHandlers.values()].map((handlers) => handlers.size), + [0, 0, 0, 0], + ); + }); + it("keeps multiple runtime registrations active until each is disposed", () => { const eventHandlers = new Map void>>(); const signalHandlers = new Map<"SIGINT" | "SIGTERM", Set<() => void>>(); diff --git a/src/services/runtime-teardown.ts b/src/services/runtime-teardown.ts index 6292a85..703adf5 100644 --- a/src/services/runtime-teardown.ts +++ b/src/services/runtime-teardown.ts @@ -36,6 +36,11 @@ type ShutdownRegistrationAdapter = { ) => void; exit?: (code?: number) => never; }; + process?: { + on?: (event: string, handler: () => void) => void; + off?: (event: string, handler: () => void) => void; + exitCode?: number; + }; }; const SHUTDOWN_EVENTS = ["unload", "beforeunload"] as const; @@ -62,7 +67,8 @@ const getForcedShutdownNotice = ( export function registerRuntimeTeardown( tasks: RuntimeTeardownTask[], - runtime: ShutdownRegistrationAdapter = globalThis, + runtime: ShutdownRegistrationAdapter = + globalThis as ShutdownRegistrationAdapter, ): RuntimeTeardownRegistration { const runtimeKey = runtime as object; let teardownPromise: Promise | null = null; @@ -80,6 +86,10 @@ export function registerRuntimeTeardown( signal: (typeof SHUTDOWN_SIGNALS)[number]; handler: () => void; }> = []; + const processEventListeners: Array<{ + event: "beforeExit" | "exit"; + handler: () => void; + }> = []; const disposeEventListeners = (): void => { if (eventListenersDisposed) return; @@ -94,6 +104,10 @@ export function registerRuntimeTeardown( signalListenersDisposed = true; for (const { signal, handler } of signalListeners) { runtime.Deno?.removeSignalListener?.(signal, handler); + runtime.process?.off?.(signal, handler); + } + for (const { event, handler } of processEventListeners) { + runtime.process?.off?.(event, handler); } }; @@ -118,7 +132,13 @@ export function registerRuntimeTeardown( if (exitRequested) return; exitRequested = true; dispose(); - runtime.Deno?.exit?.(SHUTDOWN_EXIT_CODE[signal]); + const exitCode = SHUTDOWN_EXIT_CODE[signal]; + if (runtime.Deno?.exit) { + runtime.Deno.exit(exitCode); + } + if (runtime.process) { + runtime.process.exitCode = exitCode; + } }; const run = (): Promise => { @@ -192,9 +212,23 @@ export function registerRuntimeTeardown( }; runtime.Deno?.addSignalListener?.(signal, handler); + runtime.process?.on?.(signal, handler); signalListeners.push({ signal, handler }); } + const beforeExitHandler = () => { + beginGracefulShutdown({ kind: "event", type: "beforeunload" }); + }; + const exitHandler = () => { + beginGracefulShutdown({ kind: "event", type: "unload" }); + }; + runtime.process?.on?.("beforeExit", beforeExitHandler); + runtime.process?.on?.("exit", exitHandler); + processEventListeners.push( + { event: "beforeExit", handler: beforeExitHandler }, + { event: "exit", handler: exitHandler }, + ); + const registrations = activeRegistrations.get(runtimeKey) ?? new Set(); registrations.add(dispose); activeRegistrations.set(runtimeKey, registrations); diff --git a/src/services/session-corpus.ts b/src/services/session-corpus.ts index aadb8ec..e994f37 100644 --- a/src/services/session-corpus.ts +++ b/src/services/session-corpus.ts @@ -147,7 +147,7 @@ const stemToken = (token: string): string => { const endsWithDoubleConsonant = (value: string): boolean => value.length >= 2 && - value.at(-1) === value.at(-2) && + value[value.length - 1] === value[value.length - 2] && isConsonant(value, value.length - 1); const cvc = (value: string): boolean => { @@ -823,7 +823,9 @@ export const createSessionCorpusService = (options: SessionCorpusOptions) => { if (!corpusRef) return corpusRef; const sourcePrefix = `${sessionPrefix(sourceRootSessionId)}:corpus:`; if (!corpusRef.startsWith(sourcePrefix)) return corpusRef; - const sourceCorpusId = corpusRef.split(":").at(-2) ?? ""; + const sourceCorpusParts = corpusRef.split(":"); + const sourceCorpusId = sourceCorpusParts[sourceCorpusParts.length - 2] ?? + ""; const targetCorpusId = corpusIdMap.get(sourceCorpusId); return targetCorpusId ? corpusRefFor(targetRootSessionId, targetCorpusId) @@ -1587,7 +1589,8 @@ export const createSessionCorpusService = (options: SessionCorpusOptions) => { const result = await writeCorpus(input, "index"); if (input.source && input.label) { - const corpusId = result.corpusRef.split(":").at(-2) ?? ""; + const corpusRefParts = result.corpusRef.split(":"); + const corpusId = corpusRefParts[corpusRefParts.length - 2] ?? ""; await options.redis.setString( identityKey(input.rootSessionId, input.source, input.label), corpusId, @@ -1688,7 +1691,10 @@ export const createSessionCorpusService = (options: SessionCorpusOptions) => { await refreshCorpusFamily( input.rootSessionId, - corpus.corpusRef.split(":").at(-2) ?? "", + (() => { + const corpusRefParts = corpus.corpusRef.split(":"); + return corpusRefParts[corpusRefParts.length - 2] ?? ""; + })(), ); return { diff --git a/src/services/session-executor.ts b/src/services/session-executor.ts index 9e528f6..fcdee9e 100644 --- a/src/services/session-executor.ts +++ b/src/services/session-executor.ts @@ -1,4 +1,7 @@ import path from "node:path"; +import { spawn } from "node:child_process"; +import { readFile as readFileNode } from "node:fs/promises"; +import process from "node:process"; import { createAbortError, isAbortError } from "../utils.ts"; import type { SessionMcpRequestMap, @@ -111,55 +114,69 @@ const clampTimeoutSeconds = ( defaults.maxCommandTimeoutSeconds, ); -type DenoCommandOutput = { - code: number; - stdout: Uint8Array; - stderr: Uint8Array; -}; - -type DenoCommandInstance = { - output(): Promise; +const concatChunks = (chunks: Uint8Array[]): Uint8Array => { + const totalBytes = chunks.reduce((sum, chunk) => sum + chunk.byteLength, 0); + const combined = new Uint8Array(totalBytes); + let offset = 0; + for (const chunk of chunks) { + combined.set(chunk, offset); + offset += chunk.byteLength; + } + return combined; }; -type DenoCommandConstructor = new ( - command: string | URL, - options?: { - args?: string[]; - cwd?: string; - stdin?: "null" | "piped" | "inherit"; - stdout?: "null" | "piped" | "inherit"; - stderr?: "null" | "piped" | "inherit"; - signal?: AbortSignal; - }, -) => DenoCommandInstance; - const defaultRunCommand: NonNullable = async ({ command, cwd, signal }) => { - const shell = Deno.build.os === "windows" + const shell = process.platform === "win32" ? { executable: "cmd", args: ["/d", "/s", "/c", command] } : { executable: "/bin/sh", args: ["-lc", command] }; - const DenoWithCommand = Deno as typeof Deno & { - Command: DenoCommandConstructor; - }; - const output = await new DenoWithCommand.Command(shell.executable, { - args: shell.args, - cwd, - stdin: "null", - stdout: "piped", - stderr: "piped", - signal, - }).output(); - - return { - exitCode: output.code, - stdout: textDecoder.decode(output.stdout), - stderr: textDecoder.decode(output.stderr), - }; + + return await new Promise((resolve, reject) => { + const stdoutChunks: Uint8Array[] = []; + const stderrChunks: Uint8Array[] = []; + let settled = false; + const child = spawn(shell.executable, shell.args, { + cwd, + stdio: ["ignore", "pipe", "pipe"], + signal, + }); + + const rejectOnce = (error: unknown) => { + if (settled) return; + settled = true; + reject(error); + }; + + const resolveOnce = (value: CommandExecutionResult) => { + if (settled) return; + settled = true; + resolve(value); + }; + + child.stdout.on("data", (chunk: Uint8Array | string) => { + stdoutChunks.push( + typeof chunk === "string" ? textEncoder.encode(chunk) : chunk, + ); + }); + child.stderr.on("data", (chunk: Uint8Array | string) => { + stderrChunks.push( + typeof chunk === "string" ? textEncoder.encode(chunk) : chunk, + ); + }); + child.on("error", rejectOnce); + child.on("close", (code) => { + resolveOnce({ + exitCode: code ?? (signal.aborted ? -1 : 1), + stdout: textDecoder.decode(concatChunks(stdoutChunks)), + stderr: textDecoder.decode(concatChunks(stderrChunks)), + }); + }); + }); }; const defaultReadFile: NonNullable = ( filePath, -) => Deno.readTextFile(filePath); +) => readFileNode(filePath, "utf8"); const defaultStoreArtifact: NonNullable< SessionExecutorOptions["storeArtifact"] @@ -169,7 +186,7 @@ const defaultStoreArtifact: NonNullable< }); const resolveCwd = (context: SessionExecutorContext): string => - context.worktree ?? context.directory ?? Deno.cwd(); + context.worktree ?? context.directory ?? process.cwd(); const isWithinRoot = (rootPath: string, targetPath: string): boolean => { const relative = path.relative(rootPath, targetPath); diff --git a/src/services/session-mcp-runtime.ts b/src/services/session-mcp-runtime.ts index 6b9de4e..0dd379c 100644 --- a/src/services/session-mcp-runtime.ts +++ b/src/services/session-mcp-runtime.ts @@ -25,6 +25,7 @@ import { type SessionMcpToolName, } from "./session-mcp-types.ts"; import type { RuntimeRootSessionValidator } from "../session.ts"; +import { readFile as readFileNode } from "node:fs/promises"; import path from "node:path"; export const SESSION_MCP_RESPONSE_BUDGET_BYTES = 8 * 1024; @@ -252,7 +253,7 @@ const byteLength = (value: string): number => textEncoder.encode(value).byteLength; const readTextFile = (filePath: string): Promise => - Deno.readTextFile(filePath); + readFileNode(filePath, "utf8"); const createBoundedSessionIndexError = ( code: "session_index_path_unreadable",