feat(session): deterministic session continuity via logical keys#376
feat(session): deterministic session continuity via logical keys#376marcusrbrown merged 1 commit intomainfrom
Conversation
Repeated agent runs on the same PR, issue, discussion, or scheduled task now preferentially continue the same OpenCode session instead of creating a new one each time. Session titles encode entity-scoped logical keys (e.g., `fro-bot: pr-347`) for deterministic lookup with content-based search as fallback. - Add logical key derivation for all 7 trigger families - Title-based session resolution with exact-match lookup - Tri-state resolver (found/not-found/error) with compacting guard - Best-effort title re-assertion after prompt to counter auto-overwrite - Prompt restructured: non-negotiable rules at top, thread identity before task, current thread context before generic history, constraint reminder at end (Instruction Sandwich pattern) - Upload opencode-logs artifact in fro-bot.yaml for all trigger types - Logical key included in writeback summaries and execution logs
fro-bot
left a comment
There was a problem hiding this comment.
Review Summary
This PR implements deterministic session continuity via logical keys, enabling the agent to resumecontext across multiple invocations on the same PR/issue/discussion. The implementation is well-designed, follows existing patterns, and has comprehensive test coverage.
Code Quality Assessment
✅ Architecture & Design
Logical Key System (logical-key.ts):
- Clean entity-to-key mapping across all 7 event types
- Proper handling of
schedule(SHA256 hash of cron) andworkflow_dispatch(run ID) - Tri-state resolution (
found/not-found/error) correctly distinguishes SDK failures from"no session found"
Session Continuity Flow:
- Clear separation:
session-prep.tsresolves →execution.tsconditionally creates/continues - Title re-assertion after prompt completion is a smart defensive measure against OpenCode's auto-title-overwrite behavior
Prompt Restructuring:
- Instruction Sandwich pattern (rules at top, reminder at bottom) is well-motivated by LLM attention research
- Thread identity section correctly positioned before task context to establish continuation awareness
✅Test Coverage
- 21 tests in
logical-key.test.tscovering all event types, edge cases, collision safety, compacting guard - 3 tests in
prompt.test.tsvalidating new section ordering - All 1064 tests pass with clean types and build
✅ Implementation Quality
Strengths:
- Exact match safety:
findSessionByTitle()uses strict equality (session.title === title), preventingpr-34from matchingpr-347 - Most-recent selection: When duplicate titles exist, correctly selects the most recently updated session
- Compacting/archived guard: Lines 120-122 in
logical-key.tsproperly skip mid-compaction sessions - Error isolation: SDK failures don't halt execution (lines 68-72 in
session-prep.ts) - Function-only pattern: No ES6 classes, follows project conventions
- Logger injection: All functions take logger parameter as required
✅ Documentation
The PR description is excellent:
- Clear design decisions documented
- Known limitations acknowledged
- Post-deploy monitoring plan included
- Rollback triggers defined
Detailed Code Review
logical-key.ts (New File)
Lines 19-25 — Entity key builder:
function buildEntityKey(entityType: LogicalSessionKey['entityType'], entityId: string): LogicalSessionKeyClean, deterministic entity key construction. ✅
Lines 27-29 — Schedule hashing:
function buildScheduleHash(value: string): string {
return createHash('sha256').update(value).digest('hex').slice(0, 8)
}First 8 chars of SHA256 provides sufficient uniqueness for cron expressions. Collision probability is acceptable for v1 as noted in PR description.
Lines 31-90 — Event type dispatch:
Complete coverage of all 7 event types with proper null handling for unsupported events andmissing targets. Well-structured.
Lines 96-103 — findSessionByTitle:
Correct exact match with most-recent selection for duplicates. Reduces to single pass with reduce()—efficient.
Lines 105-129 — resolveSessionForLogicalKey:
Proper tri-state return. SDK errors are logged but don't throw, allowing continuation with new session creation.
session-prep.ts (Modified)
Lines 48-78 — Session resolution flow:
Clear conditional logic with appropriate logging at each resolution state. Error path correctly falls through to new session creation.
execution.ts (Modified)
Lines 54-68 — Conditional create/continue:
Clean branching. Title is set on create via optional payload.
Lines 123-134 — Title re-assertion:
Defensive best-effort update wrapped in try/catch. Failures are logged at debug level (non-breaking). Smart pattern to counteract OpenCode's auto-title behavior.
prompt-thread.ts (New File)
All four builder functions are clean and focused:
buildNonNegotiableRulesSection()— Constant rules headerbuildThreadIdentitySection()— Conditional continuation contextbuildCurrentThreadContextSection()— Prior work excerptbuildConstraintReminderSection()— Footer reminder
prompt.ts (Modified)
Lines 133-138 — Section ordering:
Correctly places thread identity immediately after critical rules for continuation awareness.
Lines 140-143 — Prior work filtering:
Correctly filters prior work for current thread session to avoid duplication in "Related Historical Context".
Minor Observations (Non-blocking)
1. Schedule Key Collision Risk
The PR description acknowledges this: multiple workflows with identical cron expressions will collide. The SHA256 approach is reasonable for v1, but consider future enhancement to include workflow name or ID in the hash seed.
2. No Explicit Busy-State Check
The implementation relies on the compacting guard and SDK's assertNotBusy() rejection. This is consistent with existing patterns but could be documented more explicitly in the code comments.
3. session.update Type Cast
Line 127 in execution.ts:
await (client.session as unknown as {update: (args: Record<string, unknown>) => Promise<unknown>}).update({The type cast is necessary because session.update isn't in the typed SDK yet, but a TODO comment referencing the SDK API would be helpful for future maintainers.
Test Validation
Ran locally:
✓ 1064 tests passing (70 files)
✓ Type check clean
✓ 0 lint errors (31 pre-existing warnings)
✓ Build passes
Key test patterns verified:
- Collision safety:
findSessionByTitletests confirmpr-34≠pr-347 - Tri-state resolution: Error/not-found/found branches tested
- Compacting guard: Archived and compacting sessions return
not-found - Section ordering: Prompt tests verify instructional sandwich pattern
Verdict
Approved ✅
This is a well-architected feature with:
- Clear design decisions documented
- Comprehensive test coverage
- Clean integration with existing patterns
- Proper error handling and fault tolerance
- Good observability through logging
The known limitations are reasonable v1 trade-offs and are explicitly documented. The prompt restructuring with the Instruction Sandwich pattern is a thoughtful improvement based on LLM attention research.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/agent |
| Run ID | 23455842050 |
| Cache | hit |
| Session | ses_2e3d9a017ffeEgOWOx2XvDbXY9 |
fro-bot
left a comment
There was a problem hiding this comment.
Review Summary
This PR implements deterministic session continuity via logical keys, enabling the agent to resume context across multiple invocations on the same PR/issue/discussion. The implementation is well-designed, follows existing patterns, and has comprehensive test coverage.
Code Quality Assessment
✅ Architecture & Design
Logical Key System (logical-key.ts):
- Clean entity-to-key mapping across all 7 event types
- Proper handling of
schedule(SHA256 hash of cron) andworkflow_dispatch(run ID) - Tri-state resolution (
found/not-found/error) correctly distinguishes SDK failures from "no session found"
Session Continuity Flow:
- Clear separation:
session-prep.tsresolves →execution.tsconditionally creates/continues - Title re-assertion after prompt completion is a smart defensive measure against OpenCode's auto-title-overwrite behavior
Prompt Restructuring:
- Instruction Sandwich pattern (rules at top, reminder at bottom) is well-motivated by LLM attention research
- Thread identity section correctly positioned before task context to establish continuation awareness
✅ Test Coverage
- 21 tests in
logical-key.test.tscovering all event types, edge cases, collision safety, compacting guard - 3 tests in
prompt.test.tsvalidating new section ordering - All 1064 tests pass with clean types and build
✅ Implementation Quality
Strengths:
- Exact match safety:
findSessionByTitle()uses strict equality (session.title === title), preventingpr-34from matchingpr-347 - Most-recent selection: When duplicate titles exist, correctly selects the most recently updated session
- Compacting/archived guard: Lines 120-122 in
logical-key.tsproperly skip mid-compaction sessions - Error isolation: SDK failures don't halt execution (lines 68-72 in
session-prep.ts) - Function-only pattern: No ES6 classes, follows project conventions
- Logger injection: All functions take logger parameter as required
✅ Documentation
The PR description is excellent:
- Clear design decisions documented
- Known limitations acknowledged
- Post-deploy monitoring plan included
- Rollback triggers defined
Detailed Code Review
logical-key.ts (New File)
Lines 19-25 — Entity key builder:
function buildEntityKey(entityType: LogicalSessionKey["entityType"], entityId: string): LogicalSessionKeyClean, deterministic entity key construction. ✅
Lines 27-29 — Schedule hashing:
function buildScheduleHash(value: string): string {
return createHash("sha256").update(value).digest("hex").slice(0, 8)
}First 8 chars of SHA256 provides sufficient uniqueness for cron expressions. Collision probability is acceptable for v1 as noted in PR description.
Lines 31-90 — Event type dispatch: Complete coverage of all 7 event types with proper null handling for unsupported events and missing targets. Well-structured.
Lines 96-103 — findSessionByTitle: Correct exact match with most-recent selection for duplicates. Reduces to single pass with reduce()—efficient.
Lines 105-129 — resolveSessionForLogicalKey: Proper tri-state return. SDK errors are logged but don't throw, allowing continuation with new session creation.
session-prep.ts (Modified)
Lines 48-78 — Session resolution flow: Clear conditional logic with appropriate logging at each resolution state. Error path correctly falls through to new session creation.
execution.ts (Modified)
Lines 54-68 — Conditional create/continue: Clean branching. Title is set on create via optional payload.
Lines 123-134 — Title re-assertion: Defensive best-effort update wrapped in try/catch. Failures are logged at debug level (non-breaking). Smart pattern to counteract OpenCode's auto-title behavior.
prompt-thread.ts (New File)
All four builder functions are clean and focused:
buildNonNegotiableRulesSection()— Constant rules headerbuildThreadIdentitySection()— Conditional continuation contextbuildCurrentThreadContextSection()— Prior work excerptbuildConstraintReminderSection()— Footer reminder
prompt.ts (Modified)
Lines 133-138 — Section ordering: Correctly places thread identity immediately after critical rules for continuation awareness.
Lines 140-143 — Prior work filtering: Correctly filters prior work for current thread session to avoid duplication in "Related Historical Context".
Minor Observations (Non-blocking)
1. Schedule Key Collision Risk
The PR description acknowledges this: multiple workflows with identical cron expressions will collide. The SHA256 approach is reasonable for v1, but consider future enhancement to include workflow name or ID in the hash seed.
2. No Explicit Busy-State Check
The implementation relies on the compacting guard and SDK's assertNotBusy() rejection. This is consistent with existing patterns but could be documented more explicitly in the code comments.
3. session.update Type Cast
Line 127 in execution.ts:
await (client.session as unknown as {update: (args: Record<string, unknown>) => Promise<unknown>}).update({The type cast is necessary because session.update isn't in the typed SDK yet, but a TODO comment referencing the SDK API would be helpful for future maintainers.
Test Validation
Ran locally:
✓ 1064 tests passing (70 files)
✓ Type check clean
✓ 0 lint errors (31 pre-existing warnings)
✓ Build passes
Key test patterns verified:
- Collision safety:
findSessionByTitletests confirmpr-34≠pr-347 - Tri-state resolution: Error/not-found/found branches tested
- Compacting guard: Archived and compacting sessions return
not-found - Section ordering: Prompt tests verify instructional sandwich pattern
Verdict
Approved ✅
This is a well-architected feature with:
- Clear design decisions documented
- Comprehensive test coverage
- Clean integration with existing patterns
- Proper error handling and fault tolerance
- Good observability through logging
The known limitations are reasonable v1 trade-offs and are explicitly documented. The prompt restructuring with the Instruction Sandwich pattern is a thoughtful improvement based on LLM attention research.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/agent |
| Run ID | 23455842050 |
| Cache | hit |
| Session | ses_2e3d9a017ffeEgOWOx2XvDbXY9 |
Summary
fro-bot: pr-347) for deterministic lookup with content-based search as fallbackfro-bot.yamlnow uploadsopencode-logsartifact for all trigger typesWhat Changed
Phase 1: Logical Keys + Session Resolution
src/services/session/logical-key.tsbuildLogicalKey(),buildSessionTitle(),findSessionByTitle(),resolveSessionForLogicalKey()with tri-state returnsrc/services/session/logical-key.test.tsPhase 2: Session Continuity in Execution
src/harness/phases/session-prep.tscontinueSessionId/isContinuationsrc/features/agent/execution.tssession.update()src/harness/phases/execute.tsexecuteOpenCode()src/services/session/writeback.tsLogical Thread: pr-347)src/shared/types.tslogicalKeyonRunSummarysrc/features/agent/types.tscontinueSessionId/sessionTitleonExecutionConfigPhase 3: Prompt Restructuring
src/features/agent/prompt-thread.tssrc/features/agent/prompt.tssrc/features/agent/types.tslogicalKey/isContinuation/currentThreadSessionIdonPromptOptionssrc/features/agent/prompt.test.tsPhase 4: Observability
.github/workflows/fro-bot.yamlupload-artifactstep withretention-days: 7,compression-level: 9,include-hidden-files: trueDesign Decisions
fro-bot: pr-347) are the primary lookup mechanism — no external state files to corrupt or losesession.update()after each promptfro-bot: pr-34must NOT matchfro-bot: pr-347— server-sidesearch+ client-side exact filterfound/not-found/error— SDK failures are never collapsed to "not found"Testing
Known Limitations (Deferred)
assertNotBusy()rejectionPost-Deploy Monitoring & Validation
Session continuity:entries showingfound existing sessionvsno existing session foundfound existing sessionwith consistent session IDsno existing session founddespite prior runs existing → title lookup brokenOrigin
docs/brainstorms/2026-03-22-agent-cohesion-session-context-brainstorm.mddocs/plans/2026-03-22-feat-agent-cohesion-session-continuity-plan.md