Skip to content

[Feat] Add persisted environment verification state#384

Merged
brunobergher merged 23 commits into
developfrom
feat/environment-verification-state-0t2zdpugqvr9h
Jul 15, 2026
Merged

[Feat] Add persisted environment verification state#384
brunobergher merged 23 commits into
developfrom
feat/environment-verification-state-0t2zdpugqvr9h

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Opened on behalf of Bruno Bergher. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

Related issue

Internal Roomote work.

Why this PR exists

  • I am a maintainer / this is internal Roomote work

What changed

Environments now carry a persisted verification state that is separate from "a definition exists". A new environment is Configured until a follow-up verification task confirms it works, at which point it becomes Verified; onboarding stays finishable as soon as configuration completes and verification continues asynchronously.

  • Data model: added isVerified (NOT NULL DEFAULT true), verificationTaskId, verifiedAt, and verificationError to environments (migration 0012). Existing environments stay verified via the default; newly created environments start unverified.
  • Centralized reset: any runtime-affecting edit (config JSON or repository mappings) clears verification through the shared updateEnvironmentDefinition helper, while name/description-only edits preserve it, so the API, Settings, agent, and declarative write paths cannot drift. Declarative (file-managed) environments opt out of the reset.
  • Verification result contract: the manage_environments MCP tool gains a record_verification action. The recording task's identity is derived server-side from its run token and matched against the environment's current verificationTaskId, so a stale or superseded attempt cannot overwrite a newer one, and only the active verification flow for that environment can record. Failure messages are sanitized before persistence (no secrets or environment YAML).
  • Retry command: a new authenticated environments.retryVerification mutation confirms edit access, rejects a concurrent active verification, resets the environment to unverified, enqueues a normal standard task marked to verify that environment, and stores the returned task id. Verification tasks appear in normal task history with no special badge.
  • Environment-setup skill: now records the verification outcome through record_verification (success only when the environment explicitly looks ready, otherwise a user-safe failure), and treats a superseded recording as a stop signal.
  • Web UI: the environment query exposes the verification fields plus a verificationTaskActive flag. The onboarding widget distinguishes configured / verification-in-progress / verified / failed states, still lets you finish setup while verification runs, and offers Retry verification / Edit environment / View task actions. The Settings Environments page shows a per-environment verification badge, the last failure message, a Retry verification button, and a link to the related task, polling while a verification task is active. "Verification in progress" only shows while the task actually has an active run, so a crashed/unreported attempt falls back to Configured.

Concurrency hardening

  • The verification-task registration now happens atomically inside the same row-locked config update (a registerVerificationTaskId option on the shared update helper; create sets it directly in the insert), so a concurrent edit can no longer associate an older setup task with a newer configuration.
  • The retry command serializes its active-run check, task enqueue, and attempt registration under a per-environment advisory lock, so two concurrent retries cannot both enqueue duplicate verification runs. It claims the attempt in enqueueTask's beforeEnqueue hook, so the environment points at the new task id before the run is eligible to start and the run's record_verification cannot be rejected as a mismatched attempt. The active-attempt guard also recognizes an in-flight initial onboarding verification (a setup_onboarding run marked with environmentDefinitionId).
  • Env-var request follow-up: when the follow-up prompt send fails after variables were saved, a durable hidden fulfillment envelope is now persisted (taskEnvVarRequests.markFulfilled), so a reconnect or history refetch does not resurface an already-handled variable request.

How it was tested

  • New unit tests: DB verification helpers (record success/failure, stale-attempt rejection, runtime-affecting vs metadata-only reset, atomic task registration, declarative preserve), the MCP record_verification handler, the record_verification API endpoint (authorization, stale/superseded 409, and error sanitization including lowercase/YAML secret lines), the retryEnvironmentVerification command (enqueue + registration, concurrent-verification rejection, serialized-retry concurrency, missing environment), the active-verification guard against a real DB (retry marker, onboarding-setup marker, cross-environment isolation), and the env-var fulfillment persistence.
  • Updated existing environment settings/onboarding UI tests and the MCP tool-description test for the now-optional definition field.
  • pnpm lint, pnpm check-types, and pnpm knip pass locally.
  • Visual proof of the Settings verification UI is embedded below.

Screenshots

Environments settings page showing Verified, Configured, and Verification failed badges with the failure message and Retry verification action

Failed verification row showing the Verification failed badge, sanitized failure message, and Retry verification button

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

@roomote-roomote

roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

No new code issues found. See task

  • apps/api/src/handlers/environments/updateEnvironment.ts:181-188 Registering the verification task after the configuration transaction lets concurrent edits associate an older setup task with the newest configuration. If update A commits, update B commits, B registers, and A's delayed registration runs last, task A can report success and mark B's configuration as verified. Register the attempt while holding the same environment-row lock as the update, or condition the registration on the configuration version written by that task.
  • apps/web/src/trpc/commands/environments/index.ts:895-930 The active-run check and attempt registration are separated by task enqueueing, so two retries can both see no active run and enqueue. The last registration wins while the other task still consumes a sandbox and receives a 409 when recording its result. Claim the environment's verification attempt atomically before the task is eligible to run, and add a concurrency test.
  • packages/db/src/lib/environment-definitions.ts:358-361 The retry lock executes n_xact_lock(...), but PostgreSQL only provides pg_advisory_xact_lock(...). Every Retry verification request therefore fails with function n_xact_lock(integer) does not exist before the active-run check or enqueue; use the correct function name and exercise this helper against Postgres in a test.
  • apps/web/src/trpc/commands/environments/index.ts:861 The retry guard only checks verifiesEnvironmentId, but the active setup_onboarding task that performs the initial verification is marked with environmentDefinitionId and is explicitly authorized to record that result. A retry from a stale client can therefore start while setup is still verifying, replace the setup task's verificationTaskId, and force its eventual record to be rejected as superseded. Include the setup-task marker in this active-attempt check (and add a regression test) so the mutation keeps its promised concurrent-verification rejection.
  • apps/web/src/trpc/commands/environments/index.ts:946-970 enqueueTask commits and pushes the verification run before beginEnvironmentVerification stores its task id. The controller can therefore start the run and its record_verification call can be rejected as a mismatched attempt, after which the setup skill stops as superseded and leaves the retry unverified. Register through enqueueTask's beforeEnqueue hook (while holding the advisory transaction) so the attempt is claimed before the run becomes eligible.
  • apps/web/src/app/(sandbox)/task/[taskId]/PendingEnvVarRequestPanel.tsx:222-230 When sendPrompt fails, the fulfillment marker is only added to the local store. On a page refresh or history refetch, SandboxProvider rebuilds the pending request from persisted envelopes and shows the same variable request again even though the values were saved and the success toast said it was handled. Persist a hidden fulfillment envelope for this failure path, or retain a durable dismissal, so reconnection does not make users re-submit the request.
  • apps/web/src/trpc/commands/environments/index.ts:967-976 beforeEnqueue runs only after enqueueTask's internal run-creation transaction has committed, while beginEnvironmentVerification(tx, ...) writes through the outer advisory-lock transaction. After the hook returns, enqueueTask pushes the run to the controller queue before that outer transaction commits (the callback cannot return until enqueueing finishes). The controller can therefore start the task and call record_verification before its task id is visible, yielding the same mismatched-attempt rejection. Register through afterCreateInTransaction with enqueue's transaction, so the task id and environment claim commit before queueing.
  • apps/web/src/app/(sandbox)/task/[taskId]/ErrorFallback.tsx:109-111 The restored Try again button refreshes the connection target but discards it instead of invoking the sandbox reconnect callback. After initial websocket retries are exhausted, refreshing an unchanged URL/token does not restart the transport, so the button remains a no-op; a rejected refresh is also left unhandled. Restore the refresh-and-reconnect flow and handle refresh failures.
  • apps/web/src/trpc/commands/sandbox-session/index.ts:618-632 The lookup now filters to active runs, but Drizzle findFirst returns undefined when none match and undefined !== null evaluates true. Any stale verificationTaskId is therefore still exposed as active and onboarding remains stuck on Verification in progress; use a truthiness check for the matched run.
  • apps/web/src/components/system/primitives/badge.tsx:8 Removing the shared border utility leaves the outline variant, and variants that only set a border color, with zero border width. Existing outline badges therefore lose their outline across the app; keep a border width in the base or add it to the affected variants.
  • apps/web/src/components/settings/environments/EnvironmentVerificationStatus.tsx:109-127,183-195 Verified and Configured badges always say they can be clicked and always show an external-link icon, even when verificationTaskId is null and the badge is not wrapped in a link. Existing migrated environments and runtime edits commonly have no task id, so condition the copy and icon on an actual link target.

Reviewed 5f666c9

@roomote-roomote

roomote-roomote Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

All review findings addressed; branch merged up to current develop.

Verification-attempt claiming (a832748, refining earlier commits):

  • The retry command now registers the environment's verificationTaskId through enqueueTask's afterCreateInTransaction hook, so the claim commits atomically with the run row (inside enqueue's own transaction) and before the run is pushed to the controller queue. This closes the residual race the earlier beforeEnqueue approach left, where the outer advisory-lock transaction committed the claim only after the run was already created and queued — the run's record_verification can no longer be rejected as a mismatched attempt.
  • The per-environment advisory lock still serializes concurrent retries; the active-attempt guard also recognizes an in-flight initial setup_onboarding verification (environmentDefinitionId).

Earlier fixes retained: atomic registration inside the row-locked config update on create/edit; the retry lock uses pg_advisory_xact_lock (exercised against Postgres); and the env-var request panel persists a durable fulfillment envelope when the follow-up prompt send fails.

Real-DB regression tests cover the attempt-claiming, onboarding-active guard, and env-var fulfillment paths. pnpm lint, pnpm check-types, pnpm knip, the full @roomote/db suite, and the affected web/api tests pass locally.

@brunobergher brunobergher force-pushed the feat/environment-verification-state-0t2zdpugqvr9h branch from 625e31d to 1d074af Compare July 15, 2026 15:05
@brunobergher brunobergher marked this pull request as ready for review July 15, 2026 19:52
@brunobergher brunobergher merged commit 55aa93e into develop Jul 15, 2026
18 checks passed
@brunobergher brunobergher deleted the feat/environment-verification-state-0t2zdpugqvr9h branch July 15, 2026 19:53
daniel-lxs added a commit that referenced this pull request Jul 15, 2026
…s 0013

SQL unchanged, renumbered only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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