fix(typecheck): type session storage test fixtures - #1526
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{test,spec}.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (9)
📝 WalkthroughWalkthroughTest helpers converted to use UUID-typed identifiers; user messages accept either plain strings or ChangessessionStorage test refactoring
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs:
Suggested reviewers:
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
…on-storage-test # Conflicts: # src/utils/sessionStorage.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/sessionStorage.test.ts`:
- Around line 96-110: The snipBoundary helper currently types its parameters as
string but must use the UUID type to match base() and other factories; change
the signature of snipBoundary to accept uuid: UUID, parentUuid: UUID | null, and
removedUuids: UUID[] (and update any local references if needed) so calls like
snipBoundary(id(45), ...) type-check with base() and the rest of the factories
(reference functions: snipBoundary, base, id).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2217e0e2-26c1-49e6-8219-861821132431
📒 Files selected for processing (1)
src/utils/sessionStorage.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: smoke-and-tests
🧰 Additional context used
📓 Path-based instructions (3)
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/sessionStorage.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/sessionStorage.test.ts
**
⚙️ CodeRabbit configuration file
**: # Contributing to OpenClaudeThanks for contributing.
OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.
Before You Start
- Search existing issues and discussions before opening a new thread.
- Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
- Use issues for confirmed bugs and actionable feature work.
- Use discussions for setup help, ideas, and general community conversation.
- For larger changes, open an issue first so the scope is clear before implementation.
- For security reports, follow SECURITY.md.
Pull Requests
Every PR needs a reason. Your PR description must include:
- what changed and why
- the user or developer impact
- the exact checks you ran
- a linked issue when one exists, using
Fixes#123, `Closes `#123, or another clear link- screenshots when the PR touches UI, terminal presentation, or the VS Code extension
- which provider path was tested when the PR changes provider behavior
The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.
Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.
What Gets Closed Without Review
PRs may be closed without review...
Files:
src/utils/sessionStorage.test.ts
🔇 Additional comments (7)
src/utils/sessionStorage.test.ts (7)
3-3: LGTM!
18-20: LGTM!
22-94: LGTM!
120-135: LGTM!
141-181: LGTM!
283-305: LGTM!
307-330: LGTM!
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/sessionStorage.test.ts`:
- Around line 96-110: The snipBoundary helper currently types its parameters as
string but must use the UUID type to match base() and other factories; change
the signature of snipBoundary to accept uuid: UUID, parentUuid: UUID | null, and
removedUuids: UUID[] (and update any local references if needed) so calls like
snipBoundary(id(45), ...) type-check with base() and the rest of the factories
(reference functions: snipBoundary, base, id).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2217e0e2-26c1-49e6-8219-861821132431
📒 Files selected for processing (1)
src/utils/sessionStorage.test.ts
📜 Review details
🔇 Additional comments (7)
src/utils/sessionStorage.test.ts (7)
3-3: LGTM!
18-20: LGTM!
22-94: LGTM!
120-135: LGTM!
141-181: LGTM!
283-305: LGTM!
307-330: LGTM!
🛑 Comments failed to post (1)
src/utils/sessionStorage.test.ts (1)
96-110:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winType parameters as UUID, not string.
The
snipBoundaryhelper acceptsstringforuuidandparentUuid, butbase()(line 22) expectsUUID. This will fail type checking. All other factory helpers (user,assistant,compactBoundary) useUUIDconsistently.The test at line 151 calls
snipBoundary(id(45), ...)whereid()returnsUUID, so the signature should match.🔧 Proposed fix
function snipBoundary( - uuid: string, - parentUuid: string | null, - removedUuids: string[], + uuid: UUID, + parentUuid: UUID | null, + removedUuids: UUID[], ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function snipBoundary( uuid: UUID, parentUuid: UUID | null, removedUuids: UUID[], ) { return { ...base(uuid, parentUuid), type: 'system', subtype: 'snip_boundary', level: 'info', isMeta: false, content: 'Conversation history snipped', snipMetadata: { removedUuids }, } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/sessionStorage.test.ts` around lines 96 - 110, The snipBoundary helper currently types its parameters as string but must use the UUID type to match base() and other factories; change the signature of snipBoundary to accept uuid: UUID, parentUuid: UUID | null, and removedUuids: UUID[] (and update any local references if needed) so calls like snipBoundary(id(45), ...) type-check with base() and the rest of the factories (reference functions: snipBoundary, base, id).
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM
…on-storage-test # Conflicts: # src/utils/sessionStorage.test.ts
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM
* fix(typecheck): type session storage test fixtures * fix(typecheck): align snip boundary fixture ids
3way apply failed on all 3 (fork has heavily diverged from upstream context). Per skill's 'direct overwrite via git show' escape hatch, took upstream verbatim + re-evaluated byte-level. src/grpc/server.ts (upstream Twigpine#1572 gRPC stream messages): - 78 lines added (proper typing for PermissionDenyDecision, msg.event, block structures) - 15 lines removed (fork's looser typing escape hatches) - @ts-nocheck dropped src/tasks/RemoteAgentTask/RemoteAgentTask.tsx (upstream Twigpine#1573): - 33 lines added (typed message block structure, narrowed content types) - 10 lines removed (fork's any[] escape hatches) - @ts-nocheck dropped src/utils/sessionStorage.test.ts (upstream Twigpine#1526): - 322 lines added (proper UUID template literal types, typed test fixtures: SessionMessage[] instead of string) - 42 lines removed (fork's nocheck helper functions + persisted-output test cases; can be re-added in a follow-up session if needed) - @ts-nocheck dropped VERIFICATION: typecheck: 47 errors → 0 from these 3 files tests: pre-existing acorn-module-missing failure (NOT from these changes — confirmed via git stash + bun test on clean HEAD which also fails with the same error) REGRESSION RISK: sessionStorage.test.ts lost 42 lines of fork test coverage (helper functions id/base/user/assistant + 1 persisted-output test case). All TypeScript-clean, but runtime test count drops. Re-add in dedicated session if needed.
3 participants
Part of #1486.
Summary
crypto.UUIDso preserved-segment transcript helpers match the production session map contract.tool_resultcontent arrays directly instead of mutating the message shape after construction.Validation
bun test --max-concurrency=1 src/commands.test.ts src/utils/sessionStorage.test.ts(12 pass)env -u OPENAI_API_KEY -u OPENAI_BASE_URL -u OPENAI_MODEL -u CLAUDE_CODE_USE_OPENAI -u CLAUDE_CODE_USE_GEMINI -u CLAUDE_CODE_USE_MISTRAL -u CLAUDE_CODE_USE_GITHUB -u CLAUDE_CODE_USE_BEDROCK -u CLAUDE_CODE_USE_VERTEX -u CLAUDE_CODE_USE_FOUNDRY -u GEMINI_API_KEY -u GOOGLE_API_KEY -u GITHUB_TOKEN -u GH_TOKEN -u ANTHROPIC_MODEL -u ANTHROPIC_SMALL_FAST_MODEL bun run check(3433 pass, 0 fail)bun run test:provider(650 pass)env -u OPENAI_API_KEY -u OPENAI_BASE_URL -u OPENAI_MODEL -u CLAUDE_CODE_USE_OPENAI -u CLAUDE_CODE_USE_GEMINI -u CLAUDE_CODE_USE_MISTRAL -u CLAUDE_CODE_USE_GITHUB -u CLAUDE_CODE_USE_BEDROCK -u CLAUDE_CODE_USE_VERTEX -u CLAUDE_CODE_USE_FOUNDRY -u GEMINI_API_KEY -u GOOGLE_API_KEY -u GITHUB_TOKEN -u GH_TOKEN -u ANTHROPIC_MODEL -u ANTHROPIC_SMALL_FAST_MODEL npm run test:provider-recommendation(79 pass)bun install --cwd web --frozen-lockfile && bun run web:typecheckbun run typecheckstill exits on the repository baseline; no diagnostics remain forsrc/utils/sessionStorage.test.ts(1695 baseline error lines)git diff --checkSummary by CodeRabbit
Note: This release contains internal test improvements only; there are no user-facing changes.