opensource/sim
3
0
Fork 0
mirror of https://github.com/simstudioai/sim.git synced 2026-09-24 15:45:35 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(v2): stop six responses reporting less than the layer beneath them knew (#7097)

Browse Source 
* fix(v2): stop six responses reporting less than the layer beneath them knew

Findings from cubic's review of the v0.8.12 release PR, all on the v2 surface.

- `GET /workflows/{id}/runs/{runId}` documented `includeFileBase64` as
  requiring `includeOutput`, and the read honours that — files are projected
  inside the `includeOutput` branch alone. Nothing enforced it, so the flag
  parsed, was accepted, and was then dropped: a 200 carrying no files and no
  reason why. Now a 400 naming the missing flag, matching how `GET
  /billing/logs` refuses a window bound its period will not read.
- `POST /files/{id}/unzip` rendered a malformed or over-cap archive as 500.
  `ArchiveError` had no arm in the v2 policy, though the internal extract route
  beside it has mapped the same failures to 400/413 all along.
- Both workflow-MCP lists cut their tool inventory at a ceiling and published
  `nextCursor: null` regardless. The use cases were already returning
  `truncated`; only the v2 presenters dropped it, while the copilot handler
  published it. A reconciling caller read a partial set as the complete one.
- A table dispatch scoped by filter reported neither the filter nor its
  exclusions, and `rowIds: undefined` documented itself as "every eligible
  row" — so a filtered run and an unfiltered one were indistinguishable. The
  compiled filter stays unpublished, as before; `selectAll` and
  `excludeRowIds` name the distinction.
- `GET /files/uploads/{id}` promised the registered file after finalization and
  passed null unconditionally, so a caller polling a transfer it lost track of
  could watch a session reach `completed` and never learn what it created.
- `HEAD` on a run file resolved the file downstream of authorization, so it
  answered 200 for an id the `GET` beside it would 404.
- v2 bulk table delete audited `FOLDER_DELETED.resourceId` from the
  caller-keyed projection, writing a display path where the single-folder
  delete writes the canonical id.

Two further findings needed no change: the audit-log cursor scope is bound by
`member_user_id_unique` and a membership check, so the cross-organization
replay it described cannot arise; and both documentation findings had already
been fixed on staging by #7092.

* fix(v2): report a dispatch's two narrowings separately

Review follow-up. `selectAll` was set from the stored filter alone, but the
run rejects only `rowIds` *with* `excludeRowIds` — so an exclusion set with no
filter is a scope a caller can create, and the walk applies it. Those
dispatches published exclusions with no discriminator beside them, which is
the shape the flag existed to rule out.

One flag could not cover both: an exclusion-only scope is neither filtered nor
unnarrowed. So the two narrowings are reported as what they are — `filtered`
for the unpublished stored predicate, `excludeRowIds` for the deselections —
and every combination is now distinguishable from a run over every eligible
row. `excludeRowIds` mirrors the walk's own condition and is withheld beside
`rowIds`, where the dispatcher would ignore it.

* fix(v2): let a failed file read fail, and point truncation at its own check

Two review follow-ups.

`getWorkspaceFile` logs and returns null on a read failure unless told
otherwise, so a transient database error would have reported a finalized
upload as fileless — to the one caller polling to learn what it created, who
would then stop, having been told there was nothing. Read with `throwOnError`
so the failure surfaces and the poll can retry, matching how the sibling
record read loads the same row. A file genuinely deleted still answers null,
which is a different question with a different answer.

The server list pointed callers at `/tools` for an authoritative inventory
without saying that endpoint applies the same ceiling. It now names the
`truncated` flag this PR added there, so "authoritative" has a condition
attached instead of being asserted.
This commit is contained in:
Waleed
2026-08-25 21:02:59 -07:00
committed by GitHub
parent b391b28d44
commit 3645857ddb
26 changed files with 587 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/docs/openapi-v2-resources.json
+14 -4
View File
@@ -7390,9 +7390,13 @@
}
],
"description": "Opaque cursor for the next page. Send it back as `cursor`; `null` means there is nothing further to fetch. Never construct one yourself."
},
"toolNamesTruncated": {
"type": "boolean",
"description": "Whether `toolCount` and `toolNames` under-report. The names are gathered for the whole page under one ceiling, so a page whose servers publish more tools than that ceiling between them reports only part of each server's inventory. Read `GET /api/v2/workflow-mcp-servers/{serverId}/tools` for one server's inventory and check that response's own `truncated`, which reports the same ceiling applied to a single server — only an untruncated response is the authoritative set. Unrelated to `nextCursor`, which is how this list says there are further servers."
}
},
"required": ["data", "nextCursor"],
"required": ["data", "nextCursor", "toolNamesTruncated"],
"additionalProperties": false,
"title": "List workflow MCP servers response",
"description": "A cursor-paginated page of published MCP servers.",
@@ -7411,7 +7415,8 @@
"toolNames": ["triage_ticket"]
}
],
"nextCursor": null
"nextCursor": null,
"toolNamesTruncated": false
}
]
},
@@ -7653,9 +7658,13 @@
}
],
"description": "Always `null` — this list has no `cursor` or `limit` param and returns its whole bounded set in one page. Present so the list can gain pages later without a shape change."
},
"truncated": {
"type": "boolean",
"description": "Whether this inventory was cut short by the server-side ceiling on how many tools one response may carry. `nextCursor` is null either way — this list takes no `cursor`, so a truncated set cannot be paged past and this flag is the only way to tell a partial inventory from a complete one. A reconciling caller must not treat a truncated set as the full published inventory."
}
},
"required": ["data", "nextCursor"],
"required": ["data", "nextCursor", "truncated"],
"additionalProperties": false,
"title": "List workflow MCP tools response",
"description": "The tools a published MCP server exposes.",
@@ -7674,7 +7683,8 @@
"updatedAt": "2026-06-12T10:30:00.000Z"
}
],
"nextCursor": null
"nextCursor": null,
"truncated": false
}
]
},
apps/docs/openapi-v2-tables.json
+12 -1
View File
@@ -9964,7 +9964,18 @@
"description": "Workflow groups the dispatch runs."
},
"rowIds": {
"description": "Explicit rows the dispatch targets; absent means every eligible row.",
"description": "Explicit rows the dispatch targets. Absent means it was given no row list and walks every eligible row, narrowed by `filtered` and `excludeRowIds` when either is present.",
"type": "array",
"items": {
"type": "string"
}
},
"filtered": {
"description": "Present and true when a stored filter narrows which rows run. The filter itself is not published: it is held compiled, in a different grammar from the predicate the request was written in. Absent means no filter narrows the dispatch — which, with no `rowIds` and no `excludeRowIds`, is what means every eligible row.",
"type": "boolean"
},
"excludeRowIds": {
"description": "Rows the walk skips. Independent of `filtered`: a dispatch may exclude rows from a filtered set or from every eligible row. Never present alongside `rowIds`, which the run rejects and the walk would ignore.",
"type": "array",
"items": {
"type": "string"