fix(prompt): keep homepage drafts global across workspaces - #1216
Conversation
|
Warning Review limit reached
More reviews will be available in 2 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR removes the portable-draft homepage-consumption API and consolidates draft ownership to pinned + route scopes. It introduces cross-workspace draft persistence via a global homepage storage key, canonicalizes file paths with separate ChangesHomepage Draft Refactoring: Portable Removal and Path Canonicalization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/components/prompt-input/draft-carryover.test.ts, packages/app/src/components/prompt-input/draft-carryover.ts, packages/app/src/components/prompt-input/draft-isolation.integration.test.ts, packages/app/src/components/prompt-input/editor-input.ts, packages/app/src/components/prompt-input/history-comment-map.ts, packages/app/src/components/prompt-input/history-navigation.test.ts, packages/app/src/components/prompt-input/history-navigation.ts, packages/app/src/components/prompt-input/homepage-migration-storage.test.ts, packages/app/src/components/prompt-input/homepage-migration-storage.ts, packages/app/src/components/prompt-input/homepage-migration.test.ts, packages/app/src/components/prompt-input/homepage-migration.ts, packages/app/src/components/prompt-input/owner-mirror.test.ts, packages/app/src/components/prompt-input/owner-mirror.ts, packages/app/src/components/prompt-input/pinned-draft.test.ts, packages/app/src/components/prompt-input/pinned-draft.ts, packages/app/src/components/prompt-input/portable-draft.test.ts, packages/app/src/components/prompt-input/portable-draft.ts, packages/app/src/components/prompt-input/prompt-draft-lifecycle.ts, packages/app/src/components/prompt-input/submit-ownership.test.ts, packages/app/src/components/prompt-input/submit-ownership.ts, packages/app/src/components/prompt-input/submit.test.ts, packages/app/src/components/prompt-input/submit.ts, packages/app/src/context/prompt.test.ts, packages/app/src/context/prompt.tsx, packages/app/src/pages/layout/layout-homepage-migration.ts, packages/app/src/pages/session/file-tabs.tsx, packages/app/src/pages/session/use-session-commands.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request transitions the homepage draft model from a per-workspace "portable draft" model to a single global homepage draft model shared across all workspaces. The legacy PortableDraftOwner has been repurposed into a one-shot migration owner that copies legacy per-workspace drafts into the new global homepage prompt store upon first boot. The homepage owner mirror now only tracks pinned drafts, and the portable draft concept has been removed from the submit ownership, history navigation, and draft lifecycle systems. Additionally, file paths in context items and comments are now canonicalized to absolute paths to ensure they remain correctly anchored. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/app/src/components/prompt-input/homepage-migration.ts (1)
89-92:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe global completion sentinel strands legacy drafts in unopened workspaces.
This marks migration
completeafter looking at onlycurrentDirectory, and every later boot exits immediately at Line 90. Any route-scoped homepage draft that lived under another workspace will never be projected into the new global store and becomes effectively unreachable after upgrade. Consider tracking completion per directory, or postponing the globalcompletestate until each visited workspace has been checked.Also applies to: 121-127
🤖 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 `@packages/app/src/components/prompt-input/homepage-migration.ts` around lines 89 - 92, The migration currently marks the global sentinel complete after only checking the current directory (readSentinel / existing?.status === "complete"), which leaves drafts in other workspaces un-migrated; change the sentinel strategy to be directory-scoped or to defer marking global "complete" until all visited workspaces are checked — e.g., modify readSentinel/writeSentinel (and any code using them in homepage-migration.ts, including the logic around lines referenced and the other block at 121-127) to accept a directory key or to store a map of completed directories, and update the completion check to verify the currentDirectory entry (or iterate visited directories) before returning/setting complete.packages/app/src/components/prompt-input/portable-draft.ts (1)
87-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the revision counter monotonic across clear → retype cycles.
Because the revision only lives on
snapshot, an empty record/reset makes the next non-empty snapshot start back at1. If a submit captured revision1, the user clears the draft, types again, and the await resolves,clear(1)will now delete the newer draft because the revision was reused. The stale-clear guard needs a counter that survives clears.Suggested direction
export function createPortableDraftOwner(): PortableDraftOwner { const [snapshot, setSnapshot] = createSignal<PortableDraftSnapshot | null>(null) + let lastRevision = 0 ... function record(input: { sourceFilesystemDirectory: string } & PortableDraftPayload): void { if (isPayloadEmpty(input)) { + if (snapshot() !== null) lastRevision += 1 setSnapshot(null) return } ... - const nextRevision = (current?.revision ?? 0) + 1 + const nextRevision = lastRevision + 1 ... + lastRevision = nextRevision setSnapshot({ prompt: canonicalPrompt, context: canonicalContext, images: input.images, resolvedMentions: input.resolvedMentions, sourceFilesystemDirectory: input.sourceFilesystemDirectory, revision: nextRevision, }) } ... function clear(expectedRevision?: number): boolean { const current = snapshot() ... + lastRevision += 1 setSnapshot(null) return true }Also applies to: 103-105, 140-155
🤖 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 `@packages/app/src/components/prompt-input/portable-draft.ts` around lines 87 - 89, The snapshot revision must be monotonic across clears: create a persistent counter (e.g., revisionRef or nextRevision) that lives outside the transient snapshot state and is incremented whenever you produce a new non-empty snapshot (use it when calling setSnapshot) so clears/resetting snapshot to null does not reset the counter; update places that create snapshots (the branch using isPayloadEmpty and the other snapshot setters around the submit/await logic) to read and increment this persistent counter and include that revision on the snapshot object, and update the clear/stale-check code (the clear function and any resolution handlers that compare revisions) to compare against the persistent revision-based id on the snapshot rather than assuming snapshot nulling resets revision.packages/app/src/pages/session/use-session-comment-context.ts (1)
77-91:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the original absolute prompt-context path for comment updates/removals.
Lines 83-91 recompute the prompt-context key from the current
sourceFilesystemDirectory(). After a comment is added in workspace A and the global homepage draft is then viewed from workspace B,comment.fileis still relative (src/a.ts) but the stored prompt-context entry is keyed by the original absolute path (/repo-A/src/a.ts), soupdateCommentandremoveCommentwill miss the existing item and leave stale comment context behind. Pass the original absolute prompt-context path through these mutation calls instead of deriving it from ambient route state.🤖 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 `@packages/app/src/pages/session/use-session-comment-context.ts` around lines 77 - 91, The update/remove handlers recompute the prompt-context key using toAbsoluteFilePath(input.sourceFilesystemDirectory(), comment.file) which can differ from the original absolute key; change these handlers in use-session-comment-context.ts (functions update and remove) to accept and pass through the original absolute prompt-context path instead of deriving it from input.sourceFilesystemDirectory() — i.e., extend the comment payload to include the stored absolute path (e.g., originalPromptPath or promptContextPath) and call input.promptContext.updateComment(originalPromptPath, comment.id, ...) and input.promptContext.removeComment(originalPromptPath, comment.id) (keeping input.comments.update/remove calls the same) so input.promptContext.updateComment and input.promptContext.removeComment target the exact stored key rather than a recomputed one.
🤖 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 `@packages/app/src/components/prompt-input/prompt-draft-lifecycle.ts`:
- Around line 58-60: submittedDraftStillCurrent currently demands exact context
equality, but removeSubmittedCommentItems runs before clearInput so
comment-backed items have been stripped and the equality fails; update
submittedDraftStillCurrent to compare a normalized form of the prompt context
and submittedDraft by removing/comment-item-normalizing both sides (or by
running the same removeSubmittedCommentItems transformation on
prompt.context.items(scope) before JSON.stringify) so the guard matches
comment-backed submits; apply the same normalized-comparison change where the
guard is used before calling prompt.reset to ensure clearInput/prompt.reset
executes for submitted comment items.
---
Outside diff comments:
In `@packages/app/src/components/prompt-input/homepage-migration.ts`:
- Around line 89-92: The migration currently marks the global sentinel complete
after only checking the current directory (readSentinel / existing?.status ===
"complete"), which leaves drafts in other workspaces un-migrated; change the
sentinel strategy to be directory-scoped or to defer marking global "complete"
until all visited workspaces are checked — e.g., modify
readSentinel/writeSentinel (and any code using them in homepage-migration.ts,
including the logic around lines referenced and the other block at 121-127) to
accept a directory key or to store a map of completed directories, and update
the completion check to verify the currentDirectory entry (or iterate visited
directories) before returning/setting complete.
In `@packages/app/src/components/prompt-input/portable-draft.ts`:
- Around line 87-89: The snapshot revision must be monotonic across clears:
create a persistent counter (e.g., revisionRef or nextRevision) that lives
outside the transient snapshot state and is incremented whenever you produce a
new non-empty snapshot (use it when calling setSnapshot) so clears/resetting
snapshot to null does not reset the counter; update places that create snapshots
(the branch using isPayloadEmpty and the other snapshot setters around the
submit/await logic) to read and increment this persistent counter and include
that revision on the snapshot object, and update the clear/stale-check code (the
clear function and any resolution handlers that compare revisions) to compare
against the persistent revision-based id on the snapshot rather than assuming
snapshot nulling resets revision.
In `@packages/app/src/pages/session/use-session-comment-context.ts`:
- Around line 77-91: The update/remove handlers recompute the prompt-context key
using toAbsoluteFilePath(input.sourceFilesystemDirectory(), comment.file) which
can differ from the original absolute key; change these handlers in
use-session-comment-context.ts (functions update and remove) to accept and pass
through the original absolute prompt-context path instead of deriving it from
input.sourceFilesystemDirectory() — i.e., extend the comment payload to include
the stored absolute path (e.g., originalPromptPath or promptContextPath) and
call input.promptContext.updateComment(originalPromptPath, comment.id, ...) and
input.promptContext.removeComment(originalPromptPath, comment.id) (keeping
input.comments.update/remove calls the same) so
input.promptContext.updateComment and input.promptContext.removeComment target
the exact stored key rather than a recomputed one.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 94c4e783-081c-43d4-be30-f212f063f289
📒 Files selected for processing (33)
packages/app/e2e/app/composer-workspace-chip.spec.tspackages/app/src/components/prompt-input.tsxpackages/app/src/components/prompt-input/comment-routing.tspackages/app/src/components/prompt-input/draft-carryover.test.tspackages/app/src/components/prompt-input/draft-carryover.tspackages/app/src/components/prompt-input/draft-isolation.integration.test.tspackages/app/src/components/prompt-input/editor-input.tspackages/app/src/components/prompt-input/history-comment-map.tspackages/app/src/components/prompt-input/history-navigation.test.tspackages/app/src/components/prompt-input/history-navigation.tspackages/app/src/components/prompt-input/history.tspackages/app/src/components/prompt-input/homepage-migration-storage.test.tspackages/app/src/components/prompt-input/homepage-migration-storage.tspackages/app/src/components/prompt-input/homepage-migration.test.tspackages/app/src/components/prompt-input/homepage-migration.tspackages/app/src/components/prompt-input/owner-mirror.test.tspackages/app/src/components/prompt-input/owner-mirror.tspackages/app/src/components/prompt-input/pinned-draft.test.tspackages/app/src/components/prompt-input/pinned-draft.tspackages/app/src/components/prompt-input/portable-draft.test.tspackages/app/src/components/prompt-input/portable-draft.tspackages/app/src/components/prompt-input/prompt-draft-lifecycle.tspackages/app/src/components/prompt-input/submit-ownership.test.tspackages/app/src/components/prompt-input/submit-ownership.tspackages/app/src/components/prompt-input/submit.test.tspackages/app/src/components/prompt-input/submit.tspackages/app/src/context/prompt.test.tspackages/app/src/context/prompt.tsxpackages/app/src/pages/layout/layout-homepage-migration.tspackages/app/src/pages/session/file-tabs.tsxpackages/app/src/pages/session/use-session-commands.tsxpackages/app/src/pages/session/use-session-comment-context.test.tspackages/app/src/pages/session/use-session-comment-context.ts
💤 Files with no reviewable changes (3)
- packages/app/src/components/prompt-input/draft-carryover.test.ts
- packages/app/src/components/prompt-input/homepage-migration-storage.test.ts
- packages/app/src/components/prompt-input/draft-carryover.ts
Summary
Keep the homepage composer draft global while workspace switching only changes the route-backed send target. There is no linked issue; this is a user-reported regression from workspace switch recordings where the prompt text could disappear or bleed through during folder changes.
Why
The previous fix still treated homepage drafts as per-workspace state plus a runtime carryover owner. That made route changes capable of clearing, moving, or briefly exposing the wrong draft. The safer model is simpler: the homepage composer has one stable draft store, and submit captures the current route directory as the target.
Related Issue
None. User-reported regression from local workspace switch recordings.
Human Review Status
Pending
Review Focus
Please focus on the ownership boundary: ordinary homepage drafts should be route-owned/global prompt-store state, pinned deep-link prefills should remain directory-bound, and session routes should remain isolated. Also check the file/context path anchoring when context selected in one workspace is submitted after switching to another.
Risk Notes
Migration now copies the current legacy route-scoped homepage draft into the global flow without deleting the old legacy store, reducing data-loss risk at the cost of leaving an inert old key behind. Screenshots/recordings are not attached because there is no visual styling or copy change; the visible workspace-chip composer path was verified by Playwright E2E. Deleted obsolete draft carryover code and a remove-only migration storage test that no longer applies after migration stopped deleting legacy draft storage.
How To Verify
Screenshots or Recordings
Not attached. This is a behavior fix with no visual styling or copy change; the visible composer workspace switch route was checked through Playwright E2E.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements