Skip to content

[finding] registerDiscoveryEndpoints' auth branch strips only the retired /projects/:environmentId, so a scoped /discovery advertises routes.auth still scoped — and with an unsubstituted :environmentId #16538

Description

@os-sales

Found while implementing #15858 (the two stale /projects/ docblocks in packages/rest/src/rest-server.ts). ⛔ Filed unassigned and ungraded on purpose — grading and routing are triage's. Filed by the os-dev implementer on session session_01YFY46JydE1gMxQG1TqBcMZ, branch claude/issue-15858-retired-projects-spelling-in-docblocks.

This corrects a premise of the #15858 family. That card states, and its triage endorses, "This is prose only. Nothing here changes what rest-server.ts routes." That is true of the two sentences #15858 names. It is not true of the file: a third site carries the same retired ADR-0006 spelling in executable code, and there it is load-bearing.

The measurement

Two sibling unscopedBase computations inside registerDiscoveryEndpoints, eleven lines apart, both stripping the scoped segment off basePath. Located by sentence, not by line; the line numbers below are today's reading at 7a5592f5a and will drift:

packages/rest/src/rest-server.ts:4374   const isScoped = basePath.includes('/environments/:environmentId');
packages/rest/src/rest-server.ts:4491     ? basePath.replace(/\/(environments|projects)\/:environmentId$/, '')   <- mcp
packages/rest/src/rest-server.ts:4502     ? basePath.replace(/\/projects\/:environmentId$/, '')                  <- auth

isScoped is true only when basePath contains /environments/:environmentId. So the auth branch's regex, which matches only the retired spelling, can never match on the branch it guards. replace returns the string unchanged, unscopedBase stays scoped, and the advertised auth route keeps both the scope and the literal, unsubstituted route parameter.

Failing probe

unscopedBase is derived from basePath (the template), not from the realBase built a few lines earlier with the request's resolved id — so the placeholder survives into the advertised document. Pure string logic, the two lines copied verbatim:

const basePath = '/api/v1/environments/:environmentId';            // getScopedApiBasePath()
const isScoped = basePath.includes('/environments/:environmentId');            // :4374 -> true
const mcpBase  = basePath.replace(/\/(environments|projects)\/:environmentId$/, '');  // :4491
const authBase = basePath.replace(/\/projects\/:environmentId$/, '');                 // :4502
routes.mcp   : /api/v1/mcp
routes.auth  : /api/v1/environments/:environmentId/auth      <- FAIL

The contract it violates

Stated in as many words, in the comment two lines above the defect:

Align auth route with the versioned base path if present.
Auth is a control-plane concern, so use the unscoped base.

It does not use the unscoped base. The sibling MCP block, same handler, same purpose, carries the correct shape and states the same intent ("The /mcp route is mounted bare (not project-scoped), so point at the unscoped base") — so the repair shape is already pinned in-file and needs no invention.

Why it is green today

routes.auth is pinned three times, and every pin is on the unscoped base, where isScoped is false and the branch never runs:

packages/objectql/src/protocol-discovery.test.ts:47    expect(discovery.routes.auth).toBe('/api/v1/auth');
packages/objectql/src/protocol-discovery.test.ts:100   expect(discovery.routes.auth).toBe('/api/v1/auth');
packages/objectql/src/protocol-discovery.test.ts:461   expect(discovery.routes.auth).toBe('/api/v1/auth');

⇒ No test exercises routes.auth on a scoped discovery document. A repair should land with that missing case, not only with the regex.

Confidence, stated in both directions

Certain: the two regexes differ, isScoped keys on the new spelling only, and the auth regex therefore cannot match on its own branch. This is string logic with no configuration in it.

⚠️ Not measured here: what a client does with the bad value. routes.auth is only rewritten if (discovery.routes.auth) — i.e. when an auth service is advertised at all — and I did not measure which deployments serve a scoped /discovery with auth present, nor whether any live consumer follows routes.auth rather than constructing /auth itself. ⛔ Do not size the blast radius from this card without that.

⚠️ Adjacent but not the same: #9292 repaired which protocol the scoped /discovery handler builds its document from. This is the routes projection in the same handler, a different half.

Why it is filed rather than fixed under #15858

#15858's dispatch scopes that PR to two prose sentences and grades it Clause-② no, prose only, no routing change. This is executable and changes an advertised route value, so it needs a behavioural test — a new verification surface the bounded in-place exemption does not cover. Fixing it there would also falsify the card's own "prose only" classification mid-flight.

Re-check

git grep -n "unscopedBase" -- packages/rest/src/rest-server.ts
git grep -n "routes\.auth" -- packages

⚠️ Locate by the sentence, ⛔ not by line — every anchor in the #14503 / #15488 / #15858 family moved within two days, and #15858's own anchors moved 119 lines in one.

Refs: #15858 (the card this was found under) · #15488 (the same retired spelling stranded in the dispatcher's strip regex — code, not prose) · #15861 (the cloud-side reading request on the same ADR-0006 boundary) · #9292 (the other half of this handler) · ADR-0006 v4, second addendum.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions