Skip to content

feat(environments): add a lossless archive grace period - #1016

Merged
ymichael merged 10 commits into
mainfrom
bb/restore-environment-archive-grace-period-thr_ufty6npytm
Aug 11, 2026
Merged

feat(environments): add a lossless archive grace period#1016
ymichael merged 10 commits into
mainfrom
bb/restore-environment-archive-grace-period-thr_ufty6npytm

Conversation

@ymichael

@ymichael ymichael commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Problem

Archiving the last live thread in a managed environment immediately destroyed its worktree. An accidental archive could therefore lose uncommitted work before the user had a chance to undo it.

What changed

Lossless archive recovery

  • Managed worktrees remain in retiring for five minutes before destruction.
  • The grace clock is persisted in the lifecycle-owned retireRequestedAt field, so metadata writes cannot extend it and server restarts cannot bypass it.
  • Grace applies only while a non-deleted archived thread can still be revived; deleted or tombstoned-only environments clean up immediately.
  • The archive toast is emitted once, remains visible for 10 seconds, and offers Undo.
  • Toast Undo and the archived thread's normal Unarchive action both emit the existing retire.cancelled transition during the grace window, preserving the same environment and intact worktree.

Cleanup lifecycle hardening

  • Periodic cleanup continues evaluating retiring environments while the server owns the five-minute grace decision.
  • Startup recovery respects the orphan timeout instead of immediately failing an in-flight destroy command.
  • Destroy completion is correlated to its attempt id. A matching late success can converge an error row to destroyed, while a stale result cannot settle a newer attempt.
  • The removed workspace path is cleared when destruction completes; destroyed rows keep the normal seven-day retention policy.

Archived UI

  • While cleanup runs, the read-only banner says Archiving environment....
  • After cleanup, it says Environment archived.
  • There is no destroyed-environment Continue in new thread action in this PR; that workflow will be handled separately.

Boundaries and data model

  • Adds the nullable environments.retireRequestedAt column and migration, backfilled from updatedAt for rows already retiring.
  • Adds a targeted query for whether a retiring environment has a revivable archived thread.
  • Does not change the create-thread HTTP, SDK, CLI, plugin SDK, or host-daemon contracts.
  • Does not add a restore route or replace the environment attached to the archived thread.
  • destroyed remains terminal, and HOST_DAEMON_PROTOCOL_VERSION is unchanged.

Testing

  • App coverage for the single 10-second Undo toast and status-only archived-environment banner.
  • Database coverage for lifecycle timestamps, attempt correlation, pruning, and migration backfill.
  • Server coverage for grace deferral, cancellation, expiry, restart recovery, late completion, stale attempts, and deletion.
  • Integration coverage verifies that unarchiving after destruction does not reprovision the destroyed environment.
  • Affected app, server, database, domain, CLI, contract, plugin SDK, template, and integration typechecks pass locally.

Design notes: plans/environment-archive-grace-period.md.

@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.

@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.

@ymichael
ymichael force-pushed the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch from 9f70124 to d07b36b Compare August 7, 2026 21:45
@ymichael ymichael changed the title feat(environments): archive grace period + Restore environment feat(environments): add archive undo and continue in new thread Aug 7, 2026
@ymichael
ymichael force-pushed the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch from 685460b to ed16cd6 Compare August 7, 2026 23:01
@lawrenceluk

Copy link
Copy Markdown

Hihi, I ran into a possibly related failure while archiving completed delegated work.

Agent-message below:

The managed workspace was successfully removed, but bb displayed the environment as unavailable instead of showing a successfully archived run.

The logs show a race during startup recovery: bb moved an in-progress cleanup from destroying to error because it no longer expected a result. The daemon then reported that cleanup had succeeded, but bb rejected the late result because the state machine has no completion transition from error.

The corresponding log entry was:

no transition for destroy.completed from status error

I found this pattern for two separate environments locally.

This seems relevant to #1016 because that PR defines the experience after a managed thread is archived. If this race occurs, someone could see an environment error instead of the intended archived state and “Continue in new thread” option.

There may also be a longer-term issue with that continuation flow: it appears to depend on the destroyed environment’s branch and host information, while bb currently deletes destroyed environment records after seven days. Is “Continue in new thread” intended to keep working after that retention window?

Are either of these cases already covered by #1016, or would they make sense to address as part of the same lifecycle work?

@ymichael

Copy link
Copy Markdown
Collaborator Author

thanks for the comment @lawrenceluk - will fold this into the PR!

@ymichael
ymichael force-pushed the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch 3 times, most recently from 438eb6c to 07f9674 Compare August 11, 2026 03:57
@ymichael
ymichael force-pushed the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch from 07f9674 to a68c24e Compare August 11, 2026 16:52
@ymichael
ymichael force-pushed the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch from a68c24e to a7887b5 Compare August 11, 2026 16:53
@ymichael ymichael changed the title feat(environments): add archive undo and continue in new thread feat(environments): add a lossless archive grace period Aug 11, 2026
@ymichael
ymichael merged commit 99d6515 into main Aug 11, 2026
10 checks passed
@ymichael
ymichael deleted the bb/restore-environment-archive-grace-period-thr_ufty6npytm branch August 11, 2026 17:08
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.

3 participants