Skip to content

feat(desktop): Restart Tutorial deletes the previous clone - #3365

Merged
janicduplessis merged 4 commits into
mainfrom
fix/3359-restart-tutorial-cleanup
Oct 10, 2026
Merged

janicduplessis merged 4 commits into
mainfrom
fix/3359-restart-tutorial-cleanup

Conversation

@janicduplessis

Copy link
Copy Markdown
Collaborator

Description

After Restart Tutorial, the agent follows stim guide tutorial run, finds ~/stim-tutorial from the earlier run (a clean clone with node_modules) and stops to ask for another folder, as the guide says. Restart never removed the previous clone.

Solution

Desktop: Restart looks at the recorded clone path (else ~/stim-tutorial). With no folder it restarts as before. With a folder it asks "Restart deletes and its tutorial worktrees." with Cancel and Delete & Restart. The only guard is the tutorial marker (expo.extra.stimTutorial in app.json); a folder without it is left alone and the alert says so. On confirm it runs stim stop and stim worktree remove (no --force) for each linked worktree, then for the clone, then moves the folder to the Trash with FileManager.trashItem. Any Stim refusal stops the restart and is shown in an alert. Uncommitted changes are covered by the confirmation and the Trash, not a separate refusal.

Guide: stim guide tutorial run reuses an existing {base} when app.json carries this version's marker, origin is appandflow/stim-tutorial, the git root is {base}, and git status --porcelain is empty; it then runs npm ci and stim doctor and continues. Any other existing folder still stops and asks. Website docs match.

Second commit: the Make a Change step text is two sentences, has a copyable prompt box, and the leftover Run iOS button is gone.

Test plan

  • TutorialCleanupTests (plan: marker removes with its worktrees, no marker refuses; marker read from app.json), the Tutorial test filter, swift build -Xswiftc -warnings-as-errors, scripts/format.sh, check-ascii.mjs.
  • pnpm test guide tests updated for the reuse rule; lint, typecheck, knip, format:check pass.
  • Real tool calls: ran the exact sequence Restart issues (stim stop, stim worktree remove <worktree>, stim stop, stim worktree remove <clone>) against scratch clones under a private STIM_HOME; both succeeded and left the source folder for the Trash step.
  • Dev app: the confirmation dialog appears from the tutorial menu. I cancelled it. The dev app picked up another tutorial clone from the shared status instead of my scratch one, so I did not run Delete & Restart end to end in the app and have no refusal screenshot.

Fixes #3359

@janicduplessis

Copy link
Copy Markdown
Collaborator Author

Review of #3365 (read-only; nothing run against ~/stim-tutorial or ~/.stim).

Findings, most important first

  1. Confirm dialog does not show what gets removed. TutorialCleanup.plan returns every linked worktree of base (git worktree list, minus the first entry), not only the tutorial's two. TutorialRestartDialogs drops the worktrees array and says "its tutorial worktrees". A worktree the user made from the clone by hand is removed under that wording. Either list the paths in the message or filter to the tutorial's tracked worktrees.
  2. "First entry is the clone" is assumed, not checked. paths.dropFirst() skips the first listed path without comparing it to base. If base is itself a linked worktree (or a subfolder of another repo) that has the app.json marker, the first entry is a different repository and the remaining paths, including base itself and the user's other worktrees, go to stim worktree remove. The marker is the only guard. Add a check that the first path realpath-equals base (the guide's own reuse rule already requires rev-parse --show-toplevel == {base}). If it differs, refuse.
  3. A stale worktree entry blocks Restart. A prunable worktree (directory gone, still registered) is skipped for stop but still passed to stim worktree remove. If that refuses, remove throws, the blocked alert shows, and Restart stays blocked on every retry. The earlier worktrees were already removed. Consider treating a missing directory as git worktree prune territory or skipping it.
  4. No in-flight guard. confirmRestart clears the dialog and runs removal with no progress state, so Restart can be clicked again while it runs. A second plan then sees a half-removed clone and can present a second confirm, and two removals run concurrently on the same paths.
  5. Partial failure. If a later step fails (a locked worktree, trashItem), the earlier worktrees are already gone, open(beginning: true) is not called, and the record still points at the clone. This recovers on retry, but only if finding 3 does not apply.
  6. Symlinked base. hasMarker and git worktree list follow the link, but trashItem trashes only the link, so the real clone stays on disk with its environment reclaimed. Low priority. realpath the base first (AGENTS.md invariant 6).
  7. Marker mismatch. Swift accepts any non-nil expo.extra.stimTutorial; the guide requires it to equal TUTORIAL_VERSION. Low priority.

Dialog bindings look right. The confirming setter only clears for .confirm, so the .blocked that confirmRestart sets afterwards is not clobbered. The Delete action captures its own dialog value, so the setter clearing state does not affect it.

Tests and policy

  • guide.test.ts adds regexes over explanatory prose ("reuse it only when all of these hold", "git status --porcelain prints nothing"). AGENTS.md says guide tests protect contracts, not paragraphs. Keep the safety assertion (never overwrite or delete) and drop the other two or reduce them to contract identifiers.
  • TutorialCleanupTests covers a trivial pure function and the app.json read. The risky part, the git worktree list --porcelain parsing and the first-entry assumption in plan, has no test. Extract the porcelain parse to a pure function and test it, including the linked-worktree-as-base case.
  • Sources are ASCII-only. No stdout contract is affected. The comments added are doc comments on public API; the /// on remove restates the code and can go.
  • website/docs/desktop.md still says "only the optional last step removes it" next to the new Restart behavior. Reconcile.

@janicduplessis

Copy link
Copy Markdown
Collaborator Author

Fresh review of head 0f0e0bf (read-only; nothing run against ~/stim-tutorial or ~/.stim).

Earlier findings, verified fixed

  • Dialog now lists the clone and each worktree path (tilde-abbreviated).
  • plan refuses unless the first git worktree list entry realpath-equals base.
  • remove skips worktree entries whose directory is gone, so a stale entry no longer blocks Restart.
  • restarting flag guards restart() and confirmRestart (the model is @MainActor, so the flag has no race); it resets on every path via defer.
  • Marker now requires stimTutorial in supportedVersions.
  • worktreePaths parse is a tested pure function; the /// on remove and the website "only the optional last step" wording are fixed.
  • Dialog bindings: the confirming setter clears only for .confirm, so a .blocked set after a failed removal is kept; the Delete action captures its own dialog value.

Remaining, low priority (not blocking)

  1. Symlinked base: plan compares resolved paths, but remove and trashItem use the unresolved base, so a symlink to the clone would trash only the link. Resolve base once in plan/remove if wanted.
  2. The first-entry-equals-base check lives inside the async plan and has no test; only the parse is covered.
  3. Marker check in Swift accepts supportedVersions ([2]) while the guide requires TUTORIAL_VERSION; they agree today.

Nothing actionable remains for merge.

@janicduplessis

Copy link
Copy Markdown
Collaborator Author

Heads-up: #3367 (tutorial share step) merged and also regenerated apps/desktop/Sources/StimKit/TutorialSteps.swift from tutorial-data.ts. The branches merged cleanly, but after merging main please rerun node apps/desktop/scripts/generate-tokens.mjs so the generated file reflects both changes.

@janicduplessis
janicduplessis marked this pull request as ready for review October 10, 2026 17:38
@janicduplessis
janicduplessis merged commit 0c2d424 into main Oct 10, 2026
19 checks passed
@janicduplessis
janicduplessis deleted the fix/3359-restart-tutorial-cleanup branch October 10, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: Restart Tutorial leaves the previous clone, so the agent stops at the existing folder

1 participant