fix(docs): stop publishing unsettable params, fix comment blanking - #7169
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR updates documentation generation to omit hidden, unsettable tool parameters while retaining hidden parameters populated through exposed fields or parameter mappers. It also improves comment blanking during source scanning, permits numeric URL path segments, and regenerates affected integration metadata and documentation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/generate-docs.ts | Updates source scanning and visibility-aware parameter selection so generated docs describe exposed inputs rather than every tool parameter. |
| scripts/generate-docs.test.ts | Adds generator coverage using the established and resolvable relative-import convention for sibling tests under scripts/. |
| apps/sim/tools/url-path.ts | Extends guarded path-segment conversion to number and bigint inputs while retaining validation and encoding behavior. |
| apps/sim/tools/url-path.test.ts | Exercises path-segment validation, encoding, numeric inputs, Unicode errors, and size boundaries. |
| apps/sim/tools/generated/tool-metadata.ts | Regenerates tool metadata to reflect updated parameter visibility and tool definitions. |
| apps/sim/tools/generated/tool-outputs.ts | Regenerates tool output contracts corresponding to the updated integration definitions. |
| apps/sim/lib/oauth/utils.ts | Adjusts OAuth utility behavior with accompanying test coverage. |
| apps/sim/tools/daytona/utils.ts | Centralizes Daytona sandbox identifier path handling through the guarded URL-segment utility. |
| apps/docs/content/docs/en/integrations/google_drive.mdx | Regenerates Google Drive action documentation to remove unsettable inputs and align pagination descriptions with exposed behavior. |
| apps/docs/content/docs/en/integrations/firecrawl.mdx | Regenerates Firecrawl documentation to omit hidden inputs and clarify conditional outputs. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tool parameter metadata] --> B{Visible parameter?}
B -->|Yes| E[Publish input row]
B -->|No| C{Exposed by subBlock or mapper?}
C -->|Yes| E
C -->|No| F[Omit input row]
G[Tool source] --> H[Blank strings and comments]
H --> I[Scan subBlocks and parameter mappers]
I --> C
Reviews (13): Last reviewed commit: "fix(docs): cite the real Mistral options..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 30 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 39 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
generate-docs never read visibility, so every tool param appeared in the public Input table -- including params marked visibility:'hidden', which are shown to neither the user nor the LLM. Several are credential-shaped (idToken, instanceUrl, apiToken, cloudId), so the docs told integrators they could set values they cannot reach. A hidden param is now dropped only when the block declares no subBlock for it, matched on id or canonicalParamId -- a param can be hidden on the tool because the block injects it while the block still renders it as a required field the user types. blankStringsAndComments kept the first and last character of every match. That is right for a quoted string, where both are delimiters, but for a '//' comment the last character is arbitrary source text, so a commented-out '// options: [' left an unbalanced bracket that derailed the subBlock scan. Parsing now throws rather than silently reporting that a block exposes nothing, since that fallback was the destructive one. Also corrects the LinkedIn w_member_social consent-screen description, which read 'Access LinkedIn profile' for a scope that posts on the user's behalf.
The carve-out only recognized an identity match between a subBlock id and a tool param, so a block that renames or assembles the value in tools.config.params was invisible to it -- and the row was dropped even though the user types it. Cal.com's attendee (required) is assembled from attendeeName/attendeeEmail/ attendeeTimeZone; JSM's workspaceId comes from assetWorkspaceId; Textract writes parameters.file from a field whose canonicalParamId is 'document', which left the Mistral PDF Parser documenting zero inputs. Collects params written by any accumulator identifier, not just 'result', since the two real mappers use different names. Object keys are collected without proving they are top-level, so a nested key can produce a false keep -- one hard-to-set row is better than hiding a required input.
toGuardedString coerced with String(value), so an object reached the wire as %5Bobject%20Object%5D and a boolean as 'true' -- a doomed request instead of a clean error, on 44 live call sites. Accepts string, bigint, and finite non-exponential numbers; everything else throws a named error. Rejects a number whose decimal text is a rewrite rather than the caller's value: 1e21 stringifies to '1e+21', and an integer past 2^53 has already lost digits. A snowflake cannot be repaired here at all -- JSON.parse destroys it before this runs -- so the doc now says it must arrive as a string, and cites Box folderId (root = 0) instead. Corrects the claim that the parser removes only an exact '.' or '..'; the spec defines 11 removable spellings. The guards are sufficient because encodeURIComponent escapes '%', not because the others cannot occur.
Nothing guarded the consent-screen text: utils.test.ts covered only the Bitbucket and Reddit overrides, and the modal test stubs getScopeDescription to identity, so a regression to a read-only label for a posting scope would pass silently.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 44 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
extractUserSettableParamIds answered [] for a subBlocks array whose every element spreads a fields array it cannot follow (NotionV2Block's `[...NotionBlock.subBlocks, ...getTrigger(x).subBlocks]`). [] asserts the block supplies nothing, so the hidden-param filter stripped every hidden param from every tool the block owns - silently, with no parseError and so no warning. That is the exact false-drop the null UNKNOWN state exists to prevent. Return null in that case and propagate it: extractBlockSuppliedParamIds no longer folds it into [], and the block pass no longer collapses it with `supplied.ids ?? []`. A config-level spread base still narrows the filter to its readable fields plus the mapper's renames; with no base the filter is switched off. An array with at least one inline id, a genuinely empty array, and the existing throw/warn paths are unchanged - all 8 warned blocks warn identically and every generated page is byte-identical. Also pin the hidden-param filter on extractToolInfo's source-parsing path, which had no coverage at all: deleting it outright left the suite green.
…oxId start/stop/delete echo sandboxId back as the output id when the API returns no body, via params.sandboxId.trim() inside transformResponse - after the request has already gone out. sandboxId is declared type: 'string' but arrives unvalidated, and now that safeUrlPathSegment accepts a numeric id a number builds a URL, sends the DELETE/START/STOP, and only then throws an unnamed TypeError. Both the old and new behaviour fail, so this is not a regression of a working workflow, but for delete_sandbox the side effect is irreversible and the caller cannot tell what happened. Fixed with a shared resolveSandboxId in utils.ts rather than a coercion at each of the three sites: utils.ts already owns every sandbox-id helper, the three tools already import from it, and the reasoning belongs in one place. The encoded value cannot be reused - it is percent-encoded and would be wrong as an output id. Behaviour for a string is unchanged.
…aviour
The module TSDoc said the number/bigint widening fixed 'a regression for the
call sites whose pre-guard form was a bare ${params.id} template that
stringified a number fine'. It did not. Every pre-guard form in a422990
used .trim() (`/v13/deployments/${params.deploymentId.trim()}`,
`sandboxId?.trim()`), so a numeric id threw there too - no importer has ever
accepted one. The cited examples were also wrong: only Vercel and Daytona
import this module, and neither Box nor X does.
Replaced with the real motivation - params are declared type: 'string' but
nothing enforces it before the guard, and the old coercion-to-'' turned a
supplied numeric id into a misleading 'is required'. Two test comments made
the same claim ('still stringifies', 'replaced bare ${params.id} templates')
and are corrected; no assertion is weakened.
mistral_parse: the PR removed the includeImageBase64 input row but left the image_base64 output citing include_image_base64=true, so the page referenced an input it no longer documents. includeImageBase64 is visibility: 'hidden' with no subBlock, mapper or canonicalParamId, so it is annotated the same way Pulse's html and figures were. The sibling rows are a stronger defect: table_format, extract_header and extract_footer appear nowhere in the repo - not as tool params, not in the request body parser.ts builds - so tables/header/footer cited options Sim never sends. Worded accordingly rather than as hidden inputs. pulse structured_output cited 'if schema was provided', but there is no schema or structuredOutput param in the tool, in pulseParseInputSchema, or in the outgoing body, so the field is always null. No output field is deleted - removing one changes the block's output schema and could break saved workflow references.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
Formatting only — regenerating both artifacts produces a byte-identical tree, so no description text changed.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 53 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
…oter The previous wording was self-contradictory on tables: it described placeholder-referenced table objects and then asserted the list is empty. Mistral's OCR API does expose table_format, extract_header and extract_footer. table_format defaults to inline markdown, so the separate tables list stays empty; extract_header and extract_footer default to false, so neither field is returned. Sim sets none of the three. Name the option and its default in each description instead of asserting an outcome the request body alone does not establish.
|
@cubic-dev-ai review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 53 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Re-trigger cubic
1 participant
Summary
generate-docsnever readvisibility, so every tool param was published in the public Input table — includingvisibility: 'hidden'params, which are shown to neither the user nor the LLM. Several are credential-shaped (idToken,instanceUrl,apiToken,cloudId), so the docs advertised values integrators cannot set.id/canonicalParamIdmatches, or atools.config.paramsmapper that writes it. Cal.com's requiredattendeeis assembled from three fields, JSM'sworkspaceIdcomes fromassetWorkspaceId, and Textract writesparameters.filefrom a field whosecanonicalParamIdisdocument— none of which an identity match sees. The rule is biased toward keeping: measured across every block, it retains 25 mapper-only rows and all 25 are genuinely mapper-written.blankStringsAndCommentskept the first and last character of every match. Correct for a quoted string, where both are delimiters; wrong for a//comment, whose last character is arbitrary source. A commented-out// options: [left an unbalanced bracket that derailed the subBlock scan, and the failure was silent — indistinguishable from "this block exposes nothing", the destructive fallback.safeUrlPathSegmentnow acceptsnumberandbigint.On
apps/sim/tools/url-path.tsThis file is already on staging with 44 importers (Vercel ×43, Daytona ×1) — an earlier draft of this description wrongly called it a new module with none, which is the sort of claim that would let a reviewer skip auditing it.
The change is strictly more permissive. Staging's
toGuardedStringwastypeof value === 'string' ? value.trim() : '', so it already rejected every non-string; it just reported "is required" for a value the caller did supply. A numeric id from an LLM — Box's root folder is0— was one of those. A differential over 87 real call-site values (project/deployment/team/alias ids, hostnames incl. punycode, UUIDs, every URL-reserved char, unicode, all 12 dot-segment spellings) gives 87 identical, 0 differing.An earlier revision of this branch added an unused
safeUrlPath/safeOpaqueUrlSegment/SafeUrlPathOptionsAPI and then removed it again, so that churn is not visible in the diff againststaging— do not go looking for a deletion. Against the merge base this file is +156/−27. Those three names have zero references anywhere in the monorepo; the traversal call sites that need them will land separately.Type of Change
Testing
check:audits36/36 ·docs:check·test:generators·type-check·turbo run lint:check— all exit 0. Fullapps/sim/toolssuite green.Generator verified idempotent across consecutive runs; the pages with no tool-source change regenerate byte-identically.
Audited adversarially beyond the review bots, which found — and this PR now fixes — three reader-facing docs regressions (a Dataverse description mandating a removed parameter, a Vanta row whose only stated purpose was a removed sibling, five Google Drive actions documenting a pagination loop the reader cannot close), a lone-surrogate
URIErrorthat lost the param name, and a false "too large" message on values that round-trip exactly.Net docs effect: ~268 unsettable rows removed, 19 wrongly-removed rows restored, no row removed that a block exposes to the user.
Checklist