feat: add continuity ledger system for session state preservation #516
Conversation
- Add continuity-ledger feature for markdown-based state tracking - Add auto-continuity hook for context monitoring and warnings - Add 3 built-in skills: continuity_ledger, create_handoff, resume_handoff - Monitor context usage with progressive warnings (60%, 80%, 85%) - Auto-save handoffs at critical threshold to prevent token overflow - Support ledger recovery after /clear operations Inspired by Continuous Claude v2 concepts for preserving session state.
- Add 16 tests for LedgerManager (create, load, update, prune) - Add 5 tests for auto-continuity handoff structure - Add 7 tests for builtin skills registration - All 28 new tests passing
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement (CLA). To sign the CLA, please comment on this PR with: This is a one-time requirement. Once signed, all your future contributions will be automatically accepted. I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
Greptile SummaryAdds comprehensive continuity ledger system for preserving session state across Key Changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Hook as Auto-Continuity Hook
participant LM as LedgerManager
participant Skills as Builtin Skills
participant FS as File System
Note over User,FS: Session Start
User->>Hook: Start session
Hook->>LM: findLatestLedger()
LM->>FS: Read thoughts/ledgers/CONTINUITY_CLAUDE-*.md
FS-->>LM: Ledger content
LM-->>Hook: Latest ledger loaded
Note over User,FS: Context Window Monitoring
User->>Hook: Execute tool (edit/write/bash)
Hook->>Hook: Check token usage via session.messages()
alt Context < 60%
Hook-->>User: No warning
else Context 60-80%
Hook-->>User: 🟡 Yellow warning (20% chance)
else Context 80-85%
Hook-->>User: 🟠 Red warning via toast
else Context >= 85%
Hook->>LM: Save current ledger state
Hook->>FS: Write thoughts/shared/handoffs/{session}/auto-handoff-*.md
FS-->>Hook: Handoff saved
Hook-->>User: 🔴 CRITICAL - Auto-handoff saved, run /clear
end
Note over User,FS: Manual Ledger Management
User->>Skills: /continuity_ledger
Skills->>LM: createLedger() or updateState()
LM->>FS: Write thoughts/ledgers/CONTINUITY_CLAUDE-{session}.md
FS-->>LM: Ledger saved
LM-->>Skills: Updated ledger
Skills-->>User: Ledger created/updated
Note over User,FS: Manual Handoff
User->>Skills: /create_handoff
Skills->>LM: Load current ledger state
LM-->>Skills: Ledger data
Skills->>FS: Write thoughts/shared/handoffs/{session}/handoff-*.md
FS-->>Skills: Handoff saved
Skills-->>User: Handoff created
Note over User,FS: Recovery After /clear
User->>User: Run /clear
User->>Skills: /resume_handoff
Skills->>FS: Read thoughts/shared/handoffs/*/
FS-->>Skills: Latest handoff
Skills->>LM: Parse and restore state
LM-->>Skills: Restored ledger
Skills-->>User: Context restored with summary
Note over User,FS: Agent Reports
User->>Hook: Agent task completes
Hook->>LM: addAgentReport(ledger, agent, summary)
LM->>LM: Auto-prune to maxAgentReports (10)
LM->>FS: Update ledger file
FS-->>LM: Saved
LM-->>Hook: Updated ledger
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 41 files
Confidence score: 2/5
- Two separate path-traversal vulnerabilities in
src/features/continuity-ledger/manager.tslet untrustedfilenameandsessionNameescape the ledger directory, posing a concrete security risk that should block merging until sanitized. - Additional async calls in
src/tools/skill/tools.tsandsrc/tools/slashcommand/tools.tslack rejection handling, so a failing description builder could crash or leave stale state, further increasing regression risk. - Pay close attention to
src/features/continuity-ledger/manager.ts,src/tools/skill/tools.ts, andsrc/tools/slashcommand/tools.ts– these files need sanitization and defensive error handling before merge.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/auto-continuity/index.ts">
<violation number="1" location="src/hooks/auto-continuity/index.ts:348">
P2: Empty catch block silently swallows all API errors. While graceful degradation is a valid pattern, consider logging the error to aid debugging when the session messages API call fails.</violation>
</file>
<file name="src/hooks/auto-continuity/index.test.ts">
<violation number="1" location="src/hooks/auto-continuity/index.test.ts:116">
P2: These threshold tests are tautological - they test hardcoded arithmetic (`0.6 >= 0.6`) rather than actual implementation code. They provide false confidence in test coverage. Consider importing the actual threshold detection functions and testing those instead.</violation>
</file>
<file name="src/features/continuity-ledger/types.ts">
<violation number="1" location="src/features/continuity-ledger/types.ts:66">
P2: Redundant configuration: `autoSaveThreshold` and `contextThresholds.critical` both represent the auto-save trigger (both default to 0.85, and the comment on `critical` says "Auto-save trigger"). Consider removing `autoSaveThreshold` and using `contextThresholds.critical` directly, or clarify in comments that they serve different purposes.</violation>
</file>
<file name="src/tools/slashcommand/tools.ts">
<violation number="1" location="src/tools/slashcommand/tools.ts:171">
P2: Missing error handling for the async `buildDescription()` call. If the promise rejects, it will cause an unhandled promise rejection and leave `cachedDescription` stuck at the placeholder value. Consider adding a `.catch()` handler to log errors and/or retry.</violation>
</file>
<file name="src/tools/skill/tools.ts">
<violation number="1" location="src/tools/skill/tools.ts:152">
P1: Unhandled promise rejection. The async `getDescription()` call should have error handling. Consider adding `.catch()` or use `void` to indicate intentional fire-and-forget.</violation>
</file>
<file name="src/features/continuity-ledger/manager.test.ts">
<violation number="1" location="src/features/continuity-ledger/manager.test.ts:88">
P2: Flaky test: 10ms `setTimeout` may not guarantee different file modification timestamps across all filesystems. File system timestamp resolution can be as coarse as 1 second on some systems, causing intermittent test failures. Consider mocking the timestamp mechanism, using explicit timestamps if supported by the API, or increasing the delay to at least 1000ms (though mocking is preferred).</violation>
</file>
<file name="src/agents/librarian.ts">
<violation number="1" location="src/agents/librarian.ts:139">
P2: Inconsistent parallel call requirements. The inline instruction now says 5+ calls for TYPE D, but the PARALLEL EXECUTION REQUIREMENTS table at the bottom of the file still says 6+. Either update the table to match or keep both values consistent.</violation>
</file>
<file name="src/features/continuity-ledger/manager.ts">
<violation number="1" location="src/features/continuity-ledger/manager.ts:51">
P1: Path traversal vulnerability: `filename` is used directly in `path.join` without sanitization. If this public method receives untrusted input containing `../`, it could read files outside the ledger directory. Consider using `path.basename(filename)` to extract only the filename component.</violation>
<violation number="2" location="src/features/continuity-ledger/manager.ts:173">
P1: Path traversal vulnerability: `sessionName` is used directly in filename construction without sanitization. If it contains path separators (e.g., `../malicious`), files could be written outside the ledger directory. Consider sanitizing the session name to remove or replace path separators and invalid filename characters.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (notification) { | ||
| output.output += `\n\n${notification}`; | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
P2: Empty catch block silently swallows all API errors. While graceful degradation is a valid pattern, consider logging the error to aid debugging when the session messages API call fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/auto-continuity/index.ts, line 348:
<comment>Empty catch block silently swallows all API errors. While graceful degradation is a valid pattern, consider logging the error to aid debugging when the session messages API call fails.</comment>
<file context>
@@ -0,0 +1,359 @@
+ if (notification) {
+ output.output += `\n\n${notification}`;
+ }
+ } catch {
+ // Graceful degradation
+ }
</file context>
| }); | ||
|
|
||
| describe("context threshold detection", () => { | ||
| test("yellow threshold at 60%", () => { |
There was a problem hiding this comment.
P2: These threshold tests are tautological - they test hardcoded arithmetic (0.6 >= 0.6) rather than actual implementation code. They provide false confidence in test coverage. Consider importing the actual threshold detection functions and testing those instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/auto-continuity/index.test.ts, line 116:
<comment>These threshold tests are tautological - they test hardcoded arithmetic (`0.6 >= 0.6`) rather than actual implementation code. They provide false confidence in test coverage. Consider importing the actual threshold detection functions and testing those instead.</comment>
<file context>
@@ -0,0 +1,134 @@
+ });
+
+ describe("context threshold detection", () => {
+ test("yellow threshold at 60%", () => {
+ const contextPct = 0.6;
+ const isYellow = contextPct >= 0.6 && contextPct < 0.8;
</file context>
| /** Ledger configuration */ | ||
| ledger: LedgerConfig; | ||
| /** Auto-save threshold (percentage of context window, 0-1) */ | ||
| autoSaveThreshold: number; |
There was a problem hiding this comment.
P2: Redundant configuration: autoSaveThreshold and contextThresholds.critical both represent the auto-save trigger (both default to 0.85, and the comment on critical says "Auto-save trigger"). Consider removing autoSaveThreshold and using contextThresholds.critical directly, or clarify in comments that they serve different purposes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/continuity-ledger/types.ts, line 66:
<comment>Redundant configuration: `autoSaveThreshold` and `contextThresholds.critical` both represent the auto-save trigger (both default to 0.85, and the comment on `critical` says "Auto-save trigger"). Consider removing `autoSaveThreshold` and using `contextThresholds.critical` directly, or clarify in comments that they serve different purposes.</comment>
<file context>
@@ -0,0 +1,87 @@
+ /** Ledger configuration */
+ ledger: LedgerConfig;
+ /** Auto-save threshold (percentage of context window, 0-1) */
+ autoSaveThreshold: number;
+ /** Auto-clear after saving (instead of compact) */
+ autoClearAfterSave: boolean;
</file context>
| get description() { | ||
| if (!cachedDescription) { | ||
| cachedDescription = "Loading available commands and skills..." | ||
| buildDescription().then(desc => { cachedDescription = desc }) |
There was a problem hiding this comment.
P2: Missing error handling for the async buildDescription() call. If the promise rejects, it will cause an unhandled promise rejection and leave cachedDescription stuck at the placeholder value. Consider adding a .catch() handler to log errors and/or retry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/slashcommand/tools.ts, line 171:
<comment>Missing error handling for the async `buildDescription()` call. If the promise rejects, it will cause an unhandled promise rejection and leave `cachedDescription` stuck at the placeholder value. Consider adding a `.catch()` handler to log errors and/or retry.</comment>
<file context>
@@ -151,15 +138,40 @@ function formatCommandList(items: CommandInfo[]): string {
+ get description() {
+ if (!cachedDescription) {
+ cachedDescription = "Loading available commands and skills..."
+ buildDescription().then(desc => { cachedDescription = desc })
+ }
+ return cachedDescription
</file context>
| const description = skillInfos.length === 0 | ||
| ? TOOL_DESCRIPTION_NO_SKILLS | ||
| : TOOL_DESCRIPTION_PREFIX + formatSkillsXml(skillInfos) | ||
| getDescription() |
There was a problem hiding this comment.
P1: Unhandled promise rejection. The async getDescription() call should have error handling. Consider adding .catch() or use void to indicate intentional fire-and-forget.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/skill/tools.ts, line 152:
<comment>Unhandled promise rejection. The async `getDescription()` call should have error handling. Consider adding `.catch()` or use `void` to indicate intentional fire-and-forget.</comment>
<file context>
@@ -129,22 +129,38 @@ async function formatMcpCapabilities(
- const description = skillInfos.length === 0
- ? TOOL_DESCRIPTION_NO_SKILLS
- : TOOL_DESCRIPTION_PREFIX + formatSkillsXml(skillInfos)
+ getDescription()
return tool({
</file context>
| test("finds the most recent ledger", async () => { | ||
| // #given | ||
| manager.createLedger("session-old", { goal: "Old session" }); | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); |
There was a problem hiding this comment.
P2: Flaky test: 10ms setTimeout may not guarantee different file modification timestamps across all filesystems. File system timestamp resolution can be as coarse as 1 second on some systems, causing intermittent test failures. Consider mocking the timestamp mechanism, using explicit timestamps if supported by the API, or increasing the delay to at least 1000ms (though mocking is preferred).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/continuity-ledger/manager.test.ts, line 88:
<comment>Flaky test: 10ms `setTimeout` may not guarantee different file modification timestamps across all filesystems. File system timestamp resolution can be as coarse as 1 second on some systems, causing intermittent test failures. Consider mocking the timestamp mechanism, using explicit timestamps if supported by the API, or increasing the delay to at least 1000ms (though mocking is preferred).</comment>
<file context>
@@ -0,0 +1,291 @@
+ test("finds the most recent ledger", async () => {
+ // #given
+ manager.createLedger("session-old", { goal: "Old session" });
+ await new Promise((resolve) => setTimeout(resolve, 10));
+ manager.createLedger("session-new", { goal: "New session" });
+
</file context>
| **Trigger**: Complex questions, ambiguous requests, "deep dive into..." | ||
| **Execute ALL in parallel (6+ calls)**: | ||
| **Execute ALL available tools in parallel (5+ calls)**: |
There was a problem hiding this comment.
P2: Inconsistent parallel call requirements. The inline instruction now says 5+ calls for TYPE D, but the PARALLEL EXECUTION REQUIREMENTS table at the bottom of the file still says 6+. Either update the table to match or keep both values consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/agents/librarian.ts, line 139:
<comment>Inconsistent parallel call requirements. The inline instruction now says 5+ calls for TYPE D, but the PARALLEL EXECUTION REQUIREMENTS table at the bottom of the file still says 6+. Either update the table to match or keep both values consistent.</comment>
<file context>
@@ -136,21 +136,22 @@ gh api repos/owner/repo/pulls/<number>/files
**Trigger**: Complex questions, ambiguous requests, "deep dive into..."
-**Execute ALL in parallel (6+ calls)**:
+**Execute ALL available tools in parallel (5+ calls)**:
\`\`\`
-// Documentation & Web
</file context>
| return this.loadLedger(files[0]); | ||
| } | ||
|
|
||
| loadLedger(filename: string): Ledger | null { |
There was a problem hiding this comment.
P1: Path traversal vulnerability: filename is used directly in path.join without sanitization. If this public method receives untrusted input containing ../, it could read files outside the ledger directory. Consider using path.basename(filename) to extract only the filename component.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/continuity-ledger/manager.ts, line 51:
<comment>Path traversal vulnerability: `filename` is used directly in `path.join` without sanitization. If this public method receives untrusted input containing `../`, it could read files outside the ledger directory. Consider using `path.basename(filename)` to extract only the filename component.</comment>
<file context>
@@ -0,0 +1,359 @@
+ return this.loadLedger(files[0]);
+ }
+
+ loadLedger(filename: string): Ledger | null {
+ const filePath = path.join(this.ledgerDir, filename);
+ if (!fs.existsSync(filePath)) {
</file context>
- Add session name sanitization for invalid filesystem characters - Fix race condition in findLatestLedger when files are deleted - Fix state parsing to handle placeholder values correctly - Fix shallow config merging for nested contextThresholds - Include reasoning tokens in context usage calculation - Add filename sanitization for handoff files - Remove unused color variable in generateStatusLine - Fix filter condition from !== undefined to !== '' Edge case tests added: - Session names with invalid chars (<>:"/\|?*) - Very long session names (truncation) - Empty session names - Placeholder values in state parsing - Empty/malformed markdown files - Unicode content (日本語, émojis) - Concurrent rapid updates - Files with special characters All 661 tests passing (2 pre-existing MCP failures unrelated)
- Auto-create ledger on first session if none exists - Auto-inject ledger context on first user message - Auto-sync ledger state with todo list updates - Ledger now tracks: done (last 10), now (in_progress), next (first 5) No manual intervention needed - continuity just works.
- Fix disabled hook to return both handlers - Add injection guard to prevent duplicate context injection - Add session pruning to prevent memory leak (keep last 100) - Improve todo parsing with type validation - Clean UNCONFIRMED prefix duplication in context generation - Add 8 new edge case tests for hook behavior Tests: 669/671 passing (43 continuity tests)
- Add atomic writes to saveLedger() using temp file + rename - Add atomic writes to saveAutoHandoff() with same pattern - Add stale ledger recovery when todo sync fails - Replace random 20% yellow warning with deterministic once-per-session
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/auto-continuity/index.ts">
<violation number="1" location="src/hooks/auto-continuity/index.ts:280">
P2: The `yellowWarningShown` flag is not reset when a new session is created. The comment says "show once per session" but since this flag isn't reset in the `session.created` handler (where `autoSaveTriggered` is reset), users will only ever see the yellow warning once across all sessions, not once per session.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // Yellow threshold - show once per session when first crossing | ||
| if (contextPct >= yellow && contextPct < red && !state.yellowWarningShown) { | ||
| state.yellowWarningShown = true; |
There was a problem hiding this comment.
P2: The yellowWarningShown flag is not reset when a new session is created. The comment says "show once per session" but since this flag isn't reset in the session.created handler (where autoSaveTriggered is reset), users will only ever see the yellow warning once across all sessions, not once per session.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/auto-continuity/index.ts, line 280:
<comment>The `yellowWarningShown` flag is not reset when a new session is created. The comment says "show once per session" but since this flag isn't reset in the `session.created` handler (where `autoSaveTriggered` is reset), users will only ever see the yellow warning once across all sessions, not once per session.</comment>
<file context>
@@ -261,11 +275,10 @@ export function createAutoContinuityHook(
- }
+ // Yellow threshold - show once per session when first crossing
+ if (contextPct >= yellow && contextPct < red && !state.yellowWarningShown) {
+ state.yellowWarningShown = true;
+ return `🟡 Context at ${(contextPct * 100).toFixed(1)}%. Plan a good stopping point for handoff.`;
}
</file context>
1 participant
Summary
Adds a fully automatic continuity system that preserves session state across
/clearoperations with zero manual intervention.What's Automatic
Features
Continuity Ledger (
thoughts/ledgers/CONTINUITY_CLAUDE-{session}.md)Auto-Continuity Hook
Handoff System (
thoughts/shared/handoffs/{session}/)Built-in Skills (optional manual use)
/continuity_ledger- Create/update ledger/create_handoff- Manual handoff creation/resume_handoff- Load handoff after/clearEdge Cases Handled
Tests
Commits
feat: add continuity ledger system- Core implementationtest: add comprehensive tests- 28 testsfix: harden with edge case handling- Security & robustnessfeat: make continuity fully automatic- Zero-config automation