Skip to content

[codex] Structure preview session key errors#3388

Open
juliusmarminge wants to merge 1 commit into
mainfrom
codex/web-preview-session-key-errors
Open

[codex] Structure preview session key errors#3388
juliusmarminge wants to merge 1 commit into
mainfrom
codex/web-preview-session-key-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

The preview-session atom rejected malformed scoped thread keys with a generic Error. Although the message included the raw key, the failure had no stable tag or structured field, so telemetry and error inspection could not distinguish this invariant from unrelated exceptions without parsing text.

This change replaces that generic throw with a Schema.TaggedErrorClass carrying the malformed threadKey. The message remains the same and is derived from that field. The validation and construction stay inline at the existing failure boundary, avoiding a constructor-only wrapper or an extra helper module.

No behavior test was added for this pure internal-invariant refactor.

Validation

  • pnpm vp check (passes with existing repository warnings)
  • pnpm vp run typecheck

Note

Low Risk
Internal error-shape refactor only; same message and failure timing with no auth, data, or API behavior changes.

Overview
Malformed scoped thread keys in previewSessionSyncAtom now throw PreviewSessionThreadKeyParseError (Effect Schema.TaggedErrorClass) with a structured threadKey field instead of a plain Error.

The user-facing message is unchanged; the null check is tightened to threadRef === null. Validation still happens at the same inline boundary in usePreviewSession.ts.

Reviewed by Cursor Bugbot for commit 0455679. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Replace generic Error with typed PreviewSessionThreadKeyParseError for invalid preview thread keys

Introduces PreviewSessionThreadKeyParseError, a tagged error class (via effect/Schema) with a structured shape and message, to replace the generic Error thrown when an invalid scoped preview thread key is encountered in previewSessionSyncAtom. Also tightens the invalid key check from a falsy guard to an explicit === null check.

Macroscope summarized 0455679.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f367a33a-75d9-454e-8f3f-e1384cda2aab

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/web-preview-session-key-errors

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Straightforward error handling refactoring that replaces a plain Error with a structured TaggedErrorClass. The runtime behavior and error message remain unchanged; only the error type is now tagged for better Effect ecosystem integration.

You can customize Macroscope's approvability policy. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant