Bugfix spec: control-plane UI floods the console with 400s from /graph/check when a session's cwd checkout is gone
Phase 1 of 3 for a bug (bugfix → design → tasks). This phase MUST be reviewed and approved before the design is derived from it.
Summary
A session record whose checkout has been deleted makes the control-plane UI POST a repo path that no longer exists to /api/v1/graph/check, once per session per poll tick, forever. The API answers 400, and Chrome logs every one of them at the network layer where a JavaScript catch cannot suppress it.
Nothing is user-visibly broken: fetchGraphs swallows the rejection on purpose and the rail renders "no position known" from the frozen record either way. The cost is a devtools console that accumulates red lines indefinitely — noise that buries real errors and reads as a fault to anyone who opens it. Ticket: #238.
Steps to reproduce
- Run a work item so a session record is written with a workspace worktree as its
cwd. - Close the session and let the worktree be removed. The record survives with
"status": "closed"andcwdunchanged. - Open the control-plane UI against that state root and leave it polling.
The stale record on the reporter's machine:
{
"workItem": { "ref": "github:MadaraUchiha-314/devbox#2", "number": 2 },
"status": "closed",
"cwd": "/Users/…/.worktrees/github.com/MadaraUchiha-314/devbox/github-MadaraUchiha-314-devbox-2"
}Reproduced outside the browser, against the running service:
$ curl -s -w " | HTTP %{http_code}\n" -X POST http://127.0.0.1:4114/api/v1/graph/check \
-H 'Content-Type: application/json' \
-d '{"repo":"/Users/…/github-MadaraUchiha-314-devbox-2","workItem":"issue-2","prRepo":"","recompute":false}'
{"detail":"repo path is not a directory: /Users/…/github-MadaraUchiha-314-devbox-2"} | HTTP 400Expected vs actual
- Expected: a checkout that is gone is expected state on a machine that has cleaned up. The UI either does not ask about it, or gets an ordinary answer meaning "no position known" — and the console stays clean.
- Actual:
POST http://127.0.0.1:4114/api/v1/graph/check 400 (Bad Request)in the browser console on every poll tick, one line per stale session, without end.
Root cause (confirmed)
Three facts compose. None is wrong alone; together they make a doomed request that is re-sent forever.
flowchart TD
A["worktree deleted<br/>session record keeps its cwd"] --> B
B["useControlPlane.ts:150-156<br/>repo = session.cwd — guard covers a MISSING cwd,<br/>not one that existed and vanished"] --> C
C["client.ts:273-281<br/>POST /graph/check {repo, workItem, …}"] --> D
D["graphs.py:31 resolve_repo<br/>ValueError: repo path is not a directory"] --> E
E["routes.py:206<br/>ValueError → HTTP 400"] --> F
F["Chrome logs the 4xx at the network layer;<br/>fetchGraphs catch cannot suppress it"] --> G
G["next poll tick"] --> B- The record outlives the path it names. Nothing clears or blanks
cwdwhen the worktree goes, andGET /api/v1/sessionsreports no signal about whether that path still resolves — so no caller can tell a live checkout from a vanished one without asking the filesystem itself. - The UI's guard is for a different case.
if (!repo || !spec) continue(ui/src/state/useControlPlane.ts:154) skips a session that never had acwd. Its own comment describes the unhandled case exactly: "No checkout on this machine means no graph state to read." - An expected state is reported as caller error.
resolve_repo(cli/the_loop/core/graphs.py:31) raisesValueErrorfor a path that is not a directory, and the route layer maps everyValueErrorto400(cli/the_loop/api/routes.py:206). A vanished checkout is not a malformed request.
Fixing only (1) or only (2) still leaves the window between the worktree disappearing and the next reconcile, in which the request is sent and the console gets its 400.
Requirements
Requirement 1 — a checkout that is gone produces no failing request
User story: as a developer with the control-plane UI open in devtools, I want a cleaned-up checkout to cost me nothing in the console, so that the errors I do see are real.
Acceptance criteria (EARS)
- WHEN the control-plane UI polls and a session record's
cwdnames a path that does not resolve to a directory THEN the system SHALL NOT produce a browser-logged 4xx or 5xx response for that session. - WHILE a session record's
cwdnames a path that does not resolve to a directory THE SYSTEM SHALL keep that behaviour on every subsequent poll tick, with no growth in logged responses over time. - WHEN the worktree is removed while the UI is already polling THEN the system SHALL satisfy criterion 1 from the first tick after the removal, without waiting for any reconcile, cleanup or restart.
Requirement 2 — the work item still renders, and still says it does not know
User story: as the same developer, I want the quiet path to change nothing I can see, so that the fix is provably a noise fix and not a behaviour change.
Acceptance criteria (EARS)
- WHEN a session record's
cwddoes not resolve THEN the control plane SHALL render that work item's rail exactly as it does today — the position it can derive from the frozen record, and no position claimed beyond it. - WHEN a session record's
cwddoes resolve THEN the reported graph position SHALL be unchanged from the current behaviour, byte for byte in the response body.
Requirement 3 — a malformed request is still refused
User story: as the maintainer of the API contract, I want "expected state" separated from "caller error" rather than merged, so that quieting the console does not cost the service its input validation.
Acceptance criteria (EARS)
WHEN
POST /api/v1/graph/checkreceives a request whoserepois absent, blank, or of the wrong type THEN the API SHALL reject it as it does today.WHEN
POST /api/v1/graph/checkreceives arepopath that does not resolve to a directory THEN the API SHALL NOT pass that path to any core graph call.The published OpenAPI contract (
docs/api-specs/openapi/the-loop.v1.yaml) SHALL describe whatever response shape this change introduces, and the parity testcli/tests/test_api_contract_parity.pySHALL stay green.Corrected during design (see
design.md§ Trade-offs). This criterion first said the contract "SHALL be regenerated rather than hand-edited", which is false for this repository: issue-161 made the contract authored, with a test asserting the app serves it. Hand-editing it is the convention, not the violation.
Requirement 4 — the regression is pinned
User story: as a future maintainer, I want the vanished-checkout path covered by a test, so that the console cannot quietly start filling again.
Acceptance criteria (EARS)
- The fix SHALL include a regression test that fails before the fix and passes after it.
- WHEN the test suite runs THEN it SHALL cover a session record whose
cwddoes not exist, and assert the absence of an error-status response for it.
Security considerations
resolve_repo is a trust boundary, not a formatting nicety: its docstring names abuse case 3 — no core call on unvetted input — and every mutating graph verb reaches core through it. This change touches how that boundary reports, and must not touch what it admits.
- Untrusted actor: any caller that can reach the loopback API can put an arbitrary filesystem path in
repo. That is true today and unchanged here. - Trust boundary:
resolve_repoincli/the_loop/core/graphs.py. Requirement 3.2 states the invariant the fix must preserve — a path that does not resolve reaches no core graph call — so relaxing the status code must not relax the check. Fail-closed means an unresolvable path yields "position unknown", never a graph read rooted at some other directory. - Abuse case — probing the filesystem: a 200 that distinguishes "no such directory" from other outcomes is a filesystem-existence oracle. It is not a new one: today's
400body already echoes the path back withrepo path is not a directory: <path>, so the same bit is already observable to the same caller for a path they supplied. The response this change introduces MUST NOT widen it — no directory listings, no parent-path probing, no error text beyond what the caller already sent. - Abuse case — a forged session record: the session registry is a local file the agent can write, so a record could name any path. It could before this change too, and the boundary above is what contains it; no new attack surface, because the UI's decision to skip or send changes only whether a request is made, never what the server will admit.
Out of scope
- Pruning
cwdwhen a worktree is removed (the issue's suggested fix 3). It is a sound hygiene change and it does not close the window this bug lives in — the record can always be stale for one tick — so it is not required to satisfy Requirement 1.3. If the design takes it, it is an addition, not the fix. - The other
resolve_repocallers./graph,/graph/complete,/graph/advance,/graph/forceand/graph/skipshare the same helper, but only/graph/checkis polled, and the mutating verbs have a caller who wants to be told they named a nonexistent repository. Changing them is a separate decision. sessions cleanup. It already removes the record, and it is the documented workaround; nothing here changes it.
Open questions
None blocking. One decision is deliberately deferred to design.md rather than settled here: whether the fix is server-side (/graph/check answers "unknown" with a 200), client-side (the session listing reports whether cwd resolves and the UI skips those jobs), or both.
Requirement 1.3 is the constraint that separates them. The client-side fix re-reads the session listing every tick, so it does answer within one tick of the removal — but the listing and the graph checks of a single tick are two reads of a filesystem that can change between them, so a worktree removed in that gap still produces one 400. Only the server-side answer is unconditional. Whether that residual window is worth a contract change is the design's call, and the reviewer's.