feat(ask): askBrowserstackAI — mid-run permission relay, with production defaults - #379
Open
sarve-shreyas wants to merge 32 commits into
Open
feat(ask): askBrowserstackAI — mid-run permission relay, with production defaults#379sarve-shreyas wants to merge 32 commits into
sarve-shreyas wants to merge 32 commits into
Conversation
Five tools driven by a prebuilt index artifact: listProducts, listEntities,
describeEntity, searchCapability, invokeEndpoint. 173 tm endpoints reachable from
one generic invoke tool instead of a hand-written tool per operation.
WHY AN INDEX AND NOT THE SPECS. The artifact (193KB, vs ~1.6MB for the three raw
harness bundles) is generated by the Python side and contains only data that has
already passed its outbound boundary. Had this half parsed openapi.yaml it would
BECOME the boundary, and every gate — route lint, the positive vocabulary rule, the
intent lint, the discovery denylist — would have to be reimplemented and re-tested
here, with a subtle mistake publishing what the other side withholds and no test to
catch it. An index cannot leak because the internal data is not in it: a test asserts
x-atlas-permission, parameter `target` names, body `pointer`s, raw key_facts, entity
`operations[]`, page/count params and strip_prefix are all absent. No hostname is
baked in either, so one artifact ships to every environment.
Endpoints are the handle, not capability names, and arguments arrive grouped into
path_params / query / body under the spec's OWN names — so a caller passes search
output straight back with no remapping. Grouping is not cosmetic: four tm operations
declare one name in two places (bulk-move has folder_id as both a path parameter and
a body field), which a flat map cannot express. Body fields carry json_path when the
nesting differs from the field name, because that nesting is unguessable and fails
silently — tm's folder create wants {folder: {name}} while the flat {name} a reader
would assume returns 200 and discards the field.
ONE invoke tool means one set of MCP annotations, so they describe the whole surface
honestly: readOnlyHint false (it can write), destructiveHint false (it can never
delete, because destructive endpoints are refused before binding). Write consent
therefore rests on user_permission enforced here rather than on a client hint.
Parameters are validated BEFORE permission is demanded, so a typo cannot send someone
to ask a human about a call that was never going to run.
Auth forwards the caller's own credentials as `Api-Token: <username>:<access_key>`,
which every /api/v1 route validates against IAAM OAuth2 v2. HTTP Basic is NOT usable
there — authenticate_with_authorization_header never reaches the Basic path — so
Api-Token is the scheme that covers the whole surface, with no token to mint, nothing
to refresh mid-pagination, and no auth-host/environment coupling.
Ported faithfully from the Python resolver, including the rules each learned from a
live failure: rows found by SHAPE (tm uses 30 distinct row-key names; a hardcoded
list of 13 missed 24 of them), a single wrapped record counted as one row (otherwise
every stats/summary read returned ok:true count:0), an empty array kept
distinguishable from a shape with no rows, paging at the declared ceiling (880
projects walked 30 at a time was the 17 Aug incident), search penalties that reorder
without excluding (conflating them once dropped 40 valid matches), and discovery mode
publishing scalars only minus a sensitive-name denylist.
35 new tests; the full suite is 311 across 31 files. Not yet wired into
server-factory.ts — registration is opt-in until the overlap with the 17 hand-written
testmanagement.ts tools is settled.
Registered alongside the existing tool adders, conforming to their contract — (server, config) -> Record<string, RegisteredTool>, with trackMCP instrumentation in the house style. Credentials come from BrowserStackConfig and are read PER CALL, not captured, because the remote server rebuilds config per session and a captured credential would outlive its session. Fails soft on purpose: a missing or unreadable index registers nothing and logs why, rather than throwing. A packaging problem must not take every other product's tools down with it, and a test asserts the other adders still register when the artifact is absent. CAPABILITY_REGISTRY_DISABLED=true is the kill switch; CAPABILITY_REGISTRY_BASE_URL_<PRODUCT> overrides the host per environment, since no hostname is baked into the artifact. Fixes a real regression the end-to-end test caught. The port had been reading the page-size ceiling off `max_items`, but that is a total-items cap on the Python side; the ceiling lives on Operation.max_count, which `project()` deliberately withholds because publishing page controls invites a caller to drive paging itself — the 17 Aug failure. Nothing carried it into the artifact, so the resolver was about to page at the product's default: exactly the incident, reintroduced by the port. Paging controls now travel in a sibling `paging` map keyed "METHOD /path" (44 endpoints, 14 with a declared ceiling). The resolver reads it; the search tool never echoes it, so the published surface is unchanged and a test asserts no page control appears in a search hit. End-to-end coverage through the factory, with fetch stubbed: the five tools register beside the hand-written ones; a real search returns a usable endpoint; an invoke forwards `Api-Token: <username>:<access_key>` plus the attribution header, sends count=300 rather than the product's default, and projects rows to the declared returns so an undeclared field never reaches the caller; a destructive endpoint is refused with no egress at all; and a write is refused until confirmed, with a typo'd parameter surfacing as a parameter error rather than sending someone to ask a human about a call that was never going to run. capability-index.json ships at the package root and is listed in package.json `files`, because tsc compiles TS and does not copy JSON into dist. Full suite: 319 tests across 32 files, lint and tsc clean.
Precedence: config override, then the harness-declared host from the artifact, then
product-specific discovery, then refuse.
FIXES A BUG I WOULD HAVE SHIPPED. The first version hardcoded
test-management.browserstack.com for tm and invented hosts for a11y and tra from a
naming pattern. But the package already resolves TM's host per ACCOUNT: getTMBaseURL
probes test-management{,-eu,-in}.browserstack.com with the caller's credentials,
caching per process in stdio mode and deliberately never in REMOTE_MCP mode so one
tenant's region is not served to another. A fixed host would have failed every EU and
IN account on every call — and that is the live path behind the "Unable to connect to
Test Management" error users already see.
So tm now defers to getTMBaseURL, and a11y/tra are NOT guessed: an unknown host is
refused by name. A guessed host fails as a DNS error or a 404 that reads like the
caller's problem, when it is our missing configuration. Add a product here only once
its host is known rather than inferred.
The sharp edge is documented where it bites: a harness-declared host is one fixed
origin, so declaring one for a region-sharded product would send EU and IN accounts to
the wrong region. tm declares none for exactly that reason and falls through to
discovery.
Auth stays hardcoded to `Api-Token: <username>:<access_key>` by decision — it is a
BrowserStack-wide convention rather than a per-product quirk, and the artifact
therefore carries no auth information at all. The harness PR that declares the scheme
is consequently NOT a dependency of this half; it matters for the Python registry,
whose auth resolution is declaration-driven, and for the spec telling the truth.
Tests mock the resolver module (the probe is axios-based, so a fetch stub cannot reach
it) and assert each rung of the precedence, including that a request lands on the EU
host when that is the account's region.
Full suite: 325 tests across 33 files, lint and tsc clean.
Atlas resolves a product's host in three config rungs — an explicit per-session
override, then the host for the session's environment
(harness.extra_environments[env][product]), then the profile's own base_url — and all
three live in config, not in the harness bundle. This half had only the first. It now
has the same ladder:
1. CAPABILITY_REGISTRY_BASE_URL_<PRODUCT> explicit, environment-agnostic
2. CAPABILITY_REGISTRY_BASE_URL_<PRODUCT>_<ENV> this environment's host
3. CAPABILITY_REGISTRY_BASE_URLS {product:{env:url}} the same as one map, the closest
analogue of extra_environments
4. the harness-declared host from the artifact
5. product-specific discovery (tm is region-sharded)
6. refuse, by name
ENVIRONMENT AND REGION ARE DIFFERENT THINGS, and the code says so where it matters. An
environment is a property of the DEPLOYMENT (this instance talks to preprod), so it is
read once from the process. A region is a property of the ACCOUNT (this user's data is
in EU), which is why region discovery runs per request and is never cached under
REMOTE_MCP. Conflating them would either send everyone to one region or re-probe on
every call.
Two refusals rather than fallbacks, both because the silent alternative is worse than
an error: a named environment with no host defined does NOT fall through to the harness
default (that would point a preprod deployment at production), and a malformed
CAPABILITY_REGISTRY_BASE_URLS is rejected rather than read as "no override" (same
outcome, arrived at by typo).
Full suite: 331 tests across 33 files, lint and tsc clean.
`invokeEndpoint` now answers {ok, completed, http_response:{status, body}} and
nothing else. Removed: row extraction by shape, the `returns` allowlist, the
scalars-only filter for undeclared schemas, item counting, ordering, trimming,
internal paging, and the guards that reported an empty projection as a
registration defect. envelope.ts is deleted outright.
WHY, from a live failure. Every one of those was a place we could be wrong ABOUT a
correct answer, and when we were, the caller saw a confident empty result rather
than an error. `GET .../test-case/priority` returned
`capability_shape_empty: the product answered with rows, but every field on them
was an object, an array, or a name withheld as sensitive` — while in truth the
extractor had looked one level too shallow (the response is keyed by the field
asked for, `{priority: {values: […]}}`) and the scalars-only rule then dropped the
only field left. A complete answer, reported as "this endpoint can report nothing".
The same call now returns 11 option ids, which are what every priority filter and
write needs.
`ok` mirrors the HTTP status; nothing else decides it. A non-2xx returns the
product's OWN body, which usually says more than we could ("Drill-down is only
available for User Workload Reports") — inventing a message discarded it. `error`
is set only when status is 0, i.e. there was no response to speak for itself.
`completed` reads one declared field (`info.next`) so a caller knows another page
exists; it is a peek, not a reshaping.
ONE REQUEST PER CALL, so paging is the caller's. That required publishing `p`, the
page-size parameter and `max_page_size` with each paginated endpoint — they were
hidden only while this side walked the pages itself, and withholding them would
leave a 913-row read stuck on page one at the product's default of 30.
Response headers are NOT returned. Measured on preprod: 26 of them, ~20 varnish
and timing noise, and `set-cookie` present — which matters because tm's own auth
scheme IS a cookie, so that header is credential-shaped rather than metadata.
323 tests, lint and tsc clean, live-verified against preprod.
Approach 1 shipped read-only because there was no way to ask a human mid-run: the old surface refused every write and listed what it would have needed. MCP elicitation is that missing piece, so this adds one tool that hands a plain-language task to BrowserStack's agent and asks the person sitting in front of the calling client whenever the agent reaches a step that changes data. Transport is a caller-supplied callback URL, which matters more than it looks. Because the agent makes the OUTBOUND request, the decision comes back on the same connection to the same pod, so the affinity problem that would otherwise force either a single replica or a Redis nudge does not arise here at all. The listener is per tool call, on loopback, on a port the OS picks, behind a fresh 256-bit bearer compared in constant time and checked before the body is even read. The threat model is local: every other process on the developer's machine can reach that port, and a human trained to approve prompts is the exploit, so a caller that cannot present the run's token gets a 401 and no prompt appears. Everything ambiguous denies. Only accept plus confirm: true is an allow. A cancel is a deny with reason "cancelled" rather than "declined" because a headless client with nobody at the terminal returns exactly that — which is what stops an unattended run from approving its own writes, and why an elicitation is never retried. A client that cannot elicit is not a failure case. permission_relay is omitted entirely rather than sent empty, its absence selects the read-only gate, and the result says why the write was refused instead of leaving the caller with an unexplained failure. Nothing depends on sampling, which Claude Code does not declare. The result carries the approval trail and applied_before_stop, so a caller can tell "nothing happened" from "some steps applied, then stopped" and does not retry a half-applied task.
CONTRACT v1.1 replaced four inferences with facts, and two of them were traps. `needs_approval` is absent when empty rather than `[]` — Atlas's `public()` omits the key entirely, as it does for narration, artifacts, error, cost_breach and usage. And the emitted status vocabulary is ok | error | blocked | rate_limited; `interrupted` appears in a dataclass comment but never on this path. Both were already read defensively, so this pins them down with tests and comments rather than changing behaviour. Atlas now reports whether the relay it was offered actually ran, which is a fact only it knows, so its verdict is preferred over anything inferred here. The reason worth the most care is `disabled`: the knob is off server-side, so every write was refused for a configuration reason and nobody declined anything. A caller who cannot tell that from a human saying no retries forever, so each reason gets its own sentence and `disabled` says outright that nobody declined. An unrecognised reason degrades to a sentence too — a newer Atlas must not be able to throw inside a result — and is length-bounded, because it arrives off the wire and ends up in front of a person. The prompt is now framed with the product, which v1.1 approves: a bare sentence with no attribution is a worse prompt than a framed one. The product is all the callback carries, and the description still goes through verbatim, because the human must approve what the model actually said, not a paraphrase. Two judgment calls beyond the brief, both flagged for veto in the result file. Atlas's own `error` string is lifted to the top level instead of being left for the caller to dig out of `atlas_response`. And `isError` now marks a call that failed rather than one that was refused: a blocked run is the feature working, and rendering a correct refusal in red invites the retry loop these distinct reasons exist to prevent.
Atlas's delegation route accepts exactly two credentials, both in the
Authorization header: the shared delegation token, or a BrowserStack central
JWT. There is no Api-Token path. That header is right for the product APIs and
was carried over here by analogy, so as built every call would have returned
401 before the gate, the relay or the model were reached.
So /agent now sends Bearer <shared delegation token>, from its own env var
resolved per call with the same precedence as the host — a preprod deployment
must not be able to fall back to a token meant for somewhere else. It refuses
by name when unset, naming the variable and never a value, and the token
appears in no log line, no error message and no result.
Api-Token is removed rather than left as harmless clutter. Atlas never reads it
on this route, and it carries the user's access key, so sending it pushed a
secret across a trust boundary to an endpoint with no use for it and into every
request log on the way. The access key now does not leave this process on this
route at all; a call no longer needs one to succeed, and the header key set is
asserted exactly so it cannot creep back.
The shared token authenticates the caller but not the principal, so Atlas reads
the acting user from the body. user_id carries the configured username, omitted
entirely rather than sent empty. A caller can claim any user_id on this path;
that is Atlas's documented design for the shared-token route, not a hole to
plug here.
A rejected credential and a refused action are unrelated problems, and Atlas
answers a bad bearer with a bare {"detail": "unauthorized"} and no error string
of its own. A 401 therefore gets its own sentence, leading with the fact that
nobody declined anything, so nobody goes hunting for a human who said no when
the real answer is that this server never got through the door.
Atlas omits its permission_relay verdict on every refusal that dies before the
delegation layer — 401, 400 and 503 all answer with a bare {"detail": …} — and
an unreachable Atlas has no body at all. The verdict reader treated a missing
block as "an Atlas older than v1.1" and fell back to the optimistic reading, so
a 401 came back asserting that BrowserStack asked before each change and the
answers were in `approvals`, with `approvals` empty, alongside an `error` field
saying nothing had ever been asked. Three fields in one payload contradicting
each other, and zero prompts had appeared.
That is the confusion the `disabled` sentence was written to prevent, one layer
earlier: a reader who cannot tell "nobody was asked" from "somebody said no"
retries forever. So a request that never reached the agent now says so in its
own words, and says it outranks both of the other readings — including the
no-elicitation one, because a client that cannot be prompted did not "run
read-only" either when nothing ran at all.
Pre-run refusals also carry `detail` and never `error`, so a 400 or a 503 used
to arrive as status "error" with nothing whatsoever to act on. Atlas's detail is
now surfaced with the status that carried it, bounded, since it comes off the
wire and ends up in front of a person.
The withheld-placeholder fixture asserted against a string Atlas does not emit.
Runtime was never affected — the description passes through verbatim — but it
is the one string here a test can assert on and be confidently wrong about, so
it now uses the bytes collector.py actually builds.
Only Atlas can know whether an approved step's request actually landed: the gate returns before anything is sent, which was the whole of D2. So this side stops deriving applied_before_stop from "any allow preceded a deny" — a rule that counted an approval whose egress then failed as applied, and so lied in the exact direction the field exists to prevent. The derivation is deleted rather than left unused, because code that still computes a fact we no longer trust is an invitation to rewire it. A missing applied_before_stop is now null, not false. Atlas sends the field whenever a gate ran, including false and including an empty trail, so absence means either that no gate ran or that this Atlas predates the field. Reporting either as a measured false would assert something nobody checked, in the direction that makes a caller retry a task that already half-applied. Atlas's approvals trail wins whenever it sent one, and an empty trail it did send counts as one — "the relay ran and nothing was asked" is a fact, not a gap to fill from our own records. Each entry is rebuilt on arrival rather than trusted: a decision that is not exactly "allow" reports as a refusal, so a garbled trail fails closed in the reporting the same way the wire does. Our own trail is kept beside it rather than folded in, because where the two disagree the disagreement is the signal. A callback answered with no prompt appearing is a denial to Atlas and nothing at all here, and that pair is what a probe of the loopback port looks like; merging the trails would destroy the only evidence it happened. Every entry now carries a sentence, because "approved, then the request failed" is a genuinely different thing to tell a person than "somebody said no", and an approval nobody measured is neither — it is not rendered as a failure.
Atlas answers HTTP 502 with a complete result body when a delegation ran and a step then failed, and 429 carries a full body too. The not-reached rule read "any non-2xx" and ran ahead of Atlas's own verdict, so an approved write whose egress failed came back saying nothing had been asked and nothing refused — while approvals in the same payload showed the prompt shown, approved, and not applied. That is the lie the rule was added to prevent, now told on the one run where it costs the most: the reader is talked out of checking for a partial application at exactly the moment one is possible. The status code describes the outcome; the body describes whether there was a run. Only the second question decides this, so a body carrying ok, status, answer or approvals means the delegation ran, whatever code carried it, and not-reached is left to a transport failure or a body that is not a result at all. Status derivation and the error string follow the same rule: a run's own status outranks its transport code, and "refused before the agent started" is no longer said over a run that plainly started. A 2xx carrying no result used to report ok: true with an error attached. Real Atlas never emits that, but a shape that contradicts itself is not a shape to leave lying around, so it now reads as the error it is. The tests assert the whole permission_relay object beside approvals rather than picking at single fields, because this ordering has now bitten twice and the failure mode both times was two fields in one payload disagreeing.
The shared delegation token is gone from Atlas, so the only way in is a BrowserStack central JWT minted from the caller's own username and access key. That is a better door than the one it replaces, not just a different one: validate_delegation_token refuses any token without user claims, so the minted one is user-attested. Atlas sets principal_verified, takes the acting user from signed claims rather than from our request body, and re-uses the same JWT for product egress — so the write a human approves runs as that human rather than as a shared service account. The user_id we send stops being a claim anyone could forge, and is kept only because dropping a field from a frozen wire format is not a change one side gets to make alone. Both halves of the scope are load-bearing and neither can be tidied away. central_ai_s2s is what Atlas matches on, but it is a client_id+secret scope and asking for it alone with a username and access key is refused outright; it becomes obtainable only paired with oauth_user_profile. The constant says so, with the citation, because it reads like redundancy and is not. Tokens are cached rather than minted per call, and the staleness margin is the whole /agent budget plus a minute rather than the usual small skew. Atlas holds this token for the life of the run and re-uses it for egress, so handing out one with sixty seconds left would mean a human approves a write and the request that follows dies on an expired credential — the precise failure the cache exists to avoid. The key is hashed rather than stored, so rotating a credential mints immediately without leaving the secret in a map for the life of the process. The token endpoint's error body can echo the credential straight back, so only its status ever crosses. Nothing here logs the access key or the minted token, and neither appears in a result or an error. Three failures that look alike and are not: the endpoint refusing the credential, the endpoint being unreachable, and Atlas refusing a token we minted successfully. The third is what a misconfigured deployment actually hits, and it now says so — the credentials were fine and required_scope is the likely cause — rather than telling someone their password is wrong. None of the three is a permission denial, and all three still report as never having reached the agent.
The requested scope becomes oauth_user_profile ai_agent_notify, moving with Atlas's required_scope. Only the second half changes; the first stays for the reason it was always there, as what makes the pair obtainable through the username and access key flow at all. The comment on the constant now records why this is a narrower door than the one it replaces, from the merged railsApp change: ai_agent_notify is documented as client_id/secret auth, the username+access_key allow list is still only user_management and oauth_user_profile, it carries a new application registration gate, and railsApp defines it as the product-to-agent direction while we use it as an agent-to-Atlas inbound credential. The scope it replaces was deliberately excluded from that gate. Those are the facts that decide whether this can be minted at all, so they belong next to the string rather than in a report nobody reads at three in the morning. Because it may simply not be issuable, the refusal is now told apart from a rejected credential. The two need entirely different fixes — provisioning versus a password — and one message for both sends someone to the wrong place. The refusal names the scope, says outright that the credentials are not the problem, and says that this server will not quietly retry with a weaker one. It does not: a silent downgrade to a different authorization is precisely the thing nobody notices until it matters, and every refusal shape is asserted to make exactly one attempt carrying exactly the chosen scope. Classifying the refusal means reading the failure body, which the credential rule forbids surfacing. So only the OAuth2 error code is consulted, only when it is one of the fixed spec tokens, which cannot carry a credential the way the free-text description demonstrably can — and it is discarded after classifying. Nothing from the body reaches a message; where no usable code exists the status decides, since a bad request is a 400 and a bad caller is a 401.
A user approved a prompt against preprod and was told they had refused it. The schema was at fault, not the client: confirm was a required boolean defaulting to false, so a client renders one unchecked checkbox, pressing approve submits accept with confirm false, and the mapping turned that into deny/declined. The approve path was unreachable unless the user also toggled a field they had no reason to think was load-bearing. "form" is the only elicitation mode the SDK has, so there was no confirm mode to move to. For a yes/no approval the action already is the answer — accept, decline and cancel carry exactly the three outcomes needed — and a boolean inside the form duplicated that signal while contradicting it. So confirm becomes optional, with no default, and an accept that carries no confirm field is an approval. This looks like a loosening and is not. The guard against an unattended run was never that boolean; it is that a headless client returns cancel, which was measured and is unchanged, and an accept means the protocol itself says a human accepted. What the old shape actually bought was a false denial, indistinguishable in the result from a human saying no — the same confusion that two earlier fixes existed to remove, arriving this time through the front door. The comment that claimed the protection is replaced by that reasoning rather than left standing over code that no longer relies on it. An explicit false is still a refusal, because a client that does render the checkbox and a user who unticks it have said no. Only absence is consent, and nothing is coerced: a string "true" or a 1 is a malformed answer, not an approval. The emitted schema is asserted to carry neither a required array nor a default, so this cannot come back quietly.
A boolean inside a form whose accept action already means approval can only agree with that action or contradict it, and when it contradicts we cannot tell which the human meant. Accept with confirm false is either "I approved and never saw the checkbox" — the false denial a user hit on preprod — or "I unticked it deliberately". Guessing either way is wrong for the other case, and it could not be settled by inspecting what the client sends, because that binary is compiled and its strings too fragmented to read. So the question is removed rather than answered: requestedSchema now asks for nothing, and decline already offers an unambiguous refusal in the same dialog. Fail-closed is untouched and never rested on the boolean. A headless client with nobody at the terminal returns cancel, which is measured and is a deny, and that is the whole of what stops an unattended run approving itself. A volunteered confirm false is still honoured, documented as defensive only, since no client can be expected to send a field nobody asked for. The answer's shape is now logged once per ask — the action, whether content came back, and whether confirm was among it. Only a fixed enum and a boolean, so no description, credential or typed text can ride along, which is asserted rather than assumed. Next time this misbehaves the client's answer can be read instead of inferred.
A test harness is running the documented sample queries to measure which tool Claude picks, but the credentials available are preprod-only and the classic tools probe production, so they 401. That does not corrupt the selection itself, which happens before any call, but it corrupts what the selection means: a 401 can send the model off to retry with a different tool, so "chose ours because it fit" and "chose ours because the other one was broken" become the same observation. So the probe list gets an env override, named and parsed after the capability registry's, and it REPLACES the built-in list rather than extending it — appending would leave the production hosts probed first and the 401s would come straight back. With the variable unset the list is exactly the three production regions in exactly the order they were already in, which is asserted rather than assumed, because this is a harness affordance and must not be able to change what ships. An override that parses to nothing falls back to the built-in list rather than leaving an empty probe loop, which would surface as "unable to connect" with no detail. It warns when it does: quietly talking to production when someone asked for preprod is the failure this exists to prevent, so it must be visible in the log rather than inferred from a 401 much later. The module-level cache is now keyed on the list it was discovered under. Skipping it entirely under an override would have worked too, but keying it also covers the reverse — a value minted against preprod being handed to a run that has since gone back to production — where the override would appear to work while silently returning the wrong environment's host. Accessibility and observability are deliberately not covered. Their hosts are hardcoded inline at nine call sites across five files with no host-resolution module to hook, so the same shape does not drop in; giving them one means rewiring egress across the codebase for a test affordance, which is not a trade worth making here.
This package publishes as @browserstack/mcp-server and remote-mcp-server consumes it, so a version bump hands the hosted multi-tenant server a tool built entirely for stdio. Three things break there. The callback listener binds an ephemeral loopback port per tool call, which in a shared process is one listener per concurrent call with a per-run bearer as the only thing keeping tenants apart. Atlas could not reach it regardless, because a 127.0.0.1 callback names the Atlas pod's own loopback and its SSRF allowlist refuses it. And elicitation is a server-initiated message, while the remote transport is stateless by deliberate design — 841c6358 removed sessions because they broke behind two replicas, and justified it on the grounds that nothing used server-initiated messages. This feature is the exception that commit did not have to consider. So in remote mode the listener is never bound — not bound and left to fail on a callback that cannot arrive — and permission_relay is omitted, which selects the read-only gate that already works and is already tested. The reasoning sits at the guard rather than in a report, including what re-enabling it would actually take, because the next person to try will otherwise spend a day rediscovering the session problem. The result says which of three things stopped the relay, since they need different responses: nobody could be asked, the request never arrived, or this deployment cannot receive a callback. The last one is new, and it deliberately outranks the first when both hold: in the hosted mode even a client that can be prompted is no use, so telling someone to switch clients would waste their time. It still loses to a request that never reached the agent, which is the more immediate fact. What was a boolean is now a three-valued mode, because it was never really a boolean.
This was the only tool here requiring install-time host configuration. Every other one carries its production host in the code and treats env vars as an override, and the capability registry's resolveBaseUrl is the richer form of the same idea; this is that, for Atlas. Both existing overrides stay ahead of the map, so nothing already working stops working. Each host was established rather than assumed, and the comment says how: prod answers the same 401 as staging Atlas and 404s on /agent only because it runs an image without the delegation route yet; stag and preprod are the two ingress hosts serving one backend. Staging pointing at preprod's auth server looks like a copy-paste and is not — preprod is configured there as an extra environment and accepted configs include extras, so a preprod-minted token validates against staging, which was confirmed live rather than reasoned about. An unset environment refuses instead of defaulting to production. The registry already refuses when an environment is named but has no host, on the grounds that falling back would silently send a preprod deployment at production, and that argument only gets stronger for a tool that writes. The cost of refusing is one environment name at install time, reported by name the first time the tool runs; the cost of defaulting is an unconfigured install quietly changing production data. A selector that is wrong refuses by name, where a URL that is wrong talks to the wrong place in silence — which is the whole reason to prefer naming an environment over pasting a host.
The three-environment map and its selector go away. The decision is to ship a single hardcoded staging host for now and repoint at production later, so that is what this does: one default, one override, no map and no ASK_BROWSERSTACK_ENV. That also brings this tool into line with every other one here, which carries its host in the code and treats the env var as an escape hatch. A hardcoded default removes the refusal that task 13 added, and the argument for that refusal was about production: an unconfigured install quietly writing to real data. Defaulting to staging inverts it — an unconfigured install cannot touch production — but it is still wrong for a production deployment, which would read and write the wrong environment's data without any sign that it was doing so. So the resolved host is announced at info on first use, naming whether it came from the environment or from the constant, once rather than per call so it is not something to scroll past. A deployment pointing at the wrong Atlas should cost one log line to notice, not a confusing afternoon. The constant carries the rest: that this is an interim placeholder, that production is workflows.browserstack.com with its own auth endpoint and how that host was verified, that publishing to npm means an install with no configuration talks to staging, and that both the constants and the tests asserting them must change before production users get the tool. The tests assert the literals for exactly that reason — repointing should be a deliberate edit rather than something that passes quietly. The module docblock claimed no host was compiled in, which is now the opposite of what the code does, so it says what the code does instead.
Atlas is adding a per-product entitlement gate to the delegation route, the one the WebSocket path already had, and it answers 403 when an account is not on the product's agent flag. That gate fails open on its side — Redis down, a flag never seeded, an unknown product all allow the request — so a 403 is a deliberate "this account is not enabled" and never an outage, which is what makes it worth a sentence of its own. Until now it would have arrived as a generic pre-run refusal quoting Atlas's bare detail: accurate and useless. It is now its own outcome, kept apart from the four authentication failures because the fix is different from all of them. It is not a rejected credential, and saying so explicitly matters — the credentials authenticated fine, so anyone reading a vaguer message would go and rotate a working access key. It is not a permission denial either: nobody declined anything and the relay is irrelevant, so it takes precedence over both the never-reached and remote-mode readings, which are technically true of a 403 and tell the reader nothing they can act on. The message names the product, because the flags are per product and an account entitled for one is not necessarily entitled for another. A bare "not enabled" sends someone to their admin asking about the wrong thing. Classification keys on the status, not on the sentence in the body, and says so where someone might be tempted otherwise. Matching the prose would break silently the first time anyone rewords it, and silently means falling back to the generic error this change exists to replace. Atlas may add a structural code to that body; the place to prefer it is marked, with the status left as the fallback for an older Atlas.
searchCapability, invokeEndpoint, listProducts, listEntities and describeEntity are a different piece of work — Approach 2, driven by a prebuilt index — that this branch inherited by being cut from feat/capability-registry. They are not part of what is shipping here, and a release carrying five extra tools nobody asked for is a release nobody can reason about, so they come out. Nothing is lost. Those five commits are untouched on feat/capability-registry, locally and on ctoi, so this removes them from one branch rather than from the world; restoring them is a merge, not an archaeology exercise. The coupling turned out to be a single import in the server factory: the relay stopped importing the registry's auth header when /agent moved to a central JWT, and nothing else ever reached across. Also dropped: the 205KB capability-index.json artifact and its entry in the published files list, since neither has a consumer here any more, and the citation in tm-base-url that pointed at a naming precedent no longer in this tree — a comment referring to code that is not there is worse than no comment.
The tool sat among forty-four hand-written ones without saying when it should be preferred to any of them, and a description is the only thing steering that choice — the client picks from these words alone, before a single call is made. So it now leads with the case it is actually for: no other tool here fits, or the ones tried did not get there. The ordering is the point rather than the wording. When to reach for it comes first, because that is the question being asked at selection time; what it does and what consent looks like follow, because those only matter once it has been chosen. It also says plainly to prefer a specific tool when one fits, since a dedicated endpoint is faster and more predictable than an agent working out its own API calls. The assertion on it is anchored to the start of the string, so a later edit cannot quietly demote the fallback framing into a trailing sentence nobody reads.
CONTRACT v2. `callback.ts` binds 127.0.0.1:<ephemeral> and hands Atlas the URL,
which works only when Atlas shares that loopback. For a real user it cannot work
at all: this server runs on their machine, and a laptop behind NAT is not
addressable from a pod in BrowserStack's cluster. No configuration creates that
route. Every successful relay run to date used a locally-run Atlas — the shape v1
was written for, not the shape a user is in.
A1 makes both connections outbound from here: the ask arrives on the open
`POST /agent` SSE response, and each decision goes back as a fresh short
`POST /agent/{run_id}/permission`.
Adds `stream.ts` — `AgentStreamTransport` and `DecisionTransport` seams,
`splitFrames`, `parseFrame`, and fetch-based implementations of both.
This file deliberately decides NOTHING. Whether a human approved, how an
elicitation outcome maps to allow/deny, what the result looks like — all of that
stays in `relay.ts`, untouched and shared with the callback transport. Keeping the
judgement out of the transport is why swapping A2 for A1 does not put the
fail-closed behaviour at risk. `callback.ts` is untouched too: it stays the working
path for a local Atlas until the Atlas half ships (v2 §7.5), because deleting it
now would leave the feature with no working transport at all.
Chunk boundaries get most of the test attention on purpose. A frame split
mid-JSON, two frames in one read, a trailing frame with no blank line, an
unparseable frame, the heartbeat — a dropped frame is an ask that never reaches
the human, i.e. a write that silently never gets approved and never says why, and
it is the failure a hand-rolled SSE parser actually produces. An unparseable frame
is dropped rather than guessed at: the worst case is that Atlas's gate denies on
its own expiry, never that something is approved. A non-stream response throws
instead of iterating empty, because an empty iteration is indistinguishable from
"the run finished and said nothing". A decision that never left reports 0 rather
than a refusal, so a lost request is never reported as a human saying no.
Timeouts follow v2 §4: the whole-run guard is 1800s because the stream now spans a
run that may hold several 300s approvals in series, so the old 330s outer rung
meant nothing. The per-ask rung (270s elicitation inside Atlas's 300s gate) is
unchanged and still enforced where it belongs.
16 new tests, 480 passing overall. Typecheck and lint clean.
`askBrowserstackAI` now drives CONTRACT v2. The relay block it sends is
`{ mode: "stream" }` — no URL and no per-run bearer, because nothing dials in.
The ask arrives on the open `POST /agent` response and each decision goes back on
its own `POST /agent/{run_id}/permission`.
`relay.ts` is untouched, as v2 §7.3 predicted: `relayOneAsk`, `decide`,
`buildResult`, the trail precedence and every `permission_relay` reason are shared
with the callback transport. The new `runStreamed` is a loop that reads events,
elicits, posts the decision and hands the result to `buildResult`. It decides
nothing.
TWO REAL BUGS the existing tests caught, both in that loop:
1. **The HTTP status was being dropped.** I built the AgentResponse with a
hardcoded 200, which lost the 401/403 that `relay.ts` needs to tell a rejected
credential from an account without the feature from an ordinary failure — three
different sentences for the user, all collapsing into a generic error. The
transport now carries the status on a result synthesised from a non-stream
reply; a real stream is 200 by definition.
2. **A failed elicitation abandoned the run.** `relayOneAsk` RETHROWS on an
unexpected failure, which under A2 was load-bearing: the throw made the inbound
callback answer 500 and Atlas read that as a deny. Under A1 there is no inbound
request to fail, so the throw escaped and left Atlas waiting out its full 300s
gate — a client hiccup becoming a five-minute stall. Now caught and converted
into the explicit deny the throw used to imply.
Also: a fetch failure maps to "BrowserStack AI could not be reached" rather than
leaking "connection reset"/"fetch failed", matching the request/response transport
— the upstream detail names our plumbing, not anything the reader can act on.
An Atlas that predates v2 answers `POST /agent` with plain JSON. The stream
transport yields that as a single `result`, so the tool degrades to a correct
read-only answer with `permission_relay.reason: "disabled"`. No version flag and
no negotiation, which is what makes it safe to ship this before every Atlas serves
A1.
Test migration: repointing the one `atlas()` stub from A2 to A1 fixed 12 of the 30
failures on its own — which is the evidence that these assertions were about
`relay.ts` rather than the transport. The stub now streams `run`, holds each
`permission` frame open until the tool answers it on the decision endpoint, then
sends `result`. Four tests had premises A1 makes impossible and were rewritten to
assert the same invariant in the new shape rather than deleted:
* the stray-process-with-the-wrong-bearer hazard is gone (no port, no bearer), so
that test now covers what remains: a refused decision (409/404) must not be
re-sent, because a retry could land an approval on a step the run has moved past.
* listener teardown is gone (nothing is bound), so that test now pins that a
transport failure still surfaces as a clean result rather than an escaping throw.
* D4's loopback probe becomes "Atlas denies without us prompting" — same
invariant: the two trails stay separate and ours being empty is the only record
that no human was asked.
* N1's 502-carrying-a-result cannot occur under A1 (an ask needs an open 200
stream), so the failure now arrives in the `result` event of a stream that did
prompt and was approved. Same invariant: a run that asked and got a yes must not
report "nothing was asked".
480 tests passing, typecheck and lint clean.
Observed live against a real Atlas. A headless client returned `cancel`, and the
one result carried both of these:
permission_relay.detail: "This client can prompt you, so BrowserStack asked
before each change and the answers are in `approvals`."
approvals[0].outcome: "refused: nobody was there to be asked"
Two sentences in the same payload contradicting each other, and the reader pays
for it — exactly the confusion the `disabled` vs `no_human` sentences exist to
prevent, one layer up.
Cause: `relayVerdict` picks the sentence from the negotiated MODE and Atlas's
advisory field. Neither can see what the elicitation actually returned, so a client
that DECLARES elicitation capability but answers `cancel` gets a sentence claiming
a human engaged. "The channel is usable" and "a person actually answered" are
different facts, and only the second licenses the word "answers".
Fixed in `buildResult`, the one place holding both the verdict and the trail: when
the channel worked and every ask came back `cancelled`/`no_human`, the detail says
nobody was present and names what to do about it. Guarded three ways so it cannot
overreach — it does not fire when a person genuinely declined (`declined` is a
human saying no, which needs a different sentence), nor on an empty trail (nothing
was asked at all, already described correctly), nor when any ask was allowed.
484 tests, typecheck and lint clean.
Observed live. Preprod auth was down, returned 503, and the tool answered
"Your BrowserStack credentials were rejected … Check BROWSERSTACK_USERNAME
and BROWSERSTACK_ACCESS_KEY" — sending someone to audit env vars that had
worked minutes earlier. The reader's own guess ("is preprod down?") was
right and the message argued against it.
`mintOnce` had two branches: unreachable (status 0) and "not 200", and the
second one read the body to tell a scope problem from a credential one.
A 5xx has neither, so it fell through to the credential sentence. Now a
`status >= 500` check sits between them, BEFORE the refusal branch, with
its own detail that says the service is failing and states plainly that
the credentials are not the problem. The status alone settles it: OAuth2
puts a bad client at 401/403 and a bad request at 400, so nothing in the
5xx range is ever a statement about the caller.
The 401 path is asserted alongside it, because the split is only worth
having if a genuine rejection still reads as one — the two need opposite
actions from whoever reads them.
Formatting churn in this file is `npm run build`'s own prettier step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Atlas removed it, so nothing on this side has anything to talk to. A2
opened a loopback listener per tool call and handed Atlas its URL; that
only ever worked for a caller on the same host as the Atlas process, which
is not how anyone runs this — an MCP server runs on a laptop and Atlas runs
in a cluster. A1 (the ask arrives on the open `POST /agent` stream, the
decision goes back as a fresh POST) is the shipped path and is verified
end to end against a live Atlas.
Deleted: `callback.ts` in full — the listener, its bearer, its 401/400/404
handling — plus `AgentTransport`/`fetchAgentTransport` in `egress.ts` (the
one-request-one-response shape has nothing left to describe now that
`/agent` is read as a stream), and `AskDeps.transport`/`.startListener`.
`PermissionRelay` narrows to `{ mode }`; `mode` stays optional because the
field's ABSENCE is what selects Atlas's read-only gate and that has to
remain expressible.
`parseAsk` MOVED rather than went with it, into `stream.ts`, and is now
actually wired in: `runStreamed` was doing `event.data as PermissionAsk` —
a bare cast, no validation. So a frame with a blank description would have
produced a prompt asking a human to approve nothing, and one with an id
off Atlas's `perm-<32 hex>` shape would have produced a prompt whose
answer could never be routed back. Every reason that function existed was
a property of the ASK, not of the direction it arrived from.
Two tests were A2's alone. The remote-mode pair keeps its meaning and
loses its wording: "binds no port" is true by construction now, and what it
always guarded — the hosted deployment offers no relay, because a stateless
replica cannot raise a server-initiated elicitation (v2 §5) — is what it
now says. "Tears the listener down once the call ends" is gone: its subject
does not exist, and it had already gone vacuous, reading
`permission_relay.callback_url` off a body that carries only `{mode}` and
passing on the throw from probing `undefined`.
`host_not_allowed` stays mapped in `relay.ts` but is rewritten: only an
Atlas that predates A1 can send it, and such a deployment still deserves a
sentence rather than a raw enum. 479 tests pass; build and lint clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It doesn't any more. Atlas removed the guard that replaced a route-shaped `description` with "(approval request withheld: …)" — it asked a human to approve a sentence they could not read — and CONTRACT v2 §3 was amended to match. No behaviour to change on this side: the description was always passed through verbatim, deliberately, because paraphrasing it would mean the human approves something other than what the model said. Only three comments were wrong, in `register.ts` and `relay.ts`, each asserting the string had already been checked upstream. The placeholder-framing test stays and is retitled: an Atlas predating the amendment still sends one, and it must not read as a bug once framed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the relay work up to 1.3.0, which is what the hosted Remote MCP server installs. No conflicts: main's 10 commits are the per-tool MCP annotations, a docs note and the version bump, none of which touch src/tools/ask-browserstack. Done BEFORE vendoring this package into remote-mcp-server for a staging test. Without it we would have shipped 1.2.36 into a service that expects 1.3.0 — downgrading every other tool there by 10 commits and confounding any session test with unrelated regressions on a shared environment. 479 tests pass, tsc clean, and the relay is intact (`addAskBrowserstackAITool` still wired into server-factory, all seven ask-browserstack modules present). Local only. This branch is still deliberately unpushed: the repo is public and the relay is unreleased. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`relayMode` refused unconditionally in REMOTE_MCP. That refusal has become
factually wrong, and its own message said so: "this deployment has no way
to put an approval prompt in front of you". It now does.
if (appConfig.REMOTE_MCP && !allowRemoteRelay()) return "remote_mode";
MEASURED, not assumed. Against the hosted Streamable HTTP server with
per-session servers (browserstack/remote-mcp-server#96): initialize issued
an Mcp-Session-Id, the following tools/call was served by the SAME instance,
and the run completed over HTTP. The thing that used to make this
impossible was the host discarding its server after each POST, so an
elicitation answer — which arrives on a SEPARATE POST — reached an instance
that had never asked anything while the real one sat suspended.
OFF BY DEFAULT, and that is not caution. It depends on a property of the
HOST that this package cannot observe:
* the host must keep one McpServer alive per session, and
* it must pin a session to a pod. Sessions are per-process, so without
affinity the answer POST can land on a replica that has never seen the
session.
Both were demonstrated the hard way: the first hosted attempt 404'd because
it ran during a rollout, when two pods were briefly serving and the
follow-up POST hit the one without the session. That failure is
intermittent and reads like a client bug, which is exactly why it must not
be a default.
The flag only lifts the blanket refusal. `relayMode` still asks whether
THIS client declared `elicitation`, so a client that did not still gets a
read-only run — there is a test for that, because a flag that quietly
forced asks onto clients unable to show them would be worse than the
refusal it replaced.
Also rewrote the rationale above `relayMode`. It asserted the relay was
"a STDIO-ONLY FEATURE ... A config flag will not do it", which was correct
when written and is now the opposite of true. It records what actually
blocked it (a paused call cannot be moved between processes), why
841c6358 was right at the time, and what a hosted operator must have in
place before turning this on.
481 tests pass, tsc clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Over Streamable HTTP a server->client message is written to the stream of
the request it RELATES to. `elicitInput` was called with no
`relatedRequestId`, so the SDK fell back to the standalone SSE stream — and
a host that answers `GET /mcp` with 405 has none. The message was then
dropped in silence:
const standaloneSse = this._streamMapping.get(this._standaloneSseStreamId);
if (standaloneSse === undefined) {
// Stream is disconnected - event is stored for replay, nothing more to do
return;
}
The tool then waited out its 270s and Atlas's gate expired, so the run came
back `decision: "deny", reason: "timeout"` — "refused: nobody answered in
time". The person is blamed for not answering a question that was never
put in front of them, which is the worst possible shape for a
consent mechanism to fail in.
MEASURED against the hosted Remote MCP server: the relay was offered
(`permission_relay.used: true`) and the ask reached Atlas, but the client
handled ZERO elicitations and the approval timed out.
Fixed by threading the tool call's own request id through: the SDK hands it
to the tool as `extra.requestId`, so the callback now takes `extra` and
passes it to `runStreamed` -> `relayOneAsk` -> `elicitInput`.
INVISIBLE ON STDIO, which is why this shipped: one pipe, nothing to route,
so every local and stdio test passed while the hosted run timed out. Note
the shared `call()` test helper passes `{}` as `extra` and therefore could
never have caught it — the new test supplies a request id the way the SDK
does, and it FAILS against the version without `relatedRequestId`
(verified).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DEFAULT_ATLAS_URL -> https://workflows.browserstack.com DEFAULT_AUTH_TOKEN_URL -> https://auth.browserstack.com/oauth2/v2/token These were deliberate staging placeholders ("for now lets hardcode the base_url to staging only then we will point this to prod url later"); this is that step. Both hosts were verified rather than guessed - workflows.browserstack.com answers /api/profiles with 401 {"detail":"authentication required"}, byte-identical to staging Atlas. Note what this removes: an install with no env vars used to fail SAFE onto staging, where it could not touch production data. It now reaches real customer data by default, so every non-production deployment must set ASK_BROWSERSTACK_ATLAS_URL / ASK_BROWSERSTACK_AUTH_TOKEN_URL explicitly. The staging hosts are recorded in the comment block for exactly that purpose, and the resolved host is still logged at info on first use naming env-vs-default. Tests assert the literals so a future repoint stays deliberate; updated with the constants. Full suite green (482 tests). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
askBrowserstackAI— a tool that hands a plain-language task to BrowserStack'sagent (Atlas) and relays the agent's mid-run permission asks back to the calling
client, so a write can be confirmed by the human sitting in front of it rather than
refused for want of anyone to ask.
The final commit points the compiled-in defaults at production:
DEFAULT_ATLAS_URLhttps://ai-platform-service.bsstag.comhttps://workflows.browserstack.comDEFAULT_AUTH_TOKEN_URLhttps://auth-preprod.bsstag.com/oauth2/v2/tokenhttps://auth.browserstack.com/oauth2/v2/tokenBoth hosts were verified rather than guessed:
workflows.browserstack.comanswers/api/profileswith401 {"detail":"authentication required"}, byte-identical tostaging Atlas.
Until now an install with no environment variables set talked to staging, where it
could not touch production data. It now reaches real customer data by default.
Every non-production deployment must set both explicitly:
Those staging hosts are recorded in the
DEFAULT-PROD-HOSTScomment block for thatpurpose. The resolved host is logged at info on first use, naming whether it came from
the env var or the compiled default, so a deployment pointed at the wrong Atlas shows up
in a log line rather than being inferred later from confusing data.
How the relay works
The ask rides the already-open
POST /agentSSE response; each decision returns as afresh
POST /agent/{run_id}/permission. Nothing dials back into the client — an earlierdesign had Atlas POST to a caller-supplied
callback_url, which cannot work when the MCPserver is on a laptop behind NAT. That transport was removed.
The privacy boundary is structural, not textual: what crosses to the caller is a
PermissionAskof four named fields (perm_id,product,mode,description).Tool inputs, outputs, paths and statuses live on a separate type that is never
serialized, so no regex is load-bearing for redaction.
Elicitations are routed onto the originating tool call's own stream via
relatedRequestId. Without that the SDK falls back to the standalone SSE stream, and ahost answering
GET /mcpwith 405 has none — the message is then silently dropped.In hosted (
REMOTE_MCP) mode the relay is off unless explicitly opted in withASK_BROWSERSTACK_ALLOW_REMOTE_RELAY=true, because a paused tool call lives in oneprocess: with more than one replica the answer can land on a pod that never saw the
session.
Testing
askBrowserstack*.test.tsadd ~3,100 lines covering the stream transport, theelicitation path, timeout/deny handling, and the URL resolution (the constants are
asserted as literals so a future repoint has to be deliberate).
Streamable HTTP and accepted, with the write landing only after approval.
Reviewer notes
fa11a3a) and is reviewable on its own.