fix(wizard): no perder el checkpoint al cambiar de carpeta a mitad de un paso - #258
Conversation
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 55.5/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.0 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_df20f312-0d52-436e-b048-6c7fec1e02bc) |
|
Revisión independiente (worktree aislado). El fallo es real: Retargeteado de Ready for review; esperar |
…en() Pin the collection that still holds the running workflow before execute() yields. Switching folders mid-step no longer drops the finished checkpoint on the source workspace or writes it into the destination. Co-authored-by: ignaciodelcano+dcl <ignaciodelcano+dcl@gmail.com>
6426524 to
c18ef4d
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6caa67d5-d411-4124-a14a-f829f0dbdc72) |
|
El score 49.9 no lo bajó este arreglo. Era el comentario de code-health cuando la base era El parche real es +22 LOC de producción, +0 funciones ≥15, +0 complejidad máxima, ratchet OK. He rebasado sobre |
2 participants
Resumen ejecutivo
Qué cambia
El runtime del Wizard ahora guarda el checkpoint de un paso en el workspace donde empezó, aunque el usuario cambie de carpeta de salida mientras
execute()sigue en curso.Para qué sirve
Un cambio de workspace a mitad de un paso asíncrono (por ejemplo, crear escena en un workflow rítmico 3D) dejaba el origen en
runningsinoutputRefsy no escribía el resultado. Recargar mostraba el workflow interrumpido aunque el paso hubiera terminado.Impacto para el usuario
Los workflows largos (rítmico 3D, videoclip) ya no pierden el progreso si se cambia de carpeta mientras un paso trabaja.
Riesgo
Estado
Summary
persist()already pinned the workspace at the start of a save so a CAS retry could not followopen(). That pin happened too late for the common case:advanceUnlocked()awaitsstep.execute(), the footer workspace changes (busyis already false after the original ask),open()replacesthis.collection, and the later persist cloned the destination collection. The finished step mutated an orphaned source object that never reached disk.The fix captures
{workspace, collection, openSequence}before the execute loop and reuses that owner for every persist in the same advance. Destination stores stay untouched. Emit still no-ops after the live pointer moves, so the new chat does not receive the old workflow card.Overview
busybecomes false.create-scene, bake, export, …).shouldFollowWizardWorkspaceallows the switch.open(destination)replaces the in-memory collection.Without this change, source stayed
runningand destination was unchanged. Reload asked to resume a step that had already succeeded.Detailed changes
Backend
No backend changes.
UI and Wizard
WizardWorkflowRuntime.advanceUnlockedcaptures the persist owner before yielding toexecute().persist/persistAndEmitaccept that owner so post-execute checkpoints and CAS retries stay on the source store.open()coverage is unchanged.Data, provenance and compatibility
Workspace isolation is stricter: a late persist cannot adopt the destination collection. No migration.
Files and ownership
ui/src/features/agent/wizardWorkflowRuntime.ts— pin persist owner acrossexecute().ui/tests/wizardWorkflowRuntime.test.mjs— gated second step +open()mid-execute.Validation
python3 scripts/verify_clean_repo.py— PASSpython -m compileall -q app/services app/launch.py scripts— N/A (UI-only)cd ui && npm run i18n:check— not required (no catalog changes)npx tsx --tsconfig tsconfig.app.json --import ./tests/setupI18n.ts --test tests/wizardWorkflowRuntime.test.mjs tests/wizardConversationPersistence.test.mjs tests/rhythmic3dWorkflow.test.mjs— 34/34 passnpx eslint src/features/agent/wizardWorkflowRuntime.ts tests/wizardWorkflowRuntime.test.mjs --max-warnings=0— passcd ui && npm run build— not run (logic-only runtime change)git diff --check— cleanCode quality
main: pending CICI and review
Coste de la tarea
Notes and limitations
saveWizardWorkflows.execute()checkpoint, not missed task events after the switch.Follow-up work
Checklist
Note
Low Risk
UI-only wizard persistence logic with tighter workspace isolation; no auth or backend changes, covered by focused runtime tests.
Overview
Fixes a race where changing the output folder while a wizard step’s
execute()is still running could drop the finished checkpoint:open()swappedthis.collection, so a later persist cloned the destination store and never wrote the completed workflow back to the source workspace.advanceUnlockednow captures a persist owner (workspace,collection,openSequence) before the step loop and passes it into everypersistAndEmitfor that advance.persistsaves from that pinned collection, updates the owner’s revision on success/CAS merge, and only refreshes the live in-memory collection when the runtime still owns that open target—same idea as the existing in-flight persist duringopen(), extended acrossawait execute()yields.Adds a unit test that switches workspaces while the second step is blocked mid-execute and asserts the source workspace ends
completedwith mergedoutputRefswhile the destination store stays unchanged.Reviewed by Cursor Bugbot for commit c18ef4d. Configure here.