feat(editor): gist drafts, new gists, streaming edits — and per-file CI attribution - #327
Open
soroushm wants to merge 19 commits into
Open
feat(editor): gist drafts, new gists, streaming edits — and per-file CI attribution#327soroushm wants to merge 19 commits into
soroushm wants to merge 19 commits into
Conversation
…files - close #322 The gist list pages ten at a time and fetches every page behind it, so an account with more than a screenful is no longer silently truncated — GitHub has no gist-search endpoint, which is why this pages rather than searches. A gist can now be created rather than only edited. The rail's new button opens a sandbox with an id of its own, so pressing it never reopens the last one, and Publish becomes POST /gists with a public toggle that is secret by default. The app opens on such a sandbox, so a file or a description can be written without picking anything first. Every sandbox holding staged work is listed under a Drafts panel, named by its staged description, else the gist's published description, else the files it touches. An untouched one is never persisted, so opening the app and typing nothing leaves nothing behind. Files can be renamed from the panel: the old name is staged as deleted and the new one as added, which is what a rename is in a gist PATCH — a file that only exists locally simply moves. A document open on that file follows the new name, so a later save does not resurrect the old one. Adding a file is asked for rather than always on offer: a full-width button opens the name field, matching how a rename and the description already work.
… the name it has - close #322 The rail opened on a sandbox but the document did not, so what was written on startup belonged to nothing: Save wrote it to disk and it could never be published. Opening a sandbox now points the document at a file in it, named en.md and renameable from the panel, so saving stages it and Create gist has something to send. Staging into a gist that does not exist yet records as added rather than modified. Nothing is published there, so calling the first save a modification claimed GitHub had a version of the file that it does not, and the row read M. The save dialog opens on the name the document already goes by — its path on disk, or the gist file it came from — rather than making it be typed again. Save As on a disk file used to lose both the folder and the name. Creating a gist now leaves a fresh sandbox behind it. The draft was already dropped once GitHub had it, but the panel and the document stayed pointed at a sandbox that no longer existed, so nothing could be written until a gist was picked by hand.
…close #319, close #323 The bridge was one-shot and blocking: the panel showed a progress bar for up to two minutes with no way to stop it, and the answer appeared all at once when the process closed. It now spawns with --output-format stream-json and reports each run through a small typed event union - RUN_STARTED, TEXT_MESSAGE_CONTENT, RUN_FINISHED, RUN_ERROR - named after the AG-UI protocol so the vocabulary is a known quantity, without taking the dependency. The text lands in the document as it is written, rather than in a box beside it, and Cancel replaces Ask Claude while a run is going. A stopped or failed run puts back what was there: a half-written answer is not something to leave behind. RUN_FINISHED carries the whole answer rather than leaving the renderer to reassemble the deltas, so a dropped one cannot quietly corrupt the document. Runs are held by id, and their events go only to the window that started them - no other window can watch or cancel one. A gist can now be dragged out of the Gists panel onto the instruction field, and it is sent as a CONTEXT block ahead of the instruction: earlier work to build on, which the prompt says is not to be repeated or rewritten. That makes "we covered rehydration, now write the part about data fetching" a thing that can be asked for. The drag carries a private type, so dropped text or files are ignored, and the offer to drop shows in the field's own placeholder - nothing takes room for a reference that is usually absent. A gist over 60,000 characters is trimmed, and the panel says so rather than doing it silently. The panel also names the file it is about to rewrite and previews it, in place of saying that nothing is selected. The e2e suite now puts a fake claude on PATH for every launch: no test can reach the real CLI, which would need the developer signed in and would spend their tokens. Two of its tests were also failing since the startup sandbox landed - Save on a gist file stages rather than writing to disk, and the close prompt's buttons had swapped meaning. Dev runs with --watch, so a change to main or preload restarts the app instead of leaving a stale process behind.
#325 A root file meant "everything changed", and every dependency bump writes the lockfile — so adding one dependency to one package paid for the whole matrix, both workers and the web app. Each root file is now asked what it affects: the workspace file, .nvmrc and tsconfig still mean everything; the root manifest only when it changed beyond its scripts; the lockfile means the members whose importer entry moved, and everything when none did, since who inherits a transitive bump cannot be told without resolving the graph. The lockfile is compared against its previous version by importer block, sectioned by indentation rather than parsed: prepare runs before any install, so there is no parser to reach for, and a diff hunk cannot say whose dependency moved — an importer's inner lines carry no key of their own. Anything that cannot be compared — a missing base commit, an unreadable file, an importer outside apps, workers or packages — is the whole workspace again, and each decision is logged so a surprising run says why it happened. The rules move out of sixty lines of JavaScript embedded in YAML into scripts/assemble-changes.mjs, beside the two scripts already there. Every job's `if:` gate is untouched; only what prepare reports has changed. changes.json keeps its shape, but `root` now means a root change that implies the whole workspace rather than merely a top-level file being touched, so a scripts-only edit no longer redeploys the web app and both workers.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR centralizes CI change attribution, adds reusable area workflows, replaces one-shot Claude edits with streamed cancellable runs, and expands gist sandbox, draft, pagination, drag-and-drop, rename, and publishing workflows. ChangesCI, Claude, and gist workflows
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Benchmark resultsBaseline case:
|
| case | avg | p75 | alloc/iter | vs fastest |
|---|---|---|---|---|
| styled-system color() :: local-cjs | 428 ns | 427 ns | 248 B (+0.0%) | fastest |
| styled-system color() :: local-mjs | 450 ns | 451 ns | 248 B (+0.0%) | +5.1% |
| styled-system color() :: previous | 455 ns | 455 ns | 248 B (least) | +6.2% |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Bundle ReportBundle size has no change ✅ |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (4)
apps/editor/src/main/github/toGistId.test.ts (1)
24-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlign sandbox-id parsing with
isNewGist.
toNewGistId('NEW:<uuid>')returns the uppercase id, butisNewGist()only accepts lowercasenew:prefixes before the UUID. Keep the normalized output lowercase, or makeisNewGist()use the same case-insensitive rule.🤖 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 `@apps/editor/src/main/github/toGistId.test.ts` around lines 24 - 42, Align toNewGistId with isNewGist so uppercase NEW: UUID inputs produce an ID accepted by the existing validation; normalize the returned sandbox ID to lowercase, including the prefix and UUID, while preserving null results for invalid inputs.apps/editor/src/renderer/src/common/DraftList/utils/describeDraft.ts (1)
22-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
countChangesinstead of repeating the expression.Line 23 repeats the body of
countChangesinapps/editor/src/renderer/src/hooks/useGistDraft.ts(lines 12-14). That function is already exported and already imported byuseDrafts.ts. Two copies can drift if the definition of a change changes later.♻️ Proposed refactor
import { isNewGist, type GistDraft } from '../../../../../shared/ipc' +import { countChanges } from '../../../hooks/useGistDraft'const filenames = Object.keys(draft.files) - const changeCount = filenames.length + (draft.description === undefined ? 0 : 1) + const changeCount = countChanges(draft)🤖 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 `@apps/editor/src/renderer/src/common/DraftList/utils/describeDraft.ts` around lines 22 - 23, Update describeDraft to reuse the exported countChanges helper instead of calculating changeCount from draft.files and draft.description directly. Import and call countChanges with the draft so the change-count logic remains centralized.apps/editor/src/main/ipc/claudeHandlers.test.ts (1)
150-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIsolate the second registration so test order does not matter.
The mocked
handlewrites into one sharedhandlersmap. The secondregisterClaudeHandlerscall on Line 152 replaces the handlers created at module level, and it never captures a newemit. The suite passes only because thisdescriberuns last. A reordering, a--shufflerun, or a new test appended below would then use the wrong handlers.Register into a fresh handler map for this block, or read the handlers returned by this registration directly instead of the shared
starthelper.🤖 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 `@apps/editor/src/main/ipc/claudeHandlers.test.ts` around lines 150 - 157, Isolate the “registerClaudeHandlers with the default run ids” test from module-level handler state by registering against a fresh handlers map or using the handlers returned by that registration directly. Update the calls to start the runs so they use the newly registered handlers and emit function, rather than the shared start helper’s stale state.apps/editor/src/renderer/src/App.e2e.ts (1)
134-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the wait from the stub's own delay.
The
2500on Line 135 must outlast thesleep 2in the stub written bywriteClaudeStubinapps/editor/src/test/e2e/fixtures.ts. The two values live in different files with no link. If the stub delay changes, this test still passes but stops proving that the process was killed.Export the delay from
fixtures.tsand compute the wait from it. A fixed wait is correct here, because the assertion proves that no answer arrives; SonarCloud's hint to synchronize on an observable condition does not apply to an absence.♻️ Proposed change
In
apps/editor/src/test/e2e/fixtures.ts:+/** How long the stubbed CLI waits between its delta and its answer, in milliseconds. */ +export const CLAUDE_STUB_DELAY_MS = 2000Then in this file:
- // Long enough for the stub to have answered had it not been killed. - await page.waitForTimeout(2500) + // Long enough for the stub to have answered had it not been killed. + await page.waitForTimeout(CLAUDE_STUB_DELAY_MS + 500) await expect(getEditor(page)).toHaveValue('# Mine')🤖 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 `@apps/editor/src/renderer/src/App.e2e.ts` around lines 134 - 136, Replace the hard-coded 2500ms wait in the relevant App.e2e test with a duration derived from an exported stub-delay constant from writeClaudeStub in fixtures.ts, ensuring it exceeds the stub’s configured sleep and remains fixed to validate that no answer arrives when the process is killed.Source: Linters/SAST tools
🤖 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 `@apps/editor/src/main/claude/runEdit.ts`:
- Line 91: The stdout handling in runEdit must preserve UTF-8 characters split
across Buffer chunks. Replace per-chunk String conversion in the child process
flow with one shared UTF-8 decoder, flush it when the child closes, and pass
decoded text to createLineReader. In apps/editor/src/main/claude/runEdit.test.ts
lines 125-135, add a regression test that emits Buffer chunks splitting a
non-ASCII result character and verifies RUN_FINISHED receives the intact JSON.
In `@apps/editor/src/main/github/fetchGists.ts`:
- Around line 25-67: Update fetchGists so that when every page through
GISTS_MAX_PAGES is full, it returns a clear limit error instead of success with
partial data. Preserve the existing success path when a short page is received,
and add a boundary test covering GISTS_MAX_PAGES full pages.
In `@apps/editor/src/main/github/gistService.ts`:
- Around line 25-35: Update the createGist flow and GistFiles.publishDraft
contract to return the newly created remote gist ID instead of the sandbox ID.
Persist or migrate the sandbox draft to that created-ID state before reporting
success, and ensure cleanup failures retain a recoverable created-ID state so
retries do not issue another create request. Keep drafts.clear failure handling
while preserving the created gist for recovery.
In `@apps/editor/src/main/ipc/claudeHandlers.ts`:
- Around line 16-24: Update the runner event callback created by createRunner to
check contents.isDestroyed() before calling contents.send; when the WebContents
is destroyed, remove event.runId from listeners and cancel that run through the
runner without eagerly referencing runner during its own initialization, using a
deferred cancellation or an appropriate destroyed listener in startEdit.
In `@apps/editor/src/renderer/src/App.test.tsx`:
- Around line 47-49: Update the mocked gists.drafts() response in the App test
setup to return an empty GistDrafts record directly, rather than wrapping it in
a files property. Preserve the existing successful Result shape and ensure the
default data contains no fictitious gist IDs or draft values.
In `@apps/editor/src/renderer/src/App.tsx`:
- Around line 41-49: Update the new-document flow so creating a document also
clears undo history: ensure newDocument() invokes the existing reset() callback
after replacing the document, while preserving the current openGistFile behavior
and avoiding stale history from the discarded document.
In `@apps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.tsx`:
- Around line 66-69: Update the referenceName calculation in ClaudePanel to
treat whitespace-only descriptions as absent, matching toContext’s existing
behavior, so it falls back to the first filename or “that gist” and always
produces a non-empty accessible label. Add coverage beside the existing
no-description test for a description containing only whitespace.
In `@apps/editor/src/renderer/src/common/GistFiles/GistFiles.tsx`:
- Around line 123-125: Replace the separate stage calls in the rename flow with
a single atomic rename operation exposed by the draft IPC layer, ensuring
neither the deletion nor addition is applied when either step fails. Invoke
onRenamed only after the combined operation succeeds, and update the IPC
implementation and its callers consistently.
- Around line 67-73: Reset the add/rename state whenever the active gist
changes: update the component logic around the filename, isAdding, renaming, and
renamedTo state so gistId changes clear filename, renaming, and renamedTo (and
exit add mode). Alternatively, remount the panel keyed by gistId, ensuring no
pending action from the previous gist can be submitted against the new one.
In `@apps/editor/src/renderer/src/common/GistList/GistList.tsx`:
- Around line 56-58: Add a keyboard-operable action in GistList alongside the
existing draggable and onDragStart behavior that assigns the activated gist’s id
to ClaudePanel’s referenceId, using the component’s established context-setting
mechanism. Ensure the action is reachable and exposed with an appropriate
accessible label, and add an interaction test verifying keyboard activation
selects the same gist context as drag-and-drop.
In `@apps/editor/src/renderer/src/hooks/useDocument.ts`:
- Around line 128-134: Update renameOrigin and the save/stage flow to invalidate
or serialize any pending save when the gist origin filename changes, preventing
a stale stage completion from clearing isDirty after a rename from from to to.
Use an origin revision or equivalent operation coordination tied to renameOrigin
and save, and add a test covering save initiation, renaming the open file, then
resolving the stage request.
In `@apps/editor/src/renderer/src/hooks/useDrafts.ts`:
- Around line 19-39: Update the initial drafts fetch in the first useEffect to
merge the successful result into the current state via the setDrafts functional
updater, rather than replacing the entire record. Preserve any announcements
already processed by the onDraftChanged subscription, including deletions for
published or reset gists, while incorporating the fetched snapshot.
In `@scripts/assemble-changes.mjs`:
- Around line 166-178: Update the matrix construction around changed_packages so
manifest reads only use package directories present in both pkgsForCI and
allPackages, avoiding deleted or renamed importer keys. Derive both
changed_packages and has_packages from that intersection, while leaving
changes.packages unchanged for downstream deletion detection. Add coverage for
deleted and renamed package cases.
---
Nitpick comments:
In `@apps/editor/src/main/github/toGistId.test.ts`:
- Around line 24-42: Align toNewGistId with isNewGist so uppercase NEW: UUID
inputs produce an ID accepted by the existing validation; normalize the returned
sandbox ID to lowercase, including the prefix and UUID, while preserving null
results for invalid inputs.
In `@apps/editor/src/main/ipc/claudeHandlers.test.ts`:
- Around line 150-157: Isolate the “registerClaudeHandlers with the default run
ids” test from module-level handler state by registering against a fresh
handlers map or using the handlers returned by that registration directly.
Update the calls to start the runs so they use the newly registered handlers and
emit function, rather than the shared start helper’s stale state.
In `@apps/editor/src/renderer/src/App.e2e.ts`:
- Around line 134-136: Replace the hard-coded 2500ms wait in the relevant
App.e2e test with a duration derived from an exported stub-delay constant from
writeClaudeStub in fixtures.ts, ensuring it exceeds the stub’s configured sleep
and remains fixed to validate that no answer arrives when the process is killed.
In `@apps/editor/src/renderer/src/common/DraftList/utils/describeDraft.ts`:
- Around line 22-23: Update describeDraft to reuse the exported countChanges
helper instead of calculating changeCount from draft.files and draft.description
directly. Import and call countChanges with the draft so the change-count logic
remains centralized.
🪄 Autofix
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: bb532776-4cf2-4eb6-8088-9cc90a06d3e3
📒 Files selected for processing (72)
.github/workflows/ci.md.github/workflows/ci.yml.gitignoreapps/editor/package.jsonapps/editor/src/main/bootstrap.test.tsapps/editor/src/main/bootstrap.tsapps/editor/src/main/claude/editSelection.test.tsapps/editor/src/main/claude/editSelection.tsapps/editor/src/main/claude/parseStream.test.tsapps/editor/src/main/claude/parseStream.tsapps/editor/src/main/claude/runEdit.test.tsapps/editor/src/main/claude/runEdit.tsapps/editor/src/main/github/const.tsapps/editor/src/main/github/createGist.test.tsapps/editor/src/main/github/createGist.tsapps/editor/src/main/github/draftStore.test.tsapps/editor/src/main/github/draftStore.tsapps/editor/src/main/github/fetchGistFiles.test.tsapps/editor/src/main/github/fetchGistFiles.tsapps/editor/src/main/github/fetchGists.test.tsapps/editor/src/main/github/fetchGists.tsapps/editor/src/main/github/gistService.test.tsapps/editor/src/main/github/gistService.tsapps/editor/src/main/github/toGistId.test.tsapps/editor/src/main/github/toGistId.tsapps/editor/src/main/ipc/claudeHandlers.test.tsapps/editor/src/main/ipc/claudeHandlers.tsapps/editor/src/main/ipc/fileHandlers.test.tsapps/editor/src/main/ipc/fileHandlers.tsapps/editor/src/main/ipc/gistHandlers.test.tsapps/editor/src/main/ipc/gistHandlers.tsapps/editor/src/preload/index.test.tsapps/editor/src/preload/index.tsapps/editor/src/renderer/src/App.e2e.tsapps/editor/src/renderer/src/App.test.tsxapps/editor/src/renderer/src/App.tsxapps/editor/src/renderer/src/assets/PenMark.tsxapps/editor/src/renderer/src/assets/PlusMark.tsxapps/editor/src/renderer/src/assets/TrashMark.tsxapps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.test.tsxapps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.tsxapps/editor/src/renderer/src/common/ClaudePanel/utils/toContext.test.tsapps/editor/src/renderer/src/common/ClaudePanel/utils/toContext.tsapps/editor/src/renderer/src/common/ClaudePanel/utils/toPreview.test.tsapps/editor/src/renderer/src/common/ClaudePanel/utils/toPreview.tsapps/editor/src/renderer/src/common/DraftList/DraftList.test.tsxapps/editor/src/renderer/src/common/DraftList/DraftList.tsxapps/editor/src/renderer/src/common/DraftList/index.tsapps/editor/src/renderer/src/common/DraftList/utils/describeDraft.test.tsapps/editor/src/renderer/src/common/DraftList/utils/describeDraft.tsapps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.test.tsxapps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.tsxapps/editor/src/renderer/src/common/GistFiles/GistFiles.test.tsxapps/editor/src/renderer/src/common/GistFiles/GistFiles.tsxapps/editor/src/renderer/src/common/GistFiles/const.tsapps/editor/src/renderer/src/common/GistList/GistList.test.tsxapps/editor/src/renderer/src/common/GistList/GistList.tsxapps/editor/src/renderer/src/common/GistList/const.tsapps/editor/src/renderer/src/hooks/useClaudeEdit.test.tsapps/editor/src/renderer/src/hooks/useClaudeEdit.tsapps/editor/src/renderer/src/hooks/useDocument.test.tsapps/editor/src/renderer/src/hooks/useDocument.tsapps/editor/src/renderer/src/hooks/useDrafts.tsapps/editor/src/renderer/src/hooks/useGistDraft.test.tsapps/editor/src/renderer/src/hooks/useGistDraft.tsapps/editor/src/renderer/src/hooks/useGistFiles.test.tsapps/editor/src/renderer/src/hooks/useGistFiles.tsapps/editor/src/renderer/src/utils/gistDrag.test.tsapps/editor/src/renderer/src/utils/gistDrag.tsapps/editor/src/shared/ipc.tsapps/editor/src/test/e2e/fixtures.tsscripts/assemble-changes.mjs
💤 Files with no reviewable changes (2)
- apps/editor/src/main/claude/editSelection.test.ts
- apps/editor/src/main/claude/editSelection.ts
Comment on lines
+56
to
+58
| // Draggable onto the Claude panel, to write from what is in it. | ||
| draggable | ||
| onDragStart={(event) => startGistDrag(event.dataTransfer, gist.id)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add a keyboard-operated path to set gist context.
These lines make native drag-and-drop the only supplied path from GistList to ClaudePanel's referenceId. The component has no keyboard handler or equivalent action. Keyboard users cannot add a gist as Claude context.
Provide a keyboard-operable action that selects the same gist context. Add an interaction test for that action.
🤖 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 `@apps/editor/src/renderer/src/common/GistList/GistList.tsx` around lines 56 -
58, Add a keyboard-operable action in GistList alongside the existing draggable
and onDragStart behavior that assigns the activated gist’s id to ClaudePanel’s
referenceId, using the component’s established context-setting mechanism. Ensure
the action is reachable and exposed with an appropriate accessible label, and
add an interaction test verifying keyboard activation selects the same gist
context as drag-and-drop.
Member
Author
There was a problem hiding this comment.
Confirmed — draggable + onDragStart is the only route from a row into ClaudePanel's referenceId, so writing from a gist is unreachable without a pointer. Left for its own change, because closing it moves state rather than adding a handler: ClaudePanel owns referenceId privately, GistList lives in EditorSidebar, and today the two are joined only through the DOM by the drag itself. There is nowhere for a keyboard action here to put the answer.
Two ways to give it one:
A — a row action in the rail. Lift referenceId to App, make ClaudePanel controlled (referenceId + onReference), and give each row a "Write from …" button beside its existing affordances. The drop handler and the panel's "stop writing from" control go through the same callback, so there is one source of truth, and the reference still takes no space until chosen.
B — a picker in the panel. ClaudePanel grows its own "Write from…" select over useGists(). Self-contained, but it puts a permanent control in the panel for something usually absent — against the note in that file — and duplicates the list.
Leaning A. Filing it as a task against the editor epic (#295) with both options; it is an accessibility gap in new UI, so it is tracked rather than dropped.
There was a problem hiding this comment.
@soroushm, agreed. The accessibility gap is valid, but the minimal correct fix requires shared referenceId ownership.
Option A is the better design. It gives the rail action, drag-and-drop handler, and context removal control one state path. Tracking it under #295 is appropriate for this PR.
🐇
✏️ Learnings added
Learnt from: soroushm
Repo: soroush-tech/core PR: 327
File: apps/editor/src/renderer/src/common/GistList/GistList.tsx:56-58
Timestamp: 2026-08-05T01:02:43.510Z
Learning: In the editor renderer, `ClaudePanel` currently owns `referenceId` as private state, while `GistList` is rendered through `EditorSidebar`. Drag-and-drop is currently the only connection between these components for setting Claude gist context. A keyboard-accessible “Write from …” action requires lifting `referenceId` to a shared owner such as `App` and making `ClaudePanel` controlled through a common context-setting callback.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Reading the base manifest and lockfile meant three git commands resolved through PATH, which is a command the script cannot vouch for. The workflow copies both files out of the base commit instead — fetching it one deep when the shallow clone lacks it — and names them by environment variable. The script now decides and never executes: it takes the changed keys and two files, and a copy that is missing or empty is one it reads as "cannot be compared", which validates everything, exactly as an unreadable commit did.
The test waited two and a half seconds after cancelling to watch the stub's answer not arrive. That a killed run cannot write afterwards is settled in main and covered by unit tests — events carry the run they belong to, and the renderer drops any for a run it is no longer waiting on. What is left is what the page can be asked: the button is back, and the document holds what it held before. The suite is that much quicker for it.
The CLI's output was decoded a chunk at a time, so a character whose bytes arrived in two of them became a replacement character — in the middle of the answer, and in what RUN_FINISHED carried. One decoder now spans the run and is flushed when the child closes. A window closed mid-run was still sent to, which throws, and left the CLI answering nobody. A destroyed window is forgotten and its run killed. A rename staged the deletion and the new name as two changes: the first failing still moved the document, the second failing left the file gone with nothing in its place — and what is staged exists nowhere else. Main renames in one draft change now, and the document follows only once it has stuck. The rest, each small: - The file list stopped at its page cap and reported success, so a hundred full pages read as every gist there is. It says what happened instead. - A new document kept the last one's undo history: on a sandbox that is never a file on disk, Ctrl+Z walked back into what had just been discarded. - A save that finished after the open file was renamed called the document clean, though it staged under the name it no longer has. - A gist whose description is nothing but spaces named the reference it was written from as an empty string, leaving the button to stop it unlabelled. - The first draft snapshot overwrote an announcement that beat it, bringing back a draft that had just been published. - A half-typed filename, or a rename left open, carried over to the next gist and would have been staged against it. The panel is remounted with the gist instead.
Lockfile attribution names the importers that moved, and one of them can be a package that was deleted or renamed — its key is in the base file, not the head. Building the matrix from that read a manifest that is gone, which threw and left prepare with no outputs at all: nothing gated, nothing run. The matrix and has_packages are built from the packages that are still there. changes.json keeps the name, so a CD workflow can still see that it went.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/editor/src/main/claude/runEdit.test.ts (1)
246-259: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd coverage for
errorafter cancellation.This test covers late stdout and the close-time flush. It does not emit a child
errorafterrunner.cancel('run-1').Add that sequence before
close. Assert thateventsremains onlyRUN_STARTED. This verifies the cleanup required atrunEdit.tsLine 121.🤖 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 `@apps/editor/src/main/claude/runEdit.test.ts` around lines 246 - 259, Add an error event from the child after runner.cancel('run-1') and before the close event in the existing 'drops what a cancelled run writes on its way out' test, then assert events still contains only RUN_STARTED. Ensure the test exercises the cleanup path associated with the child error handler in runEdit.ts.
🤖 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 `@apps/editor/src/main/claude/runEdit.ts`:
- Around line 119-121: The child error handler in runEdit.ts must ignore
cancelled runs: when handling a child error, remove the run from cancelled and
avoid emitting RUN_ERROR or performing normal error cleanup for that cancelled
run. Add coverage in runEdit.test.ts for calling cancel() followed by a child
error instead of close, verifying no RUN_ERROR is published; the existing
close-path site requires no direct change.
---
Nitpick comments:
In `@apps/editor/src/main/claude/runEdit.test.ts`:
- Around line 246-259: Add an error event from the child after
runner.cancel('run-1') and before the close event in the existing 'drops what a
cancelled run writes on its way out' test, then assert events still contains
only RUN_STARTED. Ensure the test exercises the cleanup path associated with the
child error handler in runEdit.ts.
🪄 Autofix
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: 5c3d9a60-bb0f-4068-8572-8656f6fb7ed3
📒 Files selected for processing (11)
.github/workflows/ci-editor.md.github/workflows/ci-packages.md.github/workflows/ci-web.md.github/workflows/ci-web.ymlapps/editor/src/main/claude/runEdit.test.tsapps/editor/src/main/claude/runEdit.tsapps/editor/src/main/github/gistService.test.tsapps/editor/src/main/github/gistService.tsapps/editor/src/renderer/src/App.e2e.tsapps/editor/src/renderer/src/hooks/useGistDraft.test.tsapps/editor/src/renderer/src/hooks/useGistDraft.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/ci-web.md
- apps/editor/src/renderer/src/App.e2e.ts
- .github/workflows/ci-packages.md
- .github/workflows/ci-web.yml
- apps/editor/src/main/github/gistService.ts
- .github/workflows/ci-editor.md
- apps/editor/src/main/github/gistService.test.ts
`\s` spans newlines, so `^#\s*ci:validates\s+` would have accepted a marker whose tokens sat on a line of their own — and, on every workflow that has no marker at all, gave the engine two ways to match the same run of whitespace and a whole file to backtrack through. Horizontal space only: the marker is one line, and now says so. The wildcard's `exec(...) ?? []` left the analyzer reading the destructured prefix as a string, which made the `undefined` check look dead. Checking the match itself says the same thing in the terms the language actually has. Verified by dry run: the six attributions this file exists to make come out unchanged, and a CI edit still reports `root: false`.
`soroush-tech/bench-action@v1` sat with `actions/*` on a version tag, on the reasoning that first-party trust extends to the org that owns the repo. What SHA-pinning defends against is not the author, though — it is a tag being moved to point somewhere else, and our tags move like anyone's. Sonar had been flagging it all along; the resolution was recorded against ci.yml, and splitting that file into ci-packages.yml moved the line to an address the old resolution did not cover. A finding that comes back whenever a line changes files was never really settled. So it gets a SHA and keeps its version in the comment beside it. `actions/*` stays on tags, which is the one thing both reviewers already agree on. The cost is that an action release is now a line someone bumps and reviews, rather than one that arrives on its own — which is the same thing the pin is for.
A gist became background material for a run by being dragged onto the panel, and by nothing else. There was no second path, so anyone not working with a mouse had no way to reach the feature at all. The choice now sits in the panel beside the reference it sets, as a native select — the drag still works and still calls the same setter, but it is no longer the only way in. The panel is where the reference is used and where it is cleared, so it is also where it can be picked without lifting any state out of it. Shown only while the account has a gist to offer, which is also what being signed out looks like from here — an empty list would say nothing either way. A gist with no description is named by what it holds, since the summary the list returns carries counts rather than filenames.
Publishing the new-gist sandbox created the gist and then dropped its id, so nothing downstream could act on it: the panel opened an empty sandbox, and the document was left pointing at an address that had just stopped existing. Saving after that would stage into a draft for a sandbox already published — work filed under something nobody will look at again. `publish` now answers with the gist that holds the work: the created one for a sandbox, the same id back for a gist that already existed. The panel follows it there, and the document follows with it, so a later save stages against the gist rather than resurrecting the sandbox. The cleanup afterwards no longer decides the answer. Whether the sandbox could be tidied away says nothing about whether GitHub took the work, and reporting a failure there invited the one retry that must never happen — publishing a sandbox twice is two gists, and the second is not a correction of the first.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.tsx (1)
67-70: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the selected sandbox synchronized with the active document.
If the discard prompt is cancelled or its save fails,
Appleaves the current document open becauseloadreturnsfalse. Lines 67-70 already switchgistIdto the new sandbox. The Files panel can then stage changes for the new sandbox while the editor still saves to the old origin.Make
onOpenFilereport whether the document switch succeeded. Set the new sandbox selection only after that result is successful. Add a regression test for a rejected sandbox switch.As per coding guidelines, run
pnpm test:coverageand verify 100% coverage on touched TypeScript files.🤖 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 `@apps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.tsx` around lines 67 - 70, Update the EditorSidebar sandbox-opening flow around openSandbox so it awaits the boolean result from onOpenFile and only updates the selected sandbox after the document switch succeeds; preserve the existing selection when the switch is cancelled or saving fails. Change onOpenFile to report switch success and add a regression test covering a rejected sandbox switch, then run pnpm test:coverage and ensure touched TypeScript files remain at 100% coverage.Source: Coding guidelines
.claude/skills/ci-cd/SKILL.md (1)
42-42: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-250)
Reachability: External
Do not require
secrets: inheriton every reusable workflow hop.
secrets: inheritforwards all secrets available to the caller, including named secrets the downstream task cannot access. CI already omits it for the editor hop because those uploads are tokenless. Use named secret forwarding when possible, and require inheritance only for a documented secret need.🤖 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 @.claude/skills/ci-cd/SKILL.md at line 42, Update the reusable-workflow guidance in the scope and secrets paragraph so `secrets: inherit` is not mandatory on every hop. Prefer explicit named-secret forwarding, allow omission for tokenless workflows such as the editor hop, and require inheritance only when the downstream workflow has a documented need for caller secrets.
🤖 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 @.claude/skills/ci-cd/SKILL.md:
- Line 2: The SKILL.md description incorrectly lists web and editor as top-level
CI stages. Update the CI chain wording to `prepare → lint → packages/worker/app
→ ci-ok`, and explicitly state that the app workflow invokes web and editor,
while preserving the other documented conventions.
In `@apps/editor/src/renderer/src/common/GistFiles/GistFiles.tsx`:
- Around line 143-146: Guard the publish flow in publishDraft with an in-flight
state, disable the Create/Publish button while that state is active, and clear
it when the request settles so rapid clicks cannot start concurrent publishes.
Add a regression test that triggers two clicks and verifies only one publish
request is initiated.
---
Outside diff comments:
In @.claude/skills/ci-cd/SKILL.md:
- Line 42: Update the reusable-workflow guidance in the scope and secrets
paragraph so `secrets: inherit` is not mandatory on every hop. Prefer explicit
named-secret forwarding, allow omission for tokenless workflows such as the
editor hop, and require inheritance only when the downstream workflow has a
documented need for caller secrets.
In `@apps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.tsx`:
- Around line 67-70: Update the EditorSidebar sandbox-opening flow around
openSandbox so it awaits the boolean result from onOpenFile and only updates the
selected sandbox after the document switch succeeds; preserve the existing
selection when the switch is cancelled or saving fails. Change onOpenFile to
report switch success and add a regression test covering a rejected sandbox
switch, then run pnpm test:coverage and ensure touched TypeScript files remain
at 100% coverage.
🪄 Autofix
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: bfc6ed37-c1b1-4e3d-8e8d-3192125c620f
📒 Files selected for processing (17)
.claude/skills/ci-cd/SKILL.md.github/workflows/ci-packages.ymlapps/editor/src/main/github/gistService.test.tsapps/editor/src/main/github/gistService.tsapps/editor/src/main/ipc/gistHandlers.tsapps/editor/src/preload/index.tsapps/editor/src/renderer/src/App.tsxapps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.test.tsxapps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.tsxapps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.test.tsxapps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.tsxapps/editor/src/renderer/src/common/GistFiles/GistFiles.tsxapps/editor/src/renderer/src/hooks/useDocument.test.tsapps/editor/src/renderer/src/hooks/useDocument.tsapps/editor/src/renderer/src/hooks/useGistDraft.test.tsapps/editor/src/renderer/src/hooks/useGistDraft.tsscripts/assemble-changes.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/editor/src/renderer/src/hooks/useGistDraft.ts
- apps/editor/src/renderer/src/common/EditorSidebar/EditorSidebar.test.tsx
- apps/editor/src/renderer/src/App.tsx
- apps/editor/src/main/ipc/gistHandlers.ts
- apps/editor/src/renderer/src/common/ClaudePanel/ClaudePanel.test.tsx
- apps/editor/src/main/github/gistService.ts
- apps/editor/src/preload/index.ts
- apps/editor/src/main/github/gistService.test.ts
- apps/editor/src/renderer/src/hooks/useGistDraft.test.ts
- .github/workflows/ci-packages.yml
| @@ -1,5 +1,5 @@ | |||
| --- | |||
| description: GitHub Actions CI/CD conventions for this repo — the unified ci.yml (prepare → changes-gated lint/web/e2e/packages/worker → ci-ok), the action-pinning rule (first-party version tags, third-party SHAs), per-workspace Codecov flags with tokenless-OIDC uploads, the CI-environment approval gate (with env-scoped vars forwarded to environment-less jobs via job outputs), Cloudflare deploys via cloudflare/wrangler-action, and the standalone Chromatic workflow. Use when adding, editing, or debugging any workflow under .github/workflows/. | |||
| description: GitHub Actions CI/CD conventions for this repo — the CI entry workflow calling one workflow per area (prepare → lint → packages/worker/web/editor → ci-ok), the action-pinning rule (version tags for `actions/*`, commit SHAs for everything else including our own org), per-workspace Codecov flags with tokenless-OIDC uploads, the CI-environment approval gate (with env-scoped vars forwarded to environment-less jobs via job outputs), Cloudflare deploys via cloudflare/wrangler-action, and the standalone Chromatic workflow. Use when adding, editing, or debugging any workflow under .github/workflows/. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the CI chain in the description.
.github/workflows/ci.yml calls .github/workflows/ci-app.yml. The app workflow then calls the web and editor workflows. Describe the chain as prepare → lint → packages/worker/app → ci-ok, with app → web/editor.
🤖 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 @.claude/skills/ci-cd/SKILL.md at line 2, The SKILL.md description
incorrectly lists web and editor as top-level CI stages. Update the CI chain
wording to `prepare → lint → packages/worker/app → ci-ok`, and explicitly state
that the app workflow invokes web and editor, while preserving the other
documented conventions.
…ressed Nothing stopped a second press before the first request answered. For a gist that already exists that is a wasted PATCH, but for the sandbox it is a second gist — and nothing about the two says which one was meant, or which the panel went on to open. A duplicate that cannot be told apart from the original is not something the user can tidy up afterwards. The button is held for as long as the request is in flight, and released when it comes back. A publish that came back refused leaves the draft where it was and the button ready to try again, rather than dead. Cancelling a run had the same shape one layer down. A cancel marked the run and killed the child, but the child's own `error` event still reported the failure as the run's: a kill that goes wrong is a failure of the killing, and the panel is idle by then and asked for this. It also left the run in the cancelled set for ever, since only the close path cleared it.
The skill's description still listed web and editor as jobs of the entry workflow. They are a level further down now: ci.yml calls ci-app.yml, and that calls one workflow per app — which is the whole point of the layer, since it is what keeps a third app out of the entry file. The body was corrected when the layer went in; this line was not, and it is the line loaded to decide whether the skill is worth reading.
|
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 `@apps/editor/src/main/claude/runEdit.ts`:
- Around line 109-112: Update the cancellation handling around the cancelled-run
cleanup in the runEdit flow: maintain a separate per-run terminal flag, set it
before cancelled.delete(runId), and check it in both the stdout handler and
line-reader callback to suppress queued output and completion events after
cancellation errors. Add a regression test covering cancel() → error → stdout
data and assert that neither TEXT_MESSAGE_CONTENT nor RUN_FINISHED is emitted.
🪄 Autofix
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: f38564bb-01f7-48a5-b77f-74712d1b1157
📒 Files selected for processing (5)
.claude/skills/ci-cd/SKILL.mdapps/editor/src/main/claude/runEdit.test.tsapps/editor/src/main/claude/runEdit.tsapps/editor/src/renderer/src/common/GistFiles/GistFiles.test.tsxapps/editor/src/renderer/src/common/GistFiles/GistFiles.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/editor/src/main/claude/runEdit.test.ts
- apps/editor/src/renderer/src/common/GistFiles/GistFiles.tsx
- .claude/skills/ci-cd/SKILL.md
- apps/editor/src/renderer/src/common/GistFiles/GistFiles.test.tsx
Comment on lines
+109
to
+112
| // A run the user stopped, whose child then failed to go quietly. The | ||
| // panel is idle and asked for this: the failure is of the killing, not | ||
| // of the run, and there is nothing for anyone to do about it. | ||
| if (cancelled.delete(runId)) return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep output suppression active after a cancellation error.
Line [112] removes runId from cancelled before the child streams close. If stdout data is already queued, the reader can process it after error and emit TEXT_MESSAGE_CONTENT for a cancelled run. This can show stale Claude output after document restoration.
Keep a separate per-run terminal flag. Set it before cancelled.delete(runId). Check it in the stdout handler and the line-reader callback.
Add a regression test for cancel() → error → stdout data. Assert that the run emits neither TEXT_MESSAGE_CONTENT nor RUN_FINISHED.
Suggested guard
+ let terminal = false
+
const reader = createLineReader((line) => {
- if (cancelled.has(runId)) return
+ if (terminal || cancelled.has(runId)) return
...
- child.stdout?.on('data', (chunk: Buffer | string) =>
- reader.push(stdout.write(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)))
- )
+ child.stdout?.on('data', (chunk: Buffer | string) => {
+ if (terminal || cancelled.has(runId)) return
+ reader.push(stdout.write(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)))
+ })
...
child.on('error', (error) => {
+ terminal = true
running.delete(runId)As per coding guidelines, run pnpm test:coverage and verify 100% coverage on touched files. The PR objective also requires cancellation E2E coverage, so run pnpm test:e2e.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { spawn } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@apps/editor/src/main/claude/runEdit.ts` around lines 109 - 112, Update the
cancellation handling around the cancelled-run cleanup in the runEdit flow:
maintain a separate per-run terminal flag, set it before
cancelled.delete(runId), and check it in both the stdout handler and line-reader
callback to suppress queued output and completion events after cancellation
errors. Add a regression test covering cancel() → error → stdout data and assert
that neither TEXT_MESSAGE_CONTENT nor RUN_FINISHED is emitted.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The second half of
apps/editor, picked onto the review fixes that landed with #324, plus one CIchange that belongs to no feature.
Closes #319, #322, #323 — under Epic #295 — and #325.
What it does
Drafts have a home of their own. The rail lists every gist with unpublished work, so unfinished
edits are findable rather than remembered. A gist can be started from nothing —
new:<uuid>namesa sandbox until GitHub gives it a real id — and its files renamed from the panel, with the open
document following the rename so a later save stages under the name the file now has.
The startup document opens on the sandbox. It used to belong to nothing: what you wrote on
launch went to disk and could never be published. It is now a file in the sandbox, so saving stages
it and Create gist has something to send. Staging into a gist that does not exist yet records as
added rather than modified — nothing is published there, so calling the first save a
modification claimed GitHub had a version it does not. The save dialog also opens on the name the
document already goes by, instead of asking for it again.
Claude streams into the document. The bridge was one-shot and blocking — a progress bar for up
to two minutes with no way to stop. It now spawns with
--output-format stream-jsonand reportseach run through a typed event union (
RUN_STARTED,TEXT_MESSAGE_CONTENT,RUN_FINISHED,RUN_ERROR), named after the AG-UI protocol without taking the dependency. The answer lands in thedocument as it is written, Cancel replaces Ask Claude while a run is going, and a stopped or failed
run puts back what was there.
RUN_FINISHEDcarries the whole answer rather than leaving therenderer to reassemble deltas, so a dropped one cannot quietly corrupt the document. Runs are held
by id and their events go only to the window that started them.
A gist can be dragged onto the instruction and is sent as a CONTEXT block ahead of it: earlier
work to build on, which the prompt says is not to be repeated. That makes "we covered rehydration,
now write the part about data fetching" askable. The drag carries a private type, so dropped text
and files are ignored, and the offer shows in the field's own placeholder — nothing takes room for
a reference that is usually absent.
CI: what a root change means (#325)
A root file meant "everything changed", and every dependency bump writes the lockfile — so adding
one dependency to one package paid for the whole matrix. Each root file is now asked what it
affects:
pnpm-workspace.yaml,.nvmrcandtsconfig.jsonstill mean everything; the rootmanifest only when it changed beyond
scripts; the lockfile means the members whose importerentry moved, and everything when none did, since who inherits a transitive bump cannot be told
without resolving the graph.
The lockfile is compared against its previous version block by block, sectioned by indentation
rather than parsed —
prepareruns before any install, and a diff hunk cannot say whose dependencymoved. Anything that cannot be compared is the whole workspace again, and every decision is logged
(
changes: pnpm-lock.yaml moved app__editor). The rules moved out of sixty lines of JavaScriptembedded in YAML into
scripts/assemble-changes.mjs; every job'sif:gate is untouched.Replayed against the last eight merges to
main: #318 drops from 12 package rows to 3; #324 stillruns everything, correctly, because it changed
pnpm-workspace.yamlas well as the lockfile.One behaviour change past CI gating:
changes.jsonkeeps its shape, butrootnow means "aroot change that implies the whole workspace" rather than "a top-level file was touched", so a
scripts-only edit no longer redeploys the web app and both workers.
Reconciled with the review fixes from #324
These commits were written before that review, so the merge kept the fixes and took the feature:
the document-revision guard and the save that will not bind its result to a document that replaced
it; the serialized, atomically written draft store (
update, notwrite); theclaim()/isNewest()guard on publish; the pinned raw-gist host; the truncated-file read that fails rather than handing
back a partial.
Two collisions were not textual:
onApplyonce per delta, so the stale-answer guard would have applied the firstand refused the rest. It now remembers what it last wrote as the text to find next time, so a
delta replaces the one before it while a document that moved on is still refused.
validateIdaccepted only the hex ids GitHub issues, which rejected everynew:<uuid>sandbox atthe IPC boundary — the whole new-gist flow. It now matches the sandbox ids this app mints by
shape, so
new:../../evilstill cannot become a key in the drafts file.Verification
apps/editorpnpm lint,pnpm typecheckcleanpackage, a new workspace member, a removed member, a transitive-only bump, a scripts-only manifest
edit, and an importer outside
apps/workers/packagesKnown gaps
was merged to keep the deterministic
setDirtysync rather than a fixed wait, andexpectnowcomes from
@playwright/testbecause the merged fixtures no longer re-export it.ci.yml, so by its own rule this run validates the whole workspace — thenarrowing will first show on a later PR that only bumps a dependency.
Summary by CodeRabbit