Skip to content

feat(server): ContexGin HTTP boot context + replay fixes#365

Merged
dimakis merged 2 commits into
mainfrom
feat/contexgin-http-boot-v2
Jun 3, 2026
Merged

feat(server): ContexGin HTTP boot context + replay fixes#365
dimakis merged 2 commits into
mainfrom
feat/contexgin-http-boot-v2

Conversation

@dimakis
Copy link
Copy Markdown
Owner

@dimakis dimakis commented Jun 2, 2026

Summary

  • Fetch boot context from ContexGin HTTP API at session start, with local Python script fallback when ContexGin is unreachable
  • Extract sendBootContext() helper in ws-handler-v2 — shared hot (in-memory) + cold (EventStore) path for boot_context replay
  • Fix boot_context replay: add missing sessionId field, add cold-path fallback on reconnect, log JSON parse failures
  • Fix detectStateMismatch false alerts for CLOSING state (graceful shutdown — registry state is indeterminate)
  • Persist bootContext to EventStore on fetch so resumed sessions have cold-path recovery

Test plan

  • npx tsc -b passes
  • npx vitest run server/__tests__/ws-handler-v2.test.ts — 130/130
  • npx vitest run server/__tests__/boot-context.test.ts — passes
  • Manual: reconnect with multiple sessions, verify boot_context messages include sessionId
  • Manual: switch to an ended session, verify boot_context replays from EventStore

🤖 Generated with Claude Code

Copy link
Copy Markdown
Owner Author

@dimakis dimakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Centaur Review

Found 8 issue(s) (5 warning).

server/chat.ts

Solid refactor replacing a complex dynamic-import flow with a clean HTTP client + fallback. Main concerns: missing sessionId on initial boot_context send (inconsistent with replay path), no test coverage for the sendBootContext ws-handler helper, and a tokenBudget default inconsistency between success-fallback (8000) and total-failure-fallback (0).

  • 🟡 bugs (L916): Initial boot_context message sent via send(transport, msg) lacks a sessionId field. In contrast, the sendBootContext helper in ws-handler-v2.ts always adds sessionId to the message during reconnect/switch. The frontend may rely on sessionId for demuxing on the multiplexed WebSocket — missing it on the initial send could cause the first boot_context to be silently dropped or mis-routed by the client store. [fixable]
  • 🟡 regressions (L920): s.bootContext = msg as unknown as Record<string, unknown> stores the full BootContextMessage (which includes type: 'boot_context'). When sendBootContext in ws-handler-v2.ts replays this via { type: 'boot_context', sessionId, ...found.session.bootContext }, the spread already contains type: 'boot_context' from the cached message, so the explicit type is redundant but harmless. However, the cached object does NOT contain sessionId, so the replay correctly adds it. This is fine but the double type field is messy — consider stripping type before caching. [fixable]
  • 🟡 unsafe_assumptions (L138): localBootContextFallback hardcodes 5 source file paths (Working Style.md, Communication Style.md, Principles.md, CONSTITUTION.md, SERVICES.md). If the Python script returns different files or the workspace layout changes, the metadata will be wrong. The sources list doesn't reflect what the script actually loaded — it's a static assumption about the script's behavior. [fixable]
  • 🟡 unsafe_assumptions (L199): Default tokenBudget is 8000 when ContexGin response lacks the field, but FALLBACK_BOOT_CONTEXT (line 114) uses 0. This creates inconsistent downstream behavior: a ContexGin response missing tokenBudget implies 8000 tokens of budget, while a total failure implies 0. If downstream code uses tokenBudget for decisions (e.g., context pill display), the 0 fallback may cause unexpected UI behavior. [fixable]
  • 🔵 style (L384): The send() signature was widened to Record<string, unknown> | BootContextMessage with an as Record<string, unknown> cast inside. Since BootContextMessage is a plain object with known string keys, it's already assignable to Record<string, unknown>. The union + cast is unnecessary — just keep the original Record<string, unknown> parameter type. [fixable]
  • 🔵 style (L927): .catch(() => {}) silently swallows errors from fetchBootContext. Since fetchBootContext is documented as never-throw (returns fallback on any error), this catch should be unreachable. If it IS reachable, the silent swallow hides bugs. Consider .catch((err) => log.warn('boot context fetch unexpected error', { error: err })) for observability. [fixable]

server/ws-handler-v2.ts

Solid refactor replacing a complex dynamic-import flow with a clean HTTP client + fallback. Main concerns: missing sessionId on initial boot_context send (inconsistent with replay path), no test coverage for the sendBootContext ws-handler helper, and a tokenBudget default inconsistency between success-fallback (8000) and total-failure-fallback (0).

  • 🟡 missing_tests (L167): The new sendBootContext helper function has zero test coverage. No tests in ws-handler-v2.test.ts exercise boot_context replay (hot path from SessionRegistry or cold path from EventStore). The cold path includes JSON.parse which could fail — only the function's own catch handles it, but correctness of the hot/cold path selection and sessionId injection are untested. [fixable]

server/__tests__/boot-context.test.ts

Solid refactor replacing a complex dynamic-import flow with a clean HTTP client + fallback. Main concerns: missing sessionId on initial boot_context send (inconsistent with replay path), no test coverage for the sendBootContext ws-handler helper, and a tokenBudget default inconsistency between success-fallback (8000) and total-failure-fallback (0).

  • 🔵 missing_tests: Tests don't verify the tokenBudget field in responses. For example, the 'handles empty sources array' test checks tokenCount but not tokenBudget. The default of 8000 when ContexGin omits tokenBudget (line 199) is never explicitly tested. [fixable]

Comment thread server/chat.ts Outdated
// Fire-and-forget: fetch boot context from running ContexGin server
fetchBootContext(agentName)
.then((msg) => {
send(transport, msg);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bugs: Initial boot_context message sent via send(transport, msg) lacks a sessionId field. In contrast, the sendBootContext helper in ws-handler-v2.ts always adds sessionId to the message during reconnect/switch. The frontend may rely on sessionId for demuxing on the multiplexed WebSocket — missing it on the initial send could cause the first boot_context to be silently dropped or mis-routed by the client store. [fixable]

Comment thread server/chat.ts
}
})();
if (s) s.bootContext = msg as unknown as Record<string, unknown>;
// Persist to EventStore for cold-path recovery (resumed sessions
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 regressions: s.bootContext = msg as unknown as Record<string, unknown> stores the full BootContextMessage (which includes type: 'boot_context'). When sendBootContext in ws-handler-v2.ts replays this via { type: 'boot_context', sessionId, ...found.session.bootContext }, the spread already contains type: 'boot_context' from the cached message, so the explicit type is redundant but harmless. However, the cached object does NOT contain sessionId, so the replay correctly adds it. This is fine but the double type field is messy — consider stripping type before caching. [fixable]

Comment thread server/chat.ts
const content = parsed.additionalContext ?? '';
// Rough token estimate: ~4 chars per token
const tokens = Math.ceil(content.length / 4);
const sources = [
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 unsafe_assumptions: localBootContextFallback hardcodes 5 source file paths (Working Style.md, Communication Style.md, Principles.md, CONSTITUTION.md, SERVICES.md). If the Python script returns different files or the workspace layout changes, the metadata will be wrong. The sources list doesn't reflect what the script actually loaded — it's a static assumption about the script's behavior. [fixable]

Comment thread server/chat.ts
}

const bootTokens = typeof boot.tokens === 'number' ? boot.tokens : 0;
const bootBudget = typeof boot.tokenBudget === 'number' ? boot.tokenBudget : 8000;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 unsafe_assumptions: Default tokenBudget is 8000 when ContexGin response lacks the field, but FALLBACK_BOOT_CONTEXT (line 114) uses 0. This creates inconsistent downstream behavior: a ContexGin response missing tokenBudget implies 8000 tokens of budget, while a total failure implies 0. If downstream code uses tokenBudget for decisions (e.g., context pill display), the 0 fallback may cause unexpected UI behavior. [fixable]

Comment thread server/chat.ts
function send(transport: SessionTransport, data: Record<string, unknown>) {
if (transport.isOpen()) transport.send(data);
function send(transport: SessionTransport, data: Record<string, unknown> | BootContextMessage) {
if (transport.isOpen()) transport.send(data as Record<string, unknown>);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 style: The send() signature was widened to Record<string, unknown> | BootContextMessage with an as Record<string, unknown> cast inside. Since BootContextMessage is a plain object with known string keys, it's already assignable to Record<string, unknown>. The union + cast is unnecessary — just keep the original Record<string, unknown> parameter type. [fixable]

Comment thread server/chat.ts Outdated
eventStore.upsertSession({ sessionId: sid, bootContext: JSON.stringify(msg) });
}
})
.catch(() => {});
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 style: .catch(() => {}) silently swallows errors from fetchBootContext. Since fetchBootContext is documented as never-throw (returns fallback on any error), this catch should be unreachable. If it IS reachable, the silent swallow hides bugs. Consider .catch((err) => log.warn('boot context fetch unexpected error', { error: err })) for observability. [fixable]

Comment thread server/ws-handler-v2.ts
* Hot path: in-memory ManagedSession cache.
* Cold path: serialized JSON from EventStore (ended/restarted sessions).
*/
function sendBootContext(connectionId: string, sessionId: string, ctx: V2HandlerContext): void {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 missing_tests: The new sendBootContext helper function has zero test coverage. No tests in ws-handler-v2.test.ts exercise boot_context replay (hot path from SessionRegistry or cold path from EventStore). The cold path includes JSON.parse which could fail — only the function's own catch handles it, but correctness of the hot/cold path selection and sessionId injection are untested. [fixable]

dimakis and others added 2 commits June 3, 2026 15:26
… CLOSING mismatch

Extract sendBootContext() helper shared by handleReconnect and
handleSwitchSession. Fixes: missing sessionId field on replay, no
cold-path EventStore fallback for non-running sessions on reconnect,
silent catch on JSON.parse, and false mismatch alerts for CLOSING state.
Also persists bootContext to EventStore on fetch so resumed sessions
have cold-path recovery.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… coverage

- Add sessionId to initial boot_context send for consistency with replay
- Replace silent .catch(() => {}) with log.warn for observability
- Add 4 tests for sendBootContext helper (hot path, cold path, sessionId, invalid JSON)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dimakis dimakis force-pushed the feat/contexgin-http-boot-v2 branch from ecc2953 to c53f602 Compare June 3, 2026 14:30
@dimakis dimakis merged commit 0c9b220 into main Jun 3, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant