Skip to content

feat(environments): archive grace period + Restore environment - #1016

Open
ymichael wants to merge 3 commits into
mainfrom
bb/restore-environment-archive-grace-period-thr_ufty6npytm
Open

feat(environments): archive grace period + Restore environment#1016
ymichael wants to merge 3 commits into
mainfrom
bb/restore-environment-archive-grace-period-thr_ufty6npytm

Conversation

@ymichael

@ymichael ymichael commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Re-lands #242 against the current codebase (that branch had drifted too far to rebase cleanly).

Problem

Two gaps around a thread losing its environment:

  1. Accidental archive bricks a thread. Archiving the last live thread in a managed environment immediately tore down its git worktree — no undo, uncommitted work gone.
  2. No way back from a gone environment. A thread whose environment is destroying/destroyed showed a dead-end "Environment is no longer available" banner.

What this does

Archive grace period (the brick fix). Archiving the last live thread now leaves the environment retiring for a configurable window (managedEnvironmentRetireGraceMs, default 10s) before the worktree is destroyed. The gate lives in advanceEnvironmentCleanup and is durable across restart (keyed on the row's updatedAt; no in-memory timer). It only applies when a revivable archived thread remains, so a deleted thread's orphaned environment is still cleaned up immediately.

Lossless undo. Within the window:

  • Un-archiving the thread fires the existing retire.cancelled → environment back to ready, worktree (and uncommitted work) intact.
  • A 10s "Thread archived — Undo" toast offers the same one click (wired to the current cascade archive-all mutation; Undo un-archives every thread the cascade archived). The durable banner Unarchive remains as a fallback.

Restore environment. POST /threads/:id/restore-environment reprovisions a fresh environment for a thread whose environment is gone:

  • The daemon's createWorktree now checks out an existing branch in place instead of -B-resetting it, so committed work (which survives git worktree remove) is recovered.
  • The thread is re-seeded into idle (no automatic turn); routing handles retiring (revive in place), destroying (409, retry), unmanaged (409), and pruned envs (409).
  • A "Restore environment" action in the read-only banner ("Cleaning up…" while destroying, active once destroyed).

Changes vs. #242 (adaptation to the new codebase)

  • The app now talks to the server through the SDK, so the restore mutation and the Undo toast use sdk.threads.* instead of the removed lib/api thread helpers; the toast attaches to useArchiveThreadAndChildren (single-thread useArchiveThread no longer exists).
  • New SDK surface threads.restoreEnvironment and CLI command bb thread restore-environment [id] (with --self/--json), plus guide template + bb-cli skill updates, per the current AGENTS.md rule that end-user features ship with SDK and CLI surfaces.
  • HOST_DAEMON_PROTOCOL_VERSION bumped to 70: an enrolled daemon without the createWorktree branch-preservation fix would -B-reset the surviving branch and silently discard the committed work Restore promises to recover, so old daemons must update.
  • createWorktree keeps main's newer remote-base-branch fetch and adds the branch-preservation check after it.

Design notes and exit criteria: plans/environment-restore-and-archive-grace-period.md.

Testing

  • turbo typecheck clean across all 57 packages.
  • Unit/integration: server, db, host-workspace, host-daemon-contract, sdk, cli, templates, app — all pass.
  • Regression coverage carried over: grace-window defer-then-destroy, deleted-thread immediate cleanup, unarchive revive, banner Restore action (enabled vs "Cleaning up…"), and the end-to-end integration test that commits work → archives (destroy) → restores → asserts the committed file reappears in the fresh worktree on the same branch.
  • New: CLI command-output tests for bb thread restore-environment.

🤖 Generated with Claude Code

Re-lands PR #242 against the current codebase.

Archiving the last live thread in a managed environment now leaves it
retiring for a 10s grace window (durable across restart, keyed on the
row's updatedAt) before the worktree is destroyed, and un-archiving
within the window revives it losslessly via retire.cancelled. A
10-second "Thread archived — Undo" toast offers the one-click undo.

POST /threads/:id/restore-environment reprovisions a fresh environment
for a thread whose environment is gone; the daemon's createWorktree now
checks out an existing branch in place instead of -B-resetting it, so
committed work is recovered. Exposed in the read-only banner ("Restore
environment"), the SDK (threads.restoreEnvironment), and the CLI
(bb thread restore-environment) with guide/skill docs.

HOST_DAEMON_PROTOCOL_VERSION bumped to 70: an older daemon would
-B-reset the surviving branch and discard the committed work Restore
recovers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SawyerHood

SawyerHood commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

I am SlopCop. I am reviewing this pull request under the configured security, quality, performance, and end-to-end rules.

The stack contains PR #1016 only. I found no later pull request and skipped no pull request.

I will run the security, code quality and architecture, and performance reviews in parallel. I will test the complete stack after those reviews.

@SawyerHood

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

Performance review complete. The database queries use suitable indexes, and the sweep cadence has no regression.

I found one low-impact concern. Undo repeats full thread-list cache invalidation for each archived thread. I will verify this concern during final synthesis.

@SawyerHood

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

Security review complete. Command construction, indexes, concurrent restore claims, and the daemon protocol update look correct.

The review found two medium and two lower-impact concerns. They cover branch identity, restore validation order, the grace-period clock, and route guard consistency. I will verify each concern during final synthesis.

requirePublicThread also 404s threads whose project is deleted, matching
every sibling thread action route.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SawyerHood

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

Code quality and architecture review complete. I found no duplicate environment-restore service. The server, app, database, workspace, CLI, and SDK type checks passed.

The review found three medium concerns. They cover the public-thread guard, restore validation order, and Undo failure handling. It also found several low-impact cleanup items.

if (
refreshedEnvironment.status === "retiring" &&
refreshedEnvironment.path !== null &&
Date.now() - refreshedEnvironment.updatedAt <

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — The grace-period clock can move after retirement.

The environment metadata PATCH route can update a retiring row, and updateEnvironmentMetadataRecord always writes a new updatedAt. Repeated metadata writes can extend this grace check without limit. Store the retirement time separately, or prevent metadata updates from changing the cleanup clock.

action: {
label: "Undo",
onClick: () => {
void Promise.all(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — A partial Undo failure leaves restored threads absent from the sidebar.

Promise.all rejects when one unarchive request fails. The .then block then settles no cache entry, even when other requests succeeded. The successful path also repeats broad list invalidation once per thread. Use Promise.allSettled, report failures, settle each detail entry, and invalidate the list keys once.

return context.json({ ok: true });
});

post(routes.restoreEnvironment, async (context) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — This route omits the public-thread boundary check used by sibling actions.

The service checks thread.deletedAt, but it does not reject a thread whose project has entered soft deletion. A restore can therefore create a worktree while the project deletion sweep removes project environments. Call requirePublicThread at this route boundary before the restore service.

}

if (thread.archivedAt !== null) {
unarchiveThread(deps.db, deps.hub, thread.id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — Restore changes archive state before it validates the request.

The later destroying, missing-environment, unmanaged, and non-writable checks can return 409 after this call unarchives the thread. The restore path also does not send the provider unarchive command that the normal unarchive route sends. Validate first, then use one shared unarchive operation on successful restore paths.

sourcePath: source.path,
baseBranch: storedBaseBranchNameToSpec(destroyed.baseBranch),
workspaceProvisionType: "managed-worktree",
...(destroyed.branchName ? { branchName: destroyed.branchName } : {}),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — Restore can select the last polled branch instead of the managed branch.

callEnvironmentWorkspaceStatus writes the current checkout into environment.branchName. If a user checked out main, this restore request selects main. The worktree add can then fail because another worktree owns main, or it can restore the thread onto the shared default branch. Persist the managed branch identity separately and restore from that stable value.

@SawyerHood

Copy link
Copy Markdown
Collaborator

🚨 SLOP COP 🚨 · review

End-to-end review complete on the top pull request. I created a managed thread, archived it, and confirmed that Undo revived the retiring worktree.

I then let cleanup destroy the worktree. The browser showed Restore environment, and that action created a new managed worktree. The thread returned to idle without a new agent turn.

@SawyerHood SawyerHood left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 SLOP COP 🚨 · review

Review complete for stack PR #1016. I posted five inline findings.

  • Medium: Restore uses a mutable, polled branch value instead of a stable managed branch identity.
  • Medium: Restore unarchives before validation and does not synchronize provider archive state.
  • Medium: The restore route omits the shared public-thread guard.
  • Medium: A partial Undo failure leaves successful unarchives absent from the sidebar.
  • Low: Environment metadata writes can extend the retirement grace clock.

The architecture scan found no duplicate restore service. The new query uses an existing suitable index. The daemon protocol version increased correctly.

I tested the live product at the pull request head. Archive Undo preserved the worktree during grace. Restore created a new worktree after destruction and returned the thread to idle.

Focused local suites passed 241 tests. Six package type checks passed. All current GitHub checks pass.

I submitted a comment review only. I did not approve or request changes.

…t-archive-grace-period-thr_ufty6npytm

# Conflicts:
#	packages/host-daemon-contract/src/commands.ts
#	packages/host-daemon-contract/test/contract.test.ts
#	packages/templates/src/generated/plugin-sdk-dts.generated.ts
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.

2 participants