feat(ashby): incremental job sync, custom field writes, and application lifecycle ops - #6703
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Incremental sync: Writes & safety: Custom fields are the supported way to annotate jobs/reqs; Docs ( Reviewed by Cursor Bugbot for commit 2deb0e1. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThe PR extends the Ashby integration with incremental job synchronization, custom-field writes, application lifecycle operations, richer error extraction, and corresponding block, registry, generated metadata, documentation, and test updates.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the reviewed fixes. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/blocks/blocks/ashby.ts | Registers the new Ashby operations and maps editor inputs into typed tool parameters, including explicit source-clearing behavior. |
| apps/sim/tools/ashby/list_jobs.ts | Adds incremental synchronization input and maps Ashby's terminal sync token to a readable cursor output. |
| apps/sim/tools/ashby/change_application_source.ts | Adds source-setting and explicit source-clearing requests with mutual-exclusivity validation. |
| apps/sim/tools/ashby/set_custom_field_value.ts | Adds guarded single-field mutation while preserving explicit null as a clearing operation. |
| apps/sim/tools/ashby/set_custom_field_values.ts | Adds non-empty batched custom-field mutations and returns the resulting field values. |
| apps/sim/tools/ashby/utils.ts | Improves structured Ashby error rendering and provides narrow custom-field value parsing. |
| apps/sim/tools/ashby/ashby.test.ts | Covers new request shapes, destructive-write intent validation, response transformations, and error paths. |
| apps/docs/content/docs/en/integrations/ashby.mdx | Documents the expanded operation surface, permission requirements, incremental cursor behavior, and API limitations. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Workflow[Workflow or agent] --> AshbyBlock[Ashby block]
AshbyBlock --> Params[Operation parameter mapping]
Params --> Tool[Ashby tool]
Tool --> API[Ashby API]
API --> Tool
Tool --> Output[Normalized block output]
Output --> Cursor[nextSyncCursor for later scheduled sync]
Output --> Resources[Jobs, postings, applications, candidates, or custom fields]
Reviews (5): Last reviewed commit: "fix(blocks): stop a stale create-path so..." | Re-trigger Greptile
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a2f9156. Configure here.
list_jobs accepts Ashby's syncToken and returns it as nextSyncCursor, so a scheduled sync costs O(changed reqs) instead of rescanning every req. Ashby only returns the token once the last page is drained, which the param description states. The output is named as a cursor deliberately. It is an opaque resumption marker, not a credential, so it belongs with nextCursor - and a field literally named syncToken matches the /^.*token$/i deny-list in redaction and renders as [REDACTED], which makes an incremental sync unusable since the operator cannot read the value the next run needs. The wire name stays syncToken. list_job_postings gains includeUnpublishedJobPostings, plus the posting status field - without status a caller cannot tell a returned draft from a published posting, which makes the flag useless. Also widens the custom field valueLabel type, which MultiValueSelect returns as an array, for the write operations that follow.
Ashby documents two error shapes and uses both. The `errors` array form carries
`{ message, parameter }` objects, which stringified to '[object Object]' and hid
the real cause - including the 403 a key gets when it lacks a module permission.
Also adds the shared pieces the new write operations need: one definition of the
custom field value shape for the read and write paths to agree on, and a
normalizer for Ashby's case-sensitive objectType enum so a model emitting
'candidate' fails here with the allowed values rather than at the API.
…mize customField.setValue/setValues are the only way to annotate a job or req, since Ashby has no job notes and no job tags. Writing null clears a value, so the annotation is reversible. Because null clears, every one of these operations requires explicit intent before it can destroy data. The block's required markers do not cover the agent path - a model calls the tool directly, so tools.config.params never runs and validateRequiredParametersAfterMerge skips a param marked not-required: - set_custom_field_value rejects an absent or blank fieldValue; an explicit null still clears - change_application_source requires unsetSource to clear, and rejects a source id and an unset request together, since preferring either one silently discards the other. Ashby has no 'leave unchanged' mode, so setting and clearing are the only two intents and exactly one must be expressed - set_custom_field_values rejects an empty array locally rather than relying on Ashby to reject it application.delete needs candidatesDelete, a module permission separate from candidatesWrite. candidate.anonymize strips PII but leaves the record; Ashby exposes no candidate deletion endpoint.
Includes a gated live harness (ASHBY_LIVE=1) alongside the mocked tests. vitest.setup.ts stubs global fetch for every file in the app, so the live file restores the real implementation and asserts the restore worked - without that guard the whole suite silently passes against a mock.
fieldValue is polymorphic (boolean, number, string, array, object, null), so it
decodes structured input and otherwise passes text through. The decoding is
deliberately narrow rather than a blanket JSON.parse, which corrupts real text:
1e999 becomes Infinity and serializes back out as null, which CLEARS the field;
a long numeric id loses precision past 2^53; and prose starting with { turns into
an object. Only the literal keywords, {, [ or " prefixes, and exactly
round-tripping numbers decode.
fieldValue carries no wand generationType: json-object forces braces and
json-array forces brackets, and both would wrap a value that must stay bare.
fieldValues, whose contract really is an array, uses json-array.
Setting and clearing an application source are mutually exclusive, so the Source
ID field is conditioned off while the clear switch is on and the params mapping
sends only the intent the switch selects. A value typed before the switch was
flipped cannot reach the tool and surface as an error with no visible cause.
Ashby scopes permissions per module and they fail at runtime, not build time, so the block docs now carry the permission table. Also records the hard API limits worth designing around: no note or tag on a job, no pagination on jobPosting.list, and no delete for jobs, candidates, or custom field definitions.
a2f9156 to
734a8b6
Compare
|
@cursor review |
… change
The executor merges { ...inputs, ...transformedParams }, so any key the params
mapping leaves unset inherits whatever inputs held. The shared create-path
sourceId subblock reaches inputs even on change_application_source: it is mode
'advanced', and the serializer includes an advanced subblock whenever its value
is non-empty without ever evaluating its condition (serializer/index.ts).
So a source id typed while on Create Application survived into a source change.
With both fields blank it silently attributed a source nobody asked for, and
with the clear switch on it collided with the unset request and failed with no
visible cause, because the field producing it is hidden in that state.
sourceId is now always assigned for this operation rather than conditionally,
so it can never inherit. The regression test asserts the merged result rather
than the mapping alone, since the gap between them is where the bug lived.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2deb0e1. Configure here.
Summary
Extends the Ashby connector so a scheduled job/req sync can run incrementally, and adds the write path needed to annotate reqs and retract applications.
Reads
list_jobsaccepts Ashby'ssyncTokenand returns it asnextSyncCursor, turning a scheduled sync fromO(all reqs)intoO(changed reqs). Ashby only returns the token once the last page is drained, which the param description states.list_job_postingsgainsincludeUnpublishedJobPostings, plus the postingstatusfield. Withoutstatusa caller cannot tell a returned draft from a published posting, which makes the flag useless.Writes (new operations)
customField.setValue/setValues— the only way to annotate a job or req, since Ashby has no job notes and no job tags. Writingnullclears a value, so the annotation is reversible.application.delete— requirescandidatesDelete, a module permission separate fromcandidatesWrite.application.changeSource— corrects source attribution on programmatically created applications.candidate.anonymize— strips PII; the record itself remains, because Ashby exposes no candidate deletion endpoint.Bug fix found while testing against a real Ashby org
ashbyErrorMessagerendered Ashby's documentederrors: [{message, parameter}]shape as[object Object], hiding the real cause. That is the form a 403 for a missing module permission arrives in.Destructive-write safety
nullclears a custom field, so every write here requires explicit intent before it can destroy data. This matters because the block'srequiredmarkers only cover the editor: on the agent path a model calls the tool directly, sotools.config.paramsnever runs andvalidateRequiredParametersAfterMergeskips a param marked not-required.set_custom_field_valuerejects an absent or blankfieldValue; an explicitnullstill clears.change_application_sourcerequiresunsetSourceto clear, and rejects a source ID and an unset request together — preferring either one silently discards the other. Ashby has no "leave unchanged" mode, so setting and clearing are the only two intents and exactly one must be expressed. The editor cannot produce the pair: the Source ID field is conditioned off while the clear switch is on.set_custom_field_valuesrejects an empty array locally rather than relying on Ashby.The custom field value parser is deliberately narrow rather than a blanket
JSON.parse, which corrupts real text:1e999becomesInfinityand serializes back out asnull— a clear; long numeric ids lose precision past 2^53; prose starting with{turns into an object. Only the literal keywords,{/[/"prefixes, and exactly round-tripping numbers decode.On the sync cursor's name
Ashby calls it
syncToken. Surfacing it under that name renders it[REDACTED]in block output, becausesyncTokenmatches the/^.*token$/ideny-list inredaction.ts— and an incremental sync is unusable if the operator cannot read the cursor the next run needs.An earlier revision of this PR fixed that by exempting
syncTokenfrom redaction globally. That was the wrong trade: it would have stopped redacting that field name product-wide, in any current or future integration, across execution traces, block output, the console, and telemetry — and the sharp edge was this block's own Sync Token input, where an operator pasting an API key would have had it stored verbatim.redaction.tsis untouched by this PR. The output is namednextSyncCursorinstead, which is what it actually is — an opaque resumption marker, the same class as thecursorandnextCursorfields the redactor already leaves alone. This connector already translates Ashby's wire vocabulary on the way out (limit→perPage,results→jobs). The wire name and the request param staysyncToken, matching Ashby's docs, and the input keeps being redacted, which is the protective behaviour. A test pins the invariant:syncTokenis sensitive by name,nextSyncCursorandnextCursorare not.Test Coverage
Ashby-surface tests went 4 → 142. Full suite: 25,129 passed, 41 skipped, 0 failures (1,875 files). Every commit typechecks independently.
Verified live against a real Ashby production organization, and end-to-end through the workflow editor:
perPage=1walk), and replaying it returns a strictly smaller result set.null, confirmed by reading the object back both times.includeUnpublishedJobPostingsreturns a strict superset, withstatusreportingPublished/Draft.[object Object]before this branch.perPagebounds, all fivefieldValueencodings, empty and single-elementsetValues, and the three permission/not-found error paths.apps/sim/tools/ashby/ashby.live.test.tsis committed but inert by default:Add
ASHBY_LIVE_WRITES=1for the write phase. Note thatvitest.setup.tsstubs globalfetchfor every file in the app, so the live file restores the real implementation and asserts the restore worked — without that guard the whole suite silently passes against a mock.Review
Greptile 5/5. One Cursor Bugbot finding — a source ID and an unset request could be supplied together and the body preferred the source ID, turning an intentional clear into a set — fixed by rejecting the pair rather than picking a winner, and by making the editor unable to produce it.
A Codex prototype-pollution concern was investigated and refuted: the redactor builds a fresh object via
Object.entries, andJSON.parsenever invokes the__proto__setter.Known gaps
candidatesDelete, sodelete_applicationandchange_application_sourcehave error-path-only live verification. Their success paths are covered by mocks. In particularchange_application_source's explicit-nullunset is proven against mocks but not against the live API.delete_application's{ applicationId }response shape comes from Ashby's OpenAPI spec, not an observed live response, for the same reason.list_*tools still understateperPageas "(default 100)" when Ashby caps at 100 and silently truncates. Pre-existing; onlylist_jobswas corrected here to avoid widening scope.list_*tools still expose asyncTokenoutput, which is redacted for the reason described above. Pre-existing, and renaming them would break any workflow already chaining the value, so it is left for a separate change.candidate.anonymize's response is persisted to execution traces like any other block output. If the endpoint echoes the pre-anonymization record, log retention could outlive the erasure. That is a platform-level retention question, not specific to this diff.Evals
No prompt-related files changed — evals skipped.