Skip to content

refactor(runtime): remove legacy timestamp session read marker - #3080

Merged
Astro-Han merged 1 commit into
apache:mainfrom
CxHsin:refactor/remove-legacy-session-read-marker
Aug 15, 2026
Merged

Astro-Han merged 1 commit into
apache:mainfrom
CxHsin:refactor/remove-legacy-session-read-marker

Conversation

@CxHsin

@CxHsin CxHsin commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Remove the unused timestamp-based Session read-marker path from Runtime and storage. Runtime Host message identity remains the sole authority for mutating lastReadMessageId and hasUnread.

Fixes #3072

Verification

  • npm --workspace @maka/runtime run typecheck
  • npm --workspace @maka/storage run typecheck
  • npm run format:check
  • node --test dist/__tests__/session-store.test.js in packages/storage (16/16 passed)
  • Confirmed tracked source references only retain markSessionReadThroughMessage()

Full Runtime and Storage suites were attempted on Windows. They remain blocked by unrelated existing Windows symlink, fsutil, PowerShell, PTY, and path-containment failures. Runtime Host build is independently blocked by the existing implicit-any error at src/server/session-transcript-pager.ts:249.

Breaking change

Embedded legacy Runtime compositions can no longer mark a Session read using only a timestamp. Callers must use the supported Runtime Host message-ID operation.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex assessed issue #3072, removed the legacy Runtime/storage timestamp path and related test-double code, and assisted with verification and review. The commit includes the required Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Remove the unused timestamp-based Session read path so Runtime Host message identity remains the sole read-state mutation authority.

Generated-by: Codex
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ba0bd83f-9cae-42ba-ba1f-61c5a25915fa

📥 Commits

Reviewing files that changed from the base of the PR and between 62cded2 and 5fc5411.

📒 Files selected for processing (7)
  • packages/runtime/src/__tests__/runtime-event-read-model.test.ts
  • packages/runtime/src/__tests__/runtime-kernel-interaction.test.ts
  • packages/runtime/src/__tests__/session-manager-terminal-ledger.test.ts
  • packages/runtime/src/__tests__/session-manager.test.ts
  • packages/runtime/src/session-manager.ts
  • packages/storage/src/execution-stores.ts
  • packages/storage/src/session-store.ts
💤 Files with no reviewable changes (7)
  • packages/runtime/src/tests/session-manager-terminal-ledger.test.ts
  • packages/storage/src/execution-stores.ts
  • packages/storage/src/session-store.ts
  • packages/runtime/src/tests/runtime-kernel-interaction.test.ts
  • packages/runtime/src/session-manager.ts
  • packages/runtime/src/tests/runtime-event-read-model.test.ts
  • packages/runtime/src/tests/session-manager.test.ts

📝 Walkthrough

Summary

This PR removes the unused timestamp-based Session read-marker path from Runtime and storage. SessionManager.markSessionRead() and markSessionReadThrough() are deleted from the manager, store contracts, facade, implementation, and test fixtures.

markSessionReadThroughMessage(sessionId, messageId) remains the sole operation that mutates lastReadMessageId and hasUnread. The PR does not add a compatibility adapter or create a parallel path. Existing unread-state semantics, visible-message selection, and Session fields remain unchanged.

The change is the smallest coherent solution. It removes the obsolete API, its persistence implementation, and test-only hooks. The deleted fixtures and interleaving hook can remain deleted because they only supported the removed timestamp path and do not weaken coverage of message-ID read-state behavior.

Validation includes typecheck, formatting, and focused storage tests. Full Runtime and Storage suites remain unverified because unrelated Windows and Runtime Host issues block them.

Review-relevant risks

The diff changes internal and exported Runtime and storage contracts by removing timestamp-based read-marker methods. Callers that still use these methods require migration to the Runtime Host message-ID operation. Any material public-contract effect requires independent human review under repository policy.

No other protected-area effect was identified in the current diff. The person performing the merge must review the final diff, and a maintainer makes the final determination.

Walkthrough

The legacy timestamp-based session read-marker path was removed from runtime contracts, session management, storage wiring, and test fixtures. Message-ID-based read-marker handling remains available.

Changes

Timestamp read-marker removal

Layer / File(s) Summary
Remove runtime timestamp API
packages/runtime/src/session-manager.ts
SessionStore.markSessionReadThrough and SessionManager.markSessionRead were removed.
Retain message-ID storage wiring
packages/storage/src/session-store.ts, packages/storage/src/execution-stores.ts
Storage no longer exposes the timestamp-based operation. The execution-store facade uses markSessionReadThroughMessage(sessionId, messageId).
Remove obsolete test fixtures and hooks
packages/runtime/src/__tests__/runtime-event-read-model.test.ts, packages/runtime/src/__tests__/runtime-kernel-interaction.test.ts, packages/runtime/src/__tests__/session-manager-terminal-ledger.test.ts, packages/runtime/src/__tests__/session-manager.test.ts
Test stores no longer implement the removed method. The session-read interleave hook and helper were also removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 5fc54

This localized refactor removes an unused timestamp-based read-marker path while retaining message-ID authority; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies removal of the legacy timestamp-based session read marker, which is the main change.
Description check ✅ Passed The description includes the summary, linked issue, verification results, breaking change, AI disclosure, and behavior change.
Linked Issues check ✅ Passed The changes remove the timestamp-based Runtime and storage paths while retaining the message-ID read-state authority.
Out of Scope Changes check ✅ Passed All modified production code and test fixtures directly support removal of the legacy timestamp-based read-marker path.
Ai Use Disclosure ✅ Passed The description selects only substantive generative tooling, names Codex and its scope, and the sole introduced commit has the matching standalone Generated-by: Codex trailer.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head 5fc541124aa138ea5abe73df10f65ca706e31f2f. No P0–P3 findings.

The removal is complete and leaves Runtime Host message identity as the sole durable Session read-marker authority. I verified that no timestamp callers or compatibility paths remain, the deleted test hooks only served the removed path, and the message-ID production chain and its focused coverage remain intact. All required checks are green.

Disclosure: I used Codex to assist with source tracing and independent review passes. I reviewed the evidence and final diff and own this approval.

@Astro-Han
Astro-Han merged commit 90b78a3 into apache:main Aug 15, 2026
12 checks 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.

refactor(runtime): remove the legacy timestamp-based Session read marker

2 participants