[Spec 83] v2 client shell: apps/v2 renders the live hierarchy - #99
Merged
Conversation
Second buildable unit of the v2 program, after spec 52 shipped the server events. Scope is deliberately one thing: render the hierarchy, live, and nothing else. The reason for that scope is that spec 52's stream has never been consumed. Its tests drove fake in-process clients; no browser has opened it. The first client is therefore the first real test of the contract, and if tiling, terminals and gates land in the same unit, a wrong tree could be caused by any of them. Locked: one reducer over frames as the whole state model; tick is the only writer of buckets and absence in tick means zero; reconnect resumes rather than reloads; the four statuses map to the shipped design tokens with rust reserved for gates; containment rather than an outline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ix dark
Three defects found by the opencode review, each verified against the source
before acting.
C1+C2 froze both files that own /v2/. handleV2Route 404s every path but
GET /v2/events (v2-routes.ts:250), and tower-routes.ts is the only other
router, so no file was left that could serve the page. C2 now names the frozen
contract files and permits v2-static.ts plus a bounded dispatch prologue.
Scope is required (v2-routes.ts:260) and rev. 1 never said where the client
gets it. D7: one GET /api/workspaces at startup, never polled, never re-derived
on reconnect. Zero workspaces renders the empty site and does not open the
stream.
dark.id is workspace:<path> (v2-routes.ts:395), not a machine. D5 replaces the
claim with three states that must not look alike: empty, dark workspace,
unreachable Tower. The last has no wire signal, so it is the one that defaults
to looking empty.
Also: D6 documents why the key injection is duplicated rather than exported
(tower-routes.ts imports v2-routes.ts, so exporting cycles) and makes its three
security properties testable. C3 rules out subscribeEvents/parseSseText, which
target /api/events and a {type,body} envelope. D8 names the 01-site.html cut.
Both open questions resolved: Tower proxy in dev, light default.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…an fail Two defects from the codex review, both verified against the source. isRequestAllowed runs before dispatch and isPublicRoute (server-utils.ts:142) does not list /v2/. A browser navigating there sends no codev-tower-key header, and neither do the script and link tags the shell emits, so rev. 2's page 401s before v2-static.ts is reached and the key injection can never run. D9 adds two GET clauses, with the annotator shell as the precedent already in that function (server-utils.ts:156-165). /v2/events stays keyed. D7 told the client to bootstrap scope through listWorkspaces(). That method returns [] when result.ok is false (tower-client.ts:400-403), so 401, 403, 500 and a dead socket all arrive as an empty list, which makes D5's requirement that unreachable and empty be distinguishable unimplementable. The client does its own fetch and branches on status. I could not tell must not be spelled the same way as no. Also restores the template's Open Questions heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six defects from the codex review plus five findings from actually opening the
stream. Every claim below was measured against the running Tower on 4100, not
read off the code.
The stream emits workspace, architect and builder. There is no machine node and
there never will be from this contract, so criterion 1 was unimplementable as
written. D10 makes the local machine the page's own frame.
counts is machine-wide, not the scope's rollup. A scope of one workspace
returned 16 nodes alongside counts of 22 workspaces and 58 builders, off by 45.
D11 puts it in the footer, labelled as the machine's totals.
encodeURIComponent(paths.join(',')) is wrong and silent. parseScope splits the
raw query value on comma before decoding, so the encoded separators survive as
%2C, the split finds one part, and the server resolves one nonsense path. The
answer is HTTP 200 with nodes: [] and a dark frame. D12 states the encoding and
scenario 18 asserts it directly rather than trusting a round-trip.
All 13 builders in a real workspace returned parentId of workspace:<path>. Zero
architect parents, because discoverBuilders hardcodes spawnedByArchitect: null
(overview.ts:602,662,697). FR-3 is not satisfiable here. D13 renders the flat
truth and forbids inferring a parent by name-matching, because a guessed
hierarchy that is usually right is worse than an honest flat one.
A dark workspace appears in a dark frame and not in nodes, so there is no node
to mark. D1 adds darkPaths as a separate store and D5 says the plot is built
from the encoded id.
From the review: C3 endorsed listWorkspaces() while D7 forbade it, and scenario
14 still called it. C4 said setInterval only in backoff, which contradicts
itself since backoff is a one-shot setTimeout; now zero setInterval and exactly
one named setTimeout. D1 gains cursor-advances-on-every-frame plus defined
answers for invalid JSON, unknown type, EOF and non-2xx. D14 adds the packaging
gap: packages/codev publishes dashboard-dist from apps/web/dist and has no
equivalent for apps/v2, so /v2/ would work in-repo and 404 from the installed
CLI; an npm pack check is required because in-repo passing is not evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six internal contradictions from the codex review, plus one more measured frame.
D6 asked both key-injection implementations to comment-reference each other
while C1 freezes tower-routes.ts byte-for-byte. Only v2-static.ts carries the
reference now.
Criterion 21's allowlist omitted v2-static.ts, the file D6 requires.
D7 retried a failed bootstrap on backoff while criterion 11 demanded exactly
one request per page load. The rule is once successfully: after a 200 it is
never requested again including across reconnects, after a failure it retries.
D1 stored streamId, seq and buckets from a snapshot but not counts, and no
counts delta is guaranteed to follow, so the machine-total footer could stay
empty forever with the contract looking unexercised.
Scenario 9 still allowed setInterval in backoff, against C4 and criterion 11.
D10 required the machine hostname with no source. It is
window.location.hostname, chosen because it is what the person typed to get
here and there is no second machine until FR-16.
Measured, from a 40-second read of the live stream: a tick arrives as
{seq:1,type:tick,at:...,buckets:{}}. buckets is number[] on a node
(v2-events.ts:13) and Record<builderId, number> on a tick (:54) — one name, two
shapes, and a reducer that treats them alike breaks on the first tick, which is
also the first frame after the snapshot. The empty object is the common case,
not an edge one.
D13 now cites #97, filed against discoverBuilders.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uess Five findings from the codex review. The first is a seventh contract limitation, verified in the source and filed as #98. Rev. 5 said a recovering dark workspace stops appearing in the next snapshot and the client clears it. It cannot. handleV2Route computes darkPaths and inScope in one pass at connect (v2-routes.ts:302-314), inScopeSet excludes the dark paths, and deps.isReadable is never called again on that connection. Both directions are broken: a workspace that becomes readable stays dark forever, and one that becomes unreadable never goes dark, so it reads as live-but-quiet. FR-15 is true at connect and decays after. The client's only honest options are to replace darkPaths on a snapshot and to refuse to poll for freshness, since reconnecting to force a snapshot is polling with extra steps. A 200 from /api/workspaces whose body cannot be read — invalid JSON, no workspaces field, workspaces not an array — must surface as unreachable and retry. Never the empty state. The status code is not permission to guess. Approach A still said it reuses packages/sdk, left over from rev. 1, while C3 and D7 say no SDK behaviour is imported. The risks table still asked for cross-reference comments both ways, which C1's freeze on tower-routes.ts forbids, and pointed at criterion 18 and scenario 7 for claims that had moved to criterion 15 and scenarios 4 and 5. Scenario 10 was one Playwright case against thirteen browser-facing criteria. It now carries one scenario each, including that rust appears nowhere on the page except a gate, and that the performance budgets are measured in the browser rather than asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing tested Five findings from the codex review. FR-15 says one environment being down degrades that subtree only, and an environment is a connected Tower (FR-14, FR-16), not a workspace. This unit has exactly one environment, so when it is unreachable the whole page is the subtree and there is no sibling left running to prove anything. Goal 4 claimed FR-15 satisfied through dark workspaces; rev. 7 marks it deferred. D5's handling is FR-15's foundation, not FR-15. Rev. 6 said to drop a malformed or unknown frame without advancing the cursor. That is unsafe twice over: frame N+1 then applies on top of a tree that silently missed N, and a reconnect resumes from before N so the server replays the same bad frame forever. A bad frame is now terminal for the connection, with exactly one recovery attempt that drops since and stream to force a fresh snapshot, and a hard stop after that. The mismatch state names the seq and type that broke it. Criterion 21's whitelist forbade the test files the spec itself demands, plus the pnpm-workspace.yaml and pnpm-lock.yaml changes a new workspace necessarily makes. It now separates production code from tests and workspace metadata. The SSE reader is hand-written and had no transport tests, so it could pass every reducer test and still fail on normal network chunking. Scenario 27 covers frames split across chunks, several frames in one chunk, a partial trailing frame at EOF, CRLF, and a chunk that splits a multi-byte UTF-8 character. D6 said everything but the shell, assets and /v2/events returns 404, while the frozen handler returns 405 for a non-GET /v2/events (v2-routes.ts:256-259). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from the codex review.
Rev. 7 required the mismatch state to name the seq and type of a frame that
failed to parse. Neither is knowable from invalid JSON. The state now reports
what it actually got: invalid JSON after cursor N plus an escaped 120-byte
preview; a type verbatim when the frame parsed but the type is unknown; type,
seq and the failing field when the payload is bad.
TypeScript describes the frames and validates nothing. A decoded object with a
known type and a bad payload reached the reducer unchecked. Every frame is now
validated at runtime, with the failing conditions listed per type, and a known
type carrying a bad payload is terminal like an unknown type.
One deliberate exception, spelled out because the new terminal rule invites
getting it wrong: a structurally valid node carrying a status outside the four
still renders visibly wrong per D3 and does not enter the mismatch state. An
unexpected value in a well-formed frame is a contract drift the page should
show and survive; a malformed frame is a contract it cannot read.
D7 validated that workspaces is an array and not that each entry carries a
non-empty string path. Measured shape is {path, name, active, proxyUrl,
terminals}. Without per-entry validation, {workspaces: [{}]} becomes a scope
path of undefined, the server answers with a dark frame and reason unknown, and
a client bug arrives disguised as a server fact. A failing entry is a mismatch
for the whole bootstrap, not a workspace to skip, because a partial scope
silently drops machines the person expects to see.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… from afx cleanup Four findings from the codex review, plus the spec/plan boundary stated. Rev. 8's field validation was partial, and the reviewer offered the right choice: enumerate the whole schema or narrow the guarantee explicitly. Neither half-measure is safe. The rule is now validate every field the reducer reads and no others, with the read-set listed per frame type as a closed table. A second full schema here would duplicate packages/types and drift from it, while ignoring a field the reducer does read is how a number lands in parentId and the tree quietly reparents. Scenario 35 asserts the narrowness holds, so a harmless added server field cannot kill the page. seq validated as is not a number admitted NaN, Infinity, 1.5, -1 and 2**60. It must be a finite non-negative safe integer. Ordering is non-decreasing, not strictly increasing. Measured: a snapshot and the dark frame beside it both arrived with seq 0, so frames legitimately share a sequence and the intuitive invariant would reject a correct stream. A seq below the cursor stays terminal. The Playwright scenario for criterion 6 called afx cleanup, which would put an irreversible worktree destruction in a test runner instead of with a human. It uses a gone fixture; the real path is verified once by hand during UX verification. Eight rounds pushed file names and line numbers into this document, so the spec/plan split is now stated rather than left to the plan to re-litigate. The line-number citations stay as evidence: they are the proof behind two claims this spec got wrong before measuring, and a spec that asserts server behaviour without saying where it read it gets revised again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings from the codex review. The first would have killed the page on every Tower restart. Rev. 9 made any frame whose seq is below the cursor terminal, while D2 requires accepting a replacement snapshot with resumed: false. Those contradict. Measured: two consecutive connections returned streamId 73da9f829b466efd then 0b3e88cce99fe0b1, both starting at seq 0. So every refused resume arrives at or below the old cursor, and rev. 9 declared all of them corrupt. Ordering is now non-decreasing within one streamId and uncompared across streamIds, where a new snapshot establishes a fresh baseline at its own seq however low. Mismatch was described as terminal without saying what ends it, which reads as a latch: one bad frame would poison a tab until someone reloaded it, and a page reload is what D2 exists to avoid. A valid fresh snapshot now clears the state and resets the one-attempt budget. Rev. 9 called a malformed bootstrap 200 both unreachable and a contract mismatch, while D5 requires those two to look different. The split follows what is actually retryable: a non-200 or a thrown fetch is transient and retries on backoff indefinitely, while a 200 whose body cannot be read retries once and stops, because a server returning garbage returns garbage again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… retry can fix Four findings from the codex review. The first was a contradiction between two of this spec's own success criteria. v2-sampler.ts:196 emits every node delta as stripBuckets(node), and only the snapshot paths call withBuckets (v2-routes.ts:233, 361, 376). So the builder that appears when you spawn one arrives via a node frame with buckets absent, and rev. 10's do not touch buckets left it with no trace at all. Criterion 2 spawns that builder and criterion 5 requires it to have a sparkline. A node upsert now leaves an existing trace untouched and creates 20 zeros when the id is new and the kind is builder. That does not break tick is the only writer: a zero-filled trace is the empty state, not data. Rev. 10 sent every non-2xx stream response to unreachable plus backoff while also calling a 400 a client-built bad scope. Those contradict, and the result retries a request guaranteed to fail identically, forever. Responses are now classified by whether a retry can fix them: 400 and 404 and 405 are contract mismatches with no retry, 401 and 403 are auth-labelled unreachable with no retry, and 5xx or a thrown fetch retries on backoff indefinitely. A 400 is almost always D12's encodeURIComponent trap, and a page that retries it forever shows a spinner instead of the answer. Scenario 26 said a malformed bootstrap 200 produces the unreachable state, contradicting D7 and scenario 38 in the same document. D1 said a snapshot stores buckets, but V2SnapshotFrame has no top-level buckets — they ride on each node. It also needed to say darkPaths is cleared before that snapshot's own dark frames are applied, or a recovered workspace stays dark. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11 review rounds on the codex lane, each one catching real defects, with the sharpest findings coming from consuming the live stream rather than reading the code. Stopping here: the remaining findings are shrinking and a builder will surface real problems faster than a twelfth round will. Frontmatter records only the codex lane. No reviewer ever issued APPROVE, and this says so rather than implying a clean sweep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Approved spec for #83, the second buildable unit of the v2 UI program (#37) after spec 52 shipped the server events.
Problem
GET /v2/eventsshipped and nothing had ever consumed it. Its tests drove fake in-process clients; no browser had opened it.apps/v2did not exist.Root cause of the 11 revisions
Reading the server code was not enough. I opened the stream with
curlagainst the running Tower and got seven facts no code reading had produced, each of which changed the spec:V2NodeKindis workspace/architect/buildercountssaid 22 workspaces / 58 builderscountsis machine-wide, off by 45 in the observed caseencodeURIComponent(paths.join(','))→ HTTP 200, empty tree,darkframeparseScopesplits the raw value before decoding. Silent, and renders as a plausible empty machineparentId: "workspace:<path>"darkframe and not innodesdarkPathsis a separate storestreamIds, both atseq: 0darkPathscomputed once at connect;inScopeSetexcludes itFixes that came from the review rather than the wire
/v2/could not be served at all under the original constraints.handleV2Route404s every path butGET /v2/eventsandtower-routes.tswas frozen, so no file was left that could serve the page.isPublicRoutealso does not list/v2/, so a browser navigation would 401 before key injection ran.listWorkspaces()returns[]on 401, 403, 500 and a dead socket (tower-client.ts:400-403), which made "unreachable must not look empty" unimplementable through it.v2-sampler.ts:196stripsbucketsfrom every node delta, so the builder criterion 2 spawns arrives with no trace and fails criterion 5 on the same row.seqvalidated as "is a number" admittedNaN,-1,1.5and2**60./v2/would 404 from the installed CLI.packages/codevpublishesdashboard-distfromapps/web/distwith no equivalent forapps/v2.What ships
14 locked decisions. One reducer over frames as the whole state model. Reconnect resumes rather than reloads. Four statuses on shipped tokens with rust reserved for gates. Containment, not an outline. 41 test scenarios, including transport framing across arbitrary
ReadableStreamchunks — the reader is hand-written, so the reducer tests prove nothing about it.FR-15 is marked deferred, not satisfied: it says one environment degrades its subtree only, an environment is a connected Tower, and this unit has exactly one — so there is no sibling left running to prove the requirement.
Testing
codex; no reviewer ever issued APPROVE, and it says so rather than implying a clean sweep.Spec only. No source changes.
🤖 Generated with Claude Code