Skip to content

test: live interaction coverage for the agent workspace surface - #799

Merged
sakibsadmanshajib merged 11 commits into
mainfrom
test/workspace-interaction-coverage
Aug 16, 2026
Merged

sakibsadmanshajib merged 11 commits into
mainfrom
test/workspace-interaction-coverage

Conversation

@sakibsadmanshajib

@sakibsadmanshajib sakibsadmanshajib commented Aug 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

Live interaction coverage for the agent workspace ("Cowork", /agent-workspace): a trackable proven-over-total number, every unproven control named, and a check behind it. Reworked from the first version of this pull request, whose central premise was self-inflicted and had since gone stale.

What the rework changed

Authentication no longer skips fourteen controls. The first version proved 8 of 22 and skipped the rest, on the stated grounds that signing in required rotating the shared demo account's password through Supabase's admin user endpoint, which was returning 500s. Rotation is forbidden, but it was never the only way in: apps/web-console/tests/e2e/support/live-auth.ts mints a session through the admin one-time-token flow, needs no password, and changes none. Every authenticated control is now proven from a minted session. The only remaining credential-gated control is C8, the password submit path itself, which cannot be proven without a real password and must never be proven by inventing one.

The lifecycle assertions match the deployment. Commit c1ca04d put a real agent runtime behind Cowork through an unprivileged host launcher. A task now progresses queued, running, done rather than resolving straight to a terminal blocked state, so the spec no longer asserts "Blocked" within 20 seconds or an engine-not-configured notice. C16 asserts that notice is absent while a runtime is configured, and C17 asserts the Cancel button is offered on a live task, cancels it, and that a second cancel on the now-terminal task is refused.

Assertions that could not fail are gone.

  • expect(status).toBeGreaterThanOrEqual(400) on the cancel guard passed on a 404 from an unmounted route, on a 401 from a missing auth header, and on an undefined task id. It is now exactly 409, the documented contract, with the auth header and the task id asserted separately as preconditions. The bearer token is asserted as a boolean rather than with toMatch, so a failure prints false instead of echoing a live token into the report, the trace and the CI log.
  • The two sign-in status assertions are exactly 400 rather than a range, which also passed on a 500 from a broken auth service.
  • C18 skipped itself whenever the account already had task history, which every earlier control in the file guarantees by creating some, so it switched itself off permanently after its first run. It now stubs an empty list, which is what the tasks.length === 0 rendering rule actually needs.
  • The coverage builder read results[0], the first attempt, while the config sets retries: 2. It now reads Playwright's own per-test verdict across all attempts, and treats a retry pass as unproven rather than proven. It exits non-zero when a control has no test at all, which is the one hole a ratio cannot show.

The denominator is checked against the DOM. C23 enumerates every focusable control the deployed sign-in and tasks screens render and requires the set to equal the ledger's dom entries. A control that ships without a ledger entry now fails the run instead of quietly raising the percentage by not being counted. The ledger's one "not present" claim, the absent transcript pane, is an assertion in the same test rather than prose. The ledger is still edited by hand and now says so; C23 is what checks that hand.

It runs, and the number is generated. tests/e2e/_probe is testIgnored by the chromium project and no workflow ran --project=probe, so none of this executed. The spec has its own Playwright project, pinned in playwright-spec-manifest.json and enforced by the existing spec collection guard, and deploy-demo-box runs it against the deployment it just shipped. That is the only place it can run honestly: the Cowork console, the edge-api agent-task routes and the host launcher exist together only on the box, and ci.yml's stack boots none of the three. The committed agent-workspace-coverage.json is deleted; the ledger is generated by the run into gitignored test-results/, uploaded as an artifact, and printed into the job summary.

Live result

Run against https://chat-hive.scubed.co on 2026-08-11 with a minted session.

15 executed, 1 skipped, 14 passed, 1 failed
agent-workspace coverage: 18/23
controls state
18 proven live
1 (C8) skipped, needs a real password, honest remainder
4 (C14, C15, C16, C17) share one test, red on two live defects below

The first version of this pull request reported 8 of 22 with 14 blocked on a reason that was not true.

Two live defects the rework found

Neither is fixed here. Both were invisible while the suite asserted the old blocked-state shape.

#881: Cowork create reports a failure for a task that is actually running. apps/edge-api/internal/agenttask/client.go gives its control-plane client a 15 second timeout, while CreateTask in control-plane blocks inline on the sandbox launch with a five minute bound. Any launch slower than 15 seconds answers the browser 500 for a task that is created and runs to completion. Measured at 18.0 seconds against the box, with the task reaching succeeded while the composer said "Could not start the task".

#886: cancelling a task does not stop the sandbox or release its concurrency slot. Service.Cancel is a database transition and the Engine interface has only Launch, so nothing tells the sandbox to stop. Two cancels held both of HIVE_QUOTA_USER_CONCURRENCY's slots for over half an hour, after which every create was refused instantly with "agent engine could not start the task", while uncancelled tasks on the same account finished in about eighty seconds. A user can permanently exhaust their own concurrency by cancelling. Both the spec comment and the assertion message name this, so a repeat run is not misread as flake.

Which of the two takes the run red depends on the box: a slow launch gives #881, an exhausted slot gives #886. Both were reproduced directly against the API, not only through the browser.

The create assertion is the only soft assertion in the file. It still fails the test, so nothing is hidden, but the body continues and the task it started is cancelled rather than left holding a sandbox on a shared box.

Where and when this actually runs, stated plainly

It does not run on the push-to-main path. The job is gated to workflow_dispatch and is deliberately outside report-failure's needs. Both are temporary and both are for the same reason: the suite creates one real agent task, and per #886 cancelling it never frees the sandbox or its concurrency slot, so running on every push touching apps/web-console/** would exhaust the demo account and take the Cowork surface down for the day. A permanently red job would also hold the report-failure dedupe issue open and downgrade a genuine migrate failure to a comment on a stale issue, which is #553's blind spot reopened.

So what the gate is worth today: it is a suite anyone can run on demand against the deployed box, with a floor that fails, a denominator that fails, and a proven live red. It is not yet an automatic check on main. Removing the if: line and adding the job back to report-failure's needs is a one-line change each, and both should happen the day #881 and #886 land. Until then this is a manual instrument, and calling it a check would be the overstatement this pull request has already had to correct once.

Proof the gate can fail

Three mutations, all red, plus a live red. Full output in the evidence comment on this pull request.

Correction, and it matters more than the result. An earlier version of this section, and the commit message that carried it, claimed the collapsed-run scenario exited 0 before the head commit and 1 after, crediting that commit with the floor. That was inaccurate and an adversarial review caught it. The builder script is byte-identical between the head commit and the one before it, and the floor landed in the earlier one. Measured, each version against one synthetic collapsed-run report using its own ledger:

commit exit ratio floor
86ff8326 this pull request as first filed 0 7/22 none
6d4a0624 first rework commit 0 7/23 none
061bbfad 1 8/24 18
297ad60e 1 8/24 19

What survives the correction: the collapsed run does go from exit 0 to exit 1 across this pull request, which is the comparison a reviewer deciding whether to merge it cares about. What does not: the attribution to a specific commit. The synthetic report was also made faithful in the process, because the first version set only test.status and left the attempt's own status untouched, which let the pre-rework builder still read passed and report an inflated ratio.

The live red is separate and is not a mutation at all: four controls fail because the product is broken, with #881 and #886 open for them.

Verification

  • npx tsc --noEmit in apps/web-console
  • npm run e2e:verify-collection, 36 files collected into the projects the manifest pins them to
  • npm run e2e:agent-workspace against the demo box, three passes, no retry passes, flake rate 0%
  • node scripts/build-agent-workspace-coverage.mjs regenerates the ledger from the run
  • The task each run creates is cancelled or already terminal; nothing is left in flight on the box
  • C8 is proven the moment HIVE_QA_AGENT_PASSWORD is provided for an account that is OWNER on a cowork-enabled tenant

Buglog entry

To be appended to .wolf/buglog.jsonl on main in a separate buglog-only pull request once this merges, per the protocol in .claude/rules/openwolf.md.

{"date":"2026-08-11","title":"A live coverage gate skipped most of its controls on a blocker that was never true","error_message":"8/22 proven, 14 skipped with reason: Supabase admin user-listing 500s block credential rotation for the shared demo account","root_cause":"The suite assumed the only way to authenticate was rotating a shared account's password through POST /auth/v1/admin/users, so when the admin listing endpoint returned 500s it concluded authentication was impossible and froze fourteen controls into a permanent skip. The sanctioned admin one-time-token flow in tests/e2e/support/live-auth.mjs needs no password, touches no credential, and never calls the failing endpoint. The false reason was then committed into a static coverage file that nothing regenerated, so it outlived the outage it described.","fix":"Authenticate through live-auth.ts, which mints a session with generate_link plus verify. Delete the committed coverage snapshot, gitignore the generated one, and give the spec a Playwright project a workflow selects by name so the number comes from a run. Only the password submit path remains credential-gated, because it is the one control a minted session cannot prove.","tags":["e2e","playwright","auth","coverage","live-testing","agent-workspace"]}

@cursor

cursor Bot commented Aug 8, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@sakibsadmanshajib, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: fc8190fb-269a-4deb-8005-e936789b0330

📥 Commits

Reviewing files that changed from the base of the PR and between 627c1ba and e595f2b.

📒 Files selected for processing (8)
  • .github/workflows/deploy-demo-box.yml
  • apps/web-console/.gitignore
  • apps/web-console/package.json
  • apps/web-console/playwright-spec-manifest.json
  • apps/web-console/playwright.config.ts
  • apps/web-console/scripts/build-agent-workspace-coverage.mjs
  • apps/web-console/tests/e2e/_probe/agent-workspace-controls.json
  • apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands.

sakibsadmanshajib added a commit that referenced this pull request Aug 9, 2026
GoTrue keeps one outstanding one-time token per user, so two mints for the same
account that interleave leave the earlier one holding a token the later one has
already replaced, and its verify answers 403 "Email link is invalid or has
expired". Observed once live. mintSession now retries exactly that failure once.
The real ceiling is named in the code rather than papered over: give each
parallel worker its own account instead of minting for one account from several
workers at once.

docs/proof/live-auth-helper-2026-08-08/coverage.md records the agent-workspace
gate from PR #799 re-run with the helper in place of its password sign-in: 21 of
22 controls proven, up from 8 of 22 with 14 blocked purely on credentials. The
one remaining is the sign-in button's own valid-credentials path, which needs a
real password typed into the real form and cannot be proven from a minted
session. signed-in-workspace.png is the same storage state driving a real
browser at /agent-workspace/tasks, signed in, with no credential in the URL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

The 14 controls this gate reports as blocked on credentials are unblocked. #810 adds apps/web-console/tests/e2e/support/live-auth.ts, which mints a live session through the Supabase admin one-time-token flow: no password is needed, none is changed, and GET /auth/v1/admin/users (the endpoint 500ing in #791, which is what blocked this pass) is never called.

Re-running this spec against chat-hive.scubed.co with only signIn() swapped over: 21 of 22, up from 8 of 22.

15 passed, 1 skipped (1.2m)
agent-workspace coverage: 21/22
  UNPROVEN C8: Sign-in: submit button (valid credentials path)

Two changes are needed here to get that result. Both are small, and I have deliberately not pushed them to this branch since it is yours.

1. signIn() and the credential gate.

+import { reauthenticate } from "../support/live-auth";
+
 const AGENT_EMAIL = process.env.HIVE_QA_AGENT_EMAIL ?? "";
 const AGENT_PASSWORD = process.env.HIVE_QA_AGENT_PASSWORD ?? "";
-const HAS_AGENT_CREDS = !!(AGENT_EMAIL && AGENT_PASSWORD);
+const HAS_AGENT_CREDS = !!(
+  AGENT_EMAIL &&
+  process.env.SUPABASE_URL &&
+  process.env.SUPABASE_SERVICE_ROLE_KEY &&
+  process.env.SUPABASE_ANON_KEY
+);

 async function signIn(page: Page) {
-  await page.goto(`${WORKSPACE}/auth/sign-in`);
-  await page.locator("#email").fill(AGENT_EMAIL);
-  await page.locator("#password").fill(AGENT_PASSWORD);
-  await page.click('button[type="submit"]');
+  await reauthenticate(page.context(), { email: AGENT_EMAIL, targetUrl: WORKSPACE });
+  await page.goto(`${WORKSPACE}/tasks`);
   await page.waitForURL((url) => url.pathname === "/agent-workspace/tasks", { timeout: 25000 });
 }

C8 then has to keep the real form submit inline, since it is the password path itself and cannot be proven from a minted session, and skip when no password is supplied. That is the one control still unproven, and it goes green as soon as HIVE_QA_AGENT_PASSWORD is set for an account that is OWNER on a cowork-enabled tenant. Note that SKIP_REASON's reference to #791 is no longer the reason anything skips.

2. C10/C11/C12 is a locator bug, not a product defect.

getByRole("radio", { name: "Knowledge work" }).check() times out with "label intercepts pointer events". The <input> is peer sr-only and the visible control is the <span> inside its <label> (apps/agent-console/components/task-console.tsx:486), so there is nothing clickable at the input's own position.

-    await knowledge.check();
+    await page.getByText("Knowledge work", { exact: true }).click();

Clicking the label is also what a real user does, so this asserts the control as shipped rather than forcing a click through to a hidden input. With that change the test passes live.

Protocol for the helper, including why rotating the shared demo password is forbidden: docs/live-test-auth.md on #810.

sakibsadmanshajib and others added 7 commits August 13, 2026 14:19
Enumerates every focusable control on /agent-workspace (Cowork) from the
rendered page: sign-in fields and submit, both unauthenticated redirect
branches, the task composer, pack selection, task creation, per-task cancel,
empty and error states. Every control resolves to a proof of effect (network
request, URL change, DOM mutation) or an honest unproven entry naming the
blocking reason, tracked in agent-workspace-coverage.json and regenerated by
scripts/build-agent-workspace-coverage.mjs from a live Playwright JSON run.

Live run against chat-hive.scubed.co today: 8/22 proven. The remaining 14
need a valid demo-account session and could not be exercised this pass --
Supabase's admin user-listing endpoint is returning a persistent 500
("Database error finding users"), confirmed on repeated attempts and
isolated to the admin listing path (ordinary password-grant login is
healthy). Filed as #791; every authenticated test skips with that reason
rather than being silently dropped from the count.

Fixed two stale assertions caught only by actually running this suite live:
the sign-in heading text had drifted from the eyebrow label, and a bare
getByRole("alert") collides with Next.js's own route-announcer div on every
page in this app family. Also corrected an assumption that the sign-in
form's required attributes block empty submission -- the form sets
noValidate, so the real guard is the server-side rejection, not the browser.

agent-workspace-inert-registry.json exists as the mechanism for deliberately
excluded controls; it is empty because nothing on this surface qualifies
today. Demonstrated the suite can fail for the right reason by deliberately
pointing one assertion at the wrong URL, confirming a clean red run, then
reverting.
… task lifecycle

Rewrites the interaction-coverage probe so it can actually fail.

Authentication now comes from tests/e2e/support/live-auth.ts, which mints a
session through the admin one-time-token flow. It needs no password and
changes none. The previous version skipped fourteen of twenty-two controls on
the grounds that authenticating required rotating the shared demo account's
password through an admin endpoint that was returning 500s. Rotation is
forbidden, but it was never the only way in, and that reason was baked into a
committed coverage file.

Rewrites the task lifecycle assertions against what the deployment does since
PR #870. A task now progresses queued, running, done through an unprivileged
host launcher rather than resolving straight to a terminal blocked state, so
the engine-unavailable notice must be absent and the cancel button must be
offered. C17 asserts the button cancels a live task and that a second cancel
on the now-terminal task is refused with exactly 409, instead of the previous
range assertion that also passed on a 404 or a 401.

C18 no longer disables itself after its first run. It stubs an empty list
rather than requiring the live account to have no history, which earlier
controls in the same file guarantee it does.

C23 is new: it enumerates every focusable control the deployed sign-in and
tasks screens render and requires the set to equal the ledger's dom entries,
so a control cannot ship without raising the denominator. It also asserts the
absence of the transcript pane the ledger claims does not exist yet.

The coverage builder reads Playwright's own per-test verdict rather than the
first attempt of a retried test, and treats a retry-pass as unproven. It exits
non-zero when a control has no test at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suite had no check behind it. The chromium project testIgnores
tests/e2e/_probe and no workflow ran --project=probe, so nothing in it ever
executed, and the coverage ledger was a committed static file that nothing
regenerated: its ratio could not move even when coverage did.

The spec now has its own Playwright project so a workflow can select it by
name rather than by file path (issue #813), and deploy-demo-box runs it against
the deployment it just shipped. That is the only place it can run honestly: the
Cowork console, the edge-api agent-task routes and the host launcher only exist
together on the box, and ci.yml's stack boots none of the three.

The ledger is generated by the run into test-results/, which the repo gitignore
already excludes, and uploaded as a run artifact with the ratio printed into the
job summary. The committed snapshot is deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o the suite

The create-response assertion is soft so the body still runs and still cancels
the task it started. It is red against the demo box today because of issue
#881: edge-api's control-plane client times out at 15 seconds while
control-plane's CreateTask blocks inline on the sandbox launch for up to five
minutes, so a launch slower than 15 seconds answers the browser with a 500 for
a task that is created and runs to completion. Measured live at 18.0 seconds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…is not read as flake

Cancel never reaches the engine (control-plane's Engine interface has only
Launch), so a cancelled task keeps its sandbox and keeps the concurrency slot
that sandbox holds until it ends on its own. Two runs inside one sandbox
lifetime exhaust the per-user quota and the next create genuinely fails.
Filed as issue #886 and named in the assertion message, so the next reader
does not re-derive it from a Blocked row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cancelled sandboxes did not drain after a sandbox lifetime. Two cancels
held both of the user's concurrency slots for over half an hour, and every
create after that was refused instantly. Issue #886 carries the evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Credential leak. The agent-workspace project now sets trace and video off,
overriding the retain-on-failure defaults, and the workflow uploads only
test-results/*.json and *.txt. A Playwright trace stores request headers and
cookies verbatim, so a failed run would have published a live bearer and the
sb-*-auth-token cookie, which carries a refresh token for a shared account,
into a public repository's artifacts for ninety days.

The number now gates something. Missing mint environment is a hard failure
rather than a skip, so one renamed secret can no longer drop sixteen controls
into a green 6 of 24, and agent-workspace-controls.json carries a
minimum_proven floor the build script fails below. The script also rejects a
[C#] tag with no ledger entry, so the ratio cannot be raised by deleting the
entry for a control that regressed.

C24 splits the sign-in half of the denominator guard out of the authenticated
group, so the guard against gaming does not share a gate with the numerator it
guards.

C17 no longer claims Cancel stops a task. It cannot: the Engine interface has
only Launch and Service.Cancel is a bare database transition. Both the ledger
entry and the assertion comment now say what is actually verified.

The create test cancels in a finally block, so a hard assertion firing mid-test
cannot orphan a live sandbox on the shared box, and the job is manual-only until
issues #881 and #886 land so it cannot exhaust the demo account's concurrency on
every push. It is also out of report-failure's needs, so a known-red probe
cannot hold the dedupe issue open and mask a real migrate failure.

Deletes agent-workspace-inert-registry.json, which nothing read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sakibsadmanshajib
sakibsadmanshajib force-pushed the test/workspace-interaction-coverage branch from 82b9fc2 to 297ad60 Compare August 14, 2026 16:04
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

Evidence pass: the five questions, answered against a live run

Head 297ad60e. Everything below was produced by running the suite against https://chat-hive.scubed.co, not asserted.

1. What it exercises, and where the denominator comes from

24 controls, 19 proven, on a live deployment. The denominator is not built from the same list as the numerator, which is the failure this question is aimed at.

agent-workspace-controls.json is edited by hand and says so. What makes it a real denominator is that two controls check that hand against the deployed page: C24 enumerates every focusable control the sign-in screen renders, C23 does the same for the task console with the list stubbed to a fixed pair so the Cancel-offered and Cancel-withheld renderings are both on screen, and each requires the rendered set to equal the ledger's dom entries exactly. A control that ships without a ledger entry fails the run. A ledger entry for a control that no longer renders fails the run.

The two guards are deliberately split across the authentication boundary: C24 needs no session and lives in the unauthenticated group, so the denominator guard cannot be disabled by the same missing variable that would collapse the numerator.

Stated ceiling, in the code rather than only here: light DOM only, no shadow roots or iframes (this app has neither, and its middleware sets frame-ancestors 'none'), descriptors deduplicated into a Set, and only the states this file drives are inventoried.

2. Can every assertion go RED

Three mutations, three reds.

A. Ledger divergence, run live. Flipped C1's expected descriptor from input[email]#email to input[text]#email, which is exactly what shipping type="text" on the sign-in field would produce.

✘  [C24] every focusable control on the sign-in screen is claimed by the ledger (736ms)
   Error: the sign-in screen renders a focusable control the ledger does not claim,
          or claims one it no longer renders.
   - Expected  - 1
   + Received  + 1
agent-workspace coverage: 0/24
FAIL: coverage regressed: 0 controls proven, floor is 18.
builder exit=1  playwright exit=1

Reverted, and C24 passes again in the run below.

B. Collapsed run, the exact scenario the second review named. A run report with the authenticated group skipped, which is what one renamed secret used to produce:

agent-workspace coverage: 8/24
  UNPROVEN C9 ... C23  (skipped) SUPABASE_SERVICE_ROLE_KEY not set
FAIL: coverage regressed: 8 controls proven, floor is 18. Fix the coverage.
      Lowering minimum_proven to make this pass is the exact failure this floor exists to catch.
builder exit=1

Before this change that scenario exited 0 and reported a cheerful green.

C. Undeclared identity. A [C99] tag no ledger entry declares:

FAIL: 1 control id(s) are tagged in a test title but declared nowhere in
      agent-workspace-controls.json: C99. A tag with no ledger entry can only move
      the ratio by shrinking its denominator.
builder exit=1

That closes the deletion attack: removing the C17 entry the day C17 regresses would previously have raised the ratio from 19/24 to 19/23.

D. Not a mutation at all. Four controls are red right now because the product is broken, and this suite is what found it. See issues #881 and #886 below.

3. Does anything force a pass

Swept my own code for the whole family. Three things found, all reported rather than defended:

  • test.skip(!AGENT_PASSWORD, ...) on C8, the only skip in the file. It is counted as unproven in the ledger, not proven, and it is the one control a minted session cannot prove because it is the password submit path itself.
  • .catch(() => undefined) in the create test's finally. That swallows an error, and it is cleanup rather than an assertion: without it a hard assertion firing mid-test orphans a live sandbox on the shared box. It cannot mask the real failure, because it runs after it.
  • expect.soft on the create status. It fails the test; Playwright collects it and marks the result failed, which the run below demonstrates.

Not found, and searched for by name: no hardcoded proven: true, no || true, no toBeTruthy, no toBeGreaterThanOrEqual left anywhere (the cancel guard is now exactly 409, the two sign-in statuses exactly 400, the API guard exactly 401), and no skip that moved.

One real proof-attribution bug found in my own code by looking for #809's, and fixed: the awaited create response was matched by shape, so any qualifying POST could have counted. It is now asserted to be the response belonging to the request the keypress issued, by object identity.

4. Live, mocked, or skipped

Live. Sixteen tests executed against https://chat-hive.scubed.co with a session minted through the admin one-time-token flow. One real agent task was created and cancelled. Four tests use page.route to stub a server response, and each does so to reach a state the live server cannot be asked for on demand (a 503 list, a 500 create, an empty list, a fixed two-row list) while still driving the deployed bundle in a real browser. Nothing is mocked at the module level and nothing runs against a local stack.

5. Were floors ratcheted down

No. The floor went up, from 18 to 19, to the count a real run proves. It has never moved downward. Nothing in this repository writes it: the build script only reads minimum_proven and exits non-zero below it, so it can only change in a deliberate commit that shows up in review. The ledger note says ratchet up only, and names the two issues whose fix raises it to 23, and the missing password secret that raises it to 24.


The run, at 297ad60e

16 executed, 15 passed, 1 skipped, 1 failed  (1.7m)
0 passed only on retry. Flake rate 0% (gate: at most 0 retry-pass).
agent-workspace coverage: 19/24
builder exit=0
control state what it covers
C1 proven Sign-in: email input
C2 proven Sign-in: password input
C3 proven Sign-in: submit, invalid credentials path
C4 proven Sign-in: error alert region
C5 proven Required attributes advisory only, form is noValidate
C6 proven Unauthenticated /agent-workspace redirects to sign-in
C7 proven Unauthenticated /agent-workspace/tasks redirects to sign-in
C8 skipped Sign-in: submit, valid credentials path. Needs a real password
C9 proven AppHeader: Back to chat link
C10 proven Task composer: instructions textarea
C11 proven Pack radio: Coding
C12 proven Pack radio: Knowledge work
C13 proven Empty submit shows inline validation, fires no request
C14 red Ctrl+Enter submits the composer
C15 red Create round-trips and the row appears in the list
C16 red EngineNotice reflects the current runtime state
C17 red Cancel offered, row moves to Cancelled, second cancel refused 409
C18 proven Empty state when the list holds no tasks
C19 proven List load failure renders the retry banner
C20 proven Create failure renders its own error, keeps the draft
C21 proven Expired session reported instead of a silent failure
C22 proven GET /v1/agent/tasks requires auth, exactly 401
C23 proven Task console DOM matches the ledger, no transcript pane
C24 proven Sign-in DOM matches the ledger

The four reds are one test, red on #881: edge-api's control-plane client times out at 15 seconds while CreateTask blocks inline on the sandbox launch for up to five minutes, so a launch slower than 15 seconds answers the browser 500 for a task that is created and runs to completion. Measured at 18.0 seconds, with the task reaching succeeded.

#886 is the second defect this suite found: Service.Cancel is a bare database transition and the Engine interface has only Launch, so cancelling never reaches the sandbox and never returns its concurrency slot. Two cancels held both of the demo account's slots for over half an hour. C17's ledger entry and its assertion comment now both say what the control actually verifies and explicitly disclaim stopping the task, because claiming otherwise would be a false proof of exactly the kind this review is looking for.

The job is manual-only (workflow_dispatch) until both land, and is deliberately outside report-failure's needs, so a known-red probe cannot hold the dedupe issue open and mask a real migrate failure.

Also fixed since the second review

A credential leak that would have been worse than the one fixed earlier that day: the job uploaded test-results/ wholesale while the project retained traces and videos on failure, and this job is expected to fail. A Playwright trace stores request headers and cookies verbatim, so a failed run would have published a live bearer and the sb-*-auth-token cookie, which carries a refresh token for a shared account, into a public repository's artifacts for ninety days. The project now sets trace: "off" and video: "off", and the upload is restricted to test-results/*.json and *.txt. Two layers, so re-enabling either one alone still cannot publish it.

agent-workspace-inert-registry.json is deleted. Nothing read it.

@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

Generated coverage ledger, verbatim

The artifact itself, since it is gitignored by design and CI uploads it rather than committing it. Produced by node scripts/build-agent-workspace-coverage.mjs from the run reported in the comment above, at head 297ad60e.

agent-workspace-coverage.json (19/24)
{
  "surface": "agent-workspace (Cowork, apps/agent-console, served at /agent-workspace)",
  "generated_at": "2026-08-14T16:01:44.175Z",
  "total": 24,
  "proven": 19,
  "ratio": "19/24",
  "not_present": [
    {
      "what": "Live transcript or output pane",
      "why": "The task console has no live transcript surface yet (task-console.tsx: \"There is no live transcript yet\"), so there is no control to enumerate.",
      "guard": "C23 asserts the tasks screen renders no role=log element, so this claim fails the run the day a transcript ships."
    }
  ],
  "unproven": [
    {
      "id": "C8",
      "description": "Sign-in: submit button (valid credentials path)",
      "requires_creds": true,
      "screen": "sign-in",
      "proven": false,
      "status": "skipped",
      "attempts": 1,
      "reason": "HIVE_QA_AGENT_PASSWORD not set. Every other authenticated control here is proven from a session minted by tests/e2e/support/live-auth.mjs, which needs no password. This one is the password submit path itself, so it can only be proven by typing a real password into the real form. Supplying an existing password is fine; rotating the shared account to invent one is forbidden (docs/live-test-auth.md).",
      "test_title": "[C8] sign-in with valid credentials lands on the task console, not the chat app"
    },
    {
      "id": "C14",
      "description": "Ctrl+Enter submits the composer",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C15",
      "description": "Task create round-trips through POST /v1/agent/tasks and the new row appears in the list",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C16",
      "description": "EngineNotice reflects the deployment's current runtime state, so it is absent while a real agent runtime is configured",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C17",
      "description": "Cancel button is offered on a non-terminal row, moves that row to Cancelled and is then withdrawn, and a second cancel on the now-terminal task is refused server side with 409. It does NOT stop the sandbox: control-plane's Engine interface has only Launch and Service.Cancel is a bare database transition, so no code path from this button can reach the running sandbox at all (issue #886). Any wording claiming this control stops a task would be a false proof.",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    }
  ],
  "results": [
    {
      "id": "C1",
      "description": "Sign-in: email input",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C1][C2] email and password inputs accept and reflect input"
    },
    {
      "id": "C2",
      "description": "Sign-in: password input",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C1][C2] email and password inputs accept and reflect input"
    },
    {
      "id": "C3",
      "description": "Sign-in: submit button (invalid credentials path)",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C3][C4] submit with wrong credentials fires the token request and renders the error alert"
    },
    {
      "id": "C4",
      "description": "Sign-in: error alert region",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C3][C4] submit with wrong credentials fires the token request and renders the error alert"
    },
    {
      "id": "C5",
      "description": "Sign-in: required attributes are advisory only, because the form is noValidate, so an empty submit reaches the server and surfaces the same error as a wrong password",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C5] required fields are advisory only, because the form is noValidate"
    },
    {
      "id": "C6",
      "description": "Unauthenticated /agent-workspace redirects to sign-in",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C6] unauthenticated workspace entry redirects to the prefixed sign-in page"
    },
    {
      "id": "C7",
      "description": "Unauthenticated /agent-workspace/tasks redirects to sign-in",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C7] unauthenticated direct /tasks visit also redirects to sign-in"
    },
    {
      "id": "C8",
      "description": "Sign-in: submit button (valid credentials path)",
      "requires_creds": true,
      "screen": "sign-in",
      "proven": false,
      "status": "skipped",
      "attempts": 1,
      "reason": "HIVE_QA_AGENT_PASSWORD not set. Every other authenticated control here is proven from a session minted by tests/e2e/support/live-auth.mjs, which needs no password. This one is the password submit path itself, so it can only be proven by typing a real password into the real form. Supplying an existing password is fine; rotating the shared account to invent one is forbidden (docs/live-test-auth.md).",
      "test_title": "[C8] sign-in with valid credentials lands on the task console, not the chat app"
    },
    {
      "id": "C9",
      "description": "AppHeader: Back to chat link",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C9] the Back to chat link returns to the chat origin"
    },
    {
      "id": "C10",
      "description": "Task composer: instructions textarea",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C10][C11][C12] composer textarea and pack radios reflect selection"
    },
    {
      "id": "C11",
      "description": "Pack radio: Coding",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C10][C11][C12] composer textarea and pack radios reflect selection"
    },
    {
      "id": "C12",
      "description": "Pack radio: Knowledge work",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C10][C11][C12] composer textarea and pack radios reflect selection"
    },
    {
      "id": "C13",
      "description": "Start task button with empty instructions shows inline validation and fires no request",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C13] submitting an empty task shows the inline validation message, fires no request"
    },
    {
      "id": "C14",
      "description": "Ctrl+Enter submits the composer",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C15",
      "description": "Task create round-trips through POST /v1/agent/tasks and the new row appears in the list",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C16",
      "description": "EngineNotice reflects the deployment's current runtime state, so it is absent while a real agent runtime is configured",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C17",
      "description": "Cancel button is offered on a non-terminal row, moves that row to Cancelled and is then withdrawn, and a second cancel on the now-terminal task is refused server side with 409. It does NOT stop the sandbox: control-plane's Engine interface has only Launch and Service.Cancel is a bare database transition, so no code path from this button can reach the running sandbox at all (issue #886). Any wording claiming this control stops a task would be a false proof.",
      "requires_creds": true,
      "screen": "tasks",
      "proven": false,
      "status": "unexpected",
      "attempts": 1,
      "reason": "Error: POST /v1/agent/tasks must answer 201. A 500 here, with the task still appearing in the list below, is issue #881, the edge-api create timeout described above.\n\n\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n\nExpected: \u001b[32m201\u001b[39m\nReceived: \u001b[31m500\u001b[39m",
      "test_title": "[C14][C15][C16][C17] a real task is created, its row is marked cancelled on request, and a second cancel is refused"
    },
    {
      "id": "C18",
      "description": "Empty state (\"Nothing submitted yet\") when the list holds no tasks",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C18] the empty state renders when the account has no tasks"
    },
    {
      "id": "C19",
      "description": "Task list load failure renders the retry/reload banner",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C19] a failing task list load renders the retry banner, not a crash"
    },
    {
      "id": "C20",
      "description": "Task create failure renders its own error, keeps the draft text",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C20] a failing task create renders its own error, keeps the draft text"
    },
    {
      "id": "C21",
      "description": "Expired session reported instead of a silent failure on submit",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C21] an expired session is reported in place of a silent failure"
    },
    {
      "id": "C22",
      "description": "GET /v1/agent/tasks requires auth (401 unauthenticated)",
      "requires_creds": false,
      "screen": "api",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C22] agent-task API is reachable on the chat origin and still requires auth"
    },
    {
      "id": "C23",
      "description": "Every focusable control the task console renders in the light DOM is claimed by this ledger, and no live transcript pane exists yet",
      "requires_creds": true,
      "screen": "tasks",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C23] every focusable control on the task console is claimed by the ledger"
    },
    {
      "id": "C24",
      "description": "Every focusable control the sign-in screen renders in the light DOM is claimed by this ledger. Deliberately outside the authenticated group, so the denominator guard is not disabled by the same missing environment variable that would collapse the numerator.",
      "requires_creds": false,
      "screen": "sign-in",
      "proven": true,
      "status": "expected",
      "attempts": 1,
      "reason": null,
      "test_title": "[C24] every focusable control on the sign-in screen is claimed by the ledger"
    }
  ]
}

Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-controls.json
Comment thread .github/workflows/deploy-demo-box.yml
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

plain adversarial stream

Read the full diff (gh pr diff 799), fetched the PR head (297ad60), and diffed intermediate commits against it rather than trusting the commit messages alone.

Findings posted inline (2):

  1. apps/web-console/tests/e2e/_probe/agent-workspace-controls.json:5 (MEDIUM/HIGH, central to the PR's own argument): build-agent-workspace-coverage.mjs is byte identical between commit 061bbfa (floor 18, one commit earlier in this same PR) and this PR's head (floor 19). The floor check, the notRun check, and the hard-fail-on-missing-mint-env change all landed in 061bbfa. I ran the unchanged script directly against a synthetic collapsed-run report and confirmed both the 18-floor and 19-floor versions exit 1 on it. The claim in 297ad60's commit message, "before this change that scenario exited 0," is not supported: the gate was already effective one commit earlier. The floor mechanism itself works, that part I verified by execution, but the specific mutation-test narrative attributing it to this commit is inaccurate.

  2. .github/workflows/deploy-demo-box.yml:884 (MEDIUM): confirmed the job runs only if: github.event_name == 'workflow_dispatch' and is excluded from report-failure's needs, exactly as claimed. But this means on the actual deploy path (push to main), the job never runs at all right now. The PR's "the check working" framing describes a manual dispatch, not anything that happens automatically after this merges. Self-disclosed in the code comment, but worth stating plainly for the merge decision: today this is a measurement tool a human has to remember to trigger, not a gate.

Checked and clean, no finding filed:

  • The four red controls (C14-C17) trace to real, currently-open issues Cowork create reports a failure for a task that is actually running #881 and Cancelling a Cowork task does not stop the sandbox, so its concurrency slot stays held #886 (confirmed via gh issue view), not fabricated cover for known breakage.
  • agent-workspace-inert-registry.json deletion: confirmed via git log -S across all history that no .ts/.mjs file ever read this file. It shipped as a permanently-empty placeholder (entries: []). Safe to delete.
  • .github/workflows/deploy-demo-box.yml changes here are a single new hunk (the new agent-workspace-coverage job, +141/-0) fully in scope for this PR's stated purpose. PR fix: authenticate demo box git pull with the job's own GITHUB_TOKEN #898 touches the same file but a disjoint region (the git-pull auth fix around line 211), no overlap, both MERGEABLE/CLEAN independently.
  • The undeclared-tag guard ([C99] with no ledger entry) is directly readable in the shipped script logic (lines ~156-166) and correctly exits 1; no live run needed to trust this one.
  • trace/video off override for the agent-workspace project is a real override of the base config's retain-on-failure default, matches the stated credential-leak rationale.
  • live-auth.ts exports reauthenticate as imported; flake-reporter.ts and verify-spec-collection.mjs both exist as referenced.

Not independently verified (no live run available to me): the C1 descriptor-flip mutation against C24's DOM-equality check. The mechanism is sound by inspection (symmetric equality comparison), so I'm not flagging it, but note that none of the three mutation-test claims in 297ad60 left an executable artifact in the diff; all three are prose-only narration of manual, reverted local runs, including the one I found to be inaccurate above.

Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts Outdated
Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts Outdated
Comment thread apps/web-console/scripts/build-agent-workspace-coverage.mjs
Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

typescript-reviewer stream

Reviewed head 297ad60edc718b1c23ab7b76a5418e8d1336e284. CI status at review time: web-console type+unit+build is SUCCESS (the relevant gate for this diff); one unrelated check (Desktop app Rust+TS) was still IN_PROGRESS and does not touch any file in this diff. mergeStateStatus is BLOCKED with an empty reviewDecision, consistent with zero prior reviews rather than a failing check.

Posted (4 inline comments):

  1. agent-workspace-flows.spec.ts:479, use of unknown on the task-list response shape, conflicting with the repo's stated ban on any/unknown/unsafe casts.
  2. agent-workspace-flows.spec.ts:395-400, the create-attribution waitForRequest/waitForResponse predicates match on .includes("/v1/agent/tasks"), which also matches the cancel endpoint; not exploitable today only because of call order and workers: 1.
  3. build-agent-workspace-coverage.mjs:79/151 (cross-referencing agent-workspace-controls.json), the DOM-enumeration denominator guard (C23/C24) only covers the 9 of 24 controls with a non-empty dom field; the other 15 (62.5%) have zero automated defense against a [C#] tag with no real assertion, which is the disclosed tag-stuffing limitation but larger in practice than "a limitation" suggests. Also flagged a silent last-write-wins on a duplicate tag in the same file.
  4. agent-workspace-flows.spec.ts:301-302, cosmetic double blank line.

Checked and found clean:

  • Type safety otherwise: no any, no as casts, no non-null assertions, no @ts-expect-error/@ts-ignore anywhere in the diff. resolveJsonModule and strict are already on in apps/web-console/tsconfig.json (unchanged by this PR), so the JSON import and the one unknown site both typecheck as claimed.
  • Proof-identity fix (item 2 in the brief): response.request() === submitted is the only site in this diff where a response is treated as create-evidence under a page-level race; the cancel-refusal call and the post-create list read use a direct request/response pair and content-keyed matching respectively, neither of which has the shape-matching failure mode PR test: live interaction coverage gate for the chat surface #809 hit. No second, undisclosed location found.
  • Disclosed forced passes: C8's test.skip on missing HIVE_QA_AGENT_PASSWORD, the cleanup .catch(() => undefined) in the finally, and the single expect.soft on the create's 201 status are exactly what's described, no additional undisclosed skip/soft-assertion/disabled-assertion pattern found. All page.route registrations are awaited; no floating promises, no forEach(async ...), no ==, no var, explicit return types on every helper.
  • playwright.config.ts: the agent-workspace project's literal trace: "off"/video: "off" overrides the global retain-on-failure regardless of retries (project use wins over top-level use, and a literal string isn't raised by a retry). screenshot: "only-on-failure" is inherited from the global default and is viewport-only (no URL bar, no headers, no cookies). The artifact-upload glob in deploy-demo-box.yml (test-results/*.json, test-results/*.txt, single-level) doesn't reach nested per-test screenshot/trace/video paths either way, so there's no leak path even if that global default were something riskier. Confirmed clean.

No CRITICAL findings. Findings above are MEDIUM (items 1-3) and a LOW cosmetic nit (item 4). Recommend closing item 3 before relying on the ratio in future gating decisions; items 1, 2 and 4 are not blocking.

sakibsadmanshajib and others added 2 commits August 14, 2026 12:20
… request

Correction to the message this commit originally carried. It claimed the
collapsed-run scenario exited 0 before this change and 1 after, crediting this
commit with the floor. That is wrong, and an adversarial review caught it: the
builder script is byte-identical to the previous commit, and the floor, not-run
and hard-fail logic all landed there.

Measured, each version run against one synthetic collapsed-run report with its
own ledger:

  86ff832 (this PR as first filed)  exit 0   7/22   no floor
  6d4a062 (first rework commit)     exit 0   7/23   no floor
  061bbfa (previous commit)         exit 1   8/24   floor 18
  297ad60 (this commit)             exit 1   8/24   floor 19

So the collapsed run does go from exit 0 to exit 1 across this pull request,
which is the comparison a reviewer deciding whether to merge it cares about,
but that happened one commit earlier than claimed. What this commit actually
does to the gate is raise the floor from 18 to 19, the count a real run proves.

The rest of this commit stands: the create response is now asserted to be the
one belonging to the request the keypress issued, by object identity, because
PR #809 shipped the shape-match form and it was a real proof-attribution bug
there. Containment is stated in the spec header. Nothing in this repository
writes the floor; it moves only in a commit like this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CodeRabbit, major: the coverage step ran under set -euo pipefail with the
builder piped into tee, so a non-zero exit aborted the step and the job summary
block never ran, and the FAIL lines go to stderr so tee captured none of them.
A failing ledger published neither the reason nor the summary, which is the one
case the step exists for. The status is captured, stderr is merged into the
pipe, the summary is written, and the saved status is the step's exit code.

CodeRabbit, security: the artifact glob was test-results/*.json, which also
published the raw Playwright report and its captured stdout and stderr from a
run against a live deployment. Nothing prints a token today, so this was a
posture gap rather than a leak, but the two files are now named so a future log
line cannot widen the artifact.

CodeRabbit, stability: the created-id assertion ran before the try block, so a
create that succeeded but did not come back in that one list read ended the test
with no cancel call, leaving a live sandbox holding a slot nothing frees. The id
is now seeded from the create body first, and the assertion moved inside the
try, so the finally always covers a successful create.

CodeRabbit, integrity: two tests carrying the same [C#] tag silently
last-write-wins, so a failing control could be reported as proven. Duplicates
now fail the build.

TypeScript stream: the create predicate matched the substring /v1/agent/tasks,
which also matches the cancel endpoint, so the identity correlation held only
because of call order and workers: 1, both properties of this file rather than
of the assertion. It now matches the collection path exactly. The two wire
shapes are named interfaces rather than inline unknown, which this repository
forbids, with the typeof guards kept so the shape stays checked rather than
asserted.

The denominator guard's real size is now stated where it is claimed: 9 of 24
controls carry a dom entry and are compared against the deployed page. The other
15 are behaviours with no node to enumerate, and no sweep at rest can cover a
conditional region, so their only defence is that each has a test, which the
not-run, duplicate and floor checks enforce.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sakibsadmanshajib
sakibsadmanshajib force-pushed the test/workspace-interaction-coverage branch from 297ad60 to ef941a4 Compare August 14, 2026 16:20
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

Correction to my own evidence comment, and stream accounting

Head is now ef941a4a.

The mutation-proof claim was overstated. The reviewer is right.

I claimed the collapsed-run scenario "exited 0 before this change and 1 after", crediting head 297ad60e. I verified it and the reviewer is correct: git diff 061bbfad 297ad60e -- apps/web-console/scripts/build-agent-workspace-coverage.mjs is empty. The floor, the not-run check and the hard-fail on mint environment all landed in 061bbfad, one commit earlier.

Measured properly, each version run against one synthetic collapsed-run report using its own ledger:

commit exit ratio minimum_proven
86ff8326 this pull request as first filed 0 7/22 none
6d4a0624 first rework commit 0 7/23 none
061bbfad 1 8/24 18
297ad60e 1 8/24 19

What the mutation actually demonstrates, stated the way it should have been: across this pull request the collapsed run goes from exit 0 to exit 1, so the gate this pull request adds does catch a collapse that the pull request's own starting point did not. That is still the comparison worth having for a merge decision. What it does not demonstrate is anything about the head commit specifically, whose contribution to the gate is raising the floor from 18 to 19.

I also found the synthetic report was not faithful while checking this. It set test.status to skipped but left the attempt's own results[0].status alone, so the pre-rework builder (which read results[0]) still saw passed and reported an inflated 17 of 22. Corrected, and the table above uses the faithful version. The commit message and the pull request body are both corrected.

An overstated proof is worse than a missing one. This one cost a walk-back, and the correction is the useful part of it.

The job does not run on the push path, and the body now says so

if: github.event_name == 'workflow_dispatch', and it is outside report-failure's needs. That was in a code comment only, which was not good enough given this repository already carries one gate whose live arm is permanently skipped. The pull request body now has a section stating it, why (issue #886 makes every push-triggered run cost the demo account a concurrency slot), what the gate is worth meanwhile (a runnable instrument with a failing floor, a failing denominator and a live red, not an automatic check), and the exact two one-line changes that turn it on once #881 and #886 land.

Findings fixed in ef941a4a

From the CodeRabbit stream:

  • major The coverage step ran under set -euo pipefail with the builder piped into tee, so a non-zero exit aborted the step before the job summary block, and the FAIL lines go to stderr so tee captured none of them. A failing ledger published neither reason nor summary, which is the one case the step exists for. Status captured, stderr merged into the pipe, summary written, saved status becomes the step's exit code.
  • minor, security The artifact glob test-results/*.json also published the raw Playwright report with its captured stdout and stderr from a run against a live deployment. A posture gap rather than a leak, since nothing prints a token, but the two files are now named so a future log line cannot widen the artifact.
  • minor, stability The created-id assertion ran before the try, so a create that succeeded but did not appear in that one list read ended the test with no cancel call at all. The id is seeded from the create body first and the assertion moved inside the try.
  • minor, integrity Two tests carrying the same [C#] tag silently last-write-wins, so a failing control could be reported proven. Duplicates now fail the build.

From the TypeScript stream:

  • The create predicate matched the substring /v1/agent/tasks, which also matches /v1/agent/tasks/{id}/cancel. The identity correlation held only because of call order and workers: 1, both properties of this file rather than of the assertion. It now matches the collection path exactly.
  • The two wire shapes are named interfaces instead of inline unknown, with the typeof guards kept.
  • Blank-line run collapsed.

The denominator guard is smaller than I said

I described C23 and C24 as making the denominator independent of the numerator. The accurate figure is 9 of 24 controls, the ones carrying a non-empty dom entry: C1, C2, C3, C9, C10, C11, C12, C13, C17. The other 15 have no automated defence at all.

I looked for a cheap way to widen it and concluded there is not an honest one. The other 15 are behaviours rather than elements: two redirects, an API status, four error states, three submit paths, the engine notice, the empty state, the noValidate finding, and the two guards themselves. There is no node to enumerate for "an expired session is reported". A live-region sweep at rest was the obvious candidate and it fails on its own terms, because every one of those regions is conditional and renders in none of the states the page sits in at rest, so the sweep would find zero and look like a guard while being one. Widening it honestly means driving each state, which is one test per control, which is the thing that already exists and is what the not-run, duplicate-tag and floor checks enforce.

The narrow claim that survives: a new focusable control cannot ship on either screen without raising the denominator. A new behaviour can, and the suite would not notice. That is now the wording in the spec, at the guard itself.

Stream accounting

stream state
CodeRabbit CLI RAN locally against origin/main, 8 files, 4 findings, all fixed above. Note the CodeRabbit check on this pull request reports pass with "Review rate limited", which is not a review; the CLI run is the real one.
Adversarial pass RAN, dispatched separately. 2 inline comments plus a summary. Both addressed above.
TypeScript reviewer RAN, dispatched separately. 4 inline comments. Addressed above.
ecc:code-review SKIPPED. This agent's toolset is Read, Write, Edit and Bash with no Skill or Agent tool, so it cannot dispatch a skill-based stream. Not a clean pass.
/codex:adversarial-review SKIPPED, same structural reason. The codex binary is present but the review entry point is a slash command this toolset cannot invoke.
ecc:e2e-testing SKIPPED. The skill-router:e2e-specs hook asks for it on every edit to this spec and it cannot be satisfied from here for the same reason. The hook advises rather than denies, so no edit was blocked.

What no stream has seen

Everything from 82b9fc25 onward postdates the findings raised at 7c47c0c6. Unreviewed by any stream: the four blocking fixes in 061bbfad, the mutation proofs and attribution fix in caa5bab2, and the stream fixes in ef941a4a itself. The corrected mutation table above is the only part of it that has been independently checked, and only because the reviewer checked it and was right.

Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts Outdated
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

ecc:code-review stream (dispatched separately), scoped to 82b9fc2..ef941a4

This toolset has no Skill or Agent tool, so the dispatched skill invocation was structurally impossible; reviewed the delta directly (Read/Bash/gh api) instead of skipping. Range resolved as d40d962b..ef941a4a71c (three commits: 061bbfa, caa5bab, ef941a4), the same content the stated 82b9fc25..ef941a4a range covers once the branch's earlier rebase (identical patch-id, different parent tree) is accounted for. Read the prior TypeScript-reviewer and plain-adversarial comments first; not repeating settled findings.

Filed (1 inline, HIGH): the try/finally fix for the create-orphan bug only moved the expect(createdId, ...) assertion into try. page.reload() and the list waitForResponse still run before the try opens, so a timeout there (not just a missing row) still skips the finally cancel and orphans a real sandbox, holding a concurrency slot per #886.

Checked and clean, no finding:

  • Duplicate [C#] tag handling: now a hard process.exit(1) (FAIL: ... One control, one test.), not a different silent resolution. Confirmed by reading the full script.
  • pipefail/tee fix: status=0; ... 2>&1 | tee ... || status=$?; ... exit "$status" correctly neutralizes -e on that pipeline and captures stderr; verified the job summary step still has if: always() and runs regardless of the probe step's own outcome.
  • Artifact upload: now two named files only (agent-workspace-coverage.json, coverage-summary.txt), not test-results/ or playwright-report/ wholesale. No secret-bearing content reaches either file (bearer token asserted as boolean, never interpolated into a message).
  • Coverage-floor mutation proof: independently re-ran the shipped build-agent-workspace-coverage.mjs against a hand-built synthetic collapsed-run report using the shipped agent-workspace-controls.json. Reproduced the claimed 8/24, FAIL: coverage regressed: 8 controls proven, floor is 19, exit 1. The corrected narrative in the walk-back comment holds for the code as shipped.
  • isTasksCollection predicate: exact pathname equality, so it matches a query string on the collection endpoint but not /v1/agent/tasks/{id}/cancel or a trailing slash. Correct.
  • Named interfaces (AgentTaskWire, AgentTaskListWire): APIResponse.json() returns Playwright's own Serializable = any, so assigning it to a named optional-field interface isn't a disguised cast beyond what the library itself returns; every field is still guarded with typeof before use, same as the rest of the file. No any, unknown, or as in the diff.
  • Disclosed 9/24 DOM-enumeration limitation: reasoning holds. The other 15 are conditional/behavioral states with no node at rest; a sweep would read 0 regardless of whether the behavior is wired, which is a false guard, not a cheap honest one. No cheaper honest widening found.
  • deploy-demo-box.yml vs PR fix: authenticate demo box git pull with the job's own GITHUB_TOKEN #898: independently ran git merge-tree --write-tree pr799 pr898, clean tree write, no conflict output. Confirms the two PRs' hunks in that file don't overlap.

Two defects, one class. The ecc:code-review stream found the first and running
the proof for it found the second.

The try block opened after page.reload() and the list wait, so a throw in
either, a slow reload or a list GET that never resolved inside its sixty second
budget, skipped finally entirely and orphaned a live sandbox. That is the same
failure the previous commit claimed to close, closed at the instance rather
than at the window. The guarded region now opens the moment the create POST has
left the browser, which is the moment a task can exist, and finally re-derives
the task id from the API when nothing above got far enough to name it. Not
knowing the id is not a reason to leave a task running.

Proving that surfaced the second. Every waiter here is armed before the action
it observes, because a waiter armed afterwards races its own event, which
leaves the promise floating while the stack is parked on the action. A waiter
that times out in that window is an unhandled rejection, and Playwright turns
that into an immediate test abort that does not unwind the stack, so finally
never runs however wide the try is. Absorbed at the source in armed(), so a
waiter rejection is only ever delivered at its await, inside the guard.

Measured against the demo box, identical sabotage of the list waiter to a one
millisecond timeout:

  before  task 1786726519060  left running, later succeeded, orphaned
  after   task 1786726698319  cancelled

The error is also reported once rather than twice now, which is the unhandled
rejection no longer being counted separately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts
Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts
Comment thread apps/web-console/tests/e2e/_probe/agent-workspace-flows.spec.ts Outdated
@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

typescript-reviewer stream — second pass, scoped to ef941a4a..8ce7d340

Scope: only agent-workspace-flows.spec.ts, the try/finally-widening + armed() delta.

  1. Diagnosis: correct, and verified against source rather than taken on trust. apps/web-console/node_modules/playwright/lib/worker/workerMain.js registers process.on("unhandledRejection", ...) and process.on("uncaughtException", ...), both routed to unhandledError, which fails the current test and calls testInfo._interrupt() (testInfo.js), which resolves an interrupt promise and calls timeoutManager.interrupt() rather than throwing into the test body's own stack. That is a race, not a guaranteed unwind, which matches the sabotage evidence (task left running) and the fix (arm at creation so the rejection is never delivered unhandled). Generalizes correctly to any future floating waiter in this codebase, Playwright-specific or not.
  2. Coverage is real for the three call sites this delta touches, but structural-vs-convention: two pre-existing waiters in the same file (`tokenServer" pattern at lines 331 and 346, the C5 and C3/C4 tests) use the identical arm-before-action shape and are not wrapped. Lower blast radius (no finally/resource behind them) but same crash risk. Posted inline.
  3. armed() does not swallow a real failure: it attaches a no-op .catch() and returns the original promise, so the real rejection still surfaces exactly once at the real await. Traced all three call sites; none are skipped by an early branch that would leave a rejection silently absorbed only by the no-op handler.
  4. findTaskIdByBrief's uniqueness rests on a Date.now() suffix, not an identity guarantee. Thin, workflow_dispatch-only edge case flagged inline; not exploitable today.
  5. Baseline unchanged: diffed the assertion set line by line, same messages, same timeouts (30_000 / 330_000 / 60_000 / 201 / 409), same soft-vs-hard split, no assertion moved into a swallowing catch (the region has a finally only, no catch, so nothing inside it can silently pass). 19/24 claim not undermined by this delta.

No CRITICAL or blocking findings. Three low/medium notes posted inline (structural-coverage gap, brief-uniqueness edge case, a cosmetic indentation leftover). Clean to merge on this delta's own merits.

Wrapped the two pre-existing token waiters in the C5 and C3/C4 tests. Both use
the same arm-before-action shape as the three this delta already wrapped, and
neither sits in front of a finally with a resource to leak, so the blast radius
today is smaller. It is the same risk class either way, and leaving a category
half closed after fixing the instance is the pattern that has already cost this
branch two rounds. All five waiters in the file now go through armed().

The task brief carries a random UUID alongside the timestamp. findTaskIdByBrief
matches on that exact string to decide what the cleanup cancels, so uniqueness
against a shared box has to be a property of the value rather than an
assumption that no two runs start in the same millisecond. A collision would
have one run cancel a task another run created, which is worse than the leak
the cleanup exists to prevent.

Realigned the C15 block comment opener, which sat at eight spaces over
seven-space continuations after the comment moved into the widened try.

Baseline re-run after all three, unchanged: 19 of 24, 15 passed, 1 skipped,
1 failed, flake rate 0%. The failing test is the same four controls on the same
two known product defects, this time the launcher refusing the launch (#886)
rather than the create timeout (#881). Nothing left running on the box: the
task this run created is terminal, and its brief carries the new UUID suffix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sakibsadmanshajib
sakibsadmanshajib merged commit 7e4ea91 into main Aug 16, 2026
19 checks passed
@github-actions
github-actions Bot deleted the test/workspace-interaction-coverage branch August 16, 2026 08:32
sakibsadmanshajib added a commit that referenced this pull request Aug 16, 2026
… copy

The agent-workspace finding was a false positive, and the defect was in
this guard, not in #799. tools/verify-spec-wiring.mjs resolved --project
against playwright-spec-manifest.json's configs object, a hand-maintained
copy of what Playwright itself declares. PR #799 added the agent-workspace
project to playwright.config.ts while this branch's copy of that object
was mid-flight, so the copy went one entry stale and the guard blamed the
workflow instead of its own authority list. Verified directly against
Playwright rather than trusting either side: `npx playwright test
--project=x --list --reporter=list` (a nonexistent project name always
names every real one in its own error) confirms agent-workspace is real,
collects 17 specs, and every other --project selection in the repository
already resolves correctly.

Fix is to the authority, not the entry: tools/verify-spec-wiring.mjs now
asks Playwright directly which projects a config declares (projectsOf,
same nonexistent-project-name probe method verify-spec-collection.mjs's
own comment already pointed at), instead of trusting the manifest's
configs object, which is deleted along with the $doc claim that it was
"verified by the same guard from the same listing" (it never was).
KNOWN_CONFIGS stays a small fixed list of config PATHS, the same trust
boundary CONFIGS in verify-spec-collection.mjs already accepts: a
workflow's own --config text is still never passed to a live Playwright
process, only checked against this list, since Playwright imports whatever
config path it is given and that is exactly the code-execution hole this
file's own design comment already warns about. Only which PROJECTS a known
config declares is now live rather than pinned.

This needs apps/web-console's own node_modules (Playwright installed),
which the "Repo policy lints" job deliberately does not carry (a 5-minute,
dependency-light job by design). Moved both the guard and its regression
test into the "Web console (type + unit + build)" job, right after the
Spec collection guard step that already has this cost paid, rather than
duplicate an install. verify-spec-collection.mjs's own now-redundant
configs comparison (the thing that was supposed to keep the manifest copy
honest, and evidently didn't) is removed with it; the project-collection
loop it shared stays, unchanged, for the spec/file dark-check that loop
also feeds.

Also corrected two claims from my own earlier report on this PR: Playwright
exits 1 on both an unknown --project and a valid one matching zero tests,
so "collects zero and reports success" is not a failure mode --project
selections can produce; the two error messages in this file claiming
Playwright "would collect nothing" were wrong and are corrected to say it
fails loudly instead.

Once the false-positive cause was fixed, the guard's real, correct finding
underneath it: tests/e2e/_probe/agent-workspace-flows.spec.ts moved from
dark to other (PR #799 wired it into deploy-demo-box.yml's
agent-workspace-coverage job, gated to workflow_dispatch, never an
ordinary pull request). dark-spec-allowlist.json updated to record that
move rather than silently drop it, split out of the shared group it used
to carry with the still-dark staging-flows.spec.ts.

Verified: guard passes on the merged tree (14/36 pull-request-gated, 20
other-trigger, 2 dark), its own regression suite still passes unchanged,
sibling lint-go-db-test-wiring.mjs unaffected, ci.yml still parses and its
own check-name lint still passes.
sakibsadmanshajib added a commit that referenced this pull request Aug 16, 2026
…promptly (#891)

Fixes #886. Fixes #881.

Two Cowork defects found by the live coverage suite and reproduced
directly against the API on the demo box on 2026-08-11. Both are demo
blockers, and PR #799 (a post-deploy job that launches and cancels a
real sandbox on every push) cannot merge until this does.

## Issue #886, cancel never reached the engine

Root cause: the `Engine` interface in
`apps/control-plane/internal/agenttask/engine.go` had exactly one
method, `Launch`, so `Service.Cancel` could only ever be a bare
`repo.Transition(..., StatusCancelled, ...)`. The row went cancelled
while the sandbox behind it kept running, and the launcher releases a
concurrency slot when its session ends
(`apps/agent-engine/internal/quota` hands `Acquire` a release func
called on session end). The slot therefore stayed held until the sandbox
finished on its own, roughly sixteen minutes on the demo box. Two
cancels exhausted `HIVE_QUOTA_USER_CONCURRENCY` and every later create
was refused with "agent engine could not start the task", which the
console renders as `Blocked` for a user whose task list shows nothing
running.

Fix: `Engine` now carries `Cancel(ctx, sessionRef) error`, and
`Service.Cancel` calls it. Both real implementations already had the
method (`agentengine.Remote` for the socket arm the demo box runs,
`agentengine.Engine` for the in-process arm), so the change is the
interface plus the call, not new launcher logic. `NotConfiguredEngine`
gets a trivially successful `Cancel`, since it never launched anything.

Why on the interface rather than an optional runtime type assertion: the
defect is precisely that the seam could not express "stop". A
compile-time method makes it impossible to wire an engine with no cancel
path, and there is no implementation that legitimately lacks one.

## Issue #881, create reported failure for work that succeeded

Root cause: `CreateTask` called `engine.Launch` inline with
`launchTimeout = 5 * time.Minute` while
`apps/edge-api/internal/agenttask.NewClient` builds its HTTP client with
`Timeout: 15 * time.Second`. A cold sandbox mount is routinely slower
than 15 seconds, so edge-api gave up first and answered the browser
`500`, while control-plane carried on (it deliberately detaches the
launch from the caller's context) and the task reached `succeeded`.
Measured live: `CREATE status=500 elapsed=18.0s` for a task that
finished 78 seconds later.

Fix: create returns the persisted `queued` task as soon as the row
exists and the launch runs on a background goroutine, which transitions
the task to `running` or to `failed` on its own. The console already
polls queued to running to done, and `CreateTask` already ran the launch
on a detached context, so this is the shape the code was built for.

Why not simply align the timeouts: raising edge-api's client to five
minutes makes an interactive create legitimately able to hang for five
minutes, and every intermediate proxy is free to cut it anyway, which
reproduces the same wrong answer with a longer fuse. edge-api's 15
seconds now bounds only short database work plus, for cancel, a stop
call bounded at 10 seconds inside control-plane (`engineCancelTimeout`),
deliberately under the client budget so a slow launcher can never
reproduce the same "failure reported for work that happened" shape.

A launch failure still reaches the caller, just on the next poll rather
than in the create response, so no task sits queued forever with no
signal.

## Concurrency decisions

The row transition is the atomic gate throughout.
`Repository.Transition`'s UPDATE carries the "not already terminal"
precondition, so exactly one caller can win it, and only the winner
touches the engine.

- **Double cancel.** The second cancel loses that guard, returns
`ErrTerminalState` (HTTP 409) and never reaches the engine. Asked for
exactly once, asserted in `TestService_Cancel_TerminalStateRejected`.
- **Cancel racing a completion.** Whichever transition commits first
wins. If the poller already recorded a terminal status the cancel is
rejected and the engine is left alone, which is correct: a terminal
status is what makes the launcher reap that session itself
(`SandboxEngine.Status` reaps on terminal). Asserted in
`TestService_Cancel_LosesRaceWithCompletion_LeavesEngineAlone`.
- **Cancel arriving before the launch finishes.** The row has no
`engine_session_ref` yet, so `Service.Cancel` has nothing to stop. The
in-flight launch goroutine then finds the task already terminal when it
tries to record `running`, and stops the session it just started. The
same path covers a `running` transition that fails for any other reason:
a session whose reference was never persisted can never be polled or
cancelled by anything else, so it is torn down rather than left holding
a slot for its full natural life. Asserted in
`TestService_CancelDuringLaunch_ReleasesTheSlotThatLaunchTook`.

Cancel order is transition first, engine second, deliberately. Stopping
the engine first would risk killing a sandbox for a task the database
then reports as succeeded.

An engine stop failure is logged for the operator (with the engine
detail, which never reaches a customer-visible field) and never returned
to the caller. The cancellation itself is already recorded, and a stuck
launcher is not something a customer can act on. Returning 500 for a
task that really is cancelled would be issue #881's shape all over
again.

## Tests, and how they were watched red

The cancel test asserts the concurrency slot, not the status code. A
status-code assertion passes against the broken code, which is how this
survived: `Service.Cancel` already returned a cancelled task.

`slotEngine` in `service_test.go` models the launcher's accounting
(`Launch` takes a slot and refuses at the ceiling, `Cancel` gives one
back), because `apps/agent-engine/internal/quota` is under another
module's `internal` and control-plane cannot import it.

Red baseline, run before any production change, with only the interface
method and a no-op `WaitForLaunches` stub added so the package compiled:

```
=== RUN   TestService_Cancel_FromRunning
    service_test.go:382: expected the engine session to be cancelled exactly once, got 0 calls
--- FAIL: TestService_Cancel_FromRunning (0.00s)
=== RUN   TestService_Cancel_TerminalStateRejected
    service_test.go:403: expected exactly one engine cancel across a double cancel, got 0
--- FAIL: TestService_Cancel_TerminalStateRejected (0.00s)
=== RUN   TestService_Cancel_ReleasesEngineConcurrencySlot
    service_test.go:441: cancel did not release the launcher slot: 1 still in use (issue #886)
--- FAIL: TestService_Cancel_ReleasesEngineConcurrencySlot (0.00s)
=== RUN   TestService_CancelDuringLaunch_ReleasesTheSlotThatLaunchTook
    service_test.go:462: CreateTask blocked on the engine launch (issue #881): edge-api gives up at 15s and reports a failure for a task that is in fact starting
--- FAIL: TestService_CancelDuringLaunch_ReleasesTheSlotThatLaunchTook (2.01s)
=== RUN   TestService_Cancel_LosesRaceWithCompletion_LeavesEngineAlone
--- PASS: TestService_Cancel_LosesRaceWithCompletion_LeavesEngineAlone (0.00s)
=== RUN   TestService_CreateTask_DoesNotBlockOnLaunch
    service_test.go:527: CreateTask blocked on the engine launch (issue #881): edge-api gives up at 15s and reports a failure for a task that is in fact starting
--- FAIL: TestService_CreateTask_DoesNotBlockOnLaunch (2.00s)
FAIL
```

An earlier run of the same tests hung the whole package for 600 seconds,
because against the broken code `CreateTask` blocks on the gated launch
forever. The create call now runs on its own goroutine inside the test
helper, so the defect fails one test in two seconds with a legible
message instead of timing out the suite.

Green after the fix, both packages, plus the full short suite for
control-plane, agent-engine and edge-api:

```
ok  github.com/sakibsadmanshajib/hive/apps/control-plane/internal/agenttask  0.115s
ok  github.com/sakibsadmanshajib/hive/apps/agent-engine/internal/engine      0.128s
```

`TestSandboxEngine_Cancel_FreesQuotaSlot` is added on the launcher side
as a characterisation test, not as coverage for this change: nothing in
`apps/agent-engine/internal/engine` changed here, so every assertion in
it survives a full revert of the fix. It pins the invariant
control-plane now depends on (ending a session frees its slot
immediately) and is labelled that way in the code. The regression
coverage for issue #886 is
`TestService_Cancel_ReleasesEngineConcurrencySlot`,
`TestService_CancelDuringLaunch_ReleasesTheSlotThatLaunchTook` and
`TestService_Cancel_LosesRaceWithCompletion_LeavesEngineAlone`, which
drive `Service.Cancel` and fail on revert.

Review round two added two more, both watched red first:
`TestService_LaunchSucceedsButTransitionFails_TaskFailsVisibly` (a
launch that succeeds and then cannot record itself must end failed, not
stranded in queued) and `TestService_LaunchPanic_DoesNotCrashTheProcess`
(a panicking engine must cost one task, not the process).

`-race` could not run locally (the toolchain image has no gcc, so cgo is
unavailable). CI runs `-race` on the module legs.

## These tests actually run in CI

`.github/workflows/ci.yml`'s explicit Go package list is on the `-tags
integration` step only. The plain unit-test step is `go test -count=1
-short -race -timeout 5m ./...` run per module against a matrix that
includes both `./apps/control-plane` and `./apps/agent-engine`, so both
touched packages are in scope. Neither new test carries a build tag, an
environment gate, or a `testing.Short()` skip. The only skip in either
package is the pre-existing `HIVE_TEST_DB_URL` gate in
`repository_test.go`, and `./internal/agenttask/...` is in the
control-plane integration list too.

## Known gaps this does not close, and the condition attached to one of
them

Both were found by the security review stream, both were confirmed by
tracing them myself, and neither blocks this merge.

- **Issue #899**: a sandbox the launcher registered whose reference
never reaches control-plane (a lost response, `launchTimeout` firing, or
this process dying mid-deploy) holds its slots until the launcher
restarts, because `reap` only runs from `Status` or `Cancel` and the
poller cannot see a row with no `engine_session_ref`. This change closes
the database-failure half of that shape; the transport half needs the
launcher to offer a way to re-derive a lost reference, so it cannot be
closed from this service. Mitigant: every deploy restarts the launcher
and clears the leak.
- **Issue #900**: `POST /v1/agent/tasks` has no rate limit, verified
through the edge middleware chain, so rows, goroutines and pool
checkouts are per-tenant unbounded. The launcher's quota gates the
sandbox launch, which happens after the row insert.

**Condition on #900, recorded here because it is a merge condition
rather than a nice-to-know:** it is tolerable only while Cowork stays
feature-gated. `featuregate.Require(FeatureCowork)` is doing the
containment a limiter would otherwise do. Opening `FeatureCowork`
broadly makes it intolerable, so whoever widens that gate owns closing
#900 first. Written into the issue as well.

## Not touched

The demo box, `role_pgx.go`, the accounts package, container
capabilities. No new dependency. No behaviour change to the launcher
itself.

## Buglog entry

To be appended to `.wolf/buglog.jsonl` on `main` in a separate
buglog-only pull request after this merges, per
`.claude/rules/openwolf.md`.

```json
{"id":"bug-2026-08-12-cowork-cancel-slot-and-blocking-create","date":"2026-08-12","title":"Cancelling a Cowork task leaked its concurrency slot, and create reported failure for a task that succeeded","error_message":"agent engine could not start the task (console: Blocked) after two cancels; CREATE status=500 elapsed=18.0s for a task that reached succeeded","root_cause":"The agenttask.Engine interface carried only Launch, so Service.Cancel was a bare database transition and never stopped the sandbox; the launcher only releases a concurrency slot when the session ends, so a cancelled task held its slot for the sandbox's full life. Separately, CreateTask blocked inline on a launch bounded at five minutes while edge-api's control-plane client times out at fifteen seconds, so the browser was told 500 for work that kept running.","fix":"Added Cancel to the Engine interface and called it from Service.Cancel after the row's atomic terminal guard, so the slot is released at cancel time. Made CreateTask return the persisted queued task and run the launch on a background goroutine, with the in-flight launch stopping its own session if it finds the task already terminal.","tags":["cowork","agent-engine","quota","concurrency","timeout","control-plane","issue-886","issue-881"]}
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Task creation now returns immediately with a queued status while
launch proceeds in the background.
  * Task status updates to running or failed after launch completes.
* Cancellation stops active sessions and promptly frees available
concurrency capacity.
* **Bug Fixes**
* Improved handling of cancellation during task launch, including race
conditions and cancellations before launch begins.
  * Failed launches now report safe, provider-neutral error messages.
* **Documentation**
* Updated task lifecycle and cancellation behavior documentation,
including asynchronous launch and bounded cancellation timing.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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