fix(v2,cli): second audit pass over the v2 API and CLI by waleedlatif1 · Pull Request #7126 · simstudioai/sim

waleedlatif1 · 2026-08-26T20:54:21Z

Summary

Second audit pass over the v2 API and the CLI. Every claim was investigated by an independent agent before anything was written, then every applied change was re-reviewed against its own commit. Findings that turned out not to be real were dropped.

Data loss and destructive operations

A page-size default of 100 was attached to every request field named limit, but only cursor-paginated commands interpret it. The two filter-based row mutations declare no cursor, so it went on the wire as a row cap: a filter matching 250 rows deleted 100, exited 0, and said nothing — while the confirmation the user had just answered promised every matching row. The default now applies only where the paginator runs.
tables import --mode replace empties the table before its first batch, with the only warning in a describe. It now confirms, and the wording is per-mode: cancelling a replace leaves a prefix of the new file with the originals already gone; an append re-adds its rows if the file is imported twice.
secrets set always read a value. Off a TTY that was an immediate refusal, so a metadata-only edit exited 1 in CI for a value it was never asked for; on a TTY it prompted, and the prompt rejects empty, so there was no way to say "leave the stored value alone" short of re-typing the secret.

Authorization and secret handling

A caller-supplied conversation id was resumed with no ownership check. It now resolves through an owner-scoped lookup and answers a uniform 404 for unknown, deleted, other-user, other-workspace, and wrong-type — the last of which also closed the same omission on the web posting path.
A profile name or workspace value carrying a line break could forge a section that merged into a different profile and took over its endpoint; the next command then sent that profile's stored key to the attacker's host. Refused at the writer, which is the one place untrusted text enters the document, with flag-level validators kept for better messages.
The pre-write check on a minted key refused fewer characters than the writer, so a key it accepted could still fail at the write — after the endpoint beside it was stored, pairing a new endpoint with the previous key. The set had been spelled three times; there is now one.
Two workspace routes answered 403 for an existing-but-unreachable workspace and 404 for an absent one, which is an existence oracle. Both now conceal, byte-identically. An in-workspace role refusal still returns 403.

Errors that were wrong rather than unhelpful

A save carrying a graph id another workflow owns returned an unclassified 500 (these ids are global primary keys while the deletes are workflow-scoped). It now returns 409. The race handler behind it was dead code — it read the error code off the thrown object, but the driver error arrives wrapped with the real one on cause, and its test passed only because it threw a flat shape production never produces.
Archived table listings 500'd for the whole page when a folder archive left a table pointing at a soft-deleted folder.
A size-limit refusal collapsed every sub-kilobyte value to "0 Bytes", so a 28-byte file over a 27-byte ceiling read "is 0 Bytes, above the 0 Bytes limit".
Log-stats ignored the requested date range when bucketing, so a custom range past the last run charted mostly empty buckets.
Listing the editors of a built-in skill claimed the skill did not exist while reading it succeeded, then briefly answered a read with a modification refusal.

Descriptions, which serve both the REST reference and --help

~12 schema descriptions told callers to invoke raw HTTP endpoints. A ratchet test now walks every v2 description; the 12 that remain are allowlisted with the file that owns them.
Two OpenAPI sentences promised no conflict detection and no persistence warnings in a dry run, both of which this branch had just made false. One claimed a processed count reported only chunks that changed — the update returns every row it matched.
~59 operations refuse a workspace API key and the CLI said nothing, so callers learned it from a 403 after the request. The restriction is now read from the spec and carried in the command description.
The guides used id shapes the CLI's own help contradicts, including a wf_ prefix on workflow ids when wf_ is the file prefix.

Silent truncation

Lists truncated with no signal in any format, and the two endpoints that do report truncation had that field dropped on the way out. Three more carried it nested in the payload — a clipped file body, row search, and workflow-stats list said nothing. stdout stays byte-identical; the note is on stderr.

Type of Change

Bug fix

Testing

Every fix was proven red before green: the change was reverted by hand and the test watched to fail. Three tests that asserted a contract constant against itself were replaced with ones driving rendered help, real argv, or real render output.

packages/sim-cli: 718 passing
apps/sim affected suites: 9,041 passing
bun run lint, bun run check:audits (33 audits), both type-checks, and all three generator --checks pass

Behaviour changes worth noting on release

Filter-based row deletes and updates are now unbounded where they were silently capped at 100.
Two workspace routes return 404 where they returned 403; a graph-id conflict returns 409 where it returned 500.
Conversation ids issued before this deploy will 404 on resume — the previous code never created the row.
Bulk operations that matched nothing now exit non-zero; empty query filter values are refused rather than silently dropped.
On a TTY, secrets set NAME --description ... no longer prompts for a value.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

The v2 chat route used the caller-supplied conversationId verbatim, with no existence, owner, or workspace check, against a store keyed by bare text with no owner column. A caller who knew another user's conversation id reached that conversation. Ids now resolve through the same owner-scoped loader the web chat path uses, and anything unresolvable answers one uniform 404 before any lifecycle work runs. Omitting the id mints a server-issued conversation. The contract also accepted any 1-128 character string for a column typed uuid, so a malformed id raised a driver error and rendered 500 while an unknown but well-formed id rendered 404 - a shape oracle, and a 500 on ordinary input. The ownership predicate had no coverage anywhere: the route test mocked the module and the lifecycle test drove a chain mock that ignores its where clause, so deleting the owner condition left both suites green. It is now asserted by composition and by condition count, which is what catches a dropped condition. Also renames the reply's model identifier away from a term the project's own copy rules forbid on a user-facing surface.

… their page Two reads answered a caller more than they were entitled to know. A workspace a caller cannot reach at all returned FORBIDDEN while one that does not exist returned NOT_FOUND, so a workspace-key holder could enumerate which workspace ids exist by diffing the two. Both now answer the same absence, using the concealment policy the billing routes already use. A refusal from inside the workspace - a member whose role is too low - still answers FORBIDDEN, because that caller already knows the workspace exists. Separately, archiving a folder cascades onto its tables but leaves each table pointing at the archived folder row. The archived listing resolved those paths strictly, so one such row faulted the whole page and no cursor could step past it - which also made the ids undiscoverable and left restore unreachable for exactly the tables that need it. The archived scope now resolves leniently to the root, where a restore would place them, matching the shipped workflows behavior. Active listings still fault loudly on a dangling folder.

…live sessions recipe and lang were accepted as free strings up to their length caps, silently discarded, and echoed back nowhere, so a typo was unobservable: uploading with a misspelled recipe returned 200 and quietly used the default. Both are now validated at the boundary and a bad value answers 400 naming what is accepted. The accepted recipe set deliberately includes the sentinel every first-party caller sends today alongside the three real chunker recipes, and the three are derived from the chunker's own union so removing one there is a compile error here rather than a silent 400 in production. The same schema also parses metadata read back off a persisted upload session, so tightening it would have thrown out of resume and complete for any session created before this - a 500 on work that could then never finish. The read-back path now drops a value it no longer recognises instead of rejecting it; the request boundary stays strict. Neither field reaches chunking, so nothing here moves chunk boundaries, embeddings, or search results.

…id with a conflict Log statistics accepted a start and an end, filtered the totals by them, and then built the series against wall-clock now. Bucket width was computed over a span the caller never asked for, and every bucket past the requested end was structurally empty - so a bounded historical query returned a wrong-width series with fabricated trailing buckets, under a window label that disagreed with the request. Each edge now honours the bound it was given and keeps its previous derivation when omitted, so an unbounded request is unchanged. Separately, block, edge and subflow ids are global primary keys while the delete that precedes a state replace is scoped to one workflow. An id owned by another workflow survived that delete, the insert violated the key, and because callers pass their own transaction the driver error escaped unclassified as a server fault. The write now refuses such an id up front with a conflict naming it, and re-classifies the same violation if one races past the check, since the lock covers only the workflow being written. The dry run checks the ids a commit would insert and reports the warnings a commit would report, which is what its own contract already promised.

…nding the value Restoring redaction cost more than removing it. The only way to flip a secret back to redacted was to re-send the plaintext, because the write required a value and omitting it fell into an interactive prompt that cannot run in CI. A workspace secret can now change its description or visibility on its own; the stored value is never re-encrypted or rewritten, a write that names no existing secret answers not-found rather than creating one, and a personal secret still requires a value because it has no other writable field. The path parameter was also one shared schema across the write and the delete, so a single description had to cover both and the delete documented an argument that could create and replace. Split, mirroring the credentials pair. The metadata write is a new update against the credentials table, so its scope is asserted by composition and by condition count: an unscoped update would let one workspace flip another workspace's identically-named secret out of redaction, and the cache invalidation would then carry that flag into the other workspace's runtime catalog.

A size-limit refusal collapsed every value under a kilobyte to "0 Bytes", so a 28-byte file over a 27-byte ceiling read "is 0 Bytes, above the 0 Bytes limit" - self-contradictory, and useless for choosing a value that would work. Errors and field descriptions also told callers to invoke raw HTTP endpoints. These strings serve the REST reference and the CLI's own help equally, so they now name the operation and its object rather than a method and a path. A sweep test walks every v2 schema description and holds the line, with the remaining offenders in files this change does not own recorded explicitly rather than left to be rediscovered. Listing the editors of a built-in skill claimed the skill did not exist, while reading the same id succeeded - a well-formed request for a real resource is not malformed, so the list answers an empty roster and only the mutations refuse. Bulk folder deletion recorded only the leaf name in its audit trail while the single delete recorded the full path, leaving two same-named folders under different parents indistinguishable after the fact. Bulk chunk enable, disable and delete each treated an unmatched id differently behind one sentence of documentation. They now follow one rule. A workspace-scoped list refused with the name of a resource the caller never addressed, which reads as an empty workspace rather than an unreachable one.

… write The config file is written by joining names and values into INI lines, and nothing checked what was in them. A profile name carrying a newline and a section header wrote a section that merged into a different profile and took over its endpoint - and the next command sent that profile's stored API key there. A workspace value could do the same from the other side, since only the endpoint flag validated its input. The refusal now lives at the writer, the single place untrusted text enters the document, with the flag-level checks kept for the better message. Either alone blocks the forgery; the pair is deliberate. Rejecting rather than escaping, because the format has no escape syntax and these files are hand-edited and read by other tools that would not decode one we invented. The forbidden set covers control characters and the two Unicode line separators, which the previous guard missed - those parse as an unreadable line, so the key silently vanished on read and the next write appended a duplicate while the command reported success. Login also wrote the key before the settings, so a malformed response from the deployment could leave a key on disk with no endpoint beside it, and the next command would send it to the default host. Settings are written first, and the response is checked before anything touches disk. Name validation applies only when creating a profile, so a hand-written one that predates the rule keeps working.

…s contradicting the CLI Around sixty v2 operations refuse a workspace API key, and the CLI's help said nothing about it - the caller found out from a 403 after the request went out. The restriction is already stated in the API spec, so the generator now reads it from there and the command description carries it. The sentinel sentences are imported from the spec's own constants rather than copied, so a reword cannot silently unmark every command, and the test pins the count as well as named operations because a reword confined to one family would otherwise slip past. The generated reference also rendered an empty default as a sentence pointing at nothing - "Defaults to ." - for every repeatable filter. Omitted now, while false and zero still render, which is the trap that shape of check usually walks into. The hand-written guides used a workflow-shaped id for workflows that the CLI's own help says never names one, and five other families were equally wrong. All of them now match the scheme the CLI declares, consistently per entity across pages, with the shared ones taken from that help text so the two read as one voice. The page documenting every flag was linked from nowhere; both landing links pointed at the overview instead. And the generator's test file was absent from the hand-maintained list CI runs, so its guards never executed.

Every request field named limit inherited the pager's default of 100, but only a cursor-paginated command interprets that flag. The two filter-based row mutations declare no cursor, so the default went onto the wire as a row cap: a filter matching 250 rows deleted 100, exited 0, and said nothing - while the confirmation the user had just answered promised every matching row. The flag's own help offered 0 for everything, which those endpoints reject; the unbounded form is the field being absent. The pager's default now applies only where the pager runs, and the tests pin the omission on the request body rather than in help text. A cap typed alongside an explicit row list was silently ignored; it is now refused on the client, where refusing costs nothing to already-installed versions. Lists also truncated at a hundred with no signal in any format, and the two inventory endpoints that do report truncation had that field dropped on the way out - so a caller reconciling against a clipped list could not tell. One note now goes to stderr while stdout stays a bare array, and a flag raised on a later page survives the fold. Also: a folder whose name contains the separator no longer prints a path that resolves to a different folder; validation errors name the flag the user typed instead of the wire field; an unknown subcommand with --help exits non-zero instead of printing the parent's help; a fractional or negative page size is refused rather than floored; an empty query filter is refused rather than silently returning everything; and the two spellings of the missing-workspace message became one.

A `tables import --mode replace` empties the table before its first batch, so the only warning was in the describe. It now confirms, and the wording tells the truth per mode: cancelling a replace leaves a prefix of the new file with the originals already gone, while an append re-adds its rows if the file is imported twice. `--yes` skips the gate, and the gate runs before the file is opened. Import and export cancellation carried no describe at all; the import one now confirms, the export one records why it deliberately does not. Follow-mode output truncated cells to whatever the first row happened to measure, so a longer status or workflow name arrived clipped with no signal. Cells now clamp at a shared ceiling and pad to the lock, and the log columns carry width floors so a short first page cannot pin a column narrower than its own values. Interrupting a staged download left the staging directory behind; it is now removed on SIGINT and SIGTERM before the signal is re-raised. `--select-output` without `--follow` selected from a response that does not carry outputs, and said nothing. It is refused client-side, with a separate message for `--async`. Its describe now names what the path addresses. `secrets set` always read a value, even when only metadata flags were passed. Off a TTY that was an immediate refusal, so a metadata-only edit exited 1 in CI for a value it was never asked for; on a TTY it stopped to prompt, and the prompt rejects an empty entry, so there was no way to say "leave the stored value alone" short of re-typing the secret. The read is now skipped and the field omitted, which is what lets a metadata-only edit run unattended. On a TTY, setting only a description no longer prompts. Passing both spellings of the reveal flag is refused rather than silently resolved. Four mandatory hand-authored flags now say so, `billing logs` names its key-type scope, and the dispatch list declares its columns.

A conflict handler added earlier in this branch was dead code. It read the Postgres error code off the thrown object, but the driver error arrives wrapped with the real one on `cause`, so the check returned false on its first line and the 409 never fired. Its test passed only because it threw a flat shape production never produces. It now reads through the cause chain with the shared helpers, compares the constraint name exactly instead of matching a substring of the SQL, and its test throws the real wrapped error. Resuming a conversation checked its workflow and its workspace but not its type, so a conversation created by the web surface could be continued as a CLI turn. It now refuses through the same uniform 404 as every other mismatch, which closes the same omission on the web posting path. Minting one no longer leaves a blank untitled row at the top of the Chat list. The pre-write check on a minted API key refused fewer characters than the writer does, so a key the check accepted could still fail at the write — after the endpoint beside it was already stored, pairing a new endpoint with the previous key. The two had drifted because the set was spelled three times; there is now one. A description claimed a processed count reported only the chunks that changed. The update returns every row it matched, so re-enabling chunks that were already enabled counts them all. Two OpenAPI sentences promised no conflict detection and no persistence warnings in a dry run, both of which the same branch had just made false. A described window was wrong whenever a start was supplied without an end. Listing the editors of a built-in skill answered a read with a modification refusal on the internal surface. Archived table listings could reach the strict folder projector again through a third scope value the input type still allowed. A metadata-only secret write skipped the guard its personal-scope twin has. The internal document boundary still took the two processing fields as unbounded strings. Truncation was reported only from the response envelope, so a clipped file body, row search and workflow-stats list said nothing. A staged download stopped watching for signals before it finished removing its directory, and cleared every listener for the signal rather than its own. Three tests asserted a contract constant against itself; they now drive rendered help, real argv, or real render output.

The published reference still marked a secret value required and described the delete parameter as one that also creates, the CLI surface still lacked the marker that says which operations refuse a workspace key, and the reference rendered an empty sentence for every repeatable filter whose default is an empty list.

The audit bans a hand-rolled setTimeout promise. `sim-cli` does not depend on the shared utils package, and its own idiom is `node:timers/promises`.

vercel · 2026-08-26T20:54:28Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview	Aug 26, 2026 8:59pm

waleedlatif1 · 2026-08-26T20:54:54Z

@greptile-apps

greptile-apps · 2026-08-26T20:58:34Z

Greptile Summary

This PR performs a broad correctness and safety audit of the v2 API and CLI.

Tightens chat, workspace, secret, and workflow authorization and error handling.
Corrects CLI pagination, destructive-operation confirmation, truncation reporting, profile validation, and bulk-operation outcomes.
Aligns API contracts, generated CLI metadata, help output, and published documentation.
Adds focused regression coverage for the corrected runtime and persistence behavior.

Confidence Score: 5/5

The PR appears safe to merge; no concrete changed-code defect remained after checking the highest-risk authorization, pagination, truncation, and persistence paths.

The reviewed changes preserve chat ownership boundaries, keep generated pagination metadata aligned with request construction, report genuine truncation without altering stdout, and map graph-ID conflicts using constraint names that match the schema and migrations.

Important Files Changed

Filename	Overview
apps/sim/app/api/v2/chat/route.ts	Adds owner-, workspace-, and type-scoped conversation resolution with uniform not-found behavior; no remaining reachable authorization defect was identified.
apps/sim/lib/copilot/chat/lifecycle.ts	Centralizes accessible-chat resolution and validates stored chat scope before resuming a conversation.
packages/sim-cli/src/runtime/execute.ts	Refines pagination, bulk no-match handling, and operation execution while preserving the intended command-specific boundaries.
packages/sim-cli/src/runtime/result.ts	Reports nested and envelope-level server truncation on stderr without changing structured stdout.
packages/sim-cli/src/config/ini.ts	Hardens profile serialization against values capable of forging INI sections or fields.
apps/sim/lib/workflows/persistence/replace-normalized-state.ts	Detects graph-ID ownership conflicts and correctly unwraps database-driver errors for race-time conflict classification.
packages/sim-cli/src/commands/protocol/tables-import.ts	Adds mode-specific confirmation around destructive replace imports and clarifies append behavior.
scripts/generate-v2-cli-api.ts	Extends generated command metadata so authentication restrictions and request semantics remain aligned with v2 contracts.

Sequence Diagram

sequenceDiagram
    participant U as CLI or API caller
    participant C as Generated command contract
    participant R as Request runtime
    participant A as v2 API route
    participant S as Authorized application service
    participant D as Persistence
    U->>C: Arguments, flags, and profile
    C->>R: Typed operation metadata
    R->>A: Authenticated scoped request
    A->>S: Validated principal and input
    S->>D: Authorized read or mutation
    D-->>S: Result or classified conflict
    S-->>A: Typed response
    A-->>R: Result and truncation metadata
    R-->>U: Stable stdout, diagnostics on stderr

_{Reviews (1): Last reviewed commit: "test(cli): use the package's own delay h..." | Re-trigger Greptile}

cubic-dev-ai

11 issues found across 139 files

Confidence score: 2/5

packages/sim-cli/src/commands/auth.ts and packages/sim-cli/src/config/ini.ts can partially update a profile before rejecting a padded API key, leaving credentials in an inconsistent state; reject non-trim-equal keys before any write.
apps/sim/app/api/v2/chat/route.ts drops the loaded transcript when continuing a conversation, so headless requests lose prior context and may produce incorrect responses; pass the resolved transcript to the model.
apps/sim/lib/secrets/application/use-cases.ts can turn a metadata-only write/deletion race into a 500 after the credential disappears; make the update and follow-up lookup race-safe.
apps/sim/lib/workflows/persistence/replace-normalized-state.ts checks block-record keys different from those it inserts, allowing conflicts to slip through and making workflow persistence unreliable; collect IDs from the block values before conflict detection.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/sim-cli/src/commands/auth.ts">

<violation number="1" location="packages/sim-cli/src/commands/auth.ts:139">
P1: When the server returns a key with leading or trailing whitespace, `requireStorableKey` accepts it but `writeCredentialsProfile` rejects it after `writeConfigProfile` has changed the profile. Reject non-trim-equal keys before either file is written.</violation>
</file>

<file name="packages/sim-cli/src/config/ini.ts">

<violation number="1" location="packages/sim-cli/src/config/ini.ts:95">
P2: When the login poll returns a nonblank API key with leading or trailing whitespace, `requireStorableKey` accepts it but this writer rejects it after `writeConfigProfile` has already run. Reject padded keys during login preflight, or make the config and credential update atomic.</violation>
</file>

<file name="packages/sim-cli/src/config/profile.ts">

<violation number="1" location="packages/sim-cli/src/config/profile.ts:66">
P2: When a new profile name contains control characters, this error prints them verbatim and allows terminal/log output injection. Redact `FORBIDDEN_IN_VALUE` matches before interpolating the name.</violation>
</file>

<file name="apps/sim/lib/knowledge/upload-metadata.ts">

<violation number="1" location="apps/sim/lib/knowledge/upload-metadata.ts:27">
P3: This regex accepts invalid BCP-47 tags such as `en-a`, so `langSchema` does not enforce the contract stated in its error message. Validate with a BCP-47 parser or enforce the extension and private-use grammar before persisting analytics metadata.</violation>
</file>

<file name="packages/sim-cli/src/runtime/request.ts">

<violation number="1" location="packages/sim-cli/src/runtime/request.ts:524">
P2: When an optional numeric query filter is passed as an empty string, `coerce` turns it into `0` before this check, so the blank input bypasses the guard and reaches the API as a real filter. Normalize empty or null optional numeric inputs before coercion and omit them, while preserving an explicit `0`. 

(Based on your team's feedback about empty optional numeric parameters.)</violation>
</file>

<file name="apps/sim/app/api/v2/chat/route.ts">

<violation number="1" location="apps/sim/app/api/v2/chat/route.ts:170">
P1: When `conversationId` continues an existing chat, this resolver loads the transcript but the route discards it, so the headless model sees only the newest message and cannot maintain conversation context. Include `resolvedChat.conversationHistory` in the request messages before the current turn.</violation>
</file>

<file name="apps/sim/lib/secrets/application/use-cases.ts">

<violation number="1" location="apps/sim/lib/secrets/application/use-cases.ts:337">
P2: When a metadata-only write races with deletion, this call can return 500: the metadata update succeeds, deletion removes the credential, and the follow-up lookup throws a generic error. Make the metadata update and response lookup atomic with the same secret-map lock, or translate a disappearing row to `not_found`.</violation>
</file>

<file name="apps/sim/lib/workflows/persistence/replace-normalized-state.ts">

<violation number="1" location="apps/sim/lib/workflows/persistence/replace-normalized-state.ts:46">
P2: When a block record key differs from `block.id`, this pre-check examines different IDs from the ones `saveWorkflowToNormalizedTables` inserts. Collect the IDs from the block values so conflict detection matches the write.</violation>
</file>

<file name="apps/sim/lib/workflows/application/apply-workflow-operations.ts">

<violation number="1" location="apps/sim/lib/workflows/application/apply-workflow-operations.ts:377">
P2: When persistence preparation sanitizes an otherwise valid graph, dry-run omits that preparation warning although a committed apply returns it. Retain the prepared result and merge its warnings into the dry-run response.</violation>
</file>

<file name="apps/docs/content/docs/en/cli/scripting.mdx">

<violation number="1" location="apps/docs/content/docs/en/cli/scripting.mdx:85">
P2: The destructible-commands callout directly below the changed batch-delete example is now wrong and recommends a failing option. In this PR filtered batch-delete/batch-update no longer carry a default --limit of 100, and the NON_PAGINATED_LIMIT_HINT says '0 is not accepted', so the callout's 'carry the default --limit of 100... silently affects only the first 100. Pass --limit 0' instruction is false on both counts. Update the callout to say failing to pass a limit acts on every matching row and that omitting limit is the unbounded form, or remove the stale text.</violation>
</file>

<file name="packages/sim-cli/src/program.test.ts">

<violation number="1" location="packages/sim-cli/src/program.test.ts:101">
P3: The test 'refuses a command the group never had' probes `chat-deployments get --help`, but no `chat-deployments` group exists in the program (chat deployment operations map to `workflows chat ...`). The guard under test is the root dispatcher refusing `chat-deployments`, not a group that exists yet never had a `get` subcommand, so the test passes for a different reason than its name and comment claim. Rename it to reflect that it exercises a top-level group that does not exist, or point it at a real group that lacks the probed verb.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai · 2026-08-26T21:06:52Z

+ * writer rejected stranded the new endpoint beside the previous key.
+ */
+function requireStorableKey(apiKey: unknown): void {
+  if (typeof apiKey !== 'string' || !apiKey.trim() || FORBIDDEN_IN_VALUE.test(apiKey)) {


P1: When the server returns a key with leading or trailing whitespace, requireStorableKey accepts it but writeCredentialsProfile rejects it after writeConfigProfile has changed the profile. Reject non-trim-equal keys before either file is written.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/sim-cli/src/commands/auth.ts, line 139: <comment>When the server returns a key with leading or trailing whitespace, `requireStorableKey` accepts it but `writeCredentialsProfile` rejects it after `writeConfigProfile` has changed the profile. Reject non-trim-equal keys before either file is written.</comment> <file context> @@ -108,15 +112,33 @@ function selectedProfileName(command: Command): string { + * writer rejected stranded the new endpoint beside the previous key. + */ +function requireStorableKey(apiKey: unknown): void { + if (typeof apiKey !== 'string' || !apiKey.trim() || FORBIDDEN_IN_VALUE.test(apiKey)) { throw new SimApiError( - `Profile "${profileName}" already exists. Remove it first with: sim logout --all --profile ${profileName}`, </file context>

Suggested change

if (typeof apiKey !== 'string' || !apiKey.trim() || FORBIDDEN_IN_VALUE.test(apiKey)) {

if (

typeof apiKey !== 'string' ||

!apiKey.trim() ||

apiKey !== apiKey.trim() ||

FORBIDDEN_IN_VALUE.test(apiKey)

) {

cubic-dev-ai · 2026-08-26T21:06:52Z

+      // surface uses, and refuse every id that does not resolve with the same
+      // response so the refusal carries no information about the id. Omitting
+      // the id mints a server-issued conversation instead of trusting one.
+      const resolvedChat = await resolveOrCreateChat({


P1: When conversationId continues an existing chat, this resolver loads the transcript but the route discards it, so the headless model sees only the newest message and cannot maintain conversation context. Include resolvedChat.conversationHistory in the request messages before the current turn.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/api/v2/chat/route.ts, line 170: <comment>When `conversationId` continues an existing chat, this resolver loads the transcript but the route discards it, so the headless model sees only the newest message and cannot maintain conversation context. Include `resolvedChat.conversationHistory` in the request messages before the current turn.</comment> <file context> @@ -130,14 +152,38 @@ export const POST = withRouteHandler( + // surface uses, and refuse every id that does not resolve with the same + // response so the refusal carries no information about the id. Omitting + // the id mints a server-issued conversation instead of trusting one. + const resolvedChat = await resolveOrCreateChat({ + ...(conversationId ? { chatId: conversationId } : {}), + userId, </file context>

cubic-dev-ai · 2026-08-26T21:06:52Z

+  // as something else: the read reports the setting missing although the write
+  // reported success, and the next write appends a second block or key instead
+  // of updating the one already there.
+  if (text !== text.trim()) {


P2: When the login poll returns a nonblank API key with leading or trailing whitespace, requireStorableKey accepts it but this writer rejects it after writeConfigProfile has already run. Reject padded keys during login preflight, or make the config and credential update atomic.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/sim-cli/src/config/ini.ts, line 95: <comment>When the login poll returns a nonblank API key with leading or trailing whitespace, `requireStorableKey` accepts it but this writer rejects it after `writeConfigProfile` has already run. Reject padded keys during login preflight, or make the config and credential update atomic.</comment> <file context> @@ -28,6 +43,62 @@ export interface IniDocument { + // as something else: the read reports the setting missing although the write + // reported success, and the next write appends a second block or key instead + // of updating the one already there. + if (text !== text.trim()) { + throw new ProfileConfigError( + `Refusing to write ${what}: leading or trailing whitespace is not preserved by the ~/.sim files, so it would not read back as written.` </file context>

cubic-dev-ai · 2026-08-26T21:06:52Z

+export function validateProfileName(name: string): void {
+  if (!PROFILE_NAME_PATTERN.test(name)) {
+    throw new ProfileConfigError(
+      `Invalid profile name "${name}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.`


P2: When a new profile name contains control characters, this error prints them verbatim and allows terminal/log output injection. Redact FORBIDDEN_IN_VALUE matches before interpolating the name.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/sim-cli/src/config/profile.ts, line 66: <comment>When a new profile name contains control characters, this error prints them verbatim and allows terminal/log output injection. Redact `FORBIDDEN_IN_VALUE` matches before interpolating the name.</comment> <file context> @@ -35,11 +37,34 @@ export const DEFAULT_ENDPOINT = 'https://www.sim.ai' +export function validateProfileName(name: string): void { + if (!PROFILE_NAME_PATTERN.test(name)) { + throw new ProfileConfigError( + `Invalid profile name "${name}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.` + ) } </file context>

Suggested change

`Invalid profile name "${name}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.`

`Invalid profile name "${name.replace(FORBIDDEN_IN_VALUE_GLOBAL, ' ')}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.`

cubic-dev-ai · 2026-08-26T21:06:52Z

+       * empty path parameter already are. Scoped to the query, because an empty
+       * body string is meaningful — it clears a description.
+       */
+      if (slot === 'query' && value === '') {


P2: When an optional numeric query filter is passed as an empty string, coerce turns it into 0 before this check, so the blank input bypasses the guard and reaches the API as a real filter. Normalize empty or null optional numeric inputs before coercion and omit them, while preserving an explicit 0.

(Based on your team's feedback about empty optional numeric parameters.)

View Feedback

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/sim-cli/src/runtime/request.ts, line 524: <comment>When an optional numeric query filter is passed as an empty string, `coerce` turns it into `0` before this check, so the blank input bypasses the guard and reaches the API as a real filter. Normalize empty or null optional numeric inputs before coercion and omit them, while preserving an explicit `0`. (Based on your team's feedback about empty optional numeric parameters.) </comment> <file context> @@ -473,6 +512,19 @@ export function buildRequest( + * empty path parameter already are. Scoped to the query, because an empty + * body string is meaningful — it clears a description. + */ + if (slot === 'query' && value === '') { + throw new SimApiError(`--${flagName} cannot be empty`, 0) + } </file context>

cubic-dev-ai · 2026-08-26T21:06:52Z

+ */
+export function collectWorkflowGraphIds(state: PreparedWorkflowState): WorkflowGraphIds {
+  return {
+    blockIds: Object.keys(state.blocks),


P2: When a block record key differs from block.id, this pre-check examines different IDs from the ones saveWorkflowToNormalizedTables inserts. Collect the IDs from the block values so conflict detection matches the write.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workflows/persistence/replace-normalized-state.ts, line 46: <comment>When a block record key differs from `block.id`, this pre-check examines different IDs from the ones `saveWorkflowToNormalizedTables` inserts. Collect the IDs from the block values so conflict detection matches the write.</comment> <file context> @@ -22,6 +23,149 @@ export class WorkflowStatePersistenceError extends Error { + */ +export function collectWorkflowGraphIds(state: PreparedWorkflowState): WorkflowGraphIds { + return { + blockIds: Object.keys(state.blocks), + edgeIds: state.edges.map((edge) => edge.id), + subflowIds: [...Object.keys(state.loops), ...Object.keys(state.parallels)], </file context>

Suggested change

blockIds: Object.keys(state.blocks),

blockIds: Object.values(state.blocks).map((block) => block.id),

cubic-dev-ai · 2026-08-26T21:06:52Z

+        db,
+        context.workflowId,
+        collectWorkflowGraphIds(
+          prepareWorkflowStateForPersistence({ blocks: graph.blocks, edges: graph.edges }).state


P2: When persistence preparation sanitizes an otherwise valid graph, dry-run omits that preparation warning although a committed apply returns it. Retain the prepared result and merge its warnings into the dry-run response.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workflows/application/apply-workflow-operations.ts, line 377: <comment>When persistence preparation sanitizes an otherwise valid graph, dry-run omits that preparation warning although a committed apply returns it. Retain the prepared result and merge its warnings into the dry-run response.</comment> <file context> @@ -358,6 +364,20 @@ export const applyWorkflowOperations = defineAuthorizedWorkflowUseCase({ + db, + context.workflowId, + collectWorkflowGraphIds( + prepareWorkflowStateForPersistence({ blocks: graph.blocks, edges: graph.edges }).state + ) + ) </file context>

cubic-dev-ai · 2026-08-26T21:06:52Z

 ```bash
-sim tables rows batch-delete tbl_123 --row row_1 row_2 --yes
-sim files delete file_123 --yes
+sim tables rows batch-delete tbl_9f3c1a05d4b7426e8c2f0917ab35de64 --row row_2f81c0a94db54e6f8a13c7e0526bd94a row_6b3e59d0af1c42d7b80e94f3a271c568 --yes


P2: The destructible-commands callout directly below the changed batch-delete example is now wrong and recommends a failing option. In this PR filtered batch-delete/batch-update no longer carry a default --limit of 100, and the NON_PAGINATED_LIMIT_HINT says '0 is not accepted', so the callout's 'carry the default --limit of 100... silently affects only the first 100. Pass --limit 0' instruction is false on both counts. Update the callout to say failing to pass a limit acts on every matching row and that omitting limit is the unbounded form, or remove the stale text.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/content/docs/en/cli/scripting.mdx, line 85: <comment>The destructible-commands callout directly below the changed batch-delete example is now wrong and recommends a failing option. In this PR filtered batch-delete/batch-update no longer carry a default --limit of 100, and the NON_PAGINATED_LIMIT_HINT says '0 is not accepted', so the callout's 'carry the default --limit of 100... silently affects only the first 100. Pass --limit 0' instruction is false on both counts. Update the callout to say failing to pass a limit acts on every matching row and that omitting limit is the unbounded form, or remove the stale text.</comment> <file context> @@ -68,8 +82,8 @@ Deletions require an explicit selector **and** `--yes`. There is no "delete ```bash -sim tables rows batch-delete tbl_123 --row row_1 row_2 --yes -sim files delete file_123 --yes +sim tables rows batch-delete tbl_9f3c1a05d4b7426e8c2f0917ab35de64 --row row_2f81c0a94db54e6f8a13c7e0526bd94a row_6b3e59d0af1c42d7b80e94f3a271c568 --yes +sim files delete wf_8Kd2NpVrY6zTfQa3XwBmS --yes

</file context>

</details>

cubic-dev-ai · 2026-08-26T21:06:52Z

+ * BCP-47 language tag: a 2-8 letter primary subtag followed by any number of
+ * alphanumeric subtags, e.g. `en`, `en-US`, `zh-Hant-TW`.
+ */
+const BCP47_LANGUAGE_TAG = /^[A-Za-z]{2,8}(?:-[A-Za-z0-9]{1,8})*$/


P3: This regex accepts invalid BCP-47 tags such as en-a, so langSchema does not enforce the contract stated in its error message. Validate with a BCP-47 parser or enforce the extension and private-use grammar before persisting analytics metadata.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/knowledge/upload-metadata.ts, line 27: <comment>This regex accepts invalid BCP-47 tags such as `en-a`, so `langSchema` does not enforce the contract stated in its error message. Validate with a BCP-47 parser or enforce the extension and private-use grammar before persisting analytics metadata.</comment> <file context> @@ -1,28 +1,90 @@ + * BCP-47 language tag: a 2-8 letter primary subtag followed by any number of + * alphanumeric subtags, e.g. `en`, `en-US`, `zh-Hant-TW`. + */ +const BCP47_LANGUAGE_TAG = /^[A-Za-z]{2,8}(?:-[A-Za-z0-9]{1,8})*$/ const knowledgeDocumentUploadTagSchema = z </file context>

cubic-dev-ai · 2026-08-26T21:06:52Z

+    expect(out).not.toContain('Manage workspaces')
+  })
+
+  it('refuses a command the group never had', async () => {


P3: The test 'refuses a command the group never had' probes chat-deployments get --help, but no chat-deployments group exists in the program (chat deployment operations map to workflows chat ...). The guard under test is the root dispatcher refusing chat-deployments, not a group that exists yet never had a get subcommand, so the test passes for a different reason than its name and comment claim. Rename it to reflect that it exercises a top-level group that does not exist, or point it at a real group that lacks the probed verb.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/sim-cli/src/program.test.ts, line 101: <comment>The test 'refuses a command the group never had' probes `chat-deployments get --help`, but no `chat-deployments` group exists in the program (chat deployment operations map to `workflows chat ...`). The guard under test is the root dispatcher refusing `chat-deployments`, not a group that exists yet never had a `get` subcommand, so the test passes for a different reason than its name and comment claim. Rename it to reflect that it exercises a top-level group that does not exist, or point it at a real group that lacks the probed verb.</comment> <file context> @@ -85,3 +85,50 @@ describe('the root version flag', () => { + expect(out).not.toContain('Manage workspaces') + }) + + it('refuses a command the group never had', async () => { + const { code } = await parse(['chat-deployments', 'get', '--help']) + </file context>

waleedlatif1 added 13 commits August 26, 2026 13:52

test(cli): use the package's own delay helper in the staging poll

cb40f3c

The audit bans a hand-rolled setTimeout promise. `sim-cli` does not depend on the shared utils package, and its own idiom is `node:timers/promises`.

waleedlatif1 requested a review from a team as a code owner August 26, 2026 20:54

vercel Bot deployed to Preview August 26, 2026 20:59 View deployment

cubic-dev-ai Bot reviewed Aug 26, 2026

View reviewed changes

	`Invalid profile name "${name}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.`
	`Invalid profile name "${name.replace(FORBIDDEN_IN_VALUE_GLOBAL, ' ')}". Use letters, numbers, dots, underscores, or hyphens, starting with a letter or number.`

	blockIds: Object.keys(state.blocks),
	blockIds: Object.values(state.blocks).map((block) => block.id),

Uh oh!

Conversation

waleedlatif1 commented Aug 26, 2026

Summary

Type of Change

Testing

Behaviour changes worth noting on release

Checklist

Uh oh!

vercel Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waleedlatif1 commented Aug 26, 2026

Uh oh!

greptile-apps Bot commented Aug 26, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Aug 26, 2026 •

edited

Loading