Skip to content

Commit 405933f

Browse files
committed
docs(v2): cut duplicated and non-load-bearing comment prose
Five rationales were written three to five times each by parallel agents that could not see one another. Each now has one home and the rest point at it: - HEAD existence oracle -> the headSafe option on defineV2JsonRoute - cursor query binding -> cursorScopeKey in lib/api/cursor-binding.ts - storage-key prefix budget -> buildStorageKeySegment - NUL / U+0000 -> the containsNulCharacter predicate - blank and duplicate query values -> their own implementations Also drops changelog-in-source (prose narrating what the code used to do), anchorless module headers attached to no declaration, rejected- alternative essays, and @PARAM tags that only restate the signature. Comments only: the diff contains no executable-code change.
1 parent 7125eac commit 405933f

47 files changed

Lines changed: 232 additions & 462 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/app/api/v2/billing/logs/route.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,8 @@ describe('GET /api/v2/billing/logs', () => {
148148
})
149149

150150
/**
151-
* The envelope check used to accept any string as the inner token, so an
152-
* empty one passed it and then read as falsy in the ledger reader: no cursor
153-
* condition was applied and the caller walked the first page again — the very
151+
* An empty inner token reads as falsy in the ledger reader, so no cursor
152+
* condition is applied and the caller walks the first page again — the very
154153
* failure {@link UNKNOWN_CURSOR_MESSAGE} exists to make visible.
155154
*/
156155
it('rejects a cursor whose inner token is empty instead of restarting at page one', async () => {

apps/sim/app/api/v2/files/[fileId]/route.test.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,10 @@ describe('v2 single-file routes', () => {
121121
})
122122

123123
/**
124-
* The download `HEAD` short-circuit used to sit between admission and parsing,
125-
* so it answered before the workspace-scoped file resolution that lives in the
126-
* use case. Any valid API key therefore drew a bodiless 200 for a file id it
127-
* cannot reach, while the `GET` for the same URL answered 404. These pin the
128-
* probe to the answer the download gives, and to still not auditing one.
124+
* A download `HEAD` answered before the use case's workspace-scoped file
125+
* resolution is an existence oracle: any valid API key draws a bodiless 200
126+
* for a file id whose `GET` answers 404. These pin the probe to the answer the
127+
* download gives, and to still not auditing one.
129128
*/
130129
it('answers an authorized HEAD bodiless without auditing a download', async () => {
131130
const response = await GET(headRequest(`workspaceId=${WORKSPACE_ID}`), context)

apps/sim/app/api/v2/files/[fileId]/route.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,8 @@ export const revalidate = 0
2929
*
3030
* A generated doc whose artifact is still compiling renders `CONFLICT`; retry.
3131
*
32-
* Downloading is not a safe read: it records a `FILE_DOWNLOADED` audit event and
33-
* pulls the bytes out of object storage. Next aliases `HEAD` onto `GET`, and RFC
34-
* 9110 §9.2.1 defines `HEAD` as safe, so this route declares itself not
35-
* head-safe. A `HEAD` is admitted, parsed, and authorized through
36-
* `downloadWorkspaceFileStream.authorize` — the same workspace-scoped file
37-
* resolution and access check the `GET` performs — then answered bodiless
38-
* without auditing or fetching. Without the not-head-safe declaration an uptime
39-
* monitor walking the documented URL list would fabricate a download event on
40-
* every probe; without the authorization step the probe would instead confirm a
41-
* file id the caller has no right to know exists.
32+
* `headSafe: false` because downloading records a `FILE_DOWNLOADED` audit event
33+
* and pulls the bytes out of object storage.
4234
*/
4335
export const GET = defineV2BinaryRoute({
4436
contract: v2DownloadFileContract,

apps/sim/app/api/v2/files/route.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@ describe('/api/v2/files', () => {
106106

107107
/**
108108
* `?limit=` is not `limit` omitted. `Number('') === 0`, and this list clamps
109-
* out-of-range values, so the blank used to reach the query as `LIMIT 1` and
110-
* return a single row where the omitted param returns a hundred — a silently
111-
* wrong page, not an error. Whitespace-only is the same value.
109+
* out-of-range values, so an unrejected blank reaches the query as `LIMIT 1`
110+
* and returns a single row where the omitted param returns a hundred — a
111+
* silently wrong page, not an error. Whitespace-only is the same value.
112112
*/
113113
it.each(['limit=', 'limit=%20', 'sortBy=', 'cursor='])(
114114
'rejects the blank query value %s instead of coercing it',

apps/sim/app/api/v2/knowledge/search/route.test.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,9 @@ describe('POST /api/v2/knowledge/search', () => {
168168
})
169169

170170
/**
171-
* `rerankerEnabled` on its own used to satisfy the schema, fail the use case's
172-
* model guard, and answer 200 in plain vector order — after paying for the
173-
* widened candidate retrieval. The default closes that, matching the internal
174-
* search contract.
171+
* Without the default, `rerankerEnabled` alone satisfies the schema, fails the
172+
* use case's model guard, and answers 200 in plain vector order — after paying
173+
* for the widened candidate retrieval.
175174
*/
176175
it('defaults the reranker model so enabling reranking is enough to run it', async () => {
177176
const response = await POST(
@@ -280,9 +279,9 @@ describe('POST /api/v2/knowledge/search', () => {
280279
* The search body is strict, so an undeclared key is refused rather than
281280
* stripped. That matters most for a bring-your-own reranker key: dropping it
282281
* silently left the caller believing the secret it sent was in use. It
283-
* matters for an ordinary mis-spelling too`rerankerenabled` used to parse
284-
* to 200 with reranking off, and `topk` with `topK` back at its default, both
285-
* of which change what the search is billed.
282+
* matters for an ordinary mis-spelling too: a stripped `rerankerenabled` is a
283+
* 200 with reranking off, and a stripped `topk` leaves `topK` at its default
284+
* both change what the search is billed.
286285
*/
287286
it('refuses a caller-supplied reranker key instead of silently dropping it', async () => {
288287
const response = await POST(

apps/sim/app/api/v2/logs/route.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,8 @@ describe('GET /api/v2/logs', () => {
196196
})
197197

198198
/**
199-
* The envelope check used to accept any string as the inner token, so an
200-
* empty one passed it and then read as falsy in the domain codec: no cursor
201-
* condition was applied and the caller silently got page one back, with a
199+
* An empty inner token reads as falsy in the domain codec, so no cursor
200+
* condition is applied and the caller silently gets page one back, with a
202201
* `nextCursor` inviting it to do the same thing forever.
203202
*/
204203
it('rejects a cursor whose inner token is empty instead of restarting at page one', async () => {

apps/sim/app/api/v2/mcp-servers/[id]/tools/route.test.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,11 @@ describe('/api/v2/mcp-servers/[id]/tools', () => {
128128
})
129129

130130
/**
131-
* The `HEAD` short-circuit used to sit between admission and parsing, so it
132-
* answered before resource authorization ever ran — which lives in the use
133-
* case. Any valid API key therefore drew a bodiless 200 for a server id in a
134-
* workspace it cannot read, a server id that does not exist, and a principal
135-
* kind this operation refuses outright, while the `GET` for the same URL
136-
* answered 403 or 404. These four pin the probe to the answer the `GET` gives.
131+
* A `HEAD` answered before the use case's resource authorization is an
132+
* existence oracle: any valid API key draws a bodiless 200 for a server id in
133+
* a workspace it cannot read, for one that does not exist, and for a principal
134+
* kind this operation refuses outright. These four pin the probe to the answer
135+
* the `GET` gives.
137136
*/
138137
it('does not confirm a server to a principal kind the operation refuses', async () => {
139138
mocks.authorizeDiscover.mockRejectedValueOnce(new WorkspaceApiKeyAuthorizationError())
@@ -260,11 +259,10 @@ describe('/api/v2/mcp-servers/[id]/tools', () => {
260259
})
261260

262261
/**
263-
* The 503 wording used to be selected by searching the error message for
264-
* `cooldown`. `McpConnectionError` interpolates the server's display name into
265-
* that message, so a server the caller happened to name after the word
266-
* borrowed the negative-cache wording and told them to wait out a cooldown
267-
* that was never entered.
262+
* `McpConnectionError` interpolates the server's display name into its
263+
* message, so selecting the 503 wording by searching that message for
264+
* `cooldown` hands a server named after the word the negative-cache wording
265+
* for a cooldown it was never in.
268266
*/
269267
it('does not read cooldown wording out of a server display name', async () => {
270268
mocks.discover.mockRejectedValueOnce(new McpConnectionError('ECONNREFUSED', 'Cooldown Docs'))

apps/sim/app/api/v2/mcp-servers/[id]/tools/route.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,8 @@ export const revalidate = 0
1414
* as `mcp-<hash>` from the workspace and endpoint URL, and the registration
1515
* contract requires a URL.
1616
*
17-
* Discovery is not a safe read: it opens a live connection to the registered
18-
* endpoint and records the outcome on the server row. Next aliases `HEAD` onto
19-
* `GET`, and RFC 9110 §9.2.1 defines `HEAD` as safe, so this route declares
20-
* itself not head-safe. A `HEAD` is admitted, parsed, and authorized through
21-
* `discoverMcpServerToolsUseCase.authorize` — the same principal-kind check,
22-
* server resolution, and workspace access check the `GET` performs — then
23-
* answered bodiless without connecting or writing. Without the not-head-safe
24-
* declaration an uptime monitor walking the documented URL list would drive
25-
* outbound third-party traffic and mutate rows on every probe; without the
26-
* authorization step the probe would instead confirm that a server id exists in
27-
* a workspace the caller cannot read.
17+
* `headSafe: false` because discovery opens a live connection to the registered
18+
* endpoint and records the outcome on the server row.
2819
*/
2920
export const GET = defineV2JsonRoute({
3021
contract: v2ListMcpServerToolsContract,

apps/sim/app/api/v2/mcp-servers/utils.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,9 @@ export const MCP_SERVER_REAUTHORIZATION_REQUIRED = 'MCP_SERVER_REAUTHORIZATION_R
5454
* Every branch returns a constant, so an upstream message — which may quote a
5555
* hostname, a token endpoint, or a stack — never reaches the caller.
5656
*
57-
* Selection is typed for the same reason classification is. The cooldown branch
58-
* used to search the message for `cooldown`, but `McpConnectionError`
59-
* interpolates the server's display name into its message, so a server a caller
60-
* named after the word was told to wait out a cooldown it was never in.
57+
* Selection is typed, never matched on message text: `McpConnectionError`
58+
* interpolates the server's display name into its message, so a server named
59+
* after the word `cooldown` would select the cooldown branch it is not in.
6160
*/
6261
function unreachableServerMessage(error: unknown): string {
6362
if (isTimeoutError(error)) return 'The MCP server took too long to respond'

apps/sim/app/api/v2/skills/route.test.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,9 @@ describe('/api/v2/skills', () => {
314314
})
315315

316316
/**
317-
* The workspace-key create used to be the case this file pinned analytics
318-
* against: it succeeded, and the assertion was that no `skill_created` event
319-
* was attributed to a principal with no human subject. `skills.create` now
320-
* denies the key outright, so what needs pinning here is the surface's half of
321-
* that — the refusal reaches the caller as the operation's own 403, and a
322-
* create that never happened emits nothing.
317+
* `skills.create` denies a workspace key outright, so what this pins is the
318+
* surface's half: the refusal reaches the caller as the operation's own 403,
319+
* and a create that never happened emits no analytics.
323320
*/
324321
it('refuses a workspace-key create and records no analytics for it', async () => {
325322
mocks.create.mockRejectedValueOnce(

0 commit comments

Comments
 (0)