Skip to content

🤖 fix: make workspace resourceVersion a fingerprint of the returned representation - #115

Merged
ThomasK33 merged 3 commits into
mainfrom
fix/workspace-rv-e2e
Sep 23, 2026
Merged

ThomasK33 merged 3 commits into
mainfrom
fix/workspace-rv-e2e

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

Make CoderWorkspace.metadata.resourceVersion an opaque fingerprint of the representation this API returns: the full SHA-256 of the converted object, serialized with resourceVersion unset. On Coder 2.37.2, builds, renames, TTL and autostart changes do not advance the workspace updated_at that the old token came from. So a stale token still matched, and UPDATE and DELETE preconditions did not detect those changes.

Part of #109. The required Kind E2E now proves stale-token rejection on a real backend, using the maintained workspace driver added in #114.

Background

  • The old token was Workspace.UpdatedAt in nanoseconds. Coder 2.37.2 leaves that timestamp unchanged across builds, rename, TTL and autostart. Earlier runtime evidence showed the unchanged timestamp and the stale token still being accepted.
  • Kubernetes treats resourceVersion as opaque: clients may compare it only for equality. This server does not implement watch resumption or resourceVersionMatch, so a non-monotonic token is permitted.
  • Template resourceVersion is unchanged (template metadata updates already change it).

Implementation

  • internal/aggregated/convert/workspace.go: build the CoderWorkspace with resourceVersion unset, serialize it, and assign the hex SHA-256. Every emitted metadata/spec/status field takes part, including status.lastUsedAt and status.autoShutdown. A serialization error is an impossible state and panics with an assertion failed: message.
  • internal/aggregated/storage/workspace.go: remove the three places that patched the response with the build's UpdatedAt. GET, LIST, mutation responses and local watch events now share one conversion.
  • UPDATE (always) and DELETE (when preconditions.resourceVersion is supplied) compare against the freshly fetched object and return 409 Conflict before any Coder mutation. This is a snapshot check, not an atomic compare-and-swap; uid still guards recreation identity.
  • Tests: converter tokens follow each exposed field family and stay stable for identical input or backend-only timestamp changes. A storage flow with a frozen updated_at covers GET stability, UPDATE with a changed token, the local Modified event, LIST presence, stale UPDATE and stale DELETE returning 409 without a build, live delete, and out-of-band TTL, autostart, build status and rename.
  • docs/how-to/deploy-aggregated-apiserver.md: documents the contract, compatibility limits and watch options (current opaque token; omit sendInitialEvents and resourceVersionMatch).
  • hack/e2e-workspace-lifecycle.sh: in the stopped-workspace rename case, capture the genuine pre-rename token. After the out-of-band rename, require:
    • the same UID and a changed name and token;
    • a stale UPDATE (current object, old token) and a stale DELETE (live UID, old token) each return 409, with the backend latest build, build count and object unchanged;
    • GET and LIST agree on the renamed object's token.
  • Offline tests model the old timestamp behavior and require the driver to fail on it. Other negatives cover an accepted stale UPDATE or DELETE, stale side effects on the latest build, build count or object, a missing LIST item, and a LIST/GET token mismatch. Each stops before later mutations.

Validation

  • make test, make build, make verify-vendor, make lint, make docs-check (strict), make test-scripts, bash -n, shellcheck -x and actionlint pass on 00065225.
  • Removing each new driver guard makes the offline tests fail. The one exception is the redundant "name changed" check, which earlier rename checks already cover.
  • This PR's E2E run on the exact head is the runtime evidence for the stale-token cases. The run ID and receipt will be added in a comment.

Risks

  • Token format change: clients holding timestamp-format tokens get one 409 after upgrade and must re-read. Tokens are not monotonic and can repeat if the whole representation returns to an earlier value.
  • More conflicts from activity: lastUsedAt and build status are part of the token, so activity between a read and a write can produce a 409 without a user edit. That is intended; re-read and retry.
  • Watch: unchanged. Events cover only writes through this server, with no replay and no out-of-band notifications (🤖 fix: clarify event-only watch options after Kubernetes defaulting #112 tracks option compatibility).
  • Size: 577 changed lines, over the original ~500 target. 479 lines are the fix itself; the rest are the E2E assertions that supply its runtime evidence.

📋 Implementation Plan

Workspace representation versions (#109)

Goal and scope

Make CoderWorkspace resourceVersion detect changes in the representation returned by this API, even when Coder does not advance Workspace.UpdatedAt. Keep UID identity checks and the existing non-atomic backend mutation boundary. One cohesive PR, target <=500 changed lines including tests/docs. No template-version change, persistent counter, watch redesign, installer change, or import-readiness work.

Contract

  1. Treat workspace resourceVersion as an opaque representation fingerprint. Compute full SHA-256 over the serialized CoderWorkspace projection before assigning its resourceVersion. Cover all metadata/spec/status that the converter emits, including lastUsedAt and autoShutdown. Hash the projection, not the entire backend SDK value; backend-only timestamp changes do not describe a changed projection.
  2. Identical serialized projections have identical tokens. A different projected field must affect the fingerprint; hash collision resistance is probabilistic, not an absolute uniqueness guarantee. No exclusions for exposed activity fields: activity that changes status can cause a legitimate 409 even without a user edit.
  3. This is not a monotonic revision or history cursor. Returning to an identical projection can return the same token (for example, TTL A→B→A). No guarantee of detecting an intervening change that has reverted before the snapshot. Clients must not parse or order tokens.
  4. GET, LIST, mutation responses, and local watch events use the same converter. A mutation response and the following GET agree only if the backend projection did not change between them. Build progression or activity can legitimately change it.
  5. UPDATE and DELETE compare against the freshly fetched snapshot. A supplied old token for a currently different representation returns 409 before the backend mutation. This is not an atomic Coder compare-and-swap; a change after that fetch remains possible. UID still protects recreation identity.
  6. Existing timestamp-format workspace tokens can fail after upgrade; clients re-read before retrying. Template resourceVersion behavior remains unchanged. Existing watch is local in-process write notification only: no out-of-band Coder events, no replay, resourceVersion ignored, resourceVersionMatch rejected. This change makes no full Kubernetes storage/revision guarantees.

Implementation and quality gates

  1. Start from exact landed base 2fcd31d on a new owned branch. Preserve prior delivery/source refs. Carry the regression tests proven red on this base into normal repository style. Check: exact base, clean start, frozen evidence preserved.
  2. Build the workspace projection, serialize it with resourceVersion unset, hash using full SHA-256, assign the opaque value. Fail fast with the existing assertion style if serialization unexpectedly fails. Remove only the three storage build-to-UpdatedAt substitutions made obsolete by this change; preserve LatestBuild replacement and mutation ordering. Check: original converter/storage reproduction becomes green.
  3. Tests cover each exposed field family: rename, template/version, running/build identity/status, TTL/autostart, lastUsedAt/autoShutdown, identity/namespace/creation metadata. Preserve stable identical reads and delete/recreate UID distinction. Include quiescent GET→UPDATE→GET→LIST, stale UPDATE and stale DELETE→409 with no extra build, current UID+RV→delete proceeds, and local Modified watch event token matching the mutation response. Assert LIST presence, not only token equality. Backend-only UpdatedAt changes must not change an otherwise identical projection. Check: focused tests pass, control packages and prior identity/namespace/delete regressions pass.
  4. Update user-facing deployment docs with the contract and compatibility limits above; no API struct changes or generated-reference churn expected. Check: make test, build, verify-vendor, lint, docs-check; other established relevant gates as applicable. Every executable wrapper/helper must use new process-local output/cache paths and be audited before gates run. No old docs site or frozen packet may be regenerated.
  5. Commit the validated change with required attribution. Check clean checkout and final commit/tree, then run exact-commit hands-on validation below. No publication before required evidence is complete.

Hands-on dogfooding

Use one new owned KIND cluster, namespace, image tag, KUBECONFIG, cache/temp and evidence root. Build/load the exact committed source and run Coder v2.37.2. Prepare all scripts and an output/resource ownership audit before launch. Never use a shared deployment or change global settings.

  1. Create a workspace; capture GET/LIST with identical projection and UID. Mutate backend build, rename, TTL and autostart one at a time; fetch the new canonical name after rename. Verify same ID, changed projected fields, changed token, and GET/LIST agreement when snapshots are otherwise equal.
  2. Do not blindly require adjacent GET tokens to agree during live status progression. Capture both objects; equal projections must yield equal tokens. Establish a quiescent stopped fixture for stable-read checks. Activity fields remain in the token.
  3. For each genuine changed same-ID projection, submit the preceding token in DELETE preconditions: expect409, verify the current object/build remains intact and no delete transition occurred. Separately use live UID+RV on a quiescent fixture: expect ordinary asynchronous deletion. Recreate the same name: expect a new UID and rejection of the prior UID.
  4. Exercise API-driven UPDATE and the local Modified watch event with matching event/response token; keep the established resourceVersion/resourceVersionMatch watch-option behavior. Do not claim out-of-band notifications. Re-run canonical-name400, wrong-UID409 and namespace-isolation regressions. Check current CLI GET/apply/update flows do not parse the hash as a number.
  5. Record raw commands and native exits plus a terminal screenshot, raw cast, and playable video of the steps and verdict. Label any replay, trimming or speed changes accurately. Bind media to source SHA and receipts; preserve initial failures. Verify screenshot/video semantic frames, then remove only the exact owned cluster/processes/resources and record cleanup.

Stop conditions

Stop and report a concrete blocker on unexplained source/output mutation, setup mismatch, missing native receipts, contract conflict, or inability to demonstrate status-inclusive tokens through real backend operations. Keep implementation small; do not add persistence, polling, field exclusions or a new subsystem to force a green test. Publication, required reviews, final independent recommendation, merge queue and post-merge verification remain separate gates owned by the maintainer desk.

#109 PLAN addendum — runtime validation through the maintained E2E (the accepted PLAN.md contract is unchanged)

Why the sequencing changes

The required Kind E2E now runs a maintained CoderWorkspace lifecycle driver on the exact PR head for aggregated-API pull requests (#113, landed f8dab5b). That replaces the ad hoc local harness for runtime evidence. Earlier local packets (v5–v8, Stage-1, supplement) stay FAIL and are not relabeled; none of their scripts are reused.

Integration

  1. New branch fix/workspace-rv-e2e from main f8dab5b. Cherry-pick d4737060 and 67067993 unchanged; they touch no 🤖 ci: cover CoderWorkspace lifecycle in the Kind E2E job #113 files. Keep 67067993 and all frozen packets intact.
  2. Re-run all final-head gates on the integrated head, including docs gates. Earlier static results do not carry over to the new artifact.

New runtime assertions (maintained driver only, small extension)

Reuse the stopped-workspace rename scenario:

  • Capture the genuine pre-rename object and token.
  • After the out-of-band rename: same UID, changed canonical name, changed token.
  • Stale UPDATE (body built from the current object under the new name, with the old token) returns 409.
  • Stale DELETE (new name, old token, live UID) returns 409.
  • After both: backend latest build ID, build count and object state unchanged.
  • GET and LIST agree on the renamed object and its token when the projection is unchanged; LIST presence is asserted.
    Existing watch/update, fresh-token delete, recreate and prior-UID checks continue.

Offline qualification before publication

Stubbed-tool tests model the old timestamp behavior (token unchanged by rename) and require the driver to fail on it. Negatives: stale preconditions ignored (request succeeds), token equality where it must differ, renamed item missing from LIST, and any failure stopping later mutations.

Runtime and review bounds

One PR-triggered E2E run on the final commit is authorized after offline gates pass. Classify any failure before any further attempt; no automatic rerun. Six assessments total including automatic reviews on open; one final independent assessment reserved. Merge-queue E2E and post-merge checks still required.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh • Cost: $965.57

…epresentation

CoderWorkspace.metadata.resourceVersion was Coder's Workspace.UpdatedAt in nanoseconds. On Coder
2.37.2 builds, rename, TTL and autostart changes did not advance that timestamp, so the exposed
value did not identify those same-object changes and an old token still matched in UPDATE and
DELETE precondition checks (#109).

Compute the token from what this API returns instead: the converter builds the CoderWorkspace with
resourceVersion unset, serializes it and assigns the full hex SHA-256. Every emitted metadata, spec
and status field takes part, including lastUsedAt and autoShutdown, so an exposed change alters
the token while a backend-only timestamp change with an identical projection keeps it. The token
is opaque and not monotonic: an identical representation returns the same token again. UPDATE and
DELETE keep comparing against the freshly fetched snapshot (not an atomic compare-and-swap), and
uid still protects recreation identity. The three storage paths that patched the response with the
build's UpdatedAt are removed; the LatestBuild replacement and mutation order stay.

Tests reproduce the symptom with a mock that keeps updated_at frozen: converter tokens track each
exposed field family (rename, template and version, build id and status, TTL, autostart,
lastUsedAt, autoShutdown, identity and namespace metadata) and stay stable for identical or
backend-only changes; the storage flow covers GET stability, UPDATE with a changed token, the local
Modified watch event, LIST presence, stale UPDATE and stale DELETE returning 409 without a build
transition, a live uid+token delete, and out-of-band TTL, autostart, build status and rename
changes. The how-to documents the contract and compatibility limits (tokens from earlier releases
no longer match; template resourceVersion and watch behavior are unchanged).

Part of #109

_Generated with `mux` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$254.50`_

<!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=254.50 -->

Change-Id: I2682de8b16153df39c3456b63f680893c7ad52ce
Signed-off-by: Thomas Kosiewski <tk@coder.com>

---
_Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh`_
Correct the watch bullet added with the representation-fingerprint
resourceVersion: state the working request (supply the current opaque
token, omit sendInitialEvents and resourceVersionMatch; the token is
ignored once the options are accepted and is not a replay cursor) and
record the observed rejections under WatchList defaulting (omitted or
"0" resourceVersion -> 400 because initial events are unsupported;
resourceVersionMatch rejected; sendInitialEvents=false without a
matching option rejected upstream with 422).

Documentation only; no product, watch, or test changes. Broader HTTP
watch option coverage is tracked in #112.

_Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh`_

Change-Id: I0d310b02c8c7c709a7719a2e43e06d0a2ee99a13
Signed-off-by: Thomas Kosiewski <tk@coder.com>

Integrated the doc-only correction without changing the validated tree.

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---
_Generated with [`xum`](https://github.com/coder/xum) • Model: `coder:openai/gpt-6-astra` • Thinking: `xhigh`_
Extend the stopped-workspace rename case of hack/e2e-workspace-lifecycle.sh:
capture the genuine pre-rename object and token; after the out-of-band rename
require the same UID, a changed canonical name and a changed token; require
409 for a stale UPDATE (current object under the new name, old token) and a
stale DELETE (new name, live UID, old token); then require the backend latest
build ID, the backend build count and the object to be unchanged. A new case
requires the renamed object in LIST with the same token as GET. The receipt
records both tokens and the new cases.

Offline tests model the old timestamp behavior (rename keeps the token) and
negatives for accepted stale UPDATE/DELETE, rejected-but-mutating stale
requests, a missing LIST item and a LIST/GET token mismatch; each stops before
the wrong-UID/live deletes and the recreate.

_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `xhigh`_

Change-Id: I325f475ec99aaeab3597e5b80fc1616eb50f30a8
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-23T08:49:54.392624Z 0006522 PR opened
🔒 Security Review Completed 2026-09-23T08:57:40.803578Z 0006522 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@ThomasK33

ThomasK33 commented Sep 23, 2026

Copy link
Copy Markdown
Member Author

#109 runtime evidence on 00065225

The required E2E (Kind + CNPG + Templates) job passed on the exact PR head, including the new #109 assertions.

  • Run: 35838832500, attempt 1, job 107109501349, conclusion success. Docs run 35838832456 passed as well.
  • Source: 00065225bc2ad210fa99d33ec1e0223bd516b9b6. Backend: Coder v2.37.2+eb69e27 from the pinned digest. Image identity: built and serving image IDs are both sha256:0c48374b…9e53.
  • 🤖 fix: make workspace resource versions reflect backend changes #109 cases, on a real backend:
    • An out-of-band Coder rename of the stopped workspace kept the UID and changed resourceVersion from 781ea2b3…cfdf0e to c59006f6…bb8375. Under the old timestamp token, this rename would not have changed it.
    • A stale UPDATE (current object, pre-rename token) and a stale DELETE (live UID, pre-rename token) each returned 409. The backend's latest build, build count, and the object were unchanged afterwards.
    • GET and LIST returned the same token for the renamed object.
  • Other cases: all 9 existing lifecycle cases passed as well, 11 of 11 in total.
  • First attempt: this was the single authorized run for this PR. The earlier local runtime attempts for 🤖 fix: make workspace resource versions reflect backend changes #109 stay recorded as failures; none of their scripts were used here.

Media

Upload status update (10:55Z): both links below now load. The served files match the local SHA-256 hashes (screenshot c4e69094…, video 43f641e7…). For a while after upload they returned 404. The CI log linked above remains the primary evidence.

Both files are a terminal replay of the recorded GitHub Actions job log, not a live run. The replay prints the lifecycle step's log lines unchanged, under a header with the run, job, head SHA, and the log's SHA-256 (efea1078…3872). The video's playback timing does not reflect the CI run's timing.

Replay of the E2E lifecycle step log for issue 109, final screen

pr115-e2e-replay.webm

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: xhigh

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 00065225bc

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

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