Skip to content

fix: preserve VM agent releases and explain placement - #1808

Draft
simple-agent-manager[bot] wants to merge 18 commits into
mainfrom
sam/implement-sam-idea-01kzrmp0j6zext5kegcskfspmh-42cmtp
Draft

fix: preserve VM agent releases and explain placement#1808
simple-agent-manager[bot] wants to merge 18 commits into
mainfrom
sam/implement-sam-idea-01kzrmp0j6zext5kegcskfspmh-42cmtp

Conversation

@simple-agent-manager

@simple-agent-manager simple-agent-manager Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve the last actually published reusable-VM agent release across unrelated API/web deploys by resolving a deterministic packages/vm-agent/** + compatibility-marker fingerprint. Unchanged inputs carry the prior exact required SHA and skip the reusable-VM R2 build/upload; changed, first-install, or unproven inputs publish the target SHA before Worker enforcement and fail closed when skip_agent would bypass that transition.
  • Centralize preferred, warm, capacity, trial, and manual node evaluation behind a versioned, tenant-safe placement explanation with typed rejection/provisioning reasons. Persist the explanation through task, trial, manual, and workspace paths; expose it through REST, MCP, OpenAPI, and a default-collapsed workspace sidebar.
  • Preserve the exact VM-agent compatibility predicate. Unknown or genuinely incompatible agents remain ineligible for new placement.
  • Production incident preserved in tests/research: healthy medium/hel1 node 01KZR2JAP92AK3SKW951E4H21M (2/5 workspaces, low load) reported 23e7adc...; deploy run 31492102025 advanced enforcement to fc1e394... despite identical VM-agent fingerprints, forced task 01KZRHQ4PVD1V55YP18H3BWKBF onto a new node, and left placement evidence null.
  • Candidate-loop budget: selection reads one user's workspace nodes and active workspaces. Normal cost is O(n); a worst-case warm-claim race refreshes eligibility and performs one atomic DO claim per eligible warm candidate, yielding O(n²) rows read and O(n) DO calls before provisioning. The first successful claim returns immediately.

Validation

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • Additional validation run (if applicable)
  • If this PR changes candidate selection for a sweep/cron/alarm loop (WHERE clause, status set, join, or equivalent), expected candidate volume and worst-case per-candidate cost are stated in the summary or validation notes (see .claude/rules/47-control-loop-io-budget.md)

Additional local evidence:

  • Root lint/typecheck/test/build passed; API final suite: 539 files / 7,114 tests.
  • VM-agent release/workflow suite: 94/94, including real temporary Git repositories, invalid-ref rejection, and the two production SHAs' identical fingerprint.
  • TaskRunner SQLite vertical slice: 3/3; manual placement review suite: 41/41; shared parser: 9/9.
  • Local Playwright placement audit: 16/16 dark/light mobile/desktop scenarios with accessibility and horizontal-overflow assertions.
  • Mandatory task-completion validator: PASS at ff0b7cd69; no open findings.

Staging Verification (REQUIRED for all code changes — merge-blocking)

  • Staging deployment green — intentionally not run under the task owner's hard constraint
  • Live app verified via Playwright — intentionally not run against staging
  • Existing workflows confirmed working — covered by local tests/static analysis only
  • New feature/fix verified on staging — intentionally not run against staging
  • Infrastructure verification completed — intentionally not run against any environment
  • Mobile and desktop verification notes added for UI changes

Staging Verification Evidence

not deployed to staging. No staging, production, or other environment deployment was performed or triggered. Local tests, static analysis, specialist reviews, and normal PR CI are the authorized verification surfaces.

do not merge. This PR must remain draft/open and unmerged for human review unless Raphaël explicitly changes that instruction.

UI Compliance Checklist (Required for UI changes)

  • Mobile-first layout verified
  • Accessibility checks completed
  • Shared UI components used or exception documented
  • Playwright visual audit run locally — mock data scenarios (normal, long text, empty, many items, error, special chars) tested at mobile (375x667) and desktop (1280x800); no horizontal overflow; screenshots in .codex/tmp/playwright-screenshots/

End-to-End Verification (Required for multi-component changes)

  • Data flow traced from user input to final outcome with code path citations
  • Capability test exercises the complete happy path across system boundaries
  • All spec/doc assumptions about existing behavior verified against code
  • If any gap exists between automated test coverage and full E2E, manual verification steps documented below

Data Flow Trace

  1. Release: .github/workflows/deploy-reusable.yml calls scripts/deploy/resolve-vm-agent-release.ts, which delegates deterministic Git-tree/marker resolution to scripts/deploy/vm-agent-release.ts; the resulting exact release/fingerprint drives conditional binary publication and every sync-wrangler-config.ts Worker sync before API deployment.
  2. Task placement: handleNodeSelection() in apps/api/src/durable-objects/task-runner/node-steps.ts calls selectNodeWithExplanation() in apps/api/src/services/node-selector.ts, then persistTaskPlacement() writes the task record before step advance. Provisioning/readiness append typed outcomes, and handleWorkspaceCreation() copies the finalized explanation before workspace dispatch.
  3. Other placement paths: the trial orchestrator and manual workspace route use the same canonical selector/evaluator and persist their own pre-workspace and terminal evidence.
  4. Exposure: safe shared parsing feeds REST mappers/OpenAPI and MCP enrichment; WorkspaceSidebarInfrastructure.tsx renders the mapped explanation in the default-collapsed placement section.

Untested Gaps

The live Cloudflare settings/R2/Worker and VM provisioning boundaries were not exercised because this task explicitly prohibits every environment deployment. Those boundaries are covered by resolver/workflow contract tests, real Git-backed fingerprint tests, schema-backed SQLite vertical slices, and specialist review. Human staging verification remains required before any future merge authorization.

Post-Mortem (Required for bug fix PRs)

What broke

An unrelated application deploy changed the required VM-agent version to the deploy SHA even though VM-agent build inputs were unchanged. Healthy existing nodes then failed exact compatibility, forcing unnecessary provisioning, while null placement evidence hid the reason.

Root cause

The legacy Resolve and Verify Deployment SHA behavior in .github/workflows/deploy-reusable.yml used the application deployment SHA as the reusable-VM release identity on every normal deploy and emitted an empty requirement for skip_agent; placement logic was duplicated and did not durably explain rejections.

Class of bug

Release-identity coupling plus missing cross-boundary observability.

Why it wasn't caught

Workflow tests did not model unchanged VM-agent inputs across distinct application SHAs, and placement tests did not previously carry a real TaskRunner decision through task persistence into workspace creation.

Process fix included in this PR

  • .claude/rules/54-vm-agent-rollout-compatibility.md now documents deterministic carry-forward, explicit compatibility bumps, publication ordering, and fail-closed behavior.
  • Git-backed release tests cover source/toolchain/build/marker changes and unrelated deploys.
  • Real SQLite vertical slices cover TaskRunner, trial, and manual placement persistence boundaries.

Post-mortem file

tasks/active/2026-08-11-vm-agent-release-placement-explanation.md

Specialist Review Evidence (Required for agent-authored PRs)

  • All local reviewers completed and findings addressed before merge
  • If any reviewer did NOT complete: N/A — every reviewer completed
Reviewer Status Outcome
Cloudflare specialist PASS Release metadata, settings allowlist, migration ordering, and R2 publication behavior verified after fixes.
Environment validator PASS Generated bindings and documentation remain consistent; no manual secret prerequisite added.
Security auditor ADDRESSED Bounded/aliased placement passed; strict SHA/argv/option guards justify the documented Sonar false-positive suppression at the private Git sink.
UI/UX specialist PASS Collapsed sidebar, accessibility, responsive behavior, and 16 local Playwright scenarios passed.
Documentation validator ADDRESSED Release semantics, MCP/OpenAPI/parser contracts, and public docs synchronized.
Constitution validator ADDRESSED Removed hidden ceilings/truncation/weights and duplicated defaults; Principle XI passed.
Test engineer ADDRESSED Real SQLite reuse/provision/failure vertical slices and exact timeout persistence passed.
Task-completion validator PASS Full branch/task review passed at ff0b7cd69; no remaining implementation gap.

Exceptions (If any)

  • Scope: staging/live-environment verification and merge.
  • Rationale: Raphaël explicitly required no deployment to staging or any environment and no merge; local tests, static analysis, CI, and specialist review replace the normal /do staging phase for this draft handoff.
  • Expiration: only an explicit later instruction from Raphaël may authorize staging or merge.

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • external-api-change
  • cross-component-change
  • business-logic-change
  • public-surface-change
  • docs-sync-change
  • security-sensitive-change
  • ui-change
  • infra-change

External References

N/A: the source material was the SAM production idea/diagnosis, production run evidence, repository rules, and current implementation; no third-party API contract was introduced.

Codebase Impact Analysis

Affected paths include the reusable deploy workflow/scripts, API TaskRunner/trial/manual placement and D1 schema, shared placement types/parser, REST/OpenAPI/MCP exposure, workspace sidebar UI, tests, environment references, and rollout/public documentation. packages/vm-agent source is intentionally unchanged.

Documentation & Specs

Updated rollout compatibility rules, environment references, architecture overview, agents/self-hosting guides, API reference, configuration reference, generated OpenAPI, and the active task record.

Constitution & Risk Check

Checked Principle XI and repository rollout/security rules. Critical risks were accidental compatibility weakening, unpublished-version enforcement, leaking node/provider/version data, and stale cross-boundary placement evidence. The implementation retains exact compatibility, publishes before enforcement, fails closed, aliases unselected identifiers, allowlists evidence, and verifies task-to-workspace ordering behaviorally.

@codspeed-hq

codspeed-hq Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing sam/implement-sam-idea-01kzrmp0j6zext5kegcskfspmh-42cmtp (0731318) with main (fc1e394)

Open in CodSpeed

@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant