Skip to content

Commit 4842e05

Browse files
gcharanggcharang
committed
docs: defer the attended status gap, with the evidence to act on it
The attended status enum has three states — pending, connected, detached — for a machine with four: no socket, socket without an attachment, socket with an attachment, socket gone. connected covers the middle two, which are exactly the two that differ in whether a command can succeed. The unattended lifecycle in the same file models its machine with eight states; the attended path never got the same treatment. Two independent causes, and either alone makes a fix useless: the enum cannot express the state, and neither detach path sends anything over the wire, so the backend could not transition even if it could name it. The involuntary path is the one that matters. chrome.debugger.onDetach fires when the controlled tab is closed or the operator clicks Cancel on Chrome's debugger banner, and it notifies the backend no more than the button does. That is the unwritten reason the extension runbook says not to touch that banner. Deferred, not dismissed: adding an enum value is breaking, since consumers parse responses with the exported schema, so it needs a protocol major plus a coordinated consumer upgrade rather than a change landed mid-acceptance. Execution is correct throughout; only the reported status is wrong, so Phase 3a is not blocked.
1 parent d6fab0d commit 4842e05

2 files changed

Lines changed: 180 additions & 1 deletion

File tree

‎DEFERRED.md‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
<!-- Content type: Reference -->
2+
3+
# Deferred work
4+
5+
Known defects and design gaps that are recorded rather than fixed, with enough
6+
evidence that a fresh session can act on them without redoing the
7+
investigation. Each entry states what is wrong, how it was observed, why it was
8+
deferred, and what "done" looks like.
9+
10+
Line numbers are as-of the baseline commit named in each entry and must be
11+
re-confirmed before editing.
12+
13+
---
14+
15+
## Attended session status cannot express "connected but not attached"
16+
17+
**Baseline:** branch `dev`, commit `d6fab0de4aea6be587e158b59faea21c6c4bed06`.
18+
**Found:** 2026-07-29, during the Phase 3a attended-compatibility scenario.
19+
**Severity:** consumers can be told a session is ready when it cannot execute
20+
anything. No silent success, but a status that is affirmatively wrong.
21+
22+
### What was observed
23+
24+
With an attended session attached and working, the operator pressed **Detach
25+
tab** in the side panel. The session then reported:
26+
27+
```json
28+
{"mode":"attended","status":"connected","browser":{...},"tabs":[{"tabId":2134210639,...}]}
29+
```
30+
31+
while every command answered:
32+
33+
```json
34+
{"type":"action_result","ok":false,"error":"no active CDP session"}
35+
```
36+
37+
So `status` said `connected`, `browser` was populated, and `tabs` still listed a
38+
tab that was no longer under control.
39+
40+
### Why it happens — two independent causes
41+
42+
**1. The attended enum has no state for it.** `packages/protocol/src/index.ts:382`:
43+
44+
```ts
45+
export const AttendedSessionStatusSchema = strictObject({
46+
mode: z.literal("attended").optional(),
47+
status: z.enum(["pending", "connected", "detached"]),
48+
```
49+
50+
Three states, and the real machine has at least four: no socket; socket up with
51+
no tab attached; socket up with a tab attached; socket gone. `connected`
52+
currently covers the middle two, which are the two that differ in whether a
53+
command can succeed.
54+
55+
The unattended lifecycle in the same file (`:367`) models its machine properly,
56+
which is the contrast worth noting — the attended path was simply never given
57+
the same treatment:
58+
59+
```ts
60+
export const UnattendedSessionLifecycleSchema = z.enum([
61+
"allocating", "provisioning", "connected", "recovering",
62+
"closing", "closed", "expired", "lost",
63+
]);
64+
```
65+
66+
**2. The extension never tells the backend.** `apps/extension/src/entrypoints/background.ts:648`:
67+
68+
```ts
69+
async function detach(): Promise<void> {
70+
const active = session;
71+
try {
72+
if (active !== null) await active.detach();
73+
} catch (cause) {
74+
log(`detach error (continuing): ${errorMessage(cause)}`, "warn");
75+
}
76+
await clearAttachment();
77+
log("detached");
78+
broadcastState();
79+
}
80+
```
81+
82+
`broadcastState()` updates the side panel only. No frame goes over the
83+
WebSocket, so even with a richer enum the backend could not make the
84+
transition. The same is true of the involuntary path,
85+
`background.ts:595` — which matters more, because it is the one a user
86+
triggers by accident:
87+
88+
```ts
89+
async function onDetach(source: { tabId?: number }, reason: string): Promise<void> {
90+
await profileClient.sessions.onDebuggerDetach(source);
91+
const active = session;
92+
if (active === null || source.tabId !== active.tabId) return;
93+
await fenceStartedAttendedWrites();
94+
await clearAttachment();
95+
log(`debugger detached from tab ${active.tabId} (${reason})`);
96+
broadcastState();
97+
}
98+
```
99+
100+
`chrome.debugger.onDetach` fires when the controlled tab is closed, or when the
101+
user clicks **Cancel** on Chrome's debugger banner. Both leave the session
102+
reporting `connected`. This is the mechanical reason
103+
`apps/extension/RUNBOOK.md` instructs operators not to touch that banner — the
104+
instruction exists, but the reason is not written down anywhere.
105+
106+
For completeness, `detached` today is set only on socket-level events, in
107+
`apps/backend/src/session.ts` at lines `529`, `1132`, `1164`, `1198` and `1231`:
108+
connection close, session reconnect / browser-epoch change, device credential
109+
revocation, and terminal close. Losing the debugger attachment is not among
110+
them.
111+
112+
### Why it was deferred
113+
114+
It is not a correctness hole in execution. A consumer that sends a command gets
115+
an explicit `ok: false` with a specific reason, not a false success, and
116+
Metamind's connector surfaces that. The exposure is limited to a consumer that
117+
gates on `status` alone and concludes the session is usable.
118+
119+
It also cannot be fixed in one repo. Adding a value to
120+
`AttendedSessionStatusSchema` is a **breaking protocol change**: the schema is
121+
exported from `@understudy/protocol` and consumers parse responses with it, so
122+
`z.enum` rejects an unknown value. Shipping it needs a protocol major, a
123+
connector release, and a consumer upgrade — coordination that did not belong in
124+
the middle of a canary acceptance run.
125+
126+
### Proposed fix
127+
128+
Model the machine, then report the transition. Both halves are required; either
129+
alone changes nothing.
130+
131+
1. **Protocol.** Add a state meaning "extension socket present, no tab
132+
attached". Recommended name `idle`, giving
133+
`pending | idle | connected | detached`, and keep `connected` meaning
134+
*ready to execute*. Preserving the existing meaning of `connected` is the
135+
point: consumers that already gate on `status === "connected"` keep working
136+
and simply stop treating a detached session as usable.
137+
138+
*Rejected:* renaming the ready state to `attached` and repurposing
139+
`connected` to mean socket-up. Cleaner on paper, but it silently flips the
140+
meaning of a value existing consumers already branch on — the failure mode
141+
would be worse than the bug.
142+
143+
2. **Extension.** Send an explicit frame on both detach paths — the deliberate
144+
`detach()` and the involuntary `onDetach()` — so the backend can transition
145+
to `idle`. It must carry the tab identity so a late frame from a superseded
146+
attachment cannot clear a newer one; `sendIfPeerCurrent` in
147+
`apps/extension/src/core/peer-binding.ts` is the existing guard for exactly
148+
this class of post-await staleness and should be reused rather than
149+
reinvented.
150+
151+
3. **Backend.** Clear `tabs` and `browser` alongside the transition. Reporting a
152+
`tabId` that is no longer controlled is part of the same wrongness and should
153+
not survive the fix.
154+
155+
### Out of scope
156+
157+
- The unattended lifecycle. It already models its machine and must not be
158+
touched by this work.
159+
- `SessionAgent`'s existing `detached` transitions. They are correct for what
160+
they describe — socket-level loss — and this adds a state beside them rather
161+
than redefining them.
162+
163+
### Verification
164+
165+
- A unit test asserting the full attended sequence: `pending` on create,
166+
`connected` after hello, `idle` after a detach frame, `detached` after the
167+
socket closes.
168+
- A test that a detach frame naming a superseded tab does **not** move a live
169+
session out of `connected`.
170+
- Manually, the scenario that found it: attach, confirm `connected`, press
171+
**Detach tab**, confirm `idle` and empty `tabs`; then repeat by closing the
172+
controlled tab instead of pressing the button, which must reach the same
173+
state.
174+
- Quality gate: clean-code, architecture, and QA lanes, since this is a protocol
175+
change with cross-repo consumers.

‎docs/unattended-production-rollout.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,11 @@ The protocol-2 check is worth reusing. Sending a write **without** the contract
942942

943943
`detached` is set only when the WebSocket itself closes, or on an epoch change, device revocation, or terminal close. Detaching the debugger keeps the socket open and reports nothing to the backend, and the attended status enum — `pending | connected | detached` — has no state for "socket up, nothing attached".
944944

945-
Not a silent failure: the consumer gets an explicit per-command error rather than a false success, and Metamind's flow surfaces it. But a consumer that gates on status alone would see `connected` and be wrong. Recorded as an observation, not a defect — the status plausibly tracks the socket by design — and left for the owner to decide.
945+
Not a silent failure: the consumer gets an explicit per-command error rather than a false success, and Metamind's flow surfaces it. But a consumer that gates on status alone would see `connected` and be wrong.
946+
947+
**Ruled a defect, and deferred rather than dismissed** — see [`DEFERRED.md`](../DEFERRED.md), "Attended session status cannot express *connected but not attached*". It is deferred because the fix adds a value to `AttendedSessionStatusSchema`, which consumers parse with, so it is a breaking protocol change needing a connector release and a coordinated consumer upgrade — not something to land mid-acceptance. It does not block Phase 3a: execution is correct, only the reported status is wrong.
948+
949+
The wider case matters more than the button. `chrome.debugger.onDetach` fires when the controlled tab is **closed**, or when the operator clicks **Cancel** on Chrome's debugger banner, and neither notifies the backend either. That is the mechanical reason the extension runbook says not to touch that banner.
946950

947951
Still outstanding: hard and idle expiry, which are best evidenced by the Phase 3b soak rather than waited on. Two visual confirmations also remain — that cleanup closed exactly one tab, and that no stray popup window survived.
948952

0 commit comments

Comments
 (0)