fix: six pre-existing integration defects surfaced by the docs audit (#7195)

* fix(docs): pin the generator's sort locale to en-US

localeCompare with no locale argument uses the runtime default, which
varies with LANG and the ICU build. Against the real 254 catalog names,
tr-TR reorders 141 positions, et-EE 45, cs-CZ 2 and lt-LT diverges at
index 40 — so a contributor on any of those regenerates a different
integrations.json and fails CI with no obvious cause.

Pins en-US at all four sort sites. The committed artifacts are unchanged:
regenerating before and after leaves the tree byte-identical.

Adds a guard test asserting the committed catalog matches an explicit
en-US ordering, plus one that fails if an unpinned localeCompare returns.

* fix(vanta): remove the MIME Type field whose value was always discarded

The block rendered an advanced MIME Type input and forwarded it as the
tool's mimeType, but the upload path never reads it: file-input.ts sets
`resolved.contentType || userFile.type || input.mimeType || …`, and every
return path of downloadServableFileFromStorage yields a non-empty content
type — a literal, getMimeTypeFromExtension's GENERIC_MIME_TYPE fallback,
or resolveServableDocBytes' constants and getContentType fallback. The
placeholder claimed it was 'used when the file has no type of its own',
which never happens.

Removes the subBlock, its mapper write and its inputs entry, and marks the
tool param hidden so it is no longer advertised to the model on a path
where it cannot take effect. The param itself stays, because the base64
branch still reads it.

Letting a typed value win instead was rejected: for a compiled artifact
the storage-resolved type is the only one matching the bytes actually
sent, so an override would break the case the resolver exists to fix.

* fix(github): resolve the PR head SHA for file comments

github_comment sent commit_id as undefined for every file comment: the
param was hidden with no subBlock and no mapper write, so GitHub — which
marks commit_id required on POST /pulls/{n}/comments — answered 422 on a
path the commentType dropdown exposes.

When commitId is absent the tool now fetches the pull request first and
uses head.sha, mirroring how Jira resolves cloudId from domain.

Also removes the position param, which GitHub marks deprecated ("Use
line instead"); line is already a real subBlock.

* fix(google-drive): expose the page token so pagination is reachable

list, search, list_comments, list_permissions and list_revisions each
declared a hidden pageToken and forwarded it to Google, but the block had
no subBlock of that name and never has — so every list was capped at one
page and the nextPageToken output had nowhere to go.

Adds a per-operation Page Token field mirroring the block's existing
per-operation pageSize fields, collapses them onto the canonical
pageToken in the params mapper, and flips the tool param to user-only so
it is documented and settable.

* fix(confluence): stop documenting a cloudId users cannot supply

All 46 Confluence tools marked cloudId 'user-only', publishing it on 46
doc rows, but the block has no cloudId subBlock so no user could ever
fill it. createConfluenceClient already resolves the cloud id from the
domain through the shared Atlassian resolver, exactly as Jira does, and
Jira marks the same param hidden.

Marks cloudId hidden to match. domain stays user-only and settable — it
is now the only user-provided param on every Confluence tool.

* fix(docs): teach the source scanner about regex literals

blankStringsAndComments was a single regex with no concept of a regex
literal, so two shapes silently truncated a block's subBlock list:

  /don't/   the apostrophe opened a phantom string that swallowed the
            following entries
  /[}]/     the brace in the character class closed the enclosing object

Both returned a short list with no warning — a confident wrong answer,
which for the hidden-param filter means silently deleting a user-settable
row. No block file uses a regex literal today, so this was latent.

Replaces the regex with a linear scanner that distinguishes a regex
literal from a division by the previous significant character, blanks
regex bodies whole (their last character is arbitrary source, same reason
comments are blanked whole), and tracks ${} nesting so a backtick inside
a template expression cannot end the template early.

The scanner now returns null when it ends inside an unterminated
construct. All three call sites treat that as UNKNOWN rather than
guessing, so the filter switches off instead of stripping.

Generated artifacts are byte-identical and the warning count is unchanged.

* fix(github): gate the commit lookup on path, coerce line, name the failing field

Three corrections to the file-comment fix, from a validation sweep.

needsCommitLookup did not check `path`, and ran before the `path` branch in
request.url. A file comment with an empty File Path — reachable, since path
is not required on the block — went GET /pulls/{n} then POST /comments with
path undefined, a 422. On staging it posted to /pulls/{n}/reviews, which
GitHub documents as creating a pending review and where commit_id is
optional. The lookup is now gated on path, so only a request headed for
/comments triggers it.

The block has no tools.config.params, so `line` reached the tool as the
string the short-input produced while GitHub types it as an integer — file
comments would still have 422'd, one API call later. Coerced in
request.body, which runs at execution; anything non-finite is omitted
rather than sent as NaN.

readGitHubErrorMessage returned only the top-level message, so a 422 read
"Validation Failed" with no indication of which field was rejected. The
errors[] detail is now appended. Responses without errors[] are unchanged.

* test(github): cover the comment routing cases the gate changed

Adds the file_comment-without-a-path case (which the commit lookup now skips),
the untouched-block default where commentType is unset, a pr_comment carrying a
path, and the line coercion on both the direct and resolved-commit paths.

* fix(google-drive): let an agent feed the page token back in

A page token is an opaque continuation value produced by a previous tool
response, not an account-specific id the user has to supply, so 'user-only'
hid it from agent blocks: they saw nextPageToken in the result and could not
send it back, silently reporting page one as the whole answer. Every other
pagination token in the tool set is 'user-or-llm'.

Also covers the case the mapper guard actually defends — a per-operation page
token surviving an operation switch, which reaches inputs because
shouldSerializeSubBlock skips condition evaluation for advanced fields.

* docs(generator): name the load-bearing newline rule and report an unscannable source

- Record why '\\n' is in REGEX_ALLOWED_AFTER: formatters emit a binary '/' at
  end-of-line, so every line-leading '/' in blocks/*.ts is a real regex, including
  the ones in table.ts and table_v2.ts. Removing it silently mis-scans those two.
- Split a scanner failure out of the spread-only 'ids: null' case. Both scans come
  back empty for the same reason when blankStringsAndComments bails, so the mapper's
  renames were dropped with no warning; it now reports a parseError and warns. The
  spread case is unchanged, and its TSDoc no longer claims a cause that was false.
- Route every catalog sort through an exported compareCatalogNames so the ordering
  test exercises the generator's comparator instead of re-deriving it, and match
  localeCompare arguments whole so localeCompare() and a variable locale are caught.
- Note that downloadServableFileFromStorage guarantees a non-empty content type, so
  the Vanta mimeType fallback chain reads as deliberately defensive.

Artifacts regenerate byte-identically and the generator warning set is unchanged.

* fix(vanta): declare the removed uploadMimeType subblock as dropped

check-block-registry fails a PR that deletes a subblock id without a
migration entry, because a deployed workflow can still hold a value under
that id. The serializer already discards an orphan silently, but the repo's
contract is that the removal is declared rather than inferred.

Uses the _removed_ form, scoped to upload_document_file: the value has no
replacement field to move to.

* fix(github): run the two-phase PR comment on the secure transport

The file-comment flow posted its comment from `transformResponse` with a bare
global `fetch`, so that request carried no abort signal, no response ceiling and
no DNS/SSRF validation — cancelling a workflow still left the comment posted.

`transformResponse` cannot receive the signal; `directExecution` can. Both tools
now run the lookup and the POST through a new `secureGitHubRequest`, mirroring
`secureBitbucketRead`, with the signal forwarded to each. Routing, line coercion
and the `errors[]` detail are unchanged, and a failed response still throws an
error carrying `status`/`statusText`/`data` as the transport does.

Request bodies and the comment payload are explicitly typed instead of
`Record<string, any>`.

* fix(vanta): keep mimeType an ordinary upload parameter

`mimeType` was marked hidden, but `visibility: 'hidden'` is reserved for
system-injected params such as OAuth tokens. It also left the base64 upload path
with no way to set a content type, since `fileContent` is hidden too.

* fix(generator): lex regex-in-keyword-position and template interpolation

The scanner chose regex-vs-division from the previous significant character, so
a regex in operand position (`return /x/`, `typeof /x/`, `case /x/`, ...) lexed
as a division off the keyword's last letter and its body stayed in the
structural view; `azure_devops.ts` is inert today only because the braces in its
`return /^\d{4}-\d{2}-\d{2}$/` happen to balance.

The `${}` depth counter was also not string-aware, so a brace inside a quoted
expression miscounted, the closing backtick was lost and the block was reported
unreadable — which silently stops filtering resolver-derived hidden params.

Both scans now run on one set of lexer primitives: `${}` expressions are lexed
with the same string, comment, regex and template handling as top-level code.
Regenerated docs, tool metadata and the integration catalog are byte-identical
and the generator's warning count is unchanged.

* fix(github): stop forwarding the GitHub token across a redirect origin

secureGitHubRequest passed no redirectPolicy, and the transport only strips
credential headers when one is present, so an api.github.com redirect to another
origin carried the workspace's Authorization: Bearer header to the new host.

Adopts the standard policy already used by the internal Google Drive client.
stripAuthOnRedirect stays off: GitHub redirects same-origin for legitimate
reasons (a renamed repository answers 301), and dropping auth there would turn
a working call into a 401.

* fix(github): reject a fractional comment line instead of truncating it

toLineNumber ran Math.trunc, so line 3.9 posted the review comment on line 3 —
a silent change to what the caller asked for, on a field where landing on the
wrong line of the diff is invisible until someone reads the comment. A
non-integer now fails with a message naming the field, matching how a missing
head commit SHA fails on this path. Blank and unparseable input is still
omitted: line is optional and nothing usable was supplied.

* fix(github): select the comment endpoint by comment type, not by path

The endpoint was chosen by the presence of path, while the body was chosen by
commentType, so a pr_comment naming a file posted a review body to
POST /pulls/{n}/comments. GitHub documents body, commit_id and path as required
there, so that request can only ever 422 — it has been broken since before this
PR. The endpoint now follows the comment type: only a file comment carrying a
path uses /comments, everything else stays on /reviews. The test that codified
the broken routing is corrected, and the full type/path matrix is pinned.

* test(confluence): drop the cloudId visibility invariant test

Removed at request. The visibility change itself is unaffected; it loses
only the guard that would have caught a future edit reverting one of the
46 files.

* fix(github): send an explicit User-Agent and stop downgrading a redirected comment POST

`secureGitHubRequest` powers the GitHub comment tool's `directExecution`
path, which bypasses the declarative transport. Two behaviors the transport
provided did not survive the move.

User-Agent: the transport sets `User-Agent: Sim` on every request it formats
(`request-transport.ts`), and `secureFetchWithPinnedIP` adds none of its own —
it builds the request with raw `node:https` and passes headers through
verbatim. Production runs on Bun, whose `node:http` shim injects
`user-agent: Bun/x.y.z`, so GitHub does not reject these calls today; the
defect is that Sim's deliberate attribution is silently replaced by a runtime
version string, and that the tool depends on an undocumented runtime behavior
that does not hold under Node, where GitHub answers 403 "Request forbidden by
administrative rules". Set in the helper rather than in the tool's header map
so every future caller inherits it; a caller-supplied value still wins.

Redirect method: the policy was `mode: 'standard'`, under which
`resolveRedirectHop` rewrites a 301/302'd POST to a bodyless GET regardless of
origin. GitHub answers 301 within api.github.com for a renamed repository, so
commenting on a PR there would GET `/pulls/{n}/comments`, receive a JSON array,
fail the payload shape check, and report success with no comment created.
`legacy` keeps the method and body across that hop. Cross-origin credential
stripping is unaffected — the guarded follower strips Authorization,
Proxy-Authorization and Cookie whenever `sendCredentialsOnCrossOriginRedirect`
is false, in either mode.

* fix(vanta): drop the dead whenOperation from the removed-subblock entry

migrateBlockSubblockIds handles a _removed_ target before it consults
whenOperation, so the scope was never applied. Mine was the only _removed_
entry in the file carrying one.

Unconditional deletion is also what this case wants. Subblock values are
keyed by id and are not cleared when the operation changes, so a user who
filled the MIME field and then switched the block to another operation has
the value stored under that operation; a scoped delete would strand it
permanently. The field no longer exists for any operation, so it should go
regardless of the stored operation.

* fix(docs-gen): close a regex-vs-division gap and make three guards testable

`REGEX_ALLOWED_AFTER` was missing `'/'`, so a regex directly after a division
operator lexed as a second division: in `x / y / /[}]/` the character class was
left in the structural view and its `}` closed the enclosing object early,
truncating the block's subBlock ids with no warning. Add `'/'`, and guard the
`'+'`/`'-'` entries with a `++`/`--` lookbehind so a postfix update still reads
as a value and `i++ / 2` stays a division rather than a phantom regex that runs
to end-of-input and reports the block unreadable.

The `.`/`#` property guard and the `'\n'` entry both survived their mutants.
`counts.in / 2, m: preturn / 2` is self-cancelling — the mis-lexed regex closes
on the second slash and blanks nothing structural — so the fixtures now leave an
odd number of slashes on the line. The `'\n'` entry had no coverage at all: its
fixture is now the wrapped `.match(` newline `/re/` shape that `blocks/table.ts`
and `blocks/table_v2.ts` produce, which is what that entry (not the preceding
`(`, which the newline overwrites) actually decides. Its comment claimed a count
of line-leading regexes that drifts with the sources; restate it without one.

Drop the two locale tests that could not fail. CI runs under an `en-US` default,
where an unpinned `localeCompare` returns exactly what the pinned one does, so no
behavioural comparison discriminates; and asserting the committed
`integrations.json` against the comparator that produced it agrees by
construction. The source grep for a literal locale argument is the real guard.

Generated output is byte-identical and the extraction differential over
`apps/sim/blocks/blocks/` is empty.
This commit is contained in:
Waleed
2026-08-27 23:31:53 -07:00
committed by GitHub
parent 88a9671674
commit 46703e395f
70 changed files with 1785 additions and 191 deletions
@@ -43,7 +43,6 @@ Retrieve content from Confluence pages using the Confluence API.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | Confluence page ID to retrieve \(numeric ID from page URL or API\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -81,7 +80,6 @@ Update a Confluence page using the Confluence API.
| `pageId` | string | Yes | Confluence page ID to update \(numeric ID from page URL or API\) |
| `title` | string | No | New title for the page |
| `content` | string | No | New content for the page in Confluence storage format |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -124,7 +122,6 @@ Create a new page in a Confluence space.
| `title` | string | Yes | Title of the new page |
| `content` | string | Yes | Page content in Confluence storage format \(HTML\) |
| `parentId` | string | No | Parent page ID if creating a child page |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -165,7 +162,6 @@ Delete a Confluence page. By default moves to trash; use purge=true to permanent
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | Confluence page ID to delete |
| `purge` | boolean | No | If true, permanently deletes the page instead of moving to trash \(default: false\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -189,7 +185,6 @@ List all pages within a specific Confluence space. Supports pagination and filte
| `status` | string | No | Filter pages by status: current, archived, trashed, or draft |
| `bodyFormat` | string | No | Format for page body content: storage, atlas_doc_format, or view. If not specified, body is not included. |
| `cursor` | string | No | Pagination cursor from previous response to get the next page of results |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -235,7 +230,6 @@ Get all child pages of a specific Confluence page. Useful for navigating page hi
| `pageId` | string | Yes | The ID of the parent page to get children from |
| `limit` | number | No | Maximum number of child pages to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response to get the next page of results |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -263,7 +257,6 @@ Get the ancestor (parent) pages of a specific Confluence page. Returns the full
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | The ID of the page to get ancestors for |
| `limit` | number | No | Maximum number of ancestors to return \(default: 25, max: 250\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -290,7 +283,6 @@ List all versions (revision history) of a Confluence page.
| `pageId` | string | Yes | The ID of the page to get versions for |
| `limit` | number | No | Maximum number of versions to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -317,7 +309,6 @@ Get details about a specific version of a Confluence page.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | The ID of the page |
| `versionNumber` | number | Yes | The version number to retrieve \(e.g., 1, 2, 3\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -353,7 +344,6 @@ List all custom properties (metadata) attached to a Confluence page.
| `pageId` | string | Yes | The ID of the page to list properties from |
| `limit` | number | No | Maximum number of properties to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -385,7 +375,6 @@ Create a new custom property (metadata) on a Confluence page.
| `pageId` | string | Yes | The ID of the page to add the property to |
| `key` | string | Yes | The key/name for the property |
| `value` | json | Yes | The value for the property \(can be any JSON value\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -414,7 +403,6 @@ Delete a content property from a Confluence page by its property ID.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | The ID of the page containing the property |
| `propertyId` | string | Yes | The ID of the property to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -436,7 +424,6 @@ Search for content across Confluence pages, blog posts, and other content.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `query` | string | Yes | Search query string |
| `limit` | number | No | Maximum number of results to return \(default: 25\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -471,7 +458,6 @@ Search for content within a specific Confluence space. Optionally filter by text
| `query` | string | No | Text search query. If not provided, returns all content in the space. |
| `contentType` | string | No | Filter by content type: page, blogpost, attachment, or comment |
| `limit` | number | No | Maximum number of results to return \(default: 25, max: 250\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -508,7 +494,6 @@ List all blog posts across all accessible Confluence spaces.
| `status` | string | No | Filter by status: current, archived, trashed, or draft |
| `sort` | string | No | Sort order: created-date, -created-date, modified-date, -modified-date, title, -title |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -542,7 +527,6 @@ Get a specific Confluence blog post by ID, including its content.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `blogPostId` | string | Yes | The ID of the blog post to retrieve |
| `bodyFormat` | string | No | Format for blog post body: storage, atlas_doc_format, or view |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -586,7 +570,6 @@ Create a new blog post in a Confluence space.
| `title` | string | Yes | Title of the blog post |
| `content` | string | Yes | Blog post content in Confluence storage format \(HTML\) |
| `status` | string | No | Blog post status: current \(default\) or draft |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -630,7 +613,6 @@ List all blog posts within a specific Confluence space.
| `status` | string | No | Filter by status: current, archived, trashed, or draft |
| `bodyFormat` | string | No | Format for blog post body: storage, atlas_doc_format, or view |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -674,7 +656,6 @@ Add a comment to a Confluence page.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | Confluence page ID to comment on |
| `comment` | string | Yes | Comment text in Confluence storage format |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -697,7 +678,6 @@ List all comments on a Confluence page.
| `limit` | number | No | Maximum number of comments to return \(default: 25\) |
| `bodyFormat` | string | No | Format for the comment body: storage, atlas_doc_format, view, or export_view \(default: storage\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -735,7 +715,6 @@ Update an existing comment on a Confluence page.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `commentId` | string | Yes | Confluence comment ID to update |
| `comment` | string | Yes | Updated comment text in Confluence storage format |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -755,7 +734,6 @@ Delete a comment from a Confluence page.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `commentId` | string | Yes | Confluence comment ID to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -778,7 +756,6 @@ Upload a file as an attachment to a Confluence page.
| `file` | file | Yes | The file to upload as an attachment |
| `fileName` | string | No | Optional custom file name for the attachment |
| `comment` | string | No | Optional comment to add to the attachment |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -804,7 +781,6 @@ List all attachments on a Confluence page.
| `pageId` | string | Yes | Confluence page ID to list attachments from |
| `limit` | number | No | Maximum number of attachments to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -840,7 +816,6 @@ Delete an attachment from a Confluence page (moves to trash).
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `attachmentId` | string | Yes | Confluence attachment ID to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -862,7 +837,6 @@ List all labels on a Confluence page.
| `pageId` | string | Yes | Confluence page ID to list labels from |
| `limit` | number | No | Maximum number of labels to return \(default: 25, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -887,7 +861,6 @@ Add a label to a Confluence page for organization and categorization.
| `pageId` | string | Yes | Confluence page ID to add the label to |
| `labelName` | string | Yes | Name of the label to add |
| `prefix` | string | No | Label prefix: global \(default\), my, team, or system |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -909,7 +882,6 @@ Remove a label from a Confluence page.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `pageId` | string | Yes | Confluence page ID to remove the label from |
| `labelName` | string | Yes | Name of the label to remove |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -932,7 +904,6 @@ Retrieve all pages that have a specific label applied.
| `labelId` | string | Yes | The ID of the label to get pages for |
| `limit` | number | No | Maximum number of pages to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -968,7 +939,6 @@ List all labels associated with a Confluence space.
| `spaceId` | string | Yes | The ID of the Confluence space to list labels from |
| `limit` | number | No | Maximum number of labels to return \(default: 25, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -992,7 +962,6 @@ Get details about a specific Confluence space.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `spaceId` | string | Yes | Confluence space ID to retrieve |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1024,7 +993,6 @@ Create a new Confluence space.
| `name` | string | Yes | Name for the new space |
| `key` | string | Yes | Unique key for the space \(uppercase, no spaces\) |
| `description` | string | No | Description for the new space |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1054,7 +1022,6 @@ Update a Confluence space name or description.
| `spaceId` | string | Yes | ID of the space to update |
| `name` | string | No | New name for the space |
| `description` | string | No | New description for the space |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1081,7 +1048,6 @@ Delete a Confluence space.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `spaceId` | string | Yes | ID of the space to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1104,7 +1070,6 @@ List all Confluence spaces accessible to the user.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `limit` | number | No | Maximum number of spaces to return \(default: 25, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1137,7 +1102,6 @@ List properties on a Confluence space.
| `spaceId` | string | Yes | Space ID to list properties for |
| `limit` | number | No | Maximum number of properties to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1163,7 +1127,6 @@ Create a property on a Confluence space.
| `spaceId` | string | Yes | Space ID to create the property on |
| `key` | string | Yes | Property key/name |
| `value` | json | No | Property value \(JSON\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1186,7 +1149,6 @@ Delete a property from a Confluence space.
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `spaceId` | string | Yes | Space ID the property belongs to |
| `propertyId` | string | Yes | Property ID to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1209,7 +1171,6 @@ List permissions for a Confluence space.
| `spaceId` | string | Yes | Space ID to list permissions for |
| `limit` | number | No | Maximum number of permissions to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1239,7 +1200,6 @@ Get all descendants of a Confluence page recursively.
| `pageId` | string | Yes | Page ID to get descendants for |
| `limit` | number | No | Maximum number of descendants to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1273,7 +1233,6 @@ List inline tasks from Confluence. Optionally filter by page, space, assignee, o
| `status` | string | No | Filter tasks by status \(complete or incomplete\) |
| `limit` | number | No | Maximum number of tasks to return \(default: 50, max: 250\) |
| `cursor` | string | No | Pagination cursor from previous response |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1307,7 +1266,6 @@ Get a specific Confluence inline task by ID.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `taskId` | string | Yes | The ID of the task to retrieve |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1340,7 +1298,6 @@ Update the status of a Confluence inline task (complete or incomplete).
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `taskId` | string | Yes | The ID of the task to update |
| `status` | string | Yes | New status for the task \(complete or incomplete\) |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1374,7 +1331,6 @@ Update an existing Confluence blog post title and/or content.
| `blogPostId` | string | Yes | The ID of the blog post to update |
| `title` | string | No | New title for the blog post |
| `content` | string | No | New content for the blog post in storage format |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1398,7 +1354,6 @@ Delete a Confluence blog post.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `blogPostId` | string | Yes | The ID of the blog post to delete |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -1418,7 +1373,6 @@ Get display name and profile info for a Confluence user by account ID.
| --------- | ---- | -------- | ----------- |
| `domain` | string | Yes | Your Confluence domain \(e.g., yourcompany.atlassian.net\) |
| `accountId` | string | Yes | The Atlassian account ID of the user to look up |
| `cloudId` | string | No | Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain. |
#### Output
@@ -46,6 +46,7 @@ List files and folders in Google Drive with complete metadata
| `folderId` | string | No | The ID of the folder to list files from \(internal use\) |
| `query` | string | No | Search term to filter files by name \(e.g. "budget" finds files with "budget" in the name\). Do NOT use Google Drive query syntax here - just provide a plain search term. |
| `pageSize` | number | No | The maximum number of files to return \(default: 100\) |
| `pageToken` | string | No | The page token to use for pagination |
#### Output
@@ -484,6 +485,7 @@ Search for files in Google Drive using advanced query syntax (e.g., fullText con
| --------- | ---- | -------- | ----------- |
| `query` | string | Yes | Google Drive query string using advanced search syntax \(e.g., "fullText contains 'budget'", "mimeType = 'application/pdf'", "modifiedTime &gt; '2024-01-01'"\) |
| `pageSize` | number | No | Maximum number of files to return \(default: 100\) |
| `pageToken` | string | No | Token for fetching the next page of results |
#### Output
@@ -671,6 +673,7 @@ List all permissions (who has access) for a file in Google Drive
| Parameter | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `fileId` | string | Yes | The ID of the file to list permissions for |
| `pageToken` | string | No | The page token to use for pagination |
#### Output
@@ -720,6 +723,7 @@ List the revision history of a file in Google Drive
| --------- | ---- | -------- | ----------- |
| `fileId` | string | Yes | The ID of the file to list revisions for |
| `pageSize` | number | No | Maximum number of revisions to return \(1-1000, default 200\) |
| `pageToken` | string | No | The page token to use for pagination |
#### Output
@@ -779,6 +783,7 @@ List comments on a file in Google Drive
| `includeDeleted` | boolean | No | Whether to include deleted comments \(their content is stripped\) |
| `pageSize` | number | No | Maximum number of comments to return \(1-100, default 20\) |
| `startModifiedTime` | string | No | Only return comments modified after this RFC 3339 timestamp |
| `pageToken` | string | No | The page token to use for pagination |
#### Output
@@ -331,7 +331,7 @@ Upload an evidence file to a Vanta document. Requires credentials with the vanta
| `documentId` | string | Yes | Unique ID of the document to attach the file to |
| `file` | file | No | The evidence file to upload |
| `fileName` | string | No | Optional file name override |
| `mimeType` | string | No | MIME type of the file \(e.g., application/pdf\). Used only for base64 uploads; ignored for a file from the File input, whose content type is always resolved from storage. |
| `mimeType` | string | No | MIME type of the file \(e.g., application/pdf\). Applies only to the base64 upload path; a file from the File input always sends the content type resolved from storage. |
| `description` | string | No | Description of the uploaded evidence \(e.g., "Q3 access review evidence"\) |
| `effectiveAtDate` | string | No | ISO 8601 date indicating when the document is effective from |
@@ -0,0 +1,97 @@
/**
* @vitest-environment node
*/
import { describe, expect, it, vi } from 'vitest'
vi.mock('@/triggers', () => ({
getTrigger: () => ({ subBlocks: [] }),
}))
import { GoogleDriveBlock } from '@/blocks/blocks/google_drive'
import { listTool } from '@/tools/google_drive/list'
import { listCommentsTool } from '@/tools/google_drive/list_comments'
import { listPermissionsTool } from '@/tools/google_drive/list_permissions'
import { listRevisionsTool } from '@/tools/google_drive/list_revisions'
import { searchTool } from '@/tools/google_drive/search'
const paginationCases = [
{ operation: 'list', subBlockId: 'pageToken', tool: listTool },
{ operation: 'search', subBlockId: 'searchPageToken', tool: searchTool },
{ operation: 'list_permissions', subBlockId: 'permissionsPageToken', tool: listPermissionsTool },
{ operation: 'list_revisions', subBlockId: 'revisionsPageToken', tool: listRevisionsTool },
{ operation: 'list_comments', subBlockId: 'commentsPageToken', tool: listCommentsTool },
] as const
describe('GoogleDriveBlock pagination', () => {
const buildParams = GoogleDriveBlock.tools.config.params!
describe.each(paginationCases)('$operation', ({ operation, subBlockId, tool }) => {
it('exposes a page token field scoped to the operation', () => {
expect(GoogleDriveBlock.subBlocks.find(({ id }) => id === subBlockId)).toMatchObject({
type: 'short-input',
mode: 'advanced',
condition: { field: 'operation', value: operation },
})
})
/**
* `pageToken` is the canonical tool param, so the `list` case would forward
* through `...rest` even without the mapper. The per-operation ids are the
* ones the mapper has to translate, and none of them may survive as-is.
*/
it('forwards the page token to the tool under its own id', () => {
const params = buildParams({ operation, [subBlockId]: 'token-abc' }, undefined as never)
expect(params).toMatchObject({ pageToken: 'token-abc' })
if (subBlockId !== 'pageToken') expect(params[subBlockId]).toBeUndefined()
})
it('lets an agent feed a nextPageToken back in', () => {
expect(tool.params.pageToken?.visibility).toBe('user-or-llm')
})
})
it('does not leak a page token into operations that do not paginate', () => {
expect(
buildParams({ operation: 'get_file', pageToken: 'token-abc' }, undefined as never).pageToken
).toBeUndefined()
})
/**
* `shouldSerializeSubBlock` short-circuits for `advanced` fields in basic display
* mode without evaluating `condition`, so a page token typed under one operation
* genuinely reaches `inputs` after the user switches to another. The mapper must
* pick the token belonging to the operation being run and drop the rest.
*/
describe.each(paginationCases.filter(({ subBlockId }) => subBlockId !== 'pageToken'))(
'$subBlockId left over from a previous operation',
({ subBlockId }) => {
it.each(['upload', 'get_file', 'list'])('is dropped under %s', (operation) => {
const params = buildParams({ operation, [subBlockId]: 'stale' }, undefined as never)
expect(params.pageToken).toBeUndefined()
expect(params[subBlockId]).toBeUndefined()
})
}
)
it('prefers the operation-owned token when a stale sibling is also present', () => {
const params = buildParams(
{
operation: 'search',
searchPageToken: 'search-token',
commentsPageToken: 'stale',
pageToken: 'stale-canonical',
},
undefined as never
)
expect(params.pageToken).toBe('search-token')
expect(params.commentsPageToken).toBeUndefined()
expect(params.searchPageToken).toBeUndefined()
})
it('declares pageToken as a block input', () => {
expect(GoogleDriveBlock.inputs.pageToken).toBeDefined()
})
})
+54
View File
@@ -463,6 +463,14 @@ Return ONLY the query string - no explanations, no quotes around the whole thing
placeholder: 'Number of results (default: 100, max: 100)',
condition: { field: 'operation', value: 'list' },
},
{
id: 'pageToken',
title: 'Page Token',
type: 'short-input',
placeholder: 'Token from a previous nextPageToken',
mode: 'advanced',
condition: { field: 'operation', value: 'list' },
},
// Download File Fields - File Selector (basic mode)
{
id: 'downloadFileSelector',
@@ -905,6 +913,14 @@ Return ONLY the message text - no subject line, no greetings/signatures, no extr
condition: { field: 'operation', value: 'list_permissions' },
required: true,
},
{
id: 'permissionsPageToken',
title: 'Page Token',
type: 'short-input',
placeholder: 'Token from a previous nextPageToken',
mode: 'advanced',
condition: { field: 'operation', value: 'list_permissions' },
},
// Get File Content Fields
{
id: 'getContentFileSelector',
@@ -1073,6 +1089,14 @@ Return ONLY the query string - no explanations, no quotes around the whole thing
mode: 'advanced',
condition: { field: 'operation', value: 'search' },
},
{
id: 'searchPageToken',
title: 'Page Token',
type: 'short-input',
placeholder: 'Token from a previous nextPageToken',
mode: 'advanced',
condition: { field: 'operation', value: 'search' },
},
// Untrash File Fields
{
id: 'untrashFileSelector',
@@ -1191,6 +1215,14 @@ Return ONLY the query string - no explanations, no quotes around the whole thing
mode: 'advanced',
condition: { field: 'operation', value: 'list_revisions' },
},
{
id: 'revisionsPageToken',
title: 'Page Token',
type: 'short-input',
placeholder: 'Token from a previous nextPageToken',
mode: 'advanced',
condition: { field: 'operation', value: 'list_revisions' },
},
{
id: 'getRevisionFileSelector',
title: 'Select File',
@@ -1255,6 +1287,14 @@ Return ONLY the query string - no explanations, no quotes around the whole thing
mode: 'advanced',
condition: { field: 'operation', value: 'list_comments' },
},
{
id: 'commentsPageToken',
title: 'Page Token',
type: 'short-input',
placeholder: 'Token from a previous nextPageToken',
mode: 'advanced',
condition: { field: 'operation', value: 'list_comments' },
},
{
id: 'includeDeleted',
title: 'Include Deleted Comments',
@@ -1473,6 +1513,11 @@ Return ONLY the comment text - no explanations, no quotes, no extra formatting.`
searchPageSize,
revisionsPageSize,
commentsPageSize,
pageToken,
searchPageToken,
permissionsPageToken,
revisionsPageToken,
commentsPageToken,
getContentExportMimeType,
exportMimeType,
...rest
@@ -1586,6 +1631,13 @@ Return ONLY the comment text - no explanations, no quotes, no extra formatting.`
else if (params.operation === 'list_revisions') effectivePageSize = revisionsPageSize
else if (params.operation === 'list_comments') effectivePageSize = commentsPageSize
let effectivePageToken: string | undefined = pageToken
if (params.operation === 'search') effectivePageToken = searchPageToken
else if (params.operation === 'list_permissions') effectivePageToken = permissionsPageToken
else if (params.operation === 'list_revisions') effectivePageToken = revisionsPageToken
else if (params.operation === 'list_comments') effectivePageToken = commentsPageToken
else if (params.operation !== 'list') effectivePageToken = undefined
const effectiveQuery = params.operation === 'search' ? searchQuery : query
const effectiveMimeType =
params.operation === 'get_content'
@@ -1603,6 +1655,7 @@ Return ONLY the comment text - no explanations, no quotes, no extra formatting.`
pageSize: effectivePageSize
? Number.parseInt(effectivePageSize as string, 10)
: undefined,
pageToken: effectivePageToken?.trim() || undefined,
query: effectiveQuery,
mimeType: effectiveMimeType === 'auto' ? undefined : effectiveMimeType,
type: shareType, // Map shareType to type for share tool
@@ -1660,6 +1713,7 @@ Return ONLY the comment text - no explanations, no quotes, no extra formatting.`
// List operation inputs
query: { type: 'string', description: 'Search query' },
pageSize: { type: 'number', description: 'Results per page' },
pageToken: { type: 'string', description: 'Pagination token from a previous nextPageToken' },
// Copy operation inputs
newName: { type: 'string', description: 'New name for copied file' },
// Update operation inputs
-13
View File
@@ -286,14 +286,6 @@ export const VantaBlock: BlockConfig<ToolResponse> = {
condition: { field: 'operation', value: 'upload_document_file' },
mode: 'advanced',
},
{
id: 'uploadMimeType',
title: 'MIME Type',
type: 'short-input',
placeholder: 'e.g., application/pdf (used when the file has no type of its own)',
condition: { field: 'operation', value: 'upload_document_file' },
mode: 'advanced',
},
{
id: 'uploadDescription',
title: 'Description',
@@ -930,7 +922,6 @@ export const VantaBlock: BlockConfig<ToolResponse> = {
const normalizedFile = normalizeFileInput(rest.file, { single: true })
if (normalizedFile) result.file = normalizedFile
result.fileName = optionalString(rest.uploadFileName)
result.mimeType = optionalString(rest.uploadMimeType)
result.description = optionalString(rest.uploadDescription)
result.effectiveAtDate = optionalString(rest.effectiveAtDate)
break
@@ -993,10 +984,6 @@ export const VantaBlock: BlockConfig<ToolResponse> = {
uploadedFileId: { type: 'string', description: 'Uploaded file ID' },
file: { type: 'json', description: 'Evidence file to upload' },
uploadFileName: { type: 'string', description: 'Optional file name override' },
uploadMimeType: {
type: 'string',
description: 'MIME type override used when the uploaded content has no type of its own',
},
uploadDescription: { type: 'string', description: 'Description of the uploaded evidence' },
effectiveAtDate: { type: 'string', description: 'Effective date of the document (ISO 8601)' },
frameworkMatchesAny: { type: 'string', description: 'Comma-separated framework ID filters' },
@@ -70,6 +70,11 @@ export async function resolveVantaUploadFile(
signal: context.signal,
})
context.signal?.throwIfAborted()
/**
* Every return path of `downloadServableFileFromStorage` yields a non-empty content
* type, so `resolved.contentType` always wins. The remaining operands are defensive
* fallbacks kept in place in case that guarantee is ever relaxed.
*/
return {
buffer: resolved.buffer,
fileName: input.fileName || userFile.name,
@@ -292,6 +292,13 @@ export const SUBBLOCK_ID_MIGRATIONS: Record<string, readonly SubblockIdMigration
* dropped outright.
*/
sap_concur: [{ from: 'forwardId', to: '_removed_forwardId' }],
/**
* `uploadMimeType` was an advanced MIME Type input on Upload Document File whose
* value the upload path never read: the content type is resolved from storage and
* that resolution is never empty, so the field's value lost the `||` chain every
* time. Dropped rather than renamed there is no field for the value to move to.
*/
vanta: [{ from: 'uploadMimeType', to: '_removed_uploadMimeType' }],
}
/** Reads the value out of a stored subblock entry, tolerating a bare value. */
+1 -1
View File
@@ -68,7 +68,7 @@ export const confluenceAddLabelTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -84,7 +84,7 @@ export const confluenceCreateBlogPostTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -59,7 +59,7 @@ export const confluenceCreateCommentTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -80,7 +80,7 @@ export const confluenceCreatePageTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -72,7 +72,7 @@ export const confluenceCreatePagePropertyTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -73,7 +73,7 @@ export const confluenceCreateSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -69,7 +69,7 @@ export const confluenceCreateSpacePropertyTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -52,7 +52,7 @@ export const confluenceDeleteAttachmentTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -53,7 +53,7 @@ export const confluenceDeleteBlogPostTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -52,7 +52,7 @@ export const confluenceDeleteCommentTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -61,7 +61,7 @@ export const confluenceDeleteLabelTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -61,7 +61,7 @@ export const confluenceDeletePageTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -61,7 +61,7 @@ export const confluenceDeletePagePropertyTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -55,7 +55,7 @@ export const confluenceDeleteSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -61,7 +61,7 @@ export const confluenceDeleteSpacePropertyTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -77,7 +77,7 @@ export const confluenceGetBlogPostTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -67,7 +67,7 @@ export const confluenceGetPageAncestorsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -76,7 +76,7 @@ export const confluenceGetPageChildrenTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -77,7 +77,7 @@ export const confluenceGetPageDescendantsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -82,7 +82,7 @@ export const confluenceGetPageVersionTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -81,7 +81,7 @@ export const confluenceGetPagesByLabelTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -64,7 +64,7 @@ export const confluenceGetSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -65,7 +65,7 @@ export const confluenceGetTaskTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -57,7 +57,7 @@ export const confluenceGetUserTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -73,7 +73,7 @@ export const confluenceListAttachmentsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -88,7 +88,7 @@ export const confluenceListBlogPostsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -101,7 +101,7 @@ export const confluenceListBlogPostsInSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -80,7 +80,7 @@ export const confluenceListCommentsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -71,7 +71,7 @@ export const confluenceListLabelsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -77,7 +77,7 @@ export const confluenceListPagePropertiesTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -74,7 +74,7 @@ export const confluenceListPageVersionsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -104,7 +104,7 @@ export const confluenceListPagesInSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -72,7 +72,7 @@ export const confluenceListSpaceLabelsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -76,7 +76,7 @@ export const confluenceListSpacePermissionsTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -72,7 +72,7 @@ export const confluenceListSpacePropertiesTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -66,7 +66,7 @@ export const confluenceListSpacesTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -104,7 +104,7 @@ export const confluenceListTasksTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -43,7 +43,7 @@ export const confluenceRetrieveTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -65,7 +65,7 @@ export const confluenceSearchTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -84,7 +84,7 @@ export const confluenceSearchInSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -50,7 +50,7 @@ export const confluenceUpdateTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -71,7 +71,7 @@ export const confluenceUpdateBlogPostTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -59,7 +59,7 @@ export const confluenceUpdateCommentTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -72,7 +72,7 @@ export const confluenceUpdateSpaceTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
+1 -1
View File
@@ -72,7 +72,7 @@ export const confluenceUpdateTaskTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
@@ -77,7 +77,7 @@ export const confluenceUploadAttachmentTool: InternalToolConfig<
cloudId: {
type: 'string',
required: false,
visibility: 'user-only',
visibility: 'hidden',
description:
'Confluence Cloud ID for the instance. If not provided, it will be fetched using the domain.',
},
File diff suppressed because one or more lines are too long
+429
View File
@@ -0,0 +1,429 @@
/**
* @vitest-environment node
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { commentTool, commentV2Tool } from '@/tools/github/comment'
import type { CreateCommentParams } from '@/tools/github/types'
const { secureGitHubRequest } = vi.hoisted(() => ({ secureGitHubRequest: vi.fn() }))
vi.mock('@/tools/github/utils.server', () => ({
secureGitHubRequest,
GITHUB_MAX_RESPONSE_BYTES: 10 * 1024 * 1024,
}))
const HEAD_SHA = 'a'.repeat(40)
const OTHER_SHA = 'b'.repeat(40)
const FILE_COMMENT_PARAMS: CreateCommentParams = {
owner: 'octo',
repo: 'demo',
pullNumber: 7,
body: 'Looks good',
path: 'src/main.ts',
line: 42,
commentType: 'file_comment',
apiKey: 'ghp_test',
}
function pullRequestResponse(): Response {
return Response.json({ number: 7, head: { sha: HEAD_SHA, ref: 'feature' } })
}
function createdCommentResponse(): Response {
return Response.json({
id: 99,
body: 'Looks good',
html_url: 'https://github.com/octo/demo/pull/7#discussion_r99',
path: 'src/main.ts',
line: 42,
side: 'RIGHT',
commit_id: HEAD_SHA,
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z',
})
}
interface RecordedCall {
url: string
method: string
body: unknown
signal: AbortSignal | undefined
}
function calls(): RecordedCall[] {
return secureGitHubRequest.mock.calls.map(([url, options]) => ({
url,
method: options.method ?? 'GET',
body: options.body === undefined ? undefined : JSON.parse(options.body),
signal: options.signal,
}))
}
describe('github_comment routing', () => {
beforeEach(() => {
secureGitHubRequest.mockReset()
})
it('posts to the reviews endpoint when commentType is unset', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
const params: CreateCommentParams = {
owner: 'octo',
repo: 'demo',
pullNumber: 7,
body: 'Nice',
apiKey: 'ghp_test',
}
await commentTool.directExecution!(params)
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/reviews',
method: 'POST',
body: { body: 'Nice', event: 'COMMENT' },
signal: undefined,
},
])
})
it('leaves a general PR comment on the reviews endpoint', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
owner: 'octo',
repo: 'demo',
pullNumber: 7,
body: 'Nice',
commentType: 'pr_comment',
apiKey: 'ghp_test',
})
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/reviews',
method: 'POST',
body: { body: 'Nice', event: 'COMMENT' },
signal: undefined,
},
])
})
/**
* `POST /pulls/{n}/comments` documents `body`, `commit_id` and `path` as required,
* so a review body sent there is a guaranteed 422. The endpoint therefore follows
* the comment type, not the presence of a path.
*/
it('keeps a general PR comment carrying a path on the reviews endpoint', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
owner: 'octo',
repo: 'demo',
pullNumber: 7,
body: 'Nice',
path: 'src/main.ts',
commentType: 'pr_comment',
apiKey: 'ghp_test',
})
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/reviews',
method: 'POST',
body: { body: 'Nice', event: 'COMMENT' },
signal: undefined,
},
])
})
it('keeps a comment with no type carrying a path on the reviews endpoint', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
owner: 'octo',
repo: 'demo',
pullNumber: 7,
body: 'Nice',
path: 'src/main.ts',
apiKey: 'ghp_test',
})
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/reviews',
method: 'POST',
body: { body: 'Nice', event: 'COMMENT' },
signal: undefined,
},
])
})
it('posts a file comment left without a path to the reviews endpoint', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
const { path, ...params } = FILE_COMMENT_PARAMS
await commentTool.directExecution!(params)
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/reviews',
method: 'POST',
body: { body: 'Looks good', line: 42, side: 'RIGHT' },
signal: undefined,
},
])
})
it('looks the pull request up and posts the resolved head SHA as commit_id', async () => {
secureGitHubRequest
.mockResolvedValueOnce(pullRequestResponse())
.mockResolvedValueOnce(createdCommentResponse())
const result = await commentTool.directExecution!(FILE_COMMENT_PARAMS)
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7',
method: 'GET',
body: undefined,
signal: undefined,
},
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/comments',
method: 'POST',
body: {
body: 'Looks good',
commit_id: HEAD_SHA,
path: 'src/main.ts',
line: 42,
side: 'RIGHT',
},
signal: undefined,
},
])
expect(result.output.metadata.commit_id).toBe(HEAD_SHA)
})
it('posts directly when commitId is supplied', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({ ...FILE_COMMENT_PARAMS, commitId: OTHER_SHA })
expect(calls()).toEqual([
{
url: 'https://api.github.com/repos/octo/demo/pulls/7/comments',
method: 'POST',
body: {
body: 'Looks good',
commit_id: OTHER_SHA,
path: 'src/main.ts',
line: 42,
side: 'RIGHT',
},
signal: undefined,
},
])
})
it('resolves the head SHA for the v2 tool as well', async () => {
secureGitHubRequest
.mockResolvedValueOnce(pullRequestResponse())
.mockResolvedValueOnce(createdCommentResponse())
const result = await commentV2Tool.directExecution!(FILE_COMMENT_PARAMS)
expect(calls()[1].body).toMatchObject({ commit_id: HEAD_SHA })
expect(result.output.commit_id).toBe(HEAD_SHA)
})
it('no longer exposes the deprecated position parameter', () => {
expect(commentTool.params.position).toBeUndefined()
})
it('routes every comment type / path combination to the endpoint that accepts it', () => {
const url = commentTool.request.url as (params: CreateCommentParams) => string
const base = { owner: 'octo', repo: 'demo', pullNumber: 7, body: 'Nice', apiKey: 'ghp_test' }
const reviews = 'https://api.github.com/repos/octo/demo/pulls/7/reviews'
const comments = 'https://api.github.com/repos/octo/demo/pulls/7/comments'
expect(url({ ...base, commitId: OTHER_SHA })).toBe(reviews)
expect(url({ ...base, commitId: OTHER_SHA, path: 'src/main.ts' })).toBe(reviews)
expect(url({ ...base, commitId: OTHER_SHA, commentType: 'pr_comment' })).toBe(reviews)
expect(
url({ ...base, commitId: OTHER_SHA, commentType: 'pr_comment', path: 'src/main.ts' })
).toBe(reviews)
expect(url({ ...base, commitId: OTHER_SHA, commentType: 'file_comment' })).toBe(reviews)
expect(
url({ ...base, commitId: OTHER_SHA, commentType: 'file_comment', path: 'src/main.ts' })
).toBe(comments)
})
it('keeps the declarative request in step with the executed routing', () => {
const url = commentTool.request.url as (params: CreateCommentParams) => string
const method = commentTool.request.method as (params: CreateCommentParams) => string
expect(url(FILE_COMMENT_PARAMS)).toBe('https://api.github.com/repos/octo/demo/pulls/7')
expect(method(FILE_COMMENT_PARAMS)).toBe('GET')
expect(commentTool.request.body?.(FILE_COMMENT_PARAMS)).toBeUndefined()
})
})
describe('github_comment cancellation', () => {
beforeEach(() => {
secureGitHubRequest.mockReset()
})
it('forwards the abort signal to both the lookup and the comment request', async () => {
secureGitHubRequest
.mockResolvedValueOnce(pullRequestResponse())
.mockResolvedValueOnce(createdCommentResponse())
const controller = new AbortController()
await commentTool.directExecution!(FILE_COMMENT_PARAMS, controller.signal)
const recorded = calls()
expect(recorded).toHaveLength(2)
expect(recorded[0].signal).toBe(controller.signal)
expect(recorded[1].signal).toBe(controller.signal)
})
it('forwards the abort signal on the single-request path', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
const controller = new AbortController()
await commentTool.directExecution!(
{ ...FILE_COMMENT_PARAMS, commitId: OTHER_SHA },
controller.signal
)
expect(calls()[0].signal).toBe(controller.signal)
})
})
describe('github_comment line coercion', () => {
beforeEach(() => {
secureGitHubRequest.mockReset()
})
it('coerces a line number typed into the short input to an integer', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
commitId: OTHER_SHA,
line: '42' as unknown as number,
})
expect(calls()[0].body).toMatchObject({ line: 42 })
})
it('coerces the line on the resolved-commit path as well', async () => {
secureGitHubRequest
.mockResolvedValueOnce(pullRequestResponse())
.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
line: '42' as unknown as number,
})
expect(calls()[1].body).toMatchObject({ line: 42 })
})
it('keeps an integer line typed with surrounding whitespace', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
commitId: OTHER_SHA,
line: ' 42 ' as unknown as number,
})
expect(calls()[0].body).toMatchObject({ line: 42 })
})
it('rejects a fractional line rather than silently moving the comment', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await expect(
commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
commitId: OTHER_SHA,
line: 3.9,
})
).rejects.toThrow('GitHub line numbers are whole numbers, but line was 3.9')
expect(secureGitHubRequest).not.toHaveBeenCalled()
})
it('rejects a fractional line typed into the short input', async () => {
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await expect(
commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
commitId: OTHER_SHA,
line: '3.9' as unknown as number,
})
).rejects.toThrow('GitHub line numbers are whole numbers, but line was 3.9')
expect(secureGitHubRequest).not.toHaveBeenCalled()
})
it('omits a blank or unparseable line rather than sending NaN', async () => {
for (const line of ['', ' ', 'abc', undefined, null]) {
secureGitHubRequest.mockReset()
secureGitHubRequest.mockResolvedValueOnce(createdCommentResponse())
await commentTool.directExecution!({
...FILE_COMMENT_PARAMS,
commitId: OTHER_SHA,
line: line as unknown as number,
})
expect(calls()[0].body).not.toHaveProperty('line')
}
})
})
describe('github_comment errors', () => {
beforeEach(() => {
secureGitHubRequest.mockReset()
})
it('fails with an actionable error when the pull request has no head SHA', async () => {
secureGitHubRequest.mockResolvedValueOnce(Response.json({ number: 7 }))
await expect(commentTool.directExecution!(FILE_COMMENT_PARAMS)).rejects.toThrow(
/no head commit SHA for pull request octo\/demo#7/
)
expect(secureGitHubRequest).toHaveBeenCalledTimes(1)
})
it('surfaces the errors[] detail of a rejected comment', async () => {
secureGitHubRequest.mockResolvedValueOnce(
Response.json(
{
message: 'Validation Failed',
errors: [{ field: 'line', code: 'invalid', message: 'line must be part of the diff' }],
},
{ status: 422 }
)
)
await expect(
commentTool.directExecution!({ ...FILE_COMMENT_PARAMS, commitId: OTHER_SHA })
).rejects.toThrow('Validation Failed: line: line must be part of the diff')
})
it('carries the response status on a failed lookup', async () => {
secureGitHubRequest.mockResolvedValueOnce(
Response.json({ message: 'Not Found' }, { status: 404 })
)
await expect(commentTool.directExecution!(FILE_COMMENT_PARAMS)).rejects.toMatchObject({
message: 'Not Found',
status: 404,
})
expect(secureGitHubRequest).toHaveBeenCalledTimes(1)
})
})
+257 -46
View File
@@ -1,7 +1,233 @@
import { isRecordLike } from '@sim/utils/object'
import { formatGitHubErrorMessage } from '@/tools/github/response-parsers'
import type { CreateCommentParams, CreateCommentResponse } from '@/tools/github/types'
import { COMMENT_OUTPUT_PROPERTIES, USER_OUTPUT } from '@/tools/github/types'
import type { ToolConfig } from '@/tools/types'
const GITHUB_API_BASE = 'https://api.github.com'
/** Body GitHub accepts on `POST /pulls/{n}/reviews`. */
interface ReviewCommentBody {
body: string
event: 'COMMENT'
}
/** Body GitHub accepts on `POST /pulls/{n}/comments`. */
interface FileCommentBody {
body: string
commit_id: string | undefined
path: string | undefined
line: number | undefined
side: string
}
/** The subset of a GitHub comment payload this tool reports. */
interface GitHubCommentPayload {
id?: number
body?: string
html_url?: string
user?: unknown
path?: string
line?: number
position?: number
side?: string
commit_id?: string
created_at?: string
updated_at?: string
}
function githubHeaders(apiKey: string): Record<string, string> {
return {
Accept: 'application/vnd.github.v3+json',
Authorization: `Bearer ${apiKey}`,
'X-GitHub-Api-Version': '2022-11-28',
}
}
function pullRequestUrl(params: CreateCommentParams): string {
return `${GITHUB_API_BASE}/repos/${params.owner}/${params.repo}/pulls/${params.pullNumber}`
}
/**
* Whether the request is headed for `POST /pulls/{n}/comments`. GitHub documents
* `body`, `commit_id` and `path` as required there, so only a file comment that
* actually carries a path can use it. `path` is optional on the block, so a file
* comment left without one falls back to `/pulls/{n}/reviews`, where GitHub creates
* a pending review and neither field is required.
*/
function isFileCommentRequest(params: CreateCommentParams): boolean {
return params.commentType === 'file_comment' && Boolean(params.path)
}
/**
* GitHub requires `commit_id` on a pull request review comment. When the caller did
* not supply one, the pull request is fetched first so its head SHA can be used
* mirroring how Jira resolves a missing `cloudId` from `domain`.
*
* The lookup is gated on the endpoint, because only `/comments` needs a commit SHA.
*/
function needsCommitLookup(params: CreateCommentParams): boolean {
return isFileCommentRequest(params) && !params.commitId
}
/**
* The block renders `line` as a short input, so a typed line number reaches the tool
* as a string while GitHub types the field as an integer. Blank and unparseable input
* is omitted rather than sent as `NaN` `line` is optional, and nothing usable was
* supplied.
*
* A fractional value is rejected instead of truncated. `3.9` is not the caller asking
* for line 3, and quietly posting the review comment on a different line of the diff
* than the one they named is the failure they would never think to look for. This
* fails the way a missing head commit SHA does: loudly, naming what to set.
*/
function toLineNumber(value: unknown): number | undefined {
let parsed: number
if (typeof value === 'number') {
parsed = value
} else {
if (typeof value !== 'string' || !value.trim()) return undefined
parsed = Number(value.trim())
}
if (!Number.isFinite(parsed)) return undefined
if (!Number.isInteger(parsed)) {
throw new Error(
`GitHub line numbers are whole numbers, but line was ${parsed}. Set line to the integer line number in the diff.`
)
}
return parsed
}
function fileCommentBody(
params: CreateCommentParams,
commitId: string | undefined
): FileCommentBody {
return {
body: params.body,
commit_id: commitId,
path: params.path,
line: toLineNumber(params.line),
side: params.side || 'RIGHT',
}
}
/**
* The endpoint the comment itself is posted to. The comment TYPE selects it, not the
* mere presence of `path`: a general PR comment sends `{body, event}`, which the
* review-comment endpoint rejects with a 422 for the missing `commit_id` and `path`,
* so a `pr_comment` that happens to name a file has to stay on `/reviews`.
*/
function commentEndpointUrl(params: CreateCommentParams): string {
return isFileCommentRequest(params)
? `${pullRequestUrl(params)}/comments`
: `${pullRequestUrl(params)}/reviews`
}
function commentRequestBody(
params: CreateCommentParams,
commitId: string | undefined
): FileCommentBody | ReviewCommentBody {
if (params.commentType === 'file_comment') return fileCommentBody(params, commitId)
return { body: params.body, event: 'COMMENT' }
}
function readHeadSha(pullRequest: unknown): string | undefined {
if (!isRecordLike(pullRequest) || !isRecordLike(pullRequest.head)) return undefined
const sha = pullRequest.head.sha
return typeof sha === 'string' && sha ? sha : undefined
}
function readString(record: Record<string, unknown>, key: string): string | undefined {
const value = record[key]
return typeof value === 'string' ? value : undefined
}
function readNumber(record: Record<string, unknown>, key: string): number | undefined {
const value = record[key]
return typeof value === 'number' ? value : undefined
}
function readCommentPayload(value: unknown): GitHubCommentPayload {
if (!isRecordLike(value)) return {}
return {
id: readNumber(value, 'id'),
body: readString(value, 'body'),
html_url: readString(value, 'html_url'),
user: value.user,
path: readString(value, 'path'),
line: readNumber(value, 'line'),
position: readNumber(value, 'position'),
side: readString(value, 'side'),
commit_id: readString(value, 'commit_id'),
created_at: readString(value, 'created_at'),
updated_at: readString(value, 'updated_at'),
}
}
/**
* Projects a failed GitHub response the way the tool transport does: the thrown error
* carries `status`, `statusText`, and the parsed body on `data`, so callers that branch
* on a status (a 404 treated as a clean no-match, for one) keep working off this path.
*/
async function assertGitHubResponseOk(response: Response, fallback: string): Promise<void> {
if (response.ok) return
const text = await response.text().catch(() => '')
let data: unknown = text
try {
data = JSON.parse(text)
} catch {
data = text
}
const error = new Error(formatGitHubErrorMessage(data) ?? `${fallback} (HTTP ${response.status})`)
Object.assign(error, { status: response.status, statusText: response.statusText, data })
throw error
}
/**
* Creates the comment, resolving the pull request head SHA first when a file comment
* needs one. Both requests run on the DNS-validated, IP-pinned GitHub transport and
* carry the execution's abort signal, so cancelling a workflow cancels the POST.
*/
async function createComment(
params: CreateCommentParams,
signal?: AbortSignal
): Promise<GitHubCommentPayload> {
const { secureGitHubRequest } = await import('@/tools/github/utils.server')
const headers = githubHeaders(params.apiKey)
let commitId = params.commitId
if (needsCommitLookup(params)) {
const pullRequestResponse = await secureGitHubRequest(pullRequestUrl(params), {
headers,
signal,
})
await assertGitHubResponseOk(
pullRequestResponse,
`Failed to load pull request ${params.owner}/${params.repo}#${params.pullNumber}`
)
commitId = readHeadSha(await pullRequestResponse.json())
if (!commitId) {
throw new Error(
`GitHub returned no head commit SHA for pull request ${params.owner}/${params.repo}#${params.pullNumber}. Set commitId to comment on a specific commit.`
)
}
}
const response = await secureGitHubRequest(commentEndpointUrl(params), {
method: 'POST',
headers: { ...headers, 'Content-Type': 'application/json' },
body: JSON.stringify(commentRequestBody(params, commitId)),
signal,
})
await assertGitHubResponseOk(response, 'Failed to create comment')
return readCommentPayload(await response.json())
}
const DIRECT_EXECUTION_ONLY_ERROR = 'GitHub comments require the two-phase direct execution path'
export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse> = {
id: 'github_comment',
name: 'GitHub PR Commenter',
@@ -39,12 +265,6 @@ export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse>
visibility: 'user-or-llm',
description: 'File path for review comment',
},
position: {
type: 'number',
required: false,
visibility: 'hidden',
description: 'Line number for review comment',
},
commentType: {
type: 'string',
required: false,
@@ -68,7 +288,7 @@ export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse>
type: 'string',
required: false,
visibility: 'hidden',
description: 'The SHA of the commit to comment on',
description: 'The SHA of the commit to comment on. Defaults to the pull request head commit.',
},
apiKey: {
type: 'string',
@@ -78,46 +298,13 @@ export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse>
},
},
request: {
url: (params) => {
if (params.path) {
return `https://api.github.com/repos/${params.owner}/${params.repo}/pulls/${params.pullNumber}/comments`
}
return `https://api.github.com/repos/${params.owner}/${params.repo}/pulls/${params.pullNumber}/reviews`
},
method: 'POST',
headers: (params) => ({
Accept: 'application/vnd.github.v3+json',
Authorization: `Bearer ${params.apiKey}`,
'X-GitHub-Api-Version': '2022-11-28',
}),
body: (params) => {
if (params.commentType === 'file_comment') {
return {
body: params.body,
commit_id: params.commitId,
path: params.path,
line: params.line || params.position,
side: params.side || 'RIGHT',
}
}
return {
body: params.body,
event: 'COMMENT',
}
},
},
transformResponse: async (response) => {
const data = await response.json()
// Create a human-readable content string
const content = `Comment created: "${data.body}"`
directExecution: async (params, signal) => {
const data = await createComment(params, signal)
return {
success: true,
output: {
content,
content: `Comment created: "${data.body}"`,
metadata: {
id: data.id,
html_url: data.html_url,
@@ -132,6 +319,27 @@ export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse>
}
},
request: {
url: (params) => {
if (needsCommitLookup(params)) {
return pullRequestUrl(params)
}
return commentEndpointUrl(params)
},
method: (params) => (needsCommitLookup(params) ? 'GET' : 'POST'),
headers: (params) => githubHeaders(params.apiKey),
body: (params) => {
if (needsCommitLookup(params)) {
return undefined
}
return commentRequestBody(params, params.commitId)
},
},
transformResponse: async () => {
throw new Error(DIRECT_EXECUTION_ONLY_ERROR)
},
outputs: {
content: { type: 'string', description: 'Human-readable comment confirmation' },
metadata: {
@@ -141,15 +349,15 @@ export const commentTool: ToolConfig<CreateCommentParams, CreateCommentResponse>
},
}
export const commentV2Tool: ToolConfig = {
export const commentV2Tool: ToolConfig<CreateCommentParams> = {
id: 'github_comment_v2',
name: commentTool.name,
description: commentTool.description,
version: '2.0.0',
params: commentTool.params,
request: commentTool.request,
transformResponse: async (response: Response) => {
const data = await response.json()
directExecution: async (params, signal) => {
const data = await createComment(params, signal)
return {
success: true,
output: {
@@ -166,6 +374,9 @@ export const commentV2Tool: ToolConfig = {
},
}
},
transformResponse: async () => {
throw new Error(DIRECT_EXECUTION_ONLY_ERROR)
},
outputs: {
...COMMENT_OUTPUT_PROPERTIES,
user: USER_OUTPUT,
+34 -4
View File
@@ -137,12 +137,42 @@ export function requiredRecord(
return value
}
/**
* Renders one entry of GitHub's `errors[]` array. Entries are either a bare string or
* an object carrying some combination of `field`, `code`, and `message`.
*/
function readGitHubErrorEntry(entry: unknown): string | undefined {
if (typeof entry === 'string') return entry.trim() || undefined
if (!isRecordLike(entry)) return undefined
const field = typeof entry.field === 'string' ? entry.field.trim() : ''
const message = typeof entry.message === 'string' ? entry.message.trim() : ''
const code = typeof entry.code === 'string' ? entry.code.trim() : ''
const detail = message || code
if (!detail) return field || undefined
return field ? `${field}: ${detail}` : detail
}
/**
* A GitHub 422 names the offending field only in `errors[]` the top-level `message`
* is the useless `"Validation Failed"`. The field-level detail is appended so the user
* can tell which input was rejected. Responses without an `errors[]` array, and
* responses without a top-level `message` at all, are unchanged.
*/
export function formatGitHubErrorMessage(value: unknown): string | undefined {
if (!isRecordLike(value)) return undefined
const message = value.message
if (typeof message !== 'string' || !message.trim()) return undefined
if (!Array.isArray(value.errors)) return message
const details = value.errors
.map(readGitHubErrorEntry)
.filter((detail): detail is string => Boolean(detail))
return details.length ? `${message}: ${details.join('; ')}` : message
}
/** Reads a failed GitHub response body and renders it with {@link formatGitHubErrorMessage}. */
export async function readGitHubErrorMessage(response: Response): Promise<string | undefined> {
try {
const value: unknown = await response.json()
if (!isRecordLike(value)) return undefined
const message = value.message
return typeof message === 'string' && message.trim() ? message : undefined
return formatGitHubErrorMessage(await response.json())
} catch {
return undefined
}
-1
View File
@@ -866,7 +866,6 @@ export interface PRV2OperationParams extends PROperationParams {
export interface CreateCommentParams extends PROperationParams {
body: string
path?: string
position?: number
line?: number
side?: string
commitId?: string
+181
View File
@@ -0,0 +1,181 @@
/**
* Pins the redirect contract of the GitHub direct-execution transport: the workspace
* token must never cross an origin boundary, while a legitimate same-origin GitHub
* redirect (a renamed repository) must stay authenticated.
*
* @vitest-environment node
*/
import http from 'node:http'
import type { AddressInfo } from 'node:net'
import { afterEach, describe, expect, it, vi } from 'vitest'
vi.mock('@sim/security/dns', () => ({
resolveHostAddresses: vi.fn(async () => ({ addresses: ['127.0.0.1'] })),
preferIpv4: (addresses: string[]) => addresses[0],
}))
vi.mock('@/lib/core/config/env-flags', () => ({
isHosted: false,
isPrivateDatabaseHostsAllowed: false,
getProxyUrl: () => undefined,
}))
import { secureGitHubRequest } from '@/tools/github/utils.server'
interface RecordedHop {
url: string
method: string
body: string
headers: http.IncomingHttpHeaders
}
const servers: http.Server[] = []
afterEach(() => {
for (const server of servers.splice(0)) server.close()
})
async function startServer(handler: http.RequestListener): Promise<string> {
const server = http.createServer(handler)
servers.push(server)
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
return `http://127.0.0.1:${(server.address() as AddressInfo).port}`
}
function record(hops: RecordedHop[], req: http.IncomingMessage, res: http.ServerResponse): void {
let body = ''
req.on('data', (chunk) => {
body += chunk
})
req.on('end', () => {
hops.push({ url: req.url ?? '', method: req.method ?? '', body, headers: req.headers })
res.writeHead(200, { 'Content-Type': 'application/json' })
res.end('{"ok":true}')
})
}
/** Records every request it receives, then answers 200. */
async function startRecordingServer(hops: RecordedHop[]): Promise<string> {
return startServer((req, res) => record(hops, req, res))
}
const GITHUB_HEADERS = {
Accept: 'application/vnd.github.v3+json',
Authorization: 'Bearer ghp_workspace_token',
'X-GitHub-Api-Version': '2022-11-28',
}
describe('secureGitHubRequest redirects', () => {
it('does not forward the GitHub token across an origin boundary', async () => {
const hops: RecordedHop[] = []
const attacker = await startRecordingServer(hops)
const origin = await startServer((req, res) => {
req.resume()
res.writeHead(302, { location: `${attacker}/stolen` })
res.end()
})
const response = await secureGitHubRequest(origin, { headers: GITHUB_HEADERS })
expect(response.status).toBe(200)
expect(hops).toHaveLength(1)
expect(hops[0].url).toBe('/stolen')
expect(hops[0].headers.authorization).toBeUndefined()
})
it('does not forward the GitHub token when a comment POST crosses an origin boundary', async () => {
const hops: RecordedHop[] = []
const attacker = await startRecordingServer(hops)
const origin = await startServer((req, res) => {
req.resume()
res.writeHead(302, { location: `${attacker}/stolen` })
res.end()
})
await secureGitHubRequest(origin, {
method: 'POST',
headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
body: '{"body":"Looks good"}',
})
expect(hops).toHaveLength(1)
expect(hops[0].headers.authorization).toBeUndefined()
expect(hops[0].headers.cookie).toBeUndefined()
})
it('replays a comment POST as a POST across a same-origin renamed-repository 301', async () => {
const hops: RecordedHop[] = []
const origin = await startServer((req, res) => {
if (req.url === '/repos/octo/old/pulls/7/comments') {
req.resume()
res.writeHead(301, { location: '/repos/octo/new/pulls/7/comments' })
res.end()
return
}
record(hops, req, res)
})
await secureGitHubRequest(`${origin}/repos/octo/old/pulls/7/comments`, {
method: 'POST',
headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
body: '{"body":"Looks good"}',
})
expect(hops).toHaveLength(1)
expect(hops[0].url).toBe('/repos/octo/new/pulls/7/comments')
expect(hops[0].method).toBe('POST')
expect(hops[0].body).toBe('{"body":"Looks good"}')
expect(hops[0].headers.authorization).toBe('Bearer ghp_workspace_token')
})
it('keeps the token on a same-origin redirect, as a renamed repository needs', async () => {
const hops: RecordedHop[] = []
const origin = await startServer((req, res) => {
if (req.url === '/repos/octo/old/pulls/7') {
req.resume()
res.writeHead(301, { location: '/repos/octo/new/pulls/7' })
res.end()
return
}
record(hops, req, res)
})
const response = await secureGitHubRequest(`${origin}/repos/octo/old/pulls/7`, {
headers: GITHUB_HEADERS,
})
expect(response.status).toBe(200)
expect(hops).toHaveLength(1)
expect(hops[0].url).toBe('/repos/octo/new/pulls/7')
expect(hops[0].headers.authorization).toBe('Bearer ghp_workspace_token')
})
})
describe('secureGitHubRequest User-Agent', () => {
it('sends an explicit Sim User-Agent on the commit lookup and the comment POST', async () => {
const hops: RecordedHop[] = []
const origin = await startRecordingServer(hops)
await secureGitHubRequest(`${origin}/repos/octo/repo/pulls/7`, { headers: GITHUB_HEADERS })
await secureGitHubRequest(`${origin}/repos/octo/repo/pulls/7/comments`, {
method: 'POST',
headers: { ...GITHUB_HEADERS, 'Content-Type': 'application/json' },
body: '{"body":"Looks good"}',
})
expect(hops).toHaveLength(2)
expect(hops[0].headers['user-agent']).toBe('Sim')
expect(hops[1].headers['user-agent']).toBe('Sim')
})
it('leaves a caller-supplied User-Agent untouched', async () => {
const hops: RecordedHop[] = []
const origin = await startRecordingServer(hops)
await secureGitHubRequest(origin, {
headers: { ...GITHUB_HEADERS, 'user-agent': 'Sim-Custom' },
})
expect(hops[0].headers['user-agent']).toBe('Sim-Custom')
})
})
+85
View File
@@ -0,0 +1,85 @@
import {
secureFetchWithPinnedIP,
validateUrlWithDNS,
} from '@/lib/core/security/input-validation.server'
/**
* Response ceiling for a GitHub request issued outside the tool transport. Matches
* the transport's own `MAX_TOOL_RESPONSE_BODY_BYTES`, so a tool moved onto this
* helper keeps the exact body limit it had before.
*/
export const GITHUB_MAX_RESPONSE_BYTES = 10 * 1024 * 1024
export interface SecureGitHubRequestOptions {
method?: string
headers: Record<string, string>
body?: string
maxResponseBytes?: number
signal?: AbortSignal
}
/**
* GitHub's API rejects a request without a User-Agent with 403 "Request forbidden by
* administrative rules". The declarative transport sets `User-Agent: Sim` for every
* tool it formats; a tool on this helper bypasses that, and the guarded fetch builds
* its request with raw `node:https`, which adds no default. Bun's `node:http` shim
* does inject its own `Bun/x.y.z`, so the call happens to work in production today
* but that silently replaces Sim's attribution and does not hold under Node.
*
* Set here rather than in each caller's header map so every future tool on this
* helper inherits it. A caller that supplies its own User-Agent, in any casing, wins.
*/
function withUserAgent(headers: Record<string, string>): Record<string, string> {
const hasUserAgent = Object.keys(headers).some((name) => name.toLowerCase() === 'user-agent')
return hasUserAgent ? headers : { ...headers, 'User-Agent': 'Sim' }
}
/**
* Executes one DNS-validated, IP-pinned GitHub request for a tool that cannot use
* the declarative transport a multi-phase tool running under `directExecution`.
*
* This deliberately carries no retry loop: the tools on this path declare no
* `request.retry`, so the transport retries them zero times today, and the second
* phase of a comment flow is a non-idempotent POST that must not be replayed.
*
* The redirect policy is explicit because omitting it leaves the workspace's GitHub
* token on the request across a cross-origin hop the transport only strips
* credentials when a policy is present. `legacy` is chosen over `standard` for the
* method rules: `standard` rewrites a redirected POST to a bodyless GET on 301/302
* regardless of origin, and GitHub answers 301 within api.github.com for a renamed
* repository, so a comment POST there would be replayed as a GET of the comment
* list a JSON array that fails the tool's payload shape check and reports success
* with no comment created. `legacy` keeps the method and body across that hop.
*
* Credential stripping is unaffected by the mode: the guarded follower strips
* Authorization, Proxy-Authorization and Cookie on a cross-origin hop whenever
* `sendCredentialsOnCrossOriginRedirect` is false, in either mode.
*
* `stripAuthOnRedirect` is deliberately NOT set: it drops the token on every hop,
* including the legitimate same-origin renamed-repository 301, so an unauthenticated
* replay there would turn a working call into a 401.
*/
export async function secureGitHubRequest(
url: string,
options: SecureGitHubRequestOptions
): Promise<Response> {
const validation = await validateUrlWithDNS(url, 'githubUrl')
if (!validation.isValid || !validation.resolvedIP) {
throw new Error(`Invalid GitHub URL: ${validation.error ?? 'DNS resolution failed'}`)
}
const response = await secureFetchWithPinnedIP(url, validation.resolvedIP, {
method: options.method ?? 'GET',
headers: withUserAgent(options.headers),
body: options.body,
maxResponseBytes: options.maxResponseBytes ?? GITHUB_MAX_RESPONSE_BYTES,
redirectPolicy: { mode: 'legacy', sendCredentialsOnCrossOriginRedirect: false },
signal: options.signal,
})
return new Response(response.body, {
status: response.status,
statusText: response.statusText,
headers: response.headers.toRecord(),
})
}
+1 -1
View File
@@ -48,7 +48,7 @@ export const listTool: ToolConfig<GoogleDriveToolParams, GoogleDriveListResponse
pageToken: {
type: 'string',
required: false,
visibility: 'hidden',
visibility: 'user-or-llm',
description: 'The page token to use for pagination',
},
},
+1 -1
View File
@@ -65,7 +65,7 @@ export const listCommentsTool: ToolConfig<
pageToken: {
type: 'string',
required: false,
visibility: 'hidden',
visibility: 'user-or-llm',
description: 'The page token to use for pagination',
},
},
@@ -43,7 +43,7 @@ export const listPermissionsTool: ToolConfig<
pageToken: {
type: 'string',
required: false,
visibility: 'hidden',
visibility: 'user-or-llm',
description: 'The page token to use for pagination',
},
},
@@ -51,7 +51,7 @@ export const listRevisionsTool: ToolConfig<
pageToken: {
type: 'string',
required: false,
visibility: 'hidden',
visibility: 'user-or-llm',
description: 'The page token to use for pagination',
},
},
+1 -1
View File
@@ -50,7 +50,7 @@ export const searchTool: ToolConfig<GoogleDriveSearchParams, GoogleDriveSearchRe
pageToken: {
type: 'string',
required: false,
visibility: 'hidden',
visibility: 'user-or-llm',
description: 'Token for fetching the next page of results',
},
},
@@ -15,6 +15,14 @@ describe('Vanta internal tool declarations', () => {
}
})
it('leaves mimeType an ordinary operation parameter the base64 path can set', () => {
expect(vantaUploadDocumentFileTool.params.mimeType.visibility).toBe('user-or-llm')
})
it('hides only the system-injected file content', () => {
expect(vantaUploadDocumentFileTool.params.fileContent.visibility).toBe('hidden')
})
it('preserves resolved secrets, variables, and protected file references verbatim', () => {
const file = { key: 'workspace/file.txt', name: 'file.txt', size: 4 }
expect(
+1 -1
View File
@@ -64,7 +64,7 @@ export const vantaUploadDocumentFileTool: InternalToolConfig<
required: false,
visibility: 'user-or-llm',
description:
'MIME type of the file (e.g., application/pdf). Used only for base64 uploads; ignored for a file from the File input, whose content type is always resolved from storage.',
'MIME type of the file (e.g., application/pdf). Applies only to the base64 upload path; a file from the File input always sends the content type resolved from storage.',
},
description: {
type: 'string',
+227
View File
@@ -697,3 +697,230 @@ describe('mapper param shapes', () => {
expect(ids).not.toContain('ghostString')
})
})
describe('a source the scanner cannot get through is reported, not swallowed', () => {
/**
* When `blankStringsAndComments` bails, both the `subBlocks` scan and the mapper scan come
* back empty for the same reason. Reported as a plain `ids: null` that is indistinguishable
* from a spread-only `subBlocks` array, the block's mapper renames are dropped in silence.
*/
const unterminated = `subBlocks: [{ id: 'a' }],
tools: { config: { params: (p) => ({ renamedByMapper: p.a }) } },
longDescription: 'never closed`
it('sets parseError so the caller warns', () => {
const supplied = extractBlockSuppliedParamIds(unterminated, 'GhostBlock')
expect(supplied.parseError).not.toBeNull()
expect(supplied.parseError).toMatch(/GhostBlock: source ends inside an unterminated/)
expect(supplied.ids).toBeNull()
})
it('still reports null with no parseError for a spread-only subBlocks array', () => {
const supplied = extractBlockSuppliedParamIds(
"subBlocks: [...NotionBlock.subBlocks], tools: { config: { params: (p) => ({ renamedByMapper: p.a }) } },",
'SpreadBlock'
)
expect(supplied.parseError).toBeNull()
expect(supplied.ids).toBeNull()
expect(supplied.mapperIds).toContain('renamedByMapper')
})
})
describe('the generated catalog ordering is locale-independent', () => {
/**
* `localeCompare` with no locale argument uses the runtime default, which varies with `LANG`
* and the ICU build. Against the real catalog names, `tr-TR` (dotted/dotless I), `lt-LT`,
* `cs-CZ` (the `ch` digraph) and `et-EE` each reorder the array, so a contributor on one of
* those locales would regenerate a different `integrations.json` and fail CI with no obvious
* cause. Every `localeCompare` in the generator must therefore name its locale as a literal;
* a bare `localeCompare()`, `localeCompare(b)` or a locale read from a variable all fall
* back to the default, so the arguments are matched whole rather than pattern-matched.
*
* A source grep is the only assertion that can catch an unpinned comparator. CI runs under
* an `en-US` default, where an unpinned `localeCompare` returns exactly what the pinned one
* does, so no behavioural comparison against real catalog names discriminates there; and
* comparing the committed `integrations.json` against the comparator that produced it agrees
* by construction whatever the comparator does. Both of those were asserted here and were
* removed for claiming a guarantee they did not hold.
*/
it('leaves no unpinned localeCompare in the generator', () => {
const source = fs.readFileSync(path.join(__dirname, 'generate-docs.ts'), 'utf-8')
const calls = [...source.matchAll(/\blocaleCompare\(([^)]*)\)/g)].map(([, args]) => args)
expect(calls.length).toBeGreaterThan(0)
for (const args of calls) expect(args).toMatch(/,\s*'[a-zA-Z-]+'\s*$/)
})
})
describe('the scanner survives regex literals in a block config', () => {
/**
* `blankStringsAndComments` used to be a single regex with no concept of a regex literal, so
* `/don't/` opened a phantom string that swallowed the following subBlocks, and a character
* class like `/[}]/` closed the enclosing object early. Both returned a short list with no
* warning a confident wrong answer, which is the one outcome the filter must never produce.
*/
it('does not let an apostrophe inside a regex swallow later subBlocks', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', condition: (v) => /don't/.test(v) }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
it('does not let a brace inside a character class close the object early', () => {
const ids = extractUserSettableParamIds("subBlocks: [{ id: 'a', v: /[}]/ }, { id: 'b' }],")
expect(ids).toEqual(['a', 'b'])
})
it('still reads a division as arithmetic rather than a regex', () => {
const ids = extractUserSettableParamIds('subBlocks: [{ id: "a", n: total / 2 }, { id: "b" }],')
expect(ids).toEqual(['a', 'b'])
})
it('does not mistake a protocol slash inside a string for a comment', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', url: 'https://example.com/x' }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
it('reports UNKNOWN rather than guessing when a literal never terminates', () => {
expect(extractUserSettableParamIds("subBlocks: [{ id: 'a }],")).toBeNull()
})
/**
* A `/` directly after a division operator is an operand position, so it opens a regex.
* Without `'/'` in `REGEX_ALLOWED_AFTER` the third slash of `x / y / /re/` lexes as a
* second division, the character class is left in the structural view and its `}` closes
* the object early a short list with no warning.
*/
it('reads a regex that follows a division operator', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', v: x / y / /[}]/.source }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
/**
* `'+'` and `'-'` are in `REGEX_ALLOWED_AFTER` for the binary operators, so the previous
* significant character alone reads the `/` after a postfix `i++` as opening a regex. The
* phantom regex then runs to the end of the input and the scan reports the block unreadable.
*/
it('still reads a division after a postfix increment or decrement', () => {
for (const op of ['++', '--']) {
const ids = extractUserSettableParamIds(
`subBlocks: [{ id: 'a', n: (i) => i${op} / 2 }, { id: 'b' }],`
)
expect(ids, op).toEqual(['a', 'b'])
}
})
/**
* The shape Prettier produces when a `.match()` argument does not fit on one line, as in
* `blocks/table.ts` and `blocks/table_v2.ts`. A newline is recorded as the previous
* significant character rather than skipped, so the `(` does not carry the decision only
* the `'\n'` entry in `REGEX_ALLOWED_AFTER` keeps this lexing as a regex.
*/
it('reads a regex that a formatter has wrapped onto its own line', () => {
const ids = extractUserSettableParamIds(
["subBlocks: [{ id: 'a', v: (s) => s.match(", ' /[}]/', ") }, { id: 'b' }],"].join('\n')
)
expect(ids).toEqual(['a', 'b'])
})
})
describe('the scanner reads a regex that opens in keyword position', () => {
/**
* The scanner chose regex-vs-division from the previous significant character alone, so a
* regex in operand position was lexed as a division off the keyword's last letter and its
* body was left in the structural view a brace inside it then closed the object early.
*/
it('does not let a brace inside a regex after return close the object early', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', condition: (v) => { return /}/.test(v) } }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
it('treats every operand-position keyword as opening a regex', () => {
const keywords = [
'return',
'typeof',
'case',
'in',
'of',
'new',
'delete',
'void',
'instanceof',
'do',
'else',
'yield',
'await',
]
for (const keyword of keywords) {
const ids = extractUserSettableParamIds(
`subBlocks: [{ id: 'a', v: (x) => ${keyword} /}/.source }, { id: 'b' }],`
)
expect(ids, keyword).toEqual(['a', 'b'])
}
})
/**
* The fixture leaves an odd number of `/` on the line, so a mis-lexed regex runs on to the
* end of the input rather than closing on a second slash. A self-cancelling pair like
* `counts.in / 2, m: preturn / 2` passes with the guard removed, because the phantom regex
* spans only `2, m: preturn ` and blanks nothing structural.
*/
it('still reads a division after a property or an identifier that merely ends in a keyword', () => {
expect(
extractUserSettableParamIds("subBlocks: [{ id: 'a', n: counts.in / 2 }, { id: 'b' }],")
).toEqual(['a', 'b'])
expect(
extractUserSettableParamIds("subBlocks: [{ id: 'a', n: preturn / 2 }, { id: 'b' }],")
).toEqual(['a', 'b'])
})
})
describe('template interpolation is lexed rather than brace-counted', () => {
/**
* The `${}` depth counter was not string-aware, so a brace inside a quoted expression
* miscounted, the closing backtick was never found and the whole block was reported
* unreadable which silently stops filtering resolver-derived hidden params for it.
*/
it('does not lose the closing backtick to an opening brace inside a quoted expression', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', label: `${format('{')}` }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
it('does not let a closing brace inside a quoted expression end the interpolation', () => {
const ids = extractUserSettableParamIds(
'subBlocks: [{ id: \'a\', label: `${format("}") + "{"}` }, { id: \'b\' }],'
)
expect(ids).toEqual(['a', 'b'])
})
it('lexes a regex, a comment and a nested template inside the expression', () => {
const ids = extractUserSettableParamIds(
"subBlocks: [{ id: 'a', label: `${/[{]/.source /* { */ + `${'{'}`}` }, { id: 'b' }],"
)
expect(ids).toEqual(['a', 'b'])
})
})
+342 -27
View File
@@ -652,7 +652,7 @@ function writeIconMapping(iconMapping: Record<string, IconRef>): void {
// Generate mapping with direct references (no dynamic access for tree shaking)
const mappingEntries = Object.entries(withAliases)
.sort(([a], [b]) => a.localeCompare(b))
.sort(([a], [b]) => compareCatalogNames(a, b))
.map(([blockType, iconRef]) => ` ${formatIconMapKey(blockType)}: ${iconRef.name},`)
.join('\n')
@@ -714,6 +714,7 @@ export function extractUserSettableParamIds(
blockName = 'block'
): string[] | null {
const scannable = blankStringsAndComments(blockContent)
if (scannable === null) return null
const keyMatch = /\bsubBlocks\s*:/.exec(scannable)
if (!keyMatch) return []
@@ -997,6 +998,7 @@ function collectShorthandPropertyNames(body: string, into: Set<string>): void {
*/
export function extractMapperWrittenParamIds(blockContent: string): string[] {
const scannable = blankStringsAndComments(blockContent)
if (scannable === null) return []
const ids = new Set<string>()
for (const [start, end] of findMapperBodyRanges(scannable)) {
@@ -1076,6 +1078,21 @@ export function extractBlockSuppliedParamIds(
blockContent: string,
blockName = 'block'
): BlockSuppliedParams {
/**
* A source the blanking scanner cannot get through it ends inside an unterminated string,
* template literal or comment makes both scans below report "nothing found" for the same
* reason. It is caught here so it is reported as a parse failure. Left to the branches below
* it would be indistinguishable from a spread-only `subBlocks` array whose block has no
* mapper, and the block's renames would be dropped without a word.
*/
if (blankStringsAndComments(blockContent) === null) {
return {
ids: null,
mapperIds: [],
parseError: `${blockName}: source ends inside an unterminated string, template literal or comment, so neither its subBlocks array nor its params mapper could be read`,
}
}
const mapperIds = extractMapperWrittenParamIds(blockContent)
try {
@@ -1254,26 +1271,322 @@ function extractAuthType(blockContent: string): 'oauth' | 'api-key' | 'none' {
}
/**
* Length-preserving copy of `content` with string-literal and comment
* interiors blanked out, so delimiter scans cannot be tripped by braces or
* quotes inside them. Indices into the result line up with indices into
* `content`.
* The catalog and every generated mapping are sorted with an explicit `en-US` collation.
* `localeCompare` with no locale uses the runtime default, which varies with `LANG` and the
* ICU build: `tr-TR`, `lt-LT`, `cs-CZ` and `et-EE` each reorder the real integration names, so
* a contributor on one of those locales would regenerate a different artifact and fail CI with
* no obvious cause.
*/
function blankStringsAndComments(content: string): string {
return content.replace(
/(['"`])(?:\\[\s\S]|(?!\1)[^\\])*\1|\/\/[^\n]*|\/\*[\s\S]*?\*\//g,
(match: string, quote: string | undefined) => {
const blanked = match.replace(/[^\n]/g, ' ')
/**
* A comment has no delimiters worth preserving, so it is blanked whole. Keeping
* its final character would leak arbitrary source text commented-out code ending
* in `[` or `{` leaves an unbalanced bracket that derails every scan downstream.
* A quoted string keeps its own quotes so callers can still see where it began.
*/
if (quote === undefined) return blanked
return quote + blanked.slice(1, -1) + quote
function compareCatalogNames(a: string, b: string): number {
return a.localeCompare(b, 'en-US')
}
/**
* Characters after which a `/` begins a regex literal rather than a division.
*
* `'\n'` is deliberate and load-bearing: a line-leading `/` is treated as opening a regex.
* A newline is recorded as the previous significant character rather than skipped, so this
* entry not the `(` before it is what decides a wrapped `value.match(` newline `/re/`.
* Prettier and Biome both emit a binary `/` at end-of-line, never at the start of the next
* one, so in this repo's formatted sources every line-leading `/` really is a regex, without
* exception `blocks/table.ts` and `blocks/table_v2.ts` both wrap a `.match(` argument this
* way, and removing `'\n'` makes them lex as division and silently mis-scan. It is a
* deliberate trade: a hand-wrapped `b` newline `/ c / d` would be blanked as a regex body,
* which no formatted file in this repo produces.
*/
const REGEX_ALLOWED_AFTER = new Set([
'(',
',',
'=',
':',
'[',
'!',
'&',
'|',
'?',
'{',
'}',
';',
'+',
'-',
'*',
'/',
'%',
'~',
'^',
'<',
'>',
'\n',
])
/**
* Keywords after which a `/` begins a regex literal rather than a division. In every one of
* these positions an operand is expected, so `return /x/`, `typeof /x/` or `case /x/` opens a
* regex a check on the previous character alone reads the keyword's last letter as an
* identifier and lexes the `/` as division.
*/
const REGEX_START_KEYWORDS = new Set([
'return',
'typeof',
'case',
'in',
'of',
'new',
'delete',
'void',
'instanceof',
'do',
'else',
'yield',
'await',
])
/**
* Whether the word immediately before `index` is a {@link REGEX_START_KEYWORDS} keyword.
* A property access (`counts.in / 2`) is excluded, since there the word is an identifier
* and the `/` really is a division.
*/
function precededByRegexStartKeyword(content: string, index: number): boolean {
let j = index - 1
while (j >= 0 && /\s/.test(content[j])) j--
const wordEnd = j + 1
while (j >= 0 && /[A-Za-z0-9_$]/.test(content[j])) j--
if (!REGEX_START_KEYWORDS.has(content.slice(j + 1, wordEnd))) return false
return content[j] !== '.' && content[j] !== '#'
}
/** Index just past a `//` comment opening at `start`. */
function scanLineComment(content: string, start: number): number {
const newline = content.indexOf('\n', start)
return newline === -1 ? content.length : newline
}
/** Index just past the block comment opening at `start`, or null when it never closes. */
function scanBlockComment(content: string, start: number): number | null {
const close = content.indexOf('*/', start + 2)
return close === -1 ? null : close + 2
}
/** Index just past a regex literal (and its flags) opening at `start`, or null when unterminated. */
function scanRegexLiteral(content: string, start: number): number | null {
let j = start + 1
let inClass = false
let closed = false
while (j < content.length) {
const c = content[j]
if (c === '\\') {
j += 2
continue
}
if (c === '\n') break
if (c === '[') inClass = true
else if (c === ']') inClass = false
else if (c === '/' && !inClass) {
closed = true
break
}
j++
}
if (!closed) return null
j++
while (j < content.length && /[a-z]/.test(content[j])) j++
return j
}
/** Index OF the closing quote of the string opening at `start`, or null when unterminated. */
function scanQuoted(content: string, start: number): number | null {
const quote = content[start]
let j = start + 1
while (j < content.length) {
if (content[j] === '\\') {
j += 2
continue
}
if (content[j] === '\n') break
if (content[j] === quote) return j
j++
}
return null
}
/** Index OF the closing backtick of the template literal opening at `start`, or null. */
function scanTemplateLiteral(content: string, start: number): number | null {
let j = start + 1
while (j < content.length) {
const c = content[j]
if (c === '\\') {
j += 2
continue
}
if (c === '`') return j
if (c === '$' && content[j + 1] === '{') {
const end = scanTemplateExpression(content, j + 2)
if (end === null) return null
j = end
continue
}
j++
}
return null
}
/**
* Index just past the `}` that closes the `${` expression starting at `start`, or null.
*
* The expression is lexed with the same primitives as top-level code, so a brace inside a
* nested string, comment, regex or template never counts toward the depth a plain counter
* loses the closing backtick of `` `${ f("}") }` `` and reports the whole block unreadable.
*/
function scanTemplateExpression(content: string, start: number): number | null {
let j = start
let depth = 0
let prevSignificant = ''
while (j < content.length) {
const c = content[j]
if (c === '/' && content[j + 1] === '/') {
j = scanLineComment(content, j)
continue
}
if (c === '/' && content[j + 1] === '*') {
const end = scanBlockComment(content, j)
if (end === null) return null
j = end
continue
}
if (c === '/' && startsRegexLiteral(content, j, prevSignificant)) {
const end = scanRegexLiteral(content, j)
if (end === null) return null
prevSignificant = ')'
j = end
continue
}
if (c === "'" || c === '"') {
const close = scanQuoted(content, j)
if (close === null) return null
prevSignificant = c
j = close + 1
continue
}
if (c === '`') {
const close = scanTemplateLiteral(content, j)
if (close === null) return null
prevSignificant = '`'
j = close + 1
continue
}
if (c === '{') depth++
else if (c === '}') {
if (depth === 0) return j + 1
depth--
}
if (!/\s/.test(c)) prevSignificant = c
else if (c === '\n') prevSignificant = '\n'
j++
}
return null
}
/**
* Whether the word immediately before `index` ends in a postfix `++` or `--`.
* {@link REGEX_ALLOWED_AFTER} holds `'+'` and `'-'` for the binary operators, but a postfix
* increment produces a value, so the `/` in `i++ / a` is a division. Only the two-character
* form is matched a single `+`/`-` stays an operator position.
*/
function precededByPostfixUpdate(content: string, index: number): boolean {
let j = index - 1
while (j >= 0 && /\s/.test(content[j])) j--
const c = content[j]
return (c === '+' || c === '-') && content[j - 1] === c
}
/** Whether the `/` at `index` opens a regex literal rather than a division. */
function startsRegexLiteral(content: string, index: number, prevSignificant: string): boolean {
if (
(prevSignificant === '+' || prevSignificant === '-') &&
precededByPostfixUpdate(content, index)
)
return false
return (
prevSignificant === '' ||
REGEX_ALLOWED_AFTER.has(prevSignificant) ||
precededByRegexStartKeyword(content, index)
)
}
/**
* Blank out string literals, template literals, comments and regex literals so a structural
* scan sees only code punctuation. Length and newlines are preserved, which the `readLiteral`
* index-mapping call sites depend on.
*
* Quoted strings keep their delimiters so callers can still see where one began; comments and
* regex literals are blanked whole, because their final character is arbitrary source text
* commented-out code ending in `[`, or a character class like `/[}]/`, otherwise leaves an
* unbalanced bracket that derails every downstream scan.
*
* Returns `null` when the scan ends inside an unterminated construct, which means the input
* was not what we assumed and no structural conclusion drawn from it can be trusted.
*/
function blankStringsAndComments(content: string): string | null {
const out = content.split('')
const blank = (start: number, end: number) => {
for (let k = start; k < end && k < out.length; k++) if (out[k] !== '\n') out[k] = ' '
}
let i = 0
let prevSignificant = ''
while (i < content.length) {
const char = content[i]
if (char === '/' && content[i + 1] === '/') {
const end = scanLineComment(content, i)
blank(i, end)
i = end
continue
}
if (char === '/' && content[i + 1] === '*') {
const end = scanBlockComment(content, i)
if (end === null) return null
blank(i, end)
i = end
continue
}
if (char === '/' && startsRegexLiteral(content, i, prevSignificant)) {
const end = scanRegexLiteral(content, i)
if (end === null) return null
blank(i, end)
prevSignificant = ')'
i = end
continue
}
if (char === "'" || char === '"') {
const close = scanQuoted(content, i)
if (close === null) return null
blank(i + 1, close)
prevSignificant = char
i = close + 1
continue
}
if (char === '`') {
const close = scanTemplateLiteral(content, i)
if (close === null) return null
blank(i + 1, close)
prevSignificant = '`'
i = close + 1
continue
}
if (!/\s/.test(char)) prevSignificant = char
else if (char === '\n') prevSignificant = '\n'
i++
}
return out.join('')
}
/**
@@ -1288,6 +1601,7 @@ function extractOAuthServiceId(blockContent: string): string | undefined {
if (!typeMatch) return undefined
const scannable = blankStringsAndComments(blockContent)
if (scannable === null) return undefined
let depth = 0
let objectStart = -1
for (let i = typeMatch.index; i >= 0; i--) {
@@ -1433,7 +1747,7 @@ function writeIntegrationsIconMapping(iconMapping: Record<string, IconRef>): voi
const imports = renderIconImports(Object.values(iconMapping))
const mappingEntries = Object.entries(iconMapping)
.sort(([a], [b]) => a.localeCompare(b))
.sort(([a], [b]) => compareCatalogNames(a, b))
.map(([blockType, iconRef]) => ` ${formatIconMapKey(blockType)}: ${iconRef.name},`)
.join('\n')
@@ -1608,7 +1922,7 @@ async function writeIntegrationsJson(iconMapping: Record<string, IconRef>): Prom
}
}
integrations.sort((a, b) => a.name.localeCompare(b.name))
integrations.sort((a, b) => compareCatalogNames(a.name, b.name))
const jsonPath = path.join(INTEGRATIONS_CATALOG_PATH, 'integrations.json')
// `JSON.stringify` always expands every array across multiple lines, but Biome's
@@ -1829,12 +2143,13 @@ function extractBlockConfigFromContent(
userSettableParamIds = null
} else if (supplied.ids === null) {
/**
* The block's `subBlocks` array holds only spreads of fields arrays this scanner cannot
* follow. A config-level spread base still contributes its readable fields, so the filter
* stays on against those plus the mapper's renames; with no base there is nothing to
* filter against and the filter is switched off. No warning: unlike the `parseError`
* cases the array itself parsed fine, and every field it names is documented through the
* spread source's own page.
* With `parseError` null, the only remaining cause is a `subBlocks` array holding just
* spreads of fields arrays this scanner cannot follow a source the scanner could not
* get through at all is reported as a `parseError` by `extractBlockSuppliedParamIds` and
* handled above. A config-level spread base still contributes its readable fields, so the
* filter stays on against those plus the mapper's renames; with no base there is nothing
* to filter against and the filter is switched off. No warning: the array itself parsed
* fine, and every field it names is documented through the spread source's own page.
*/
userSettableParamIds =
baseSettableParamIds.length > 0
@@ -4322,7 +4637,7 @@ function groupTriggersByProvider(
}
groups.set(
provider,
[...byName.values()].sort((a, b) => a.name.localeCompare(b.name))
[...byName.values()].sort((a, b) => compareCatalogNames(a.name, b.name))
)
}
return groups