feat: return actionable next steps with every error and add two workflow prompts

[scrollDynasty]

What

Every application error now carries a nextStep telling the caller what to do about it, two workflow prompts join diagnose_page, and the union-typed tool arguments describe their shape in prose.

Actionable errors (ADR 0022)

applicationErrorResult replaces every raw message with a fixed per-code string. Correct for the boundary, but eighteen of twenty codes then said only what broke — the remediation lived solely in docs-site/reference/errors.md, which a client choosing its next tool call never reads.

A constant nextStep per code now ships on the wire. Every value is a compile-time constant naming BrowserMesh tools and arguments; nothing is interpolated from an error, a page, a locator, or configuration, so the field cannot become a leak channel however the failure arose. An integration test asserts a distinct next step for all twenty codes and proves a hostile locator cannot reach it.

INTERNAL_ERROR deliberately says nothing about where to report. Bug reporting reaches an agent through the MCP instructions, which an operator drops with BROWSERMESH_AGENT_GUIDELINES=false (ADR 0021); repeating the solicitation on the error channel would put it back past that opt-out, and an unattended run has nobody to ask.

Union arguments described in prose

A client that flattens oneOf/$ref during schema ingestion leaves the caller an untyped object, and nothing else in the published surface stated the shape. locator, capture, condition, action, and wait now carry their form in a description as well as in the union. That costs bytes — tools/list goes from roughly 87 KB to 94 KB — which is the argument for ADR 0024's budget rather than a reason to leave the unions unusable.

Two prompts

• compare_page_states — load one URL in two isolated sessions and report only what differs. The two sessions are the point: one session would carry the first state into the second reading.
• form_validation — drive a form to its validation errors, in the page and in the console. It says explicitly never to submit real credentials.

Documentation

docs-site/reference/limits.md is new: every bound BrowserMesh enforces in one table, including the ones browser_runtime_info does not report. errors.md now quotes the strings actually sent. README gains a "when to use BrowserMesh, and when not to" section that names the workflows other servers serve better, and the first-run Chromium download that can trip a short client connect timeout.

AUDIT.md records the measurements behind all of this. Three findings it raises are proposed but not applied, because each changes a published contract: ADR 0023 (support-request opt-in), ADR 0024 (a discovery-cost budget), ADR 0025 (element-action failure classification — a locator that matches nothing currently reports OPERATION_TIMEOUT, not ELEMENT_NOT_FOUND).

Verification

npm run verify green: 231 tests under the coverage thresholds, plus the build. The tools/list byte budget and the $ref round-trip equivalence both still pass with the added descriptions.

🤖 Generated with Claude Code

[claude]

Reviewed the diff against correctness, security, the AGENTS.md/SPEC contracts, and test coverage. The change is mostly additive and low-risk: a constant lookup table, five .describe() calls on existing unions, two static prompts, and documentation. CI is green across all 19 checks.

Contracts — clean. NEXT_STEPS lives in the MCP adapter, is typed Readonly<Record<BrowserMeshErrorCode, string>> so the compiler enforces exhaustiveness, and every value is a literal, so nextStep genuinely cannot become a leak channel — I checked that no error, locator, URL, or config value is interpolated anywhere in it. No new Playwright or MCP import reaches domain or runtime. Both prompts state explicit sessionId/pageId addressing and compare_page_states is built on two isolated sessions rather than two readings, which is the right side of the isolation invariant. Nothing touches the serial queue or lifecycle.

I spot-checked the new limits.md against the source rather than taking it on trust: SNAPSHOT_LIMITS (50,000/100,000 chars, 65,536/131,072 bytes, depth 100, refs 50/100, children 1,000, 20,000 nodes, 2,000,000 chars, 4 retained, 30s cursor), the observability defaults and ranges in config.ts, the limit-vs-maxPageSize discrepancy, the 32-wildcard glob cap, the 1–5 iframe chain, the 1–160 cursor, and the claim that browser_visible_text has no per-call bound argument. All accurate.

Two findings, both on the wire guidance itself rather than the mechanism, left as inline comments:

1. OPERATION_CANCELLED (and INTERNAL_ERROR) promise the operation left nothing behind and tell the agent to reissue. SerialQueue.run checks the abort signal after await task(), so a cancellation can be reported for an action that already landed — and asBrowserMeshError maps any post-action throw to INTERNAL_ERROR. That contradicts the Recovery paragraph this PR writes in errors.md, and the wire channel is the one an agent actually reads.
2. INVALID_ARGUMENT is raised from ~31 sites but its next step names only the locator/ref case — including on browser_observe with an out-of-range limit, which errors.md calls the clearest example of this code and which has no locator or ref at all.

One documentation gap outside the diff hunks, so no inline: README.md still opens its Prompts and resources section with "BrowserMesh publishes two MCP prompts" and lists only parallel_roles and diagnose_page. docs/IMPLEMENTATION_STATUS.md line 23 has the same stale list. tests/integration/prompts.test.ts now asserts four, so the README is the only place a user is told the wrong number.

Minor, no action needed: the PR description says the new test "asserts a distinct next step for all twenty codes". It asserts each code has a non-empty step differing from its own message — exhaustiveness is covered by the Record type and the min(1) schema, but two codes sharing an identical string would still pass.

🤖 Generated with Claude Code

[claude]

Review

The core of this is sound. NEXT_STEPS is a readonly record of compile-time constants indexed by error code alone, so exhaustiveness is enforced by the type and there is no path by which a locator, URL, raw cause, or config value reaches the field — the hostile-locator assertion in mcp.test.ts is the right shape for proving that. Nothing here touches session or page addressing, the serial queue, or lifecycle; prompts.ts and results.ts are both adapter-layer, and the two new prompts stay static templates that render text and return it. No resource is acquired, so nothing new can leak.

I checked the factual claims the new strings and docs make against the source, and the substantive ones hold:

• 30s ref lifetime (ELEMENT_REF_TTL_MS) and 30s cursor lifetime (SNAPSHOT_LIMITS.cursorTtlMs), both invalidated by main-frame navigation and by page close (playwright-browser-engine.ts:1004-1010, browsermesh-runtime.ts:444/488/496/504).
• Every tool and argument name cited by a nextStep or by a prompt exists in the published surface; 35 registered tools matches the README's count.
• limits.md matches config.ts, resource-limits.ts and snapshots.ts on every default and range I spot-checked, including the browser_observe limit case: schema max 200, runtime INVALID_ARGUMENT above the configured maxPageSize.
• left/right bounded at 128 does match safeLabel(DEFAULT_RESOURCE_LIMITS.session.maxNameChars), so the comment's reasoning is right.
• The first Chromium download does precede initialize — ensureBrowser runs before the transport connects in serveMcp.
• browserActionSchema's new description says 'a locator object' unquoted, so the existing not.toContain('"locator"') assertion on browser_action_and_wait still holds.

Three findings, all posted inline, none blocking:

1. browser_runtime_info cannot report the bounds two next steps send the caller to it for. LIMIT_EXCEEDED names maxChars, maxBytes, maxRefs and limit, then says browser_runtime_info reports the effective limits — it reports none of those four. INVALID_ARGUMENT makes the same claim, and lands on the exact case the new errors.md calls the clearest one. The limits.md added in this PR states the contradiction outright.
2. The distinctness the PR description claims is not asserted. The loop compares each nextStep to its own message; twenty identical strings would pass it.
3. docs/IMPLEMENTATION_STATUS.md still reads 87,367 bytes and lists two prompts. README's figure moved, that file's did not.

I could not run npm run verify in this environment, so the green result in the description is taken as reported rather than reproduced.

🤖 Generated with Claude Code

[claude]

Review

Reviewed the whole diff. The change is sound in its core: nextStep is a closed table keyed only by BrowserMeshErrorCode, so NEXT_STEPS[mapped.code] is total (every path into applicationErrorResult goes through asBrowserMeshError, which falls back to INTERNAL_ERROR), and every value is a literal — there is no interpolation path from an error, a locator, a URL, or configuration into the field. safeDetails is untouched. Three findings, one of which is worth fixing before merge.

Correctness

• SESSION_NOT_READY tells the caller to retry a state that can be terminal — inline on src/adapters/mcp/results.ts:52. readySession raises this code for a session whose creation failed as well as for one still being created; failed entries are retained and published by browser_session_list and browsermesh://sessions, so a client can address one and retry forever. This is exactly the single-cause assumption the INVALID_ARGUMENT comment in the same table deliberately avoids. docs-site/reference/errors.md:26 carries the same wording.

Test coverage

• The five union descriptions are untested on the wire — inline on src/adapters/mcp/server.ts:128. Only tool-level descriptions are asserted today; the byte budget is an upper bound and the $ref round-trip compares compacted against uncompacted, so a serialization change that drops union-level description would pass everything.
• Test counts in docs/IMPLEMENTATION_STATUS.md do not match the diff — inline on line 33. Four integration tests are added and no unit tests, but the line moves 156/72 to 179/82.

Verified and fine

• Every tool name and argument named in NEXT_STEPS and in the two prompts exists: browser_session_get, browser_page_list, browser_state_list/browser_state_save, browser_action_and_wait, interactiveOnly, includeRefs, maxRefs, limit, includeText, contextSettings, stateId, source requestFailed. BROWSERMESH_PERSISTENCE is the real variable name, and browser_runtime_info really does return browserLaunchState and exactly the four resourceLimits groups the two long next steps claim it does — and not the snapshot or observability bounds, as they say.
• The OPERATION_CANCELLED / INTERNAL_ERROR caveat matches SerialQueue.run: the signal is re-checked only after task() resolves, so an in-flight action is genuinely not aborted. Good that both codes are qualified rather than saying "reissue".
• The capture description saying {"kind":"viewport"} is the default when omitted matches the handler — an omitted capture falls through to {}.
• left/right bounded at 128 chars is exactly right against maxNameChars: 128 / maxNameBytes: 512: 128 codepoints can never exceed 512 UTF-8 bytes, so the prompt cannot produce a label browser_session_create would reject.
• Spot-checked docs-site/reference/limits.md against SNAPSHOT_LIMITS, DEFAULT_RESOURCE_LIMITS, and the config schema — the snapshot, session-label, observability, and screenshot numbers and ranges all match.
• Architecture contracts are unaffected: no runtime or domain file is touched, the prompts stay static templates with no runtime state and no LLM call, and both new prompts instruct explicit sessionId/pageId addressing (compare_page_states is built on two contexts precisely to avoid a shared one). No new Playwright or MCP import crosses a boundary.
• tools/list at ~94 KB stays well under the 115,000-byte budget, and AUDIT.md is not in the npm files list.

[claude]

Reviewed for correctness, leaks, the addressing/isolation contracts, and test coverage. The change is sound — two notes, both posted inline, neither blocking.

What I verified

nextStep cannot leak. NEXT_STEPS in src/adapters/mcp/results.ts is a Readonly<Record<BrowserMeshErrorCode, string>> of compile-time literals, indexed only by mapped.code. asBrowserMeshError can only produce a code from errorCodes, so the lookup is total and TypeScript enforces exhaustiveness — a new code fails the build rather than emitting undefined. Nothing is interpolated from the error, its cause, details, the page, or configuration, so the field is structurally incapable of carrying a locator, URL, token, or path. The added assertion in tests/integration/mcp.test.ts that a hostile LOCATOR_AMBIGUOUS payload cannot reach nextStep is a real guard, and expect(new Set(steps).size).toBe(errorCodes.length) catches the copy-paste regression this table is most prone to.

The strings are accurate, which for this feature is the whole risk. A nextStep that names the wrong tool or the wrong argument is worse than none, so I checked each claim against the code rather than the prose:

• Every tool named (browser_session_list/get/create, browser_page_list, browser_state_list/save, browser_snapshot, browser_observe, browser_runtime_info) is registered in server.ts, and every argument named (interactiveOnly, includeRefs, maxChars, maxBytes, maxRefs, limit, timeoutMs, exact, stateId) exists with the spelling used.
• SESSION_NOT_READY sending the caller to browser_session_get before retrying matches readySession (browsermesh-runtime.ts:1027): closing, closed, and failed-after-disconnect are peeled off first, so the code genuinely covers both the transient creating and the terminal failed. Telling the caller to retry unconditionally would have been an infinite loop.
• The OPERATION_CANCELLED / INTERNAL_ERROR caveat is correct. SerialQueue.run (serial-queue.ts:16-18) re-checks the signal only after await task(), so a cancelled click can already have landed. The does not promise a rollback the runtime cannot perform test pins it.
• BROWSERMESH_PERSISTENCE is real and defaults to true (config.ts:57); the 30-second cursor and ref lifetimes match SNAPSHOT_LIMITS.cursorTtlMs; browser_runtime_info really does not report the snapshot or observability bounds, so the hedge in INVALID_ARGUMENT and LIMIT_EXCEEDED is right rather than merely cautious.

Declining to solicit bug reports from INTERNAL_ERROR is the right call — routing it through the error channel would reinstate the request past the BROWSERMESH_AGENT_GUIDELINES=false opt-out.

Union descriptions cover the whole surface. Walking every inputSchema in server.ts, the union-typed top-level arguments are locator (19 element tools plus browser_drag_and_drops source/target), scope, capture, condition, action, and wait. All six now carry prose, since scope and drag_and_drop reuse locatorSchema. {"kind":"viewport"} really is what an omitted capture produces (server.ts:833-838). The budget is untouched at 115,000 against a payload the PR measures at ~94 KB, and .describe() on a discriminatedUnion changes no validation.

Contracts hold. prompts.ts and results.ts are adapter-layer; no Playwright type crosses into domain or runtime, no global current session or page appears, no serialization or lifecycle code is touched. Prompts stay static templates — text rendered and returned, no runtime state, no agent entity.

limits.md is accurate. I checked the tables against config.ts, resource-limits.ts, and snapshots.ts rather than trusting them: defaults, ranges, and variable names all match, including the limit 200-vs-100 schema/runtime split it calls out.

Notes

1. tests/integration/browser-runtime.test.ts swaps back() for navigate() to dodge an intermittent Chromium failure. The comment says back/forward/reload are covered below, but the surviving test only does back() after navigate() — never after a click-driven navigation, which is the sequence that flakes, and which looks reachable from a real client since click does not await the navigation it triggers. Details inline.
2. Neither new prompt restates the explicit-pageId rule that parallel_roles carries. Details inline.

Test coverage for the behaviour introduced is good: all twenty codes, the leak channel, the two rollback caveats, the five described unions, both prompt renderings, the optional-argument branch, and URL rejection.

I could not execute npm run verify in this environment, so the green run is taken from the PR description rather than reproduced.

[claude]

Reviewed for correctness, leaks, the addressing/isolation contracts, and coverage of the new behaviour.

What holds up

nextStep cannot leak. NEXT_STEPS is Readonly<Record<BrowserMeshErrorCode, string>>, so the compiler forces one entry per code and asBrowserMeshError guarantees mapped.code is always in the table — no undefined slipping into the payload. Every value is a string literal; nothing is interpolated from mapped.message, mapped.details, a locator, a URL, or configuration, so the new field genuinely cannot become a channel the way publicMessage/safeDetails have to be defended. The hostile-locator assertion in mcp.test.ts pins that, and the distinctness check (new Set(steps).size === errorCodes.length) pins the failure mode a per-code table actually has.

I checked every tool and argument name the strings mention against the registered surface — browser_session_get, browser_page_list, browser_state_list/browser_state_save, browser_observe, interactiveOnly, includeRefs, maxRefs, maxChars, maxBytes, includeText, the console/pageError/requestFailed sources, the creating/failed session statuses, and BROWSERMESH_PERSISTENCE — all exist and all read correctly. The two qualified retries (OPERATION_CANCELLED, INTERNAL_ERROR) match what SerialQueue actually does, and the second test locks that qualification in rather than leaving it to a reviewer.

Prompts stay inside the boundary. Both are static templates over validated arguments; no runtime state, no LLM call, no notion of an agent. Both spell out the addressing rule. left/right are bounded at 128 to match session.maxNameChars, and 128 UTF-16 units cannot exceed maxNameBytes (512) at 3 bytes per BMP unit, so the label really is passable verbatim. form_validation saying "Never submit real credentials" is the right call for a prompt that walks an agent onto a login form, and the test asserts it rather than trusting it.

Union descriptions. Reasonable response to schema flattening, and the new test is honest about why it exists: the byte budget is an upper bound and the $ref round trip compares compacted against uncompacted, so neither would catch a silent loss of the prose.

Two things

1. settleBeforeHistoryMove waits for load on the current document (inline comment). That turns browser_back into an OPERATION_TIMEOUT on a page that never settles — the page a caller most wants to leave. domcontentloaded covers the commit window the docblock describes without waiting on subresources.

2. CLAUDE.md test counts (inline comment) — 156 to 179 unit tests with no file under tests/unit/ touched.

Coverage note

The new browser-runtime test is a five-attempt race repro, so it can pass against the unfixed engine — it demonstrates the sequence rather than pinning the fix. Nothing wrong with keeping it, but the behaviour this change introduces (a settle before the history move) is not covered by anything deterministic, and forward gets the same new wait with no test at all. Worth at least a fake-engine assertion that back/forward await the load state before calling goBack/goForward.

One scope note: the back/forward engine fix is a real bug fix but is not mentioned anywhere in the PR description, which covers only errors, prompts, and docs. It deserves its own line there, and arguably its own commit.