feat: sync private provider updates (batch 1) - #222
Conversation
Summary by CodeRabbit
WalkthroughAdds Cartes.io, Heartbeat, Pod AI, Postalytics, Salesloft, TemplateFox, and TRIGGERcmd provider integrations with action schemas, API runtimes, credential validation, executor wiring, and proxy configuration. Extends DataForSEO with asynchronous Amazon task actions and normalization. Expands Shopify with blog article tag and author actions while narrowing page-count filters. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
|
按维护者安排，改为本地合并整理，关闭此拆分 PR。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
src/providers/cartes/runtime.ts (1)
420-439: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
is_spamfrommarkerBody.
create_marker's input schema (actions.tslines 142-161) has noisSpamproperty, so this is alwaysundefinedand stripped bycompactObject.markerUpdateBodyalready owns the spam flag.♻️ Proposed refactor
roll: input.roll, speed: input.speed, - is_spam: input.isSpam, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/cartes/runtime.ts` around lines 420 - 439, Remove the is_spam property from the object returned by markerBody, since create_marker does not provide isSpam and markerUpdateBody owns the spam flag. Leave the remaining marker fields unchanged.src/providers/cartes/executors.ts (1)
16-19: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
customizeRequestclobbers caller-suppliedcontent-type.
defineProviderProxyrunscustomizeRequestafter normalizing caller headers (provider-runtime.tslines 388-393), so a proxy caller sendingtext/csvormultipart/form-datagets overwritten withapplication/json. Prefer setting it only when absent.♻️ Proposed refactor
customizeRequest({ headers }) { headers.set("accept", "application/json"); - headers.set("content-type", "application/json"); + if (!headers.has("content-type")) { + headers.set("content-type", "application/json"); + } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/cartes/executors.ts` around lines 16 - 19, Update customizeRequest in defineProviderProxy so the default content-type is set only when the caller has not supplied one, preserving caller-provided values such as text/csv or multipart/form-data. Keep the existing application/json default and accept-header behavior unchanged.src/providers/cartes/actions.ts (1)
194-196: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist heading/pitch/roll schemas like the neighbouring coordinate constants.
These three are byte-identical to lines 136-138; the file already hoists
latitudeSchema,zoomSchema,speedSchema, etc., so keeping these inline is inconsistent and invites drift.♻️ Proposed refactor
+const headingSchema = s.number("Heading in degrees.", { minimum: 0, maximum: 360 }); +const pitchSchema = s.number("Pitch in degrees.", { minimum: -90, maximum: 90 }); +const rollSchema = s.number("Roll in degrees.", { minimum: -180, maximum: 180 });- heading: s.number("Heading in degrees.", { minimum: 0, maximum: 360 }), - pitch: s.number("Pitch in degrees.", { minimum: -90, maximum: 90 }), - roll: s.number("Roll in degrees.", { minimum: -180, maximum: 180 }), + heading: headingSchema, + pitch: pitchSchema, + roll: rollSchema,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/cartes/actions.ts` around lines 194 - 196, Hoist the heading, pitch, and roll schemas into reusable constants alongside the existing coordinate schema constants, then replace the inline definitions in the affected action schema with those constants. Reuse the existing schema descriptions and bounds exactly, and avoid changing validation behavior.src/providers/templatefox/runtime.ts (2)
225-235: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the request options into a named interface.
The inline object type spans ten lines and is reused by every handler; a
TemplatefoxRequestOptionsinterface reads better and documents the contract.As per coding guidelines, "Prefer named options/input interfaces over inline object types when a function signature spans multiple lines or crosses module boundaries."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/templatefox/runtime.ts` around lines 225 - 235, Define a named TemplatefoxRequestOptions interface containing the fields currently declared in the inline input object for requestTemplatefoxJson, then update that function signature to accept the interface while preserving all field types and optionality.Source: Coding guidelines
238-251: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAbort/cancellation is reclassified as an upstream 502.
When
context.signalfires, theAbortErroris caught and reported astemplatefox request failed, hiding client cancellation and timeouts behind a provider-failure status.♻️ Rethrow aborts
} catch (error) { + if (error instanceof Error && error.name === "AbortError") { + throw error; + } throw new ProviderRequestError(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/templatefox/runtime.ts` around lines 238 - 251, Update the catch block around the fetch and readTemplatefoxPayload calls to detect abort/cancellation errors associated with input.signal and rethrow them unchanged. Only wrap non-abort failures in ProviderRequestError with the existing 502 behavior and message.src/providers/templatefox/actions.ts (1)
83-83: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
pdfUrl/pdfUrlsare user-supplied fetch targets with no public-URL enforcement.The description promises a public HTTPS URL, but
s.urlalone won't rejecthttp://, loopback, or metadata hosts. Validate these in the runtime handlers with the sharedassertPublicHttpUrlbefore building the request body.As per coding guidelines, "User-supplied content or download URLs must always use public-only
assertPublicHttpUrland public-onlyproviderFetch" and "Prefer sharedassertPublicHttpUrlandisBlockedIpAddressover bespoke provider hostname guards".Also applies to: 180-180, 193-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/templatefox/actions.ts` at line 83, Update the runtime handlers for the pdfUrl/pdfUrls inputs to call the shared assertPublicHttpUrl validation before constructing each TemplateFox request body, covering the paths around the relevant handlers near lines 83, 180, and 193. Ensure all resulting outbound requests use the public-only providerFetch, and do not add bespoke hostname or IP checks.Source: Coding guidelines
src/providers/heartbeat/actions.ts (1)
148-154: 📐 Maintainability & Code Quality | 🔵 TrivialInconsistent schema builder for
list_eventsinput.Every other action in this file builds its input with
s.actionInput({...}, [requiredKeys], description), butlist_eventsusess.object(...)with an{ optional: [...] }config instead. Confirm this still satisfies theActionDefinition.inputSchemacontract expected bydefineProviderAction(e.g. any action-input-specific metadatas.actionInputadds thats.objectmight not). If equivalent, consider switching tos.actionInput({ groupId: s.uuid(...) }, [], "...")for consistency with the rest of the file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/heartbeat/actions.ts` around lines 148 - 154, Update the list_events inputSchema to use s.actionInput with groupId as the schema field, no required keys, and the existing description. Preserve groupId as optional and ensure the result remains compatible with the ActionDefinition.inputSchema contract used by defineProviderAction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/cartes/runtime.ts`:
- Around line 392-402: Update normalizeSuccess to treat a valid response body
without a literal success field as successful, preserving an explicit success:
false value when the API provides it. Keep the existing fallback for non-record
payloads and continue returning the raw record.
In `@src/providers/heartbeat/executors.ts`:
- Around line 22-102: Update the Heartbeat handlers in the executor object to
unwrap each requestHeartbeatJson response before passing it to
requireResourceArray or requireResourceObject, using the appropriate payload
property for users, groups, channels, events, and user/group lookups. Apply the
same extraction in the /channels credential-check path so wrapped valid
responses are validated correctly.
In `@src/providers/salesloft/runtime.ts`:
- Around line 156-179: Update requestSalesloft to create and apply a
provider-level timeout signal in addition to input.signal, matching the timeout
pattern used by sibling providers. Ensure the timeout resource is cleaned up
after the fetch completes while preserving the existing request, response
parsing, and error mapping behavior.
In `@src/providers/templatefox/runtime.ts`:
- Around line 393-403: Update normalizeTemplateField to read help text from the
provider’s snake_case help_text key while preserving support for the existing
camelCase helpText key, using the established nullable string parsing behavior.
- Around line 300-301: Update the error-message fallback in the runtime request
handling to treat an empty response.statusText as missing, allowing the
descriptive status-based template message to be used. Preserve
readTemplatefoxErrorMessage(payload) as the highest-priority message and keep
the existing statusText behavior for non-empty values.
- Around line 420-439: Update readOptionalModifications and readPdfUrls to use
an input-side array reader that raises the caller-facing 400 validation error
for non-array values, rather than readArray’s 502 response-error handling.
Preserve the existing undefined handling for optional modifications and retain
the current item-level validation and mapping behavior.
In `@src/providers/triggercmd/runtime.ts`:
- Line 11: Update the listCommandsPath constant used by list_commands and
credential validation to the account-wide /api/command/commandlist endpoint
instead of the per-computer /api/command/list path, preserving both callers’
existing behavior.
---
Nitpick comments:
In `@src/providers/cartes/actions.ts`:
- Around line 194-196: Hoist the heading, pitch, and roll schemas into reusable
constants alongside the existing coordinate schema constants, then replace the
inline definitions in the affected action schema with those constants. Reuse the
existing schema descriptions and bounds exactly, and avoid changing validation
behavior.
In `@src/providers/cartes/executors.ts`:
- Around line 16-19: Update customizeRequest in defineProviderProxy so the
default content-type is set only when the caller has not supplied one,
preserving caller-provided values such as text/csv or multipart/form-data. Keep
the existing application/json default and accept-header behavior unchanged.
In `@src/providers/cartes/runtime.ts`:
- Around line 420-439: Remove the is_spam property from the object returned by
markerBody, since create_marker does not provide isSpam and markerUpdateBody
owns the spam flag. Leave the remaining marker fields unchanged.
In `@src/providers/heartbeat/actions.ts`:
- Around line 148-154: Update the list_events inputSchema to use s.actionInput
with groupId as the schema field, no required keys, and the existing
description. Preserve groupId as optional and ensure the result remains
compatible with the ActionDefinition.inputSchema contract used by
defineProviderAction.
In `@src/providers/templatefox/actions.ts`:
- Line 83: Update the runtime handlers for the pdfUrl/pdfUrls inputs to call the
shared assertPublicHttpUrl validation before constructing each TemplateFox
request body, covering the paths around the relevant handlers near lines 83,
180, and 193. Ensure all resulting outbound requests use the public-only
providerFetch, and do not add bespoke hostname or IP checks.
In `@src/providers/templatefox/runtime.ts`:
- Around line 225-235: Define a named TemplatefoxRequestOptions interface
containing the fields currently declared in the inline input object for
requestTemplatefoxJson, then update that function signature to accept the
interface while preserving all field types and optionality.
- Around line 238-251: Update the catch block around the fetch and
readTemplatefoxPayload calls to detect abort/cancellation errors associated with
input.signal and rethrow them unchanged. Only wrap non-abort failures in
ProviderRequestError with the existing 502 behavior and message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c9d9138-ad86-4b9e-94b4-ace42286d58b
📒 Files selected for processing (33)
src/providers/cartes/actions.tssrc/providers/cartes/definition.tssrc/providers/cartes/executors.tssrc/providers/cartes/runtime.tssrc/providers/dataforseo/actions.tssrc/providers/dataforseo/runtime.tssrc/providers/heartbeat/actions.tssrc/providers/heartbeat/definition.tssrc/providers/heartbeat/executors.tssrc/providers/pod_ai/actions.tssrc/providers/pod_ai/definition.tssrc/providers/pod_ai/executors.tssrc/providers/pod_ai/runtime.tssrc/providers/postalytics/actions.tssrc/providers/postalytics/definition.tssrc/providers/postalytics/executors.tssrc/providers/postalytics/runtime.tssrc/providers/qdrant/runtime.test.tssrc/providers/salesloft/actions.tssrc/providers/salesloft/definition.tssrc/providers/salesloft/executors.tssrc/providers/salesloft/runtime.tssrc/providers/shopify/actions.tssrc/providers/shopify/definition.tssrc/providers/shopify/runtime.tssrc/providers/templatefox/actions.tssrc/providers/templatefox/definition.tssrc/providers/templatefox/executors.tssrc/providers/templatefox/runtime.tssrc/providers/triggercmd/actions.tssrc/providers/triggercmd/definition.tssrc/providers/triggercmd/executors.tssrc/providers/triggercmd/runtime.ts
💤 Files with no reviewable changes (1)
- src/providers/qdrant/runtime.test.ts
| function normalizeSuccess(payload: unknown): Record<string, unknown> { | ||
| const record = optionalRecord(payload); | ||
| if (!record) { | ||
| return { success: true }; | ||
| } | ||
|
|
||
| return { | ||
| success: record.success === true, | ||
| raw: record, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
normalizeSuccess can report success: false for a successful delete.
This runs only after response.ok passed (line 309). If Cartes.io returns a 2xx body without a literal success: true field (e.g. {"message":"Deleted"}), the action reports success: false while the resource was actually deleted — callers may retry a destructive op.
🐛 Proposed fix
function normalizeSuccess(payload: unknown): Record<string, unknown> {
const record = optionalRecord(payload);
if (!record) {
return { success: true };
}
return {
- success: record.success === true,
+ success: record.success !== false,
raw: record,
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function normalizeSuccess(payload: unknown): Record<string, unknown> { | |
| const record = optionalRecord(payload); | |
| if (!record) { | |
| return { success: true }; | |
| } | |
| return { | |
| success: record.success === true, | |
| raw: record, | |
| }; | |
| } | |
| function normalizeSuccess(payload: unknown): Record<string, unknown> { | |
| const record = optionalRecord(payload); | |
| if (!record) { | |
| return { success: true }; | |
| } | |
| return { | |
| success: record.success !== false, | |
| raw: record, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/cartes/runtime.ts` around lines 392 - 402, Update
normalizeSuccess to treat a valid response body without a literal success field
as successful, preserving an explicit success: false value when the API provides
it. Keep the existing fallback for non-record payloads and continue returning
the raw record.
| async list_users(_input, context) { | ||
| return { | ||
| users: requireResourceArray( | ||
| await requestHeartbeatJson({ context, path: "/users", phase: "execute" }), | ||
| "Heartbeat users response", | ||
| ), | ||
| }; | ||
| }, | ||
| async get_user(input, context) { | ||
| const userId = requiredString(input.userId, "userId", providerInputError); | ||
| return { | ||
| user: requireResourceObject( | ||
| await requestHeartbeatJson({ | ||
| context, | ||
| path: `/users/${encodeURIComponent(userId)}`, | ||
| phase: "execute", | ||
| }), | ||
| "Heartbeat user response", | ||
| ), | ||
| }; | ||
| }, | ||
| async find_users_by_email(input, context) { | ||
| return { | ||
| users: requireResourceArray( | ||
| await requestHeartbeatJson({ | ||
| context, | ||
| path: "/find/users", | ||
| phase: "execute", | ||
| query: { | ||
| email: requiredString(input.email, "email", providerInputError), | ||
| }, | ||
| }), | ||
| "Heartbeat user search response", | ||
| ), | ||
| }; | ||
| }, | ||
| async list_groups(_input, context) { | ||
| return { | ||
| groups: requireResourceArray( | ||
| await requestHeartbeatJson({ context, path: "/groups", phase: "execute" }), | ||
| "Heartbeat groups response", | ||
| ), | ||
| }; | ||
| }, | ||
| async get_group(input, context) { | ||
| const groupId = requiredString(input.groupId, "groupId", providerInputError); | ||
| return { | ||
| group: requireResourceObject( | ||
| await requestHeartbeatJson({ | ||
| context, | ||
| path: `/groups/${encodeURIComponent(groupId)}`, | ||
| phase: "execute", | ||
| }), | ||
| "Heartbeat group response", | ||
| ), | ||
| }; | ||
| }, | ||
| async list_channels(_input, context) { | ||
| return { | ||
| channels: requireResourceArray( | ||
| await requestHeartbeatJson({ context, path: "/channels", phase: "execute" }), | ||
| "Heartbeat channels response", | ||
| ), | ||
| }; | ||
| }, | ||
| async list_events(input, context) { | ||
| return { | ||
| events: requireResourceArray( | ||
| await requestHeartbeatJson({ | ||
| context, | ||
| path: "/events", | ||
| phase: "execute", | ||
| query: { | ||
| groupID: optionalString(input.groupId), | ||
| }, | ||
| }), | ||
| "Heartbeat events response", | ||
| ), | ||
| }; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
What does the Heartbeat chat API (api.heartbeat.chat/v0) return for GET /users, /groups, /channels — a raw array or a wrapped object?
💡 Result:
The Heartbeat chat API (api.heartbeat.chat/v0) endpoints for /users, /groups, and /channels return a wrapped object rather than a raw array. While the official API documentation reference pages for these endpoints [1][2][3] currently display loading states for their example responses, common patterns in the Heartbeat API implementation—such as those observed in third-party integrations [4] and the use of cursor-based pagination for other collection endpoints [5]—indicate that these responses are wrapped objects. These objects typically contain a data field (or similar) holding the requested list, often accompanied by metadata required for pagination, such as cursors [4][5].
Citations:
- 1: https://heartbeat.readme.io/reference/getchannels
- 2: https://heartbeat.readme.io/reference/getusers
- 3: https://heartbeat.readme.io/reference/getgroups
- 4: https://docs.composio.dev/toolkits/heartbeat.md
- 5: https://docs.pinkfish.ai/api-reference/mcp-servers/application/heartbeat/heartbeat-content
Unwrap Heartbeat responses before validating them
requestHeartbeatJson is passed straight into requireResourceArray/requireResourceObject across these handlers, but Heartbeat collection endpoints return wrapped payloads. These calls will reject valid responses unless the list/object is extracted first. This also affects the /channels credential check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/heartbeat/executors.ts` around lines 22 - 102, Update the
Heartbeat handlers in the executor object to unwrap each requestHeartbeatJson
response before passing it to requireResourceArray or requireResourceObject,
using the appropriate payload property for users, groups, channels, events, and
user/group lookups. Apply the same extraction in the /channels credential-check
path so wrapped valid responses are validated correctly.
| async function requestSalesloft(input: SalesloftRequestInput): Promise<SalesloftPayload> { | ||
| const url = new URL(`${salesloftApiBaseUrl}${input.path}`); | ||
| if (input.query) { | ||
| appendQueryObject(url, input.query); | ||
| } | ||
|
|
||
| const response = await input.fetcher(url, { | ||
| method: "GET", | ||
| headers: { | ||
| accept: "application/json", | ||
| authorization: `Bearer ${input.apiKey}`, | ||
| "user-agent": providerUserAgent, | ||
| }, | ||
| signal: input.signal, | ||
| }); | ||
|
|
||
| const rawBody = await response.text(); | ||
| const payload = parseSalesloftJson(rawBody, response.status); | ||
| if (!response.ok) { | ||
| throw mapSalesloftError(response.status, payload, rawBody); | ||
| } | ||
|
|
||
| return payload; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether context.signal already carries an enforced timeout centrally,
# and whether other providers implement it locally at the transport layer.
rg -n -A5 -B5 'AbortController|setTimeout|signal:' src/providers/provider-runtime.ts
rg -n -A5 -B5 'AbortController|setTimeout' src/providers/heartbeat/executors.ts src/providers/postalytics/runtime.ts src/providers/triggercmd/runtime.tsRepository: oomol-lab/open-connector
Length of output: 3850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== provider-runtime timeout plumbing ==\n'
rg -n -A8 -B8 'createProviderTimeout|signal:' src/providers/provider-runtime.ts
printf '\n== defineApiKeyProviderExecutors / context wiring ==\n'
rg -n -A10 -B10 'defineApiKeyProviderExecutors|createContext\(' src/providers/provider-runtime.ts src/providers/salesloft/runtime.ts
printf '\n== Salesloft transport ==\n'
sed -n '1,260p' src/providers/salesloft/runtime.tsRepository: oomol-lab/open-connector
Length of output: 20305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== executor creation path ==\n'
rg -n -A20 -B20 'createProviderTimeout\(|ProviderTimeout|executionContext\.signal|signal: context\.signal|signal: executionContext\.signal' src/providers/provider-runtime.ts
printf '\n== executor entrypoint around defineProviderExecutors ==\n'
sed -n '720,820p' src/providers/provider-runtime.ts
printf '\n== salesloft runtime tail for any timeout handling ==\n'
sed -n '260,420p' src/providers/salesloft/runtime.tsRepository: oomol-lab/open-connector
Length of output: 14206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== action execution context definitions ==\n'
rg -n -A6 -B6 'interface .*ExecutionContext|type .*ExecutionContext|signal: AbortSignal|AbortSignal' src | sed -n '1,240p'
printf '\n== executor runtime signal creation ==\n'
rg -n -A10 -B10 'new AbortController|AbortController\(|timeout|signal:' src | sed -n '1,260p'Repository: oomol-lab/open-connector
Length of output: 34128
Add a request timeout to Salesloft transport
requestSalesloft only forwards context.signal; unlike the sibling providers in this stack, it doesn’t add its own timeout, so a slow Salesloft call can stall until the parent signal aborts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/salesloft/runtime.ts` around lines 156 - 179, Update
requestSalesloft to create and apply a provider-level timeout signal in addition
to input.signal, matching the timeout pattern used by sibling providers. Ensure
the timeout resource is cleaned up after the fetch completes while preserving
the existing request, response parsing, and error mapping behavior.
| const message = | ||
| readTemplatefoxErrorMessage(payload) ?? response.statusText ?? `templatefox request failed with ${response.status}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Empty statusText defeats the fallback message.
response.statusText is always a string (commonly "" over HTTP/2), so ?? never reaches the final template and callers can receive a blank error message.
🐛 Proposed fix
- const message =
- readTemplatefoxErrorMessage(payload) ?? response.statusText ?? `templatefox request failed with ${response.status}`;
+ const message =
+ readTemplatefoxErrorMessage(payload) ||
+ response.statusText ||
+ `templatefox request failed with ${response.status}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const message = | |
| readTemplatefoxErrorMessage(payload) ?? response.statusText ?? `templatefox request failed with ${response.status}`; | |
| const message = | |
| readTemplatefoxErrorMessage(payload) || | |
| response.statusText || | |
| `templatefox request failed with ${response.status}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/templatefox/runtime.ts` around lines 300 - 301, Update the
error-message fallback in the runtime request handling to treat an empty
response.statusText as missing, allowing the descriptive status-based template
message to be used. Preserve readTemplatefoxErrorMessage(payload) as the
highest-priority message and keep the existing statusText behavior for non-empty
values.
| function normalizeTemplateField(record: Record<string, unknown>): Record<string, unknown> { | ||
| return { | ||
| ...record, | ||
| key: readRequiredResponseString(record.key, "field.key"), | ||
| label: readRequiredResponseString(record.label, "field.label"), | ||
| type: optionalRawString(record.type) ?? "string", | ||
| required: optionalBoolean(record.required) ?? false, | ||
| helpText: readNullableRawString(record.helpText), | ||
| spec: readNullableFieldSpec(record.spec), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
helpText likely never populates.
The rest of this file reads snake_case provider keys (credits_remaining, created_at, expires_in), and normalizeTemplate accepts both casings, but here only record.helpText is read — so provider help_text is dropped and the field is always null.
🐛 Proposed fix
- helpText: readNullableRawString(record.helpText),
+ helpText: readNullableRawString(record.help_text ?? record.helpText),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function normalizeTemplateField(record: Record<string, unknown>): Record<string, unknown> { | |
| return { | |
| ...record, | |
| key: readRequiredResponseString(record.key, "field.key"), | |
| label: readRequiredResponseString(record.label, "field.label"), | |
| type: optionalRawString(record.type) ?? "string", | |
| required: optionalBoolean(record.required) ?? false, | |
| helpText: readNullableRawString(record.helpText), | |
| spec: readNullableFieldSpec(record.spec), | |
| }; | |
| } | |
| function normalizeTemplateField(record: Record<string, unknown>): Record<string, unknown> { | |
| return { | |
| ...record, | |
| key: readRequiredResponseString(record.key, "field.key"), | |
| label: readRequiredResponseString(record.label, "field.label"), | |
| type: optionalRawString(record.type) ?? "string", | |
| required: optionalBoolean(record.required) ?? false, | |
| helpText: readNullableRawString(record.help_text ?? record.helpText), | |
| spec: readNullableFieldSpec(record.spec), | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/templatefox/runtime.ts` around lines 393 - 403, Update
normalizeTemplateField to read help text from the provider’s snake_case
help_text key while preserving support for the existing camelCase helpText key,
using the established nullable string parsing behavior.
| function readOptionalModifications(value: unknown): Array<Record<string, unknown>> | undefined { | ||
| if (value === undefined) { | ||
| return undefined; | ||
| } | ||
| return readArray(value, "modifications").map((item) => { | ||
| const record = readInputObject(item, "modification"); | ||
| return compactObject({ | ||
| name: readRequiredInputString(record.name, "modification.name"), | ||
| text: optionalRawString(record.text), | ||
| image_url: optionalRawString(record.imageUrl), | ||
| color: optionalRawString(record.color), | ||
| background: optionalRawString(record.background), | ||
| hidden: optionalBoolean(record.hidden), | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function readPdfUrls(value: unknown): string[] { | ||
| return readArray(value, "pdfUrls").map((item, index) => readRequiredInputString(item, `pdfUrls[${index}]`)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Client input validated with the response-error helper.
readArray throws ProviderRequestError(502, "malformed templatefox response: ..."). Non-array modifications or pdfUrls is a caller error, so this reports a bogus upstream failure with a misleading message instead of a 400.
🐛 Add an input-side array reader
+function readInputArray(value: unknown, fieldName: string): unknown[] {
+ if (!Array.isArray(value)) {
+ throw new ProviderRequestError(400, `${fieldName} must be an array`);
+ }
+ return value;
+}
+
function readOptionalModifications(value: unknown): Array<Record<string, unknown>> | undefined {
if (value === undefined) {
return undefined;
}
- return readArray(value, "modifications").map((item) => {
+ return readInputArray(value, "modifications").map((item) => { function readPdfUrls(value: unknown): string[] {
- return readArray(value, "pdfUrls").map((item, index) => readRequiredInputString(item, `pdfUrls[${index}]`));
+ return readInputArray(value, "pdfUrls").map((item, index) => readRequiredInputString(item, `pdfUrls[${index}]`));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function readOptionalModifications(value: unknown): Array<Record<string, unknown>> | undefined { | |
| if (value === undefined) { | |
| return undefined; | |
| } | |
| return readArray(value, "modifications").map((item) => { | |
| const record = readInputObject(item, "modification"); | |
| return compactObject({ | |
| name: readRequiredInputString(record.name, "modification.name"), | |
| text: optionalRawString(record.text), | |
| image_url: optionalRawString(record.imageUrl), | |
| color: optionalRawString(record.color), | |
| background: optionalRawString(record.background), | |
| hidden: optionalBoolean(record.hidden), | |
| }); | |
| }); | |
| } | |
| function readPdfUrls(value: unknown): string[] { | |
| return readArray(value, "pdfUrls").map((item, index) => readRequiredInputString(item, `pdfUrls[${index}]`)); | |
| } | |
| function readInputArray(value: unknown, fieldName: string): unknown[] { | |
| if (!Array.isArray(value)) { | |
| throw new ProviderRequestError(400, `${fieldName} must be an array`); | |
| } | |
| return value; | |
| } | |
| function readOptionalModifications(value: unknown): Array<Record<string, unknown>> | undefined { | |
| if (value === undefined) { | |
| return undefined; | |
| } | |
| return readInputArray(value, "modifications").map((item) => { | |
| const record = readInputObject(item, "modification"); | |
| return compactObject({ | |
| name: readRequiredInputString(record.name, "modification.name"), | |
| text: optionalRawString(record.text), | |
| image_url: optionalRawString(record.imageUrl), | |
| color: optionalRawString(record.color), | |
| background: optionalRawString(record.background), | |
| hidden: optionalBoolean(record.hidden), | |
| }); | |
| }); | |
| } | |
| function readPdfUrls(value: unknown): string[] { | |
| return readInputArray(value, "pdfUrls").map((item, index) => readRequiredInputString(item, `pdfUrls[${index}]`)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/templatefox/runtime.ts` around lines 420 - 439, Update
readOptionalModifications and readPdfUrls to use an input-side array reader that
raises the caller-facing 400 validation error for non-array values, rather than
readArray’s 502 response-error handling. Preserve the existing undefined
handling for optional modifications and retain the current item-level validation
and mapping behavior.
| export const triggercmdApiBaseUrl = "https://www.triggercmd.com"; | ||
|
|
||
| const triggercmdRequestTimeoutMs = 30_000; | ||
| const listCommandsPath = "/api/command/list"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced file around the constant and its callers.
sed -n '1,220p' src/providers/triggercmd/runtime.ts
# Search for the endpoint constant and any related path usage.
rg -n 'listCommandsPath|command/list|commandlist|computer_id' src/providers/triggercmd -SRepository: oomol-lab/open-connector
Length of output: 6080
🌐 Web query:
TRIGGERcmd API list commands endpoint commandlist computer_id documentation
💡 Result:
To list commands via the TRIGGERcmd API, you can use two primary endpoints depending on whether you want to list all commands or commands specific to a single computer [1][2]. For all commands across all computers: Endpoint: GET https://www.triggercmd.com/api/command/commandlist Header: Authorization: Bearer (your token) Content-Type: application/json This endpoint returns a list of all commands for your account. Each command typically includes a did field, which combines the computer_id and command_id [1][2]. For commands on a specific computer: Endpoint: POST https://www.triggercmd.com/api/command/list Header: Authorization: Bearer (your token) Header: Content-Type: application/json Body: {"computer_id": "YOUR_COMPUTER_ID"} This endpoint allows you to retrieve only the commands associated with the specified computer_id [1][2]. To obtain your authorization token, check the Instructions page within your TRIGGERcmd account portal [3]. You can also retrieve a list of your computer IDs using the endpoint GET https://www.triggercmd.com/api/computer/list [1][2].
Citations:
- 1: https://www.triggercmd.com/forum/topic/370/install-from-github-repo/13
- 2: https://www.triggercmd.com/forum/topic/370/install-from-github-repo/13?page=2
- 3: https://www.triggercmd.com/forum/topic/59/api-call-to-list-your-commands-on-each-computer
Use the account-wide command list endpoint.
/api/command/list is the per-computer endpoint and requires computer_id; both callers omit it. Switch listCommandsPath to /api/command/commandlist so list_commands and credential validation can return the full account command list.
Proposed fix
-const listCommandsPath = "/api/command/list";
+const listCommandsPath = "/api/command/commandlist";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const listCommandsPath = "/api/command/list"; | |
| const listCommandsPath = "/api/command/commandlist"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/triggercmd/runtime.ts` at line 11, Update the listCommandsPath
constant used by list_commands and credential validation to the account-wide
/api/command/commandlist endpoint instead of the per-computer /api/command/list
path, preserving both callers’ existing behavior.
1 participant
概要
同步第一批 10 个 provider 边界变更：
边界
验证
npm run generate:catalognpm run fix-checknpm run buildgit diff --check origin/main...HEAD未运行全量测试；纯 provider 移植按仓库规则使用 catalog、typecheck/build 与逐 provider probes 验证。