From 3437e60d5972ba5b8196f1d943ce80c373c92af4 Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Sat, 1 Aug 2026 08:51:38 -0700 Subject: [PATCH 1/7] fix: stop handoff tools 400ing on a first push into an empty org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools are applied before assistants, because assistants reference tools. A handoff/transfer tool references an assistant, inverting the dependency for that subset — a genuine cycle. The create path already handled it: unresolved assistant destinations are stripped, and updateToolAssistantRefs links them once every assistant exists. The update path sent the raw slug instead, so the API answered 400 "Assistant with ID not found" and the push aborted before the linking pass could run. The update payload now omits the whole destinations key when any entry is unresolved. Omitting rather than filtering matters: PATCH replaces the keys it receives, so a filtered array would wipe destinations that are live on the dashboard whenever the referenced assistant is merely untracked locally. --- docs/learnings/tools.md | 15 ++++ improvements.md | 57 ++++++++++++++ src/push.ts | 66 +++++++++++++--- tests/tool-assistant-cycle.test.ts | 116 +++++++++++++++++++++++++++++ 4 files changed, 244 insertions(+), 10 deletions(-) create mode 100644 tests/tool-assistant-cycle.test.ts diff --git a/docs/learnings/tools.md b/docs/learnings/tools.md index 9cb4dbe..ed1cb70 100644 --- a/docs/learnings/tools.md +++ b/docs/learnings/tools.md @@ -466,3 +466,18 @@ The LLM produces only `name` and `email` (what the caller spoke). The orchestrat For belt-and-braces, pair static parameters with HMAC body signing so the backend verifies sender + content, not just channel. For the trust-tier breakdown of which Liquid variables are safe to use here (`{{ customer.number }}`, `{{ call.id }}`, etc.) vs. which are LLM-derived and not, see [assistants.md → Liquid Variable Bag and Trust Tiers](assistants.md#liquid-variable-bag-and-trust-tiers). + +## Handoff/transfer tools reference assistants (dependency cycle) + +Tools are pushed before assistants, because assistants reference tools. A +handoff or transfer tool references an assistant, which inverts that for the +tool in question. The engine handles it in two passes: the tool is created (or +updated) without its unresolved assistant destinations, then a linking pass +PATCHes the real destinations once every assistant exists. + +Consequence worth knowing: on a push where the referenced assistant is not in +state, the tool's `destinations` are deliberately **not** sent. They are set by +the linking pass at the end of the same push. If you scope a push to +`--type tools` while the assistant is untracked, the destinations on the +dashboard are left as-is rather than cleared — the engine omits the key instead +of sending a partial array, because PATCH replaces whatever it receives. diff --git a/improvements.md b/improvements.md index 442ab8e..97e8a15 100644 --- a/improvements.md +++ b/improvements.md @@ -79,6 +79,7 @@ you which stack PR closes the row.** | 25 | Interactive flows lack automated coverage | Picker/conflict-prompt regressions ship silently | None | Open — scheduled for the test-update iteration | | 26 | Rollback is snapshot replay, not transaction rollback | Creates/deletes/state drift are not fully undone | #3 | Open — document/plan transactional rollback | | 27 | List endpoints read one unpaginated page at a time | >100-resource fleets got truncated orphan detection | None | RESOLVED 2026-08-01 (consumers gap open) | +| 28 | Handoff tools 400 on first push into an empty org | Push aborts before the assistant-linking pass runs | None | RESOLVED 2026-08-01 | **Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated. @@ -1364,6 +1365,62 @@ so they still report confidently on a partial view. `cleanup.ts` has its own loc `vapiGet` and stays unpaginated (`src/cleanup.ts:193`); it fails safe, since a truncated listing finds *fewer* dashboard orphans to delete. +## 28. Handoff/transfer tools 400 on a first push into an empty org + +**[RESOLVED 2026-08-01]** + +**Discovered:** on a customer repo. `PATCH /tool/d787c351… → 400 Assistant with +ID "clinical-stage-1-a4598432" not found`, aborting the whole push. + +### Problem + +Tools are applied before assistants, because assistants reference tools. A +handoff/transfer tool references an *assistant*, which inverts the dependency for +that subset — a genuine cycle. The update path sent the unresolved assistant slug +to the API, which rejected it. + +### Current behavior (Verified) + +The engine already resolves the cycle in two passes. `applyTool` +(`src/push.ts`) strips unresolved assistant destinations from the **create** +payload, and `updateToolAssistantRefs` PATCHes the real destinations once every +assistant exists. The **update** payload had no equivalent: it was +`removeExcludedKeys(payload, "tools")` with the raw slug still in +`destinations[].assistantId`. Any tool that already existed on the platform while +its referenced assistant was not yet in state produced a 400, and because +`applyTool` rethrows, the push aborted before the linking pass ran. + +Reproduces whenever a tool exists remotely and its assistant does not exist +locally in state — a first push into an empty org, a re-pointed handoff, or a +`--type tools` push. + +### Risk + +A first push into a fresh org fails partway with an error that names an assistant +rather than the tool, so the cause reads as an assistant problem. Resources +applied before the failing tool stay applied, so the org is left half-configured. + +### Current mitigation + +Push assistants first (`npm run push -- --type assistants`), then push +everything. + +### Possible fix + +Implemented: `omitUnresolvedDestinations` drops the whole `destinations` key from +the update payload when any entry is unresolved, letting the existing linking pass +set the real value. + +Omitting the key matters more than filtering the array. Vapi PATCH replaces the +keys it receives, so sending a filtered array would wipe destinations that are +live on the dashboard whenever the referenced assistant is merely untracked +locally. An absent key is left alone. Covered by +`tests/tool-assistant-cycle.test.ts`. + +### Status + +**RESOLVED 2026-08-01.** + --- ## Out of scope (intentionally not improvements) diff --git a/src/push.ts b/src/push.ts index c25223e..002727d 100644 --- a/src/push.ts +++ b/src/push.ts @@ -604,12 +604,30 @@ export async function applyTool( stateSection: state.tools, fullState: state, updateEndpoint: `/tool/${existingUuid}`, - updatePayload: removeExcludedKeys(payload, "tools"), + updatePayload: omitUnresolvedDestinations( + removeExcludedKeys(payload, "tools"), + data as Record, + ), createEndpoint: "/tool", createPayload: payloadForCreate, }); } +// A destination is unresolved when reference resolution left the assistantId +// exactly as the file wrote it — i.e. the slug is not in state yet, so no UUID +// could be substituted. +function isUnresolvedDestination( + resolvedDest: Record | undefined, + originalDest: Record | undefined, +): boolean { + if (!resolvedDest || typeof resolvedDest.assistantId !== "string") + return false; + if (!originalDest || typeof originalDest.assistantId !== "string") + return false; + const originalId = (originalDest.assistantId as string).split("##")[0]?.trim(); + return resolvedDest.assistantId === originalId; +} + // Strip destinations with unresolved assistantIds (where original equals resolved = not found in state) function stripUnresolvedAssistantDestinations( resolved: Record, @@ -622,19 +640,47 @@ function stripUnresolvedAssistantDestinations( const originalDests = original.destinations as Record[]; const resolvedDests = resolved.destinations as Record[]; - // Filter out destinations where assistantId wasn't resolved (still matches original) - const filteredDests = resolvedDests.filter((dest, idx) => { - if (typeof dest.assistantId !== "string") return true; - const origDest = originalDests[idx]; - if (!origDest || typeof origDest.assistantId !== "string") return true; - // Keep if resolved (UUID format) or no original assistantId - const originalId = (origDest.assistantId as string).split("##")[0]?.trim(); - return dest.assistantId !== originalId; - }); + const filteredDests = resolvedDests.filter( + (dest, idx) => !isUnresolvedDestination(dest, originalDests[idx]), + ); return { ...resolved, destinations: filteredDests }; } +// The update-path counterpart, and the reason a first push into an empty org +// used to fail with `400 Assistant with ID "" not found`. +// +// Tools are applied before assistants (tools are a dependency of assistants), +// but a handoff/transfer tool references an assistant — a genuine cycle. On a +// CREATE the unresolved destinations are stripped and `updateToolAssistantRefs` +// links them once every assistant exists. The UPDATE path had no equivalent, so +// it sent the raw slug, the API rejected it, and the push aborted before the +// linking pass could run. +// +// This omits the whole `destinations` key rather than sending a filtered array: +// PATCH replaces the keys it receives, so a filtered array would wipe +// destinations that are live on the dashboard whenever the local assistant is +// merely untracked (a `--type tools` push, for instance). Omitting the key +// leaves the platform value untouched, and the linking pass sets the real value. +export function omitUnresolvedDestinations( + payload: Record, + original: Record, +): Record { + if (!Array.isArray(payload.destinations)) return payload; + + const originalDests = Array.isArray(original.destinations) + ? (original.destinations as Record[]) + : []; + const anyUnresolved = ( + payload.destinations as Record[] + ).some((dest, idx) => isUnresolvedDestination(dest, originalDests[idx])); + + if (!anyUnresolved) return payload; + + const { destinations: _omitted, ...rest } = payload; + return rest; +} + export async function applyStructuredOutput( resource: ResourceFile, state: StateFile, diff --git a/tests/tool-assistant-cycle.test.ts b/tests/tool-assistant-cycle.test.ts new file mode 100644 index 0000000..7cb4f14 --- /dev/null +++ b/tests/tool-assistant-cycle.test.ts @@ -0,0 +1,116 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// ───────────────────────────────────────────────────────────────────────────── +// Tools are applied before assistants because assistants reference tools. But a +// handoff/transfer tool references an assistant, so that subset inverts the +// dependency — a genuine cycle. +// +// The engine already resolves it in two passes: the CREATE path strips +// unresolved assistant destinations, and `updateToolAssistantRefs` links them +// once every assistant exists. The UPDATE path had no equivalent, so a first +// push into an empty org sent the raw slug and the API answered: +// +// PATCH /tool/ → 400 Assistant with ID "clinical-stage-1-a4598432" not found +// +// which aborted the push before the linking pass could run. +// ───────────────────────────────────────────────────────────────────────────── + +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { omitUnresolvedDestinations } = await import("../src/push.ts"); + +const UUID = "8f14e45f-ceea-467a-9f1b-1a1b2c3d4e5f"; + +test("an unresolved assistant destination drops the whole destinations key", async () => { + // Unresolved = resolution left the slug exactly as written, because the + // assistant is not in state yet. + const original = { + type: "transferCall", + destinations: [{ type: "assistant", assistantId: "clinical-stage-1" }], + }; + const payload = { + type: "transferCall", + destinations: [{ type: "assistant", assistantId: "clinical-stage-1" }], + }; + + const result = omitUnresolvedDestinations(payload, original); + + assert.ok( + !("destinations" in result), + "the key must be absent so PATCH leaves the platform value untouched", + ); + assert.equal(result.type, "transferCall", "the rest of the payload survives"); +}); + +test("a resolved destination is sent unchanged", async () => { + const original = { + destinations: [{ type: "assistant", assistantId: "clinical-stage-1" }], + }; + const payload = { + destinations: [{ type: "assistant", assistantId: UUID }], + }; + + const result = omitUnresolvedDestinations(payload, original); + + assert.deepEqual(result.destinations, [ + { type: "assistant", assistantId: UUID }, + ]); +}); + +test("a trailing YAML comment on the reference still counts as resolved", async () => { + // `assistantId: clinical-stage-1 ## human note` — the comment is part of the + // authored string, so the comparison has to strip it or every reference would + // look unresolved. + const original = { + destinations: [ + { type: "assistant", assistantId: "clinical-stage-1 ## stage one" }, + ], + }; + const payload = { destinations: [{ type: "assistant", assistantId: UUID }] }; + + const result = omitUnresolvedDestinations(payload, original); + + assert.ok("destinations" in result, "a resolved reference is still sent"); +}); + +test("one unresolved destination among several omits the key, not just that entry", async () => { + // Sending a filtered array would PATCH-replace `destinations` and drop the + // resolved sibling from the dashboard. The linking pass rewrites the whole + // array afterwards, so omitting is both safe and complete. + const original = { + destinations: [ + { type: "assistant", assistantId: "stage-one" }, + { type: "assistant", assistantId: "stage-two" }, + ], + }; + const payload = { + destinations: [ + { type: "assistant", assistantId: UUID }, + { type: "assistant", assistantId: "stage-two" }, + ], + }; + + const result = omitUnresolvedDestinations(payload, original); + + assert.ok( + !("destinations" in result), + "a partially resolved array must not be sent", + ); +}); + +test("a tool with no destinations is untouched", async () => { + const payload = { type: "function", function: { name: "lookup" } }; + const result = omitUnresolvedDestinations(payload, payload); + assert.deepEqual(result, payload); +}); + +test("non-assistant destinations never block the update", async () => { + // Number/SIP destinations carry no assistantId, so they are always sendable. + const payload = { + destinations: [{ type: "number", number: "+15550000000" }], + }; + const result = omitUnresolvedDestinations(payload, payload); + assert.ok("destinations" in result); +}); From d038051a952de1ec39b5fd49b6b36e8a30845ee9 Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Sat, 1 Aug 2026 12:24:24 -0700 Subject: [PATCH 2/7] feat(pull): add advisory timing context to 3-way conflict reports (#52) A both-diverged report showed three 8-character hash prefixes and nothing else, which tells an operator nothing about which side to keep. Each entry now also carries the dashboard's updatedAt, the local file's mtime, and which is newer. It stays advisory and the engine still refuses to choose, because none of these timestamps is authoritative: - updatedAt is bumped by our own pushes, so a newer dashboard often just means you pushed recently rather than that a teammate changed something. - local mtime is reset by git clone and git checkout, so on a fresh checkout every file looks edited seconds ago. - later does not mean supersedes. An edit to the prompt and an edit to the voice both deserve to survive; last-write-wins would discard one silently. Sub-minute gaps report as within a minute of each other rather than naming a winner, since clock skew is the same order of magnitude as the gap. Nothing is persisted: a previous schema stored lastPulledAt and it was deliberately dropped in favour of content hashes. --- docs/learnings/sync-behavior.md | 29 +++++++++ src/pull.ts | 105 +++++++++++++++++++++++++++++++- tests/conflict-timing.test.ts | 92 ++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 tests/conflict-timing.test.ts diff --git a/docs/learnings/sync-behavior.md b/docs/learnings/sync-behavior.md index edaf375..bbb3eed 100644 --- a/docs/learnings/sync-behavior.md +++ b/docs/learnings/sync-behavior.md @@ -99,6 +99,35 @@ UUIDs → names): | `apply` (default `--resolve=defer`) | pull defers → push prompts for **exactly the conflicted resources**; clean ones flow silently | | `apply --resolve=ours` | no questions: pull re-baselines, push runs with `--overwrite` (CI semantics; dashboard edits lose) | +### Conflict timing context (advisory) + +When a 3-way conflict is reported, each entry carries a timing line beside the +hashes: + +``` + - assistants/intake + local-hash: 3f9a1c2b… platform-hash: 8e01d4aa… last-pulled: 3f9a1c2b… + dashboard changed 2026-08-01 15:00Z, your file 2026-08-01 12:00Z — dashboard is 3h newer +``` + +It is a hint, never a verdict, and the engine still refuses to choose. Three +reasons it cannot be trusted as one: + +- `updatedAt` is bumped by **our own pushes**, so a "newer" dashboard often just + means you pushed a few minutes ago. +- The local mtime is reset by `git clone` and `git checkout`, so on a fresh + checkout every file looks edited seconds ago. +- **Later does not mean supersedes.** If you changed the prompt and a teammate + changed the voice, both edits deserve to survive; last-write-wins would discard + one silently. + +Sub-minute gaps are reported as "within a minute of each other" rather than +picking a winner, because clock skew is the same order of magnitude as the gap. + +A previous state schema stored `lastPulledAt` for this purpose and it was +deliberately removed in favour of content hashes. This line reads `updatedAt` +from the live response and the file's mtime at report time; it persists nothing. + ### 5. Both changed identically (L = D, stale baseline) Both `pull` and `push` treat this as clean (live sides agree — nothing to diff --git a/src/pull.ts b/src/pull.ts index 85a111c..4e71fea 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -667,6 +667,93 @@ function findLocalResourcePath( ].find((p) => existsSync(p)); } +// ───────────────────────────────────────────────────────────────────────────── +// Conflict timing context +// +// A 3-way conflict report used to show three 8-character hash prefixes, which +// tell a human nothing about which side to keep. Timestamps do — but only as a +// hint, never as the decision: +// +// - `updatedAt` is bumped by OUR OWN pushes, so a "newer" dashboard often just +// means you pushed a few minutes ago, not that a teammate changed anything. +// - the local mtime is reset by `git clone` and `git checkout`, so on a fresh +// checkout every file looks like it was edited seconds ago. +// - "later" does not mean "supersedes". Two edits to different fields both +// deserve to survive, and last-write-wins would silently discard one. +// +// So this is printed to help a human pick a `--resolve` mode. The engine still +// refuses to choose. A previous schema stored `lastPulledAt` for this and it was +// deliberately dropped in favour of content hashes; nothing here brings it back. +// ───────────────────────────────────────────────────────────────────────────── + +function formatAge(ms: number): string { + const mins = Math.round(ms / 60_000); + if (mins < 60) return `${mins}m`; + const hours = Math.round(mins / 60); + if (hours < 48) return `${hours}h`; + return `${Math.round(hours / 24)}d`; +} + +function isoMinute(date: Date): string { + return `${date.toISOString().slice(0, 16).replace("T", " ")}Z`; +} + +/** + * Human-readable timing for one conflicted resource, or `undefined` when + * neither side offers a usable timestamp. Exported for tests. + */ +export function conflictTimingHint(options: { + dashboardUpdatedAt?: unknown; + localModifiedMs?: number; +}): string | undefined { + const remote = + typeof options.dashboardUpdatedAt === "string" + ? new Date(options.dashboardUpdatedAt) + : undefined; + const remoteOk = remote && !Number.isNaN(remote.getTime()); + const local = + typeof options.localModifiedMs === "number" && + Number.isFinite(options.localModifiedMs) + ? new Date(options.localModifiedMs) + : undefined; + + if (!remoteOk && !local) return undefined; + if (remoteOk && !local) return `dashboard changed ${isoMinute(remote)}`; + if (!remoteOk && local) return `your file changed ${isoMinute(local)}`; + + const delta = remote!.getTime() - local!.getTime(); + const which = + Math.abs(delta) < 60_000 + ? "within a minute of each other" + : delta > 0 + ? `dashboard is ${formatAge(delta)} newer` + : `your file is ${formatAge(-delta)} newer`; + return `dashboard changed ${isoMinute(remote!)}, your file ${isoMinute(local!)} — ${which}`; +} + +// Reads the local file's mtime for the timing hint. Returns undefined rather +// than throwing: a missing file or an unreadable stat must never break the +// conflict report. +function localModifiedMs( + resourceType: ResourceType, + resourceId: string, +): number | undefined { + try { + const path = findLocalResourcePath(FOLDER_MAP[resourceType], resourceId); + return path ? statSync(path).mtimeMs : undefined; + } catch { + return undefined; + } +} + +function timingLine(entry: BothDivergedResource): string { + const hint = conflictTimingHint({ + dashboardUpdatedAt: entry.resource.updatedAt, + localModifiedMs: localModifiedMs(entry.resourceType, entry.resourceId), + }); + return hint ? `\n ${hint}` : ""; +} + export interface PullOptions { force?: boolean; bootstrap?: boolean; @@ -1057,7 +1144,8 @@ async function resolveBothDivergedResources(options: { ); for (const entry of bothDiverged) { console.log( - ` - ${FOLDER_MAP[entry.resourceType]}/${entry.resourceId}`, + ` - ${FOLDER_MAP[entry.resourceType]}/${entry.resourceId}` + + timingLine(entry), ); } return { exitCode: 0 }; @@ -1070,7 +1158,8 @@ async function resolveBothDivergedResources(options: { for (const entry of bothDiverged) { console.error( ` - ${entry.resourceType}/${entry.resourceId}\n` + - ` local-hash: ${entry.localHash.slice(0, 8)}… platform-hash: ${entry.platformHash.slice(0, 8)}… last-pulled: ${entry.lastPulledHash.slice(0, 8)}…`, + ` local-hash: ${entry.localHash.slice(0, 8)}… platform-hash: ${entry.platformHash.slice(0, 8)}… last-pulled: ${entry.lastPulledHash.slice(0, 8)}…` + + timingLine(entry), ); } return { exitCode: 1 }; @@ -1084,7 +1173,8 @@ async function resolveBothDivergedResources(options: { for (const entry of bothDiverged) { console.error( ` - ${FOLDER_MAP[entry.resourceType]}/${entry.resourceId}\n` + - ` local-hash: ${entry.localHash.slice(0, 8)}… platform-hash: ${entry.platformHash.slice(0, 8)}… last-pulled: ${entry.lastPulledHash.slice(0, 8)}…`, + ` local-hash: ${entry.localHash.slice(0, 8)}… platform-hash: ${entry.platformHash.slice(0, 8)}… last-pulled: ${entry.lastPulledHash.slice(0, 8)}…` + + timingLine(entry), ); } console.error( @@ -1096,6 +1186,15 @@ async function resolveBothDivergedResources(options: { console.error( " --resolve=fail exit non-zero without writing anything (CI mode — fail the build so a human investigates)", ); + console.error( + "\n Timestamps above are a hint, not a verdict: your own pushes bump the dashboard's", + ); + console.error( + " updatedAt, and git clone/checkout resets local file times. Newer does not mean correct —", + ); + console.error( + " two edits to different fields both deserve to survive.", + ); return { exitCode: 1 }; } diff --git a/tests/conflict-timing.test.ts b/tests/conflict-timing.test.ts new file mode 100644 index 0000000..9771af4 --- /dev/null +++ b/tests/conflict-timing.test.ts @@ -0,0 +1,92 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// ───────────────────────────────────────────────────────────────────────────── +// A 3-way conflict report used to show three 8-character hash prefixes and +// nothing else, which tells a human nothing about which side to keep. These +// tests pin the advisory timing line that replaced that gap. +// +// "Advisory" is the whole contract. The engine must keep refusing to choose, +// because neither timestamp is authoritative: `updatedAt` is bumped by our own +// pushes, local mtime is reset by clone and checkout, and two edits to different +// fields both deserve to survive regardless of which landed last. +// ───────────────────────────────────────────────────────────────────────────── + +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { conflictTimingHint } = await import("../src/pull.ts"); + +const T0 = Date.parse("2026-08-01T12:00:00.000Z"); + +test("reports both sides and which is newer when the dashboard moved later", () => { + const hint = conflictTimingHint({ + dashboardUpdatedAt: new Date(T0 + 3 * 3_600_000).toISOString(), + localModifiedMs: T0, + }); + + assert.match(hint as string, /dashboard changed 2026-08-01 15:00Z/); + assert.match(hint as string, /your file 2026-08-01 12:00Z/); + assert.match(hint as string, /dashboard is 3h newer/); +}); + +test("reports the local file as newer when it moved later", () => { + const hint = conflictTimingHint({ + dashboardUpdatedAt: new Date(T0).toISOString(), + localModifiedMs: T0 + 2 * 86_400_000, + }); + + assert.match(hint as string, /your file is 2d newer/); +}); + +test("edits within a minute are not called a winner", () => { + // Sub-minute ordering is noise: clock skew between your machine and the + // platform is the same order of magnitude as the gap. + const hint = conflictTimingHint({ + dashboardUpdatedAt: new Date(T0 + 20_000).toISOString(), + localModifiedMs: T0, + }); + + assert.match(hint as string, /within a minute of each other/); + assert.doesNotMatch(hint as string, /newer/); +}); + +test("minutes are used below an hour", () => { + const hint = conflictTimingHint({ + dashboardUpdatedAt: new Date(T0 + 25 * 60_000).toISOString(), + localModifiedMs: T0, + }); + + assert.match(hint as string, /dashboard is 25m newer/); +}); + +test("one side alone still produces a usable line", () => { + // A resource with no local file, or an unreadable stat, must not blank the + // whole report. + assert.match( + conflictTimingHint({ dashboardUpdatedAt: new Date(T0).toISOString() }) as string, + /^dashboard changed 2026-08-01 12:00Z$/, + ); + assert.match( + conflictTimingHint({ localModifiedMs: T0 }) as string, + /^your file changed 2026-08-01 12:00Z$/, + ); +}); + +test("no usable timestamp yields no line at all", () => { + // The caller appends this to an existing bullet, so an empty hint has to be + // distinguishable from a hint that happens to be short. + assert.equal(conflictTimingHint({}), undefined); + assert.equal(conflictTimingHint({ dashboardUpdatedAt: 1754049600000 }), undefined); + assert.equal(conflictTimingHint({ dashboardUpdatedAt: "not a date" }), undefined); + assert.equal(conflictTimingHint({ localModifiedMs: Number.NaN }), undefined); +}); + +test("a malformed dashboard timestamp falls back to the local side", () => { + const hint = conflictTimingHint({ + dashboardUpdatedAt: "2026-13-45T99:99:99Z", + localModifiedMs: T0, + }); + + assert.match(hint as string, /^your file changed 2026-08-01 12:00Z$/); +}); From a050ecdf27a209ded62b2ffb2983b041278209f6 Mon Sep 17 00:00:00 2001 From: Vitali Korezki Date: Mon, 3 Aug 2026 14:42:35 -0700 Subject: [PATCH 3/7] fix: skip tool destination linking when the assistant is genuinely absent Co-Authored-By: Claude Fable 5 --- src/push.ts | 38 +++++++++ tests/tool-assistant-cycle.test.ts | 131 ++++++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 1 deletion(-) diff --git a/src/push.ts b/src/push.ts index 002727d..81b62ca 100644 --- a/src/push.ts +++ b/src/push.ts @@ -681,6 +681,30 @@ export function omitUnresolvedDestinations( return rest; } +// Destinations that survived resolution still carrying a slug — the +// referenced assistant is in neither state nor the local repo. Cleaned of +// any trailing `## comment` the author left on the reference. +export function unresolvedDestinationSlugs(destinations: unknown): string[] { + if (!Array.isArray(destinations)) return []; + + const slugs: string[] = []; + for (const dest of destinations as unknown[]) { + if ( + !dest || + typeof dest !== "object" || + typeof (dest as Record).assistantId !== "string" + ) { + continue; + } + const cleaned = (dest as Record).assistantId as string; + const slug = cleaned.split("##")[0]?.trim() ?? ""; + if (!UUID_REGEX.test(slug)) { + slugs.push(slug); + } + } + return slugs; +} + export async function applyStructuredOutput( resource: ResourceFile, state: StateFile, @@ -901,6 +925,20 @@ export async function updateToolAssistantRefs( // Resolve destinations now that all assistants exist const resolved = resolveReferences(rawData, state); + // A destination can still carry a slug here if the referenced assistant + // is genuinely absent (not in state, not in the local repo) rather than + // merely not-yet-applied. Sending that slug in the PATCH would 400 the + // whole push at the very end, after every other resource already + // applied — skip this tool and let a future push link it once the + // assistant exists. + const unresolvedSlugs = unresolvedDestinationSlugs(resolved.destinations); + if (unresolvedSlugs.length > 0) { + console.warn( + ` ⚠️ Tool "${resourceId}" still references unresolved assistant destination(s): ${unresolvedSlugs.join(", ")}. Leaving this tool's destinations untouched on the platform — they will link on a future push once those assistants exist.`, + ); + continue; + } + console.log(` 🔗 Linking tool ${resourceId} to assistant destinations`); const result = await vapiRequest("PATCH", `/tool/${uuid}`, { destinations: resolved.destinations, diff --git a/tests/tool-assistant-cycle.test.ts b/tests/tool-assistant-cycle.test.ts index 7cb4f14..1868f02 100644 --- a/tests/tool-assistant-cycle.test.ts +++ b/tests/tool-assistant-cycle.test.ts @@ -19,7 +19,13 @@ import test from "node:test"; process.argv = ["node", "test", "test-fixture-org"]; process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; -const { omitUnresolvedDestinations } = await import("../src/push.ts"); +const { + omitUnresolvedDestinations, + unresolvedDestinationSlugs, + updateToolAssistantRefs, +} = await import("../src/push.ts"); + +import type { ResourceFile, StateFile } from "../src/types.ts"; const UUID = "8f14e45f-ceea-467a-9f1b-1a1b2c3d4e5f"; @@ -114,3 +120,126 @@ test("non-assistant destinations never block the update", async () => { const result = omitUnresolvedDestinations(payload, payload); assert.ok("destinations" in result); }); + +// ───────────────────────────────────────────────────────────────────────────── +// `updateToolAssistantRefs` runs AFTER every other resource has already +// applied — it is the linking pass, not the initial create. Today it +// unconditionally PATCHes `resolved.destinations`, so a destination whose +// assistant is genuinely absent (not in state, not in the local repo) still +// carries a raw slug and the API answers `400 Assistant with ID "" not +// found`, aborting the whole push at the very end. `unresolvedDestinationSlugs` +// is the guard: it reports which destinations still carry a slug post- +// resolution so the linking pass can skip that tool and warn instead of +// PATCHing garbage. +// ───────────────────────────────────────────────────────────────────────────── + +test("unresolvedDestinationSlugs: a slug entry is reported", () => { + const result = unresolvedDestinationSlugs([ + { type: "assistant", assistantId: "clinical-stage-1" }, + ]); + assert.deepEqual(result, ["clinical-stage-1"]); +}); + +test("unresolvedDestinationSlugs: a trailing YAML comment is stripped before reporting", () => { + const result = unresolvedDestinationSlugs([ + { type: "assistant", assistantId: "clinical-stage-1 ## stage one" }, + ]); + assert.deepEqual(result, ["clinical-stage-1"]); +}); + +test("unresolvedDestinationSlugs: an array of only UUID assistantIds reports nothing", () => { + // Raw UUIDs are never "unresolved" here — the CREATE path already strips + // any destination whose resolved value equals the original, so a UUID that + // reaches this helper is either a genuinely resolved reference or an + // untracked-but-valid raw UUID the author wrote directly. Either way it must + // flow to the PATCH, not get reported as unresolved. + const result = unresolvedDestinationSlugs([ + { type: "assistant", assistantId: UUID }, + ]); + assert.deepEqual(result, []); +}); + +test("unresolvedDestinationSlugs: non-assistant destinations are not reported", () => { + const result = unresolvedDestinationSlugs([ + { type: "number", number: "+15550000000" }, + ]); + assert.deepEqual(result, []); +}); + +test("unresolvedDestinationSlugs: non-array input returns an empty list", () => { + assert.deepEqual(unresolvedDestinationSlugs(undefined), []); + assert.deepEqual(unresolvedDestinationSlugs(null), []); + assert.deepEqual(unresolvedDestinationSlugs("not-an-array"), []); +}); + +function emptyState(): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +test("updateToolAssistantRefs: skips the PATCH and warns when the referenced assistant is genuinely absent", async () => { + const state = emptyState(); + state.tools["router"] = { uuid: UUID }; + // Deliberately no entry under state.assistants for "clinical-stage-1" — the + // assistant is genuinely absent, not merely not-yet-applied. + + const tool: ResourceFile = { + resourceId: "router", + filePath: "/fake/tools/router.yml", + data: { + type: "transferCall", + destinations: [{ type: "assistant", assistantId: "clinical-stage-1" }], + }, + }; + + const fetchCalls: unknown[] = []; + const originalFetch = globalThis.fetch; + const originalWarn = console.warn; + const warnings: unknown[][] = []; + console.warn = (...args: unknown[]) => { + warnings.push(args); + }; + globalThis.fetch = (async (...args: unknown[]) => { + fetchCalls.push(args); + return { + ok: true, + status: 200, + json: async () => ({ id: UUID }), + text: async () => "{}", + } as unknown as Response; + }) as typeof globalThis.fetch; + + try { + await updateToolAssistantRefs([tool], state); + + assert.equal( + fetchCalls.length, + 0, + "no PATCH should be sent when the assistant is genuinely absent", + ); + assert.ok( + warnings.some((args) => + args.some( + (arg) => + typeof arg === "string" && + arg.includes("router") && + arg.includes("clinical-stage-1"), + ), + ), + "a warning naming the tool and the unresolved slug should be logged", + ); + } finally { + globalThis.fetch = originalFetch; + console.warn = originalWarn; + } +}); From 51b7f1c7bb67c934a4aa637e594dec0fb5ccfde8 Mon Sep 17 00:00:00 2001 From: Vitali Korezki Date: Mon, 3 Aug 2026 14:55:14 -0700 Subject: [PATCH 4/7] fix: stop structured-output pushes wiping links to untracked assistants Co-Authored-By: Claude Fable 5 --- src/push.ts | 71 ++++++++- tests/so-assistant-omit.test.ts | 247 ++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 tests/so-assistant-omit.test.ts diff --git a/src/push.ts b/src/push.ts index 81b62ca..9b8f018 100644 --- a/src/push.ts +++ b/src/push.ts @@ -705,6 +705,39 @@ export function unresolvedDestinationSlugs(destinations: unknown): string[] { return slugs; } +// Count of authored `assistant_ids` entries — strings, cleaned of any +// trailing `## comment`, non-empty after trim. Shared counting rule between +// `omitUnresolvedAssistantIds` and `updateStructuredOutputAssistantRefs`. +function countAuthoredAssistantRefs(assistantIds: unknown): number { + if (!Array.isArray(assistantIds)) return 0; + return assistantIds.filter( + (ref) => typeof ref === "string" && (ref.split("##")[0]?.trim() ?? "") !== "", + ).length; +} + +// Same omit-not-filter rationale as `omitUnresolvedDestinations`: PATCH +// replaces the keys it receives, so a partially-resolved `assistantIds` +// array would wipe assistant links that are live on the dashboard whenever a +// referenced assistant is merely untracked locally (a `--type structuredOutputs` +// push, for instance). Omitting the key leaves the platform value untouched; +// `updateStructuredOutputAssistantRefs` sets the real value once every +// assistant exists. +export function omitUnresolvedAssistantIds( + payload: Record, + original: Record, +): Record { + if (!Array.isArray(original.assistant_ids)) return payload; + if (!Array.isArray(payload.assistantIds)) return payload; + + const authoredCount = countAuthoredAssistantRefs(original.assistant_ids); + if ((payload.assistantIds as unknown[]).length >= authoredCount) { + return payload; + } + + const { assistantIds: _omitted, ...rest } = payload; + return rest; +} + export async function applyStructuredOutput( resource: ResourceFile, state: StateFile, @@ -725,7 +758,10 @@ export async function applyStructuredOutput( stateSection: state.structuredOutputs, fullState: state, updateEndpoint: `/structured-output/${existingUuid}?schemaOverride=true`, - updatePayload: removeExcludedKeys(payload, "structuredOutputs"), + updatePayload: omitUnresolvedAssistantIds( + removeExcludedKeys(payload, "structuredOutputs"), + data as Record, + ), createEndpoint: "/structured-output", createPayload: payloadWithoutAssistants, }); @@ -954,6 +990,17 @@ export async function updateToolAssistantRefs( // Post-Apply: Update Structured Outputs with Assistant References // ───────────────────────────────────────────────────────────────────────────── +// Local mirror of the untracked-UUID / missing-slug check `resolveAssistantId` +// (src/resolver.ts) applies, used only to name which authored refs failed to +// resolve in the warning below — the skip decision itself is the plain length +// comparison against `countAuthoredAssistantRefs`. +function assistantRefIsTracked(ref: string, state: StateFile): boolean { + if (UUID_REGEX.test(ref)) { + return Object.values(state.assistants).some((entry) => entry.uuid === ref); + } + return !!state.assistants[ref]?.uuid; +} + export async function updateStructuredOutputAssistantRefs( structuredOutputs: ResourceFile[], state: StateFile, @@ -973,12 +1020,34 @@ export async function updateStructuredOutputAssistantRefs( const uuid = state.structuredOutputs[resourceId]?.uuid; if (!uuid) continue; + const authoredRefs = (rawData.assistant_ids as unknown[]) + .filter((ref): ref is string => typeof ref === "string") + .map((ref) => ref.split("##")[0]?.trim() ?? "") + .filter((ref) => ref !== ""); + // Resolve assistant IDs now that all assistants exist const assistantIds = resolveAssistantIds( rawData.assistant_ids as string[], state, ); + if (assistantIds.length < authoredRefs.length) { + // A referenced assistant is genuinely absent (not in state, not in the + // local repo) — including a raw UUID that is untracked in state, which + // `resolveAssistantId` treats as "possibly deleted" and resolves to + // null. Sending the partial list would PATCH-replace `assistantIds` and + // wipe whatever assistant links are live on the dashboard; unlike tool + // destinations, there is no later pass that repairs a structured + // output's assistant links, so skip this one and warn instead. + const unresolved = authoredRefs.filter( + (ref) => !assistantRefIsTracked(ref, state), + ); + console.warn( + ` ⚠️ Structured output "${resourceId}" still references unresolved assistant(s): ${unresolved.join(", ")}. Leaving this structured output's assistant links untouched on the platform — they will link on a future push once those assistants exist.`, + ); + continue; + } + if (assistantIds.length > 0) { console.log(` 🔗 Linking structured output ${resourceId} to assistants`); const result = await vapiRequest("PATCH", `/structured-output/${uuid}`, { diff --git a/tests/so-assistant-omit.test.ts b/tests/so-assistant-omit.test.ts new file mode 100644 index 0000000..820320b --- /dev/null +++ b/tests/so-assistant-omit.test.ts @@ -0,0 +1,247 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// ───────────────────────────────────────────────────────────────────────────── +// Structured outputs link to assistants via `assistant_ids` (snake_case, +// slugs), resolved to camelCase `assistantIds` (UUIDs) at push time. +// `resolveAssistantIds` (src/resolver.ts:103) silently drops any ref it can't +// resolve, so a structured output referencing an untracked assistant (a +// `--type structuredOutputs` push, say, or an assistant that simply hasn't +// been pushed yet) ends up with a shorter `assistantIds` array than what was +// authored. +// +// PATCH replaces the keys it receives, so sending that shorter array wipes +// whichever assistant links are already live on the dashboard: +// +// PATCH /structured-output/ { assistantIds: [] } // "resolved" nothing +// +// silently un-links every assistant the dashboard had, even though the file +// still authors every one of them. `omitUnresolvedAssistantIds` is the same +// omit-not-filter guard Task 1 added for tool destinations +// (`omitUnresolvedDestinations`), applied to the update path; the +// `updateStructuredOutputAssistantRefs` linking pass gets the analogous +// skip-and-warn instead of PATCHing a partial list. +// ───────────────────────────────────────────────────────────────────────────── + +process.argv = ["node", "test", "test-fixture-org"]; +process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; + +const { omitUnresolvedAssistantIds, updateStructuredOutputAssistantRefs } = + await import("../src/push.ts"); + +import type { ResourceFile, StateFile } from "../src/types.ts"; + +const UUID = "8f14e45f-ceea-467a-9f1b-1a1b2c3d4e5f"; +const UUID_2 = "1a2b3c4d-5e6f-4a1b-9c2d-3e4f5a6b7c8d"; + +test("one unresolved ref among the authored assistant_ids drops the whole assistantIds key", () => { + const original = { + assistant_ids: ["front-desk", "clinical-stage-1"], + }; + const payload = { + name: "intake-schema", + assistantIds: [UUID], // only one of the two resolved + }; + + const result = omitUnresolvedAssistantIds(payload, original); + + assert.ok( + !("assistantIds" in result), + "the key must be absent so PATCH leaves the platform value untouched", + ); + assert.equal(result.name, "intake-schema", "the rest of the payload survives"); +}); + +test("a fully resolved assistantIds array is sent unchanged", () => { + const original = { assistant_ids: ["front-desk", "clinical-stage-1"] }; + const payload = { assistantIds: [UUID, UUID_2] }; + + const result = omitUnresolvedAssistantIds(payload, original); + + assert.deepEqual(result.assistantIds, [UUID, UUID_2]); +}); + +test("an authored ref with a trailing YAML comment still counts as one authored ref", () => { + // `assistant_ids: [front-desk ## front desk bot]` — the comment is part of + // the authored string, so the count has to strip it before comparing + // lengths, or a fully resolved single ref would look like a partial one. + const original = { assistant_ids: ["front-desk ## front desk bot"] }; + const payload = { assistantIds: [UUID] }; + + const result = omitUnresolvedAssistantIds(payload, original); + + assert.ok( + "assistantIds" in result, + "one authored ref, one resolved ref — nothing to omit", + ); +}); + +test("no assistant_ids on the original payload leaves the update untouched", () => { + const payload = { name: "intake-schema", assistantIds: [] }; + const result = omitUnresolvedAssistantIds(payload, {}); + assert.deepEqual(result, payload); +}); + +test("assistant_ids: [] on the original payload leaves the update untouched", () => { + const original = { assistant_ids: [] }; + const payload = { name: "intake-schema", assistantIds: [] }; + + const result = omitUnresolvedAssistantIds(payload, original); + + assert.deepEqual(result, payload); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// `updateStructuredOutputAssistantRefs` runs after every other resource has +// applied — the linking pass, not the initial create. Before this change it +// PATCHed `resolveAssistantIds(...)` unconditionally whenever the result was +// non-empty, so a structured output with two authored refs where only one +// resolved would silently PATCH `assistantIds: []` and wipe the +// other link on the dashboard — with no warning at all, since the "all +// refs unresolved" case was the only one that skipped silently. Now any +// shortfall skips the PATCH and warns, naming which authored refs failed. +// ───────────────────────────────────────────────────────────────────────────── + +function emptyState(): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +async function withFetchAndWarnRecorders( + fn: (recorders: { + fetchCalls: unknown[]; + warnings: unknown[][]; + }) => Promise, +): Promise { + const fetchCalls: unknown[] = []; + const warnings: unknown[][] = []; + const originalFetch = globalThis.fetch; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(args); + }; + globalThis.fetch = (async (...args: unknown[]) => { + fetchCalls.push(args); + return { + ok: true, + status: 200, + json: async () => ({ id: UUID }), + text: async () => "{}", + } as unknown as Response; + }) as typeof globalThis.fetch; + + try { + return await fn({ fetchCalls, warnings }); + } finally { + globalThis.fetch = originalFetch; + console.warn = originalWarn; + } +} + +test("updateStructuredOutputAssistantRefs: skips the PATCH and warns when the single authored ref is absent from state", async () => { + const state = emptyState(); + state.structuredOutputs["intake-schema"] = { uuid: UUID }; + // Deliberately no entry under state.assistants for "front-desk" — the + // assistant is genuinely absent, not merely not-yet-applied. + + const so: ResourceFile = { + resourceId: "intake-schema", + filePath: "/fake/structured-outputs/intake-schema.yml", + data: { assistant_ids: ["front-desk"] }, + }; + + await withFetchAndWarnRecorders(async ({ fetchCalls, warnings }) => { + await updateStructuredOutputAssistantRefs([so], state); + + assert.equal( + fetchCalls.length, + 0, + "no PATCH should be sent when the assistant is genuinely absent", + ); + assert.ok( + warnings.some((args) => + args.some( + (arg) => + typeof arg === "string" && + arg.includes("intake-schema") && + arg.includes("front-desk"), + ), + ), + "a warning naming the structured output and the unresolved ref should be logged", + ); + }); +}); + +test("updateStructuredOutputAssistantRefs: a partial resolution (one of two) skips the PATCH and names only the failing ref", async () => { + const state = emptyState(); + state.structuredOutputs["intake-schema"] = { uuid: UUID }; + state.assistants["front-desk"] = { uuid: UUID_2 }; + // "clinical-stage-1" is deliberately untracked. + + const so: ResourceFile = { + resourceId: "intake-schema", + filePath: "/fake/structured-outputs/intake-schema.yml", + data: { assistant_ids: ["front-desk", "clinical-stage-1"] }, + }; + + await withFetchAndWarnRecorders(async ({ fetchCalls, warnings }) => { + await updateStructuredOutputAssistantRefs([so], state); + + assert.equal( + fetchCalls.length, + 0, + "sending the partial array would wipe the resolved link on the dashboard", + ); + assert.ok( + warnings.some((args) => + args.some( + (arg) => + typeof arg === "string" && + arg.includes("intake-schema") && + arg.includes("clinical-stage-1") && + !arg.includes("clinical-stage-1, front-desk"), + ), + ), + "the warning should name the unresolved ref but not the resolved one", + ); + }); +}); + +test("updateStructuredOutputAssistantRefs: an untracked raw UUID counts as unresolved and skips the PATCH", async () => { + // A raw UUID that isn't in state resolves to null (resolveAssistantId + // treats it as "possibly deleted") — for structured outputs that must + // trigger the same skip-and-warn as an unresolved slug, since there is no + // later pass to repair a wiped link. + const state = emptyState(); + state.structuredOutputs["intake-schema"] = { uuid: UUID }; + + const so: ResourceFile = { + resourceId: "intake-schema", + filePath: "/fake/structured-outputs/intake-schema.yml", + data: { assistant_ids: [UUID_2] }, + }; + + await withFetchAndWarnRecorders(async ({ fetchCalls, warnings }) => { + await updateStructuredOutputAssistantRefs([so], state); + + assert.equal(fetchCalls.length, 0); + assert.ok( + warnings.some((args) => + args.some( + (arg) => typeof arg === "string" && arg.includes(UUID_2), + ), + ), + "the untracked UUID should be named in the warning", + ); + }); +}); From 49d5c4e3bfb177bc03fdd208fb9139745cf028ba Mon Sep 17 00:00:00 2001 From: Vitali Korezki Date: Mon, 3 Aug 2026 15:09:04 -0700 Subject: [PATCH 5/7] docs: record linking-pass guard and SO omit semantics Co-Authored-By: Claude Fable 5 --- docs/learnings/structured-outputs.md | 44 ++++++ docs/learnings/tools.md | 20 ++- improvements.md | 225 ++++++++++++++++++++++++++- 3 files changed, 286 insertions(+), 3 deletions(-) diff --git a/docs/learnings/structured-outputs.md b/docs/learnings/structured-outputs.md index 9621761..7685a9a 100644 --- a/docs/learnings/structured-outputs.md +++ b/docs/learnings/structured-outputs.md @@ -97,3 +97,47 @@ Structured outputs are the primary way to measure voice agent performance. Commo **Recommendation:** For squad-wide KPIs that must populate on **every** ending (VM-only, classifier-only, and live-agent), use **`squad.membersOverrides.analysisPlan.structuredDataPlan`** plus **`membersOverrides.artifactPlan.fullMessageHistoryEnabled: true`**. Remove per-assistant duplicate plans. Keep standalone structured outputs for evals, dashboard analytics, or schemas you want versioned as separate resources. Full YAML pattern and merge-order notes: [squads.md → Squad-level post-call extraction via `membersOverrides`](squads.md#squad-level-post-call-extraction-via-membersoverrides-multi-member-squads). + +--- + +## assistant_ids and the update/linking cycle (dependency cycle) + +Structured outputs are pushed before assistants, same as tools — see +[tools.md → "Handoff/transfer tools reference assistants (dependency +cycle)"](tools.md#handofftransfer-tools-reference-assistants-dependency-cycle) +for the general shape. A structured output's `assistant_ids` references an +assistant, which inverts the push order for that field. The engine handles it +the same two-pass way: on create, `assistantIds` is stripped entirely (no +assistants exist yet); on update, it's resolved to whatever already exists in +state. Either way, a linking pass then sets the real value once every +assistant in the push has been applied. + +The difference from tool destinations is what happens when a reference still +doesn't resolve. `assistantIds` is a flat array with no shape to signal "leave +this one for later" — so instead of sending a shorter array, the engine omits +the whole `assistantIds` key from the update PATCH whenever any authored +`assistant_ids` entry fails to resolve. Sending a filtered array would PATCH- +replace the platform's current list, silently unlinking every live-but- +untracked assistant the array left out — the update never shrinks the +dashboard's array as a side effect of an unrelated edit. + +The linking pass (which runs once every assistant in the push has been +applied) can still find an unresolved reference — genuinely absent from both +state and the local repo, not just not-yet-applied. When that happens it +**skips that structured output** rather than PATCHing the partial list, and +logs a warning naming the unresolved reference(s): + +``` +⚠️ Structured output "" still references unresolved assistant(s): . Leaving this structured output's assistant links untouched on the platform — they will link on a future push once those assistants exist. +``` + +**One asymmetry worth knowing versus tool destinations:** for structured +outputs, an **untracked raw UUID counts as unresolved**, not as a legitimate +platform-only reference. Tool destinations let a raw UUID through even when +it's untracked in state (it might be a real dashboard assistant the repo +just doesn't manage). Structured outputs can't take that risk — there's no +later repair pass for a structured output's assistant links the way there is +for tool destinations, so if the linking pass got it wrong here, the wipe +would be permanent until someone noticed and re-pushed by hand. Treating an +untracked UUID as unresolved means it skips-and-warns instead of silently +shipping a wipe. diff --git a/docs/learnings/tools.md b/docs/learnings/tools.md index ed1cb70..592fdec 100644 --- a/docs/learnings/tools.md +++ b/docs/learnings/tools.md @@ -476,8 +476,24 @@ updated) without its unresolved assistant destinations, then a linking pass PATCHes the real destinations once every assistant exists. Consequence worth knowing: on a push where the referenced assistant is not in -state, the tool's `destinations` are deliberately **not** sent. They are set by -the linking pass at the end of the same push. If you scope a push to +state, the tool's `destinations` are deliberately **not** sent by the main +create/update. They are set by the linking pass at the end of the same push, +once every assistant in the push has been applied. If you scope a push to `--type tools` while the assistant is untracked, the destinations on the dashboard are left as-is rather than cleared — the engine omits the key instead of sending a partial array, because PATCH replaces whatever it receives. + +The linking pass itself can still find an unresolved destination — not +"not yet applied this push" but genuinely absent from both state and the +local repo. In that case it does **not** send the raw slug. It **skips that +tool** and logs a warning naming the unresolved reference(s): + +``` +⚠️ Tool "" still references unresolved assistant destination(s): . Leaving this tool's destinations untouched on the platform — they will link on a future push once those assistants exist. +``` + +The rest of the push continues normally, and the destinations link +automatically on a later push once the assistant exists. Previously the +linking pass sent the raw slug straight through, and the API's `400 +Assistant not found` aborted the whole push at the very end — after every +other resource had already applied. diff --git a/improvements.md b/improvements.md index 97e8a15..a421b3a 100644 --- a/improvements.md +++ b/improvements.md @@ -80,8 +80,11 @@ you which stack PR closes the row.** | 26 | Rollback is snapshot replay, not transaction rollback | Creates/deletes/state drift are not fully undone | #3 | Open — document/plan transactional rollback | | 27 | List endpoints read one unpaginated page at a time | >100-resource fleets got truncated orphan detection | None | RESOLVED 2026-08-01 (consumers gap open) | | 28 | Handoff tools 400 on first push into an empty org | Push aborts before the assistant-linking pass runs | None | RESOLVED 2026-08-01 | +| 29 | SO linking sent filtered `assistantIds` arrays | Silent unlink of live-but-untracked assistants | None | RESOLVED 2026-08-03 (#51) | +| 30 | Tool-linking pass could PATCH a raw assistant slug | Mid-push 400 naming the wrong resource | None | RESOLVED 2026-08-03 (#51) | +| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open | -**Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated. +**Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, `#31`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated. --- @@ -1423,6 +1426,226 @@ locally. An absent key is left alone. Covered by --- +## 29. Structured-output assistant-link update/linking sent filtered `assistantIds` arrays — silent unlink of live-but-untracked assistants + +**[RESOLVED 2026-08-03] (#51)** + +**Discovered:** during the same audit that produced #30 — the tool-destination +linking pass and the structured-output linking pass share the exact same +circular-dependency shape, but the structured-output side had a materially +worse version of the gap: `assistantIds` is a flat array with no per-entry +"leave this one alone" marker, so the pre-fix code had no way to skip just the +unresolved entry. + +### Problem + +`applyStructuredOutput`'s update path and `updateStructuredOutputAssistantRefs`'s +linking pass both resolved `assistant_ids` with `resolveAssistantIds` +(`src/resolver.ts:103-110`), which silently drops any entry that fails to +resolve, then PATCHed the resulting array onto `assistantIds` regardless of +whether it was shorter than what was authored. Vapi PATCH replaces the key it +receives rather than merging, so a filtered array didn't just fail to add the +unresolved assistant — on a resource that already existed, it removed that +assistant's link on the dashboard if one was already there, with no warning +that anything had changed. + +### Current behavior (Verified) + +- `omitUnresolvedAssistantIds` (`src/push.ts:725-739`) compares the resolved + `assistantIds` length against `countAuthoredAssistantRefs` + (`src/push.ts:711-716`, counts non-empty authored `assistant_ids` entries) + and omits the `assistantIds` key from the update payload entirely when the + resolved array is shorter. Wired into `applyStructuredOutput` at + `src/push.ts:761-764`. +- `updateStructuredOutputAssistantRefs` (`src/push.ts:1004-1061`) applies the + same length check at `src/push.ts:1034-1049`: when `resolveAssistantIds` + returns fewer entries than the cleaned, non-empty authored refs, it skips + the PATCH for that structured output and logs a warning naming the + unresolved reference(s), via `assistantRefIsTracked` (`src/push.ts:997-1002`): + + ``` + ⚠️ Structured output "" still references unresolved assistant(s): . Leaving this structured output's assistant links untouched on the platform — they will link on a future push once those assistants exist. + ``` + +- Unlike tool destinations, an **untracked raw UUID counts as unresolved** + here: `resolveAssistantId` (`src/resolver.ts:79-101`) warns and returns + `null` for an untracked UUID rather than passing it through. There is no + later pass that repairs a structured output's assistant links the way the + tool-linking pass repairs destinations, so the stricter rule is + intentional. Covered by `tests/so-assistant-omit.test.ts`. + +### Risk + +Silent — no error, no push-time signal. A structured output's live +`assistant_ids` could shrink on any push where one authored reference was +untracked, whether that reference was a typo, an assistant not yet pulled +locally, or one that was deleted from the repo. The only symptom is a KPI or +eval that quietly stops running against calls it used to cover. + +### Current mitigation + +Prior to the fix: audit each structured output's live `assistant_ids` against +the dashboard after any push where an assistant reference changed, and keep +`assistant_ids` and state in lockstep (see #11). + +### Possible fix + +Implemented: the omit-on-update / skip-and-warn-on-link guards above. + +### Status + +**RESOLVED 2026-08-03 (#51).** + +--- + +## 30. Tool-destination linking pass could still PATCH a raw assistant slug when the assistant was genuinely absent + +**[RESOLVED 2026-08-03] (#51)** + +**Discovered:** while auditing #28's fix — that entry closed the gap in the +tool's own create/update PATCH, but the separate linking pass +(`updateToolAssistantRefs`), which runs after every assistant in the push has +been applied, had the identical gap for the same reason. + +### Problem + +`updateToolAssistantRefs` (`src/push.ts:939-987`) resolves a tool's +`destinations` again once all assistants in the push have been applied, then +PATCHed `{ destinations: resolved.destinations }` unconditionally. If a +destination's `assistantId` is genuinely absent — not in state and not in the +local repo, as opposed to merely not-yet-applied earlier in the same push — +`resolveReferences` leaves the raw slug in place (`src/resolver.ts:244-256`: +`if (resolvedId) { destination.assistantId = resolvedId }`, no `else`). The +linking pass sent that raw slug straight to the API. + +### Current behavior (Verified) + +`unresolvedDestinationSlugs` (`src/push.ts:687-706`) scans +`resolved.destinations` for any `assistantId` that still isn't a UUID after +resolution, and `updateToolAssistantRefs` (`src/push.ts:970-976`) skips that +tool's PATCH entirely when any are found, logging: + +``` +⚠️ Tool "" still references unresolved assistant destination(s): . Leaving this tool's destinations untouched on the platform — they will link on a future push once those assistants exist. +``` + +A destination that resolves to an untracked-but-valid UUID still flows +through unchanged (untracked UUIDs may legitimately exist on the platform; +only a still-a-slug value counts as unresolved — see #29 for why structured +outputs made the opposite call). Covered by +`tests/tool-assistant-cycle.test.ts`. + +### Risk + +`PATCH /tool/{uuid}` returned `400 Assistant with ID "" not found`, and +because the linking pass runs as the last stage of `push`, the failure landed +after every other resource in the push had already applied — the error named +an assistant, not the tool, making the actual cause non-obvious. + +### Current mitigation + +Prior to the fix: push assistants first +(`npm run push -- --type assistants`) so no destination is ever +genuinely unresolved by the time the linking pass runs. + +### Possible fix + +Implemented: the skip-and-warn guard above. No further action needed — the +destinations link automatically once the referenced assistant is added to +the repo and a subsequent push runs. + +### Status + +**RESOLVED 2026-08-03 (#51).** + +--- + +## 31. Unresolved references are handled three different ways depending on the field, and `validate.ts` has no dangling-reference check + +**Discovered:** while fixing #29 and #30 — those two entries close the +loudest and quietest failure modes for their specific fields, but the +underlying question ("what happens when a reference resolves to nothing") +still has a different answer per field, and nothing validates references +before push time. + +### Problem + +Whether an unresolved reference is dropped, deferred, or 400s depends +entirely on which field it's in. There's no single engine-wide rule, and +`validate.ts` never checks that a referenced id resolves to anything before +push. + +### Current behavior (Verified) + +Three distinct behaviors exist today, none of them a validation error: + +- **Silently filtered (array shrinks, no warning about the shrink itself):** + `model.toolIds` / root `toolIds` (`resolveToolIds`, `src/resolver.ts:46-50`), + `artifactPlan.structuredOutputIds` (`resolveStructuredOutputIds`, + `src/resolver.ts:52-77`), and structured-output `assistant_ids` → + `assistantIds` (`resolveAssistantIds`, `src/resolver.ts:103-110`) at the + point of first resolution in `resolveReferences` + (`src/resolver.ts:184-219`). Each resolver logs a per-item `⚠️` warning but + still returns a shorter array; callers other than the two guarded in + #29/#30 (e.g. `applyAssistant`'s `model.toolIds` / + `artifactPlan.structuredOutputIds`) send that shorter array straight + through with no further check. +- **Left raw → mid-push 400:** `hooks[].do[].toolId` + (`src/resolver.ts:227-241`), squad `members[].assistantId` / + `assistantDestinations[].assistantId` (`src/resolver.ts:258-282`), + `personalityId` (`src/resolver.ts:284-290`), and `scenarioId` + (`src/resolver.ts:292-298`) all follow the pattern + `if (resolvedId) { field = resolvedId }` with no `else` — an unresolved + reference keeps its original slug and is sent to the API as-is, which 400s + on whichever endpoint receives it. +- **Deferred to a linking pass:** tool `destinations[].assistantId` and + structured-output `assistant_ids`, per #29 and #30. + +`validate.ts` has no check that a referenced id resolves to something. Its +existing reference-shaped checks stop short of it: `checkLockstep`'s forward +pass explicitly steps around a missing reference — +`if (!assistant) continue; // missing-reference is a different class` +(`src/validate.ts:120`) — because the SO↔assistant lockstep check (#11) only +validates that both sides of an edge agree, not that either side exists. +`checkResourceRefs` (`src/validate.ts:456-483`, Check 6) already walks every +reference field via `extractReferencedIds` (`src/resolver.ts:351-457`), but +only to flag a reference matching `.vapi-ignore` — it doesn't flag a +reference that resolves to nothing at all. + +### Risk + +The same authoring mistake — a typo'd resource id, a reference to a file that +was deleted, a reference to a resource in the wrong org — surfaces three +different ways depending on which field it's in: a silently smaller array, a +mid-push 400 naming the wrong resource, or a skipped resource with a warning. +Fixing one field's dangling reference gives no reason to expect a different +field behaves completely differently. + +### Current mitigation + +Read `docs/learnings/tools.md` and `docs/learnings/structured-outputs.md` for +the two guarded fields; for everything else, treat any reference to a +resource that doesn't yet exist as unsafe to push until the referenced +resource exists in state. + +### Possible fix + +Extend `checkResourceRefs` (`src/validate.ts:456-483`) — it already walks +every reference field via `extractReferencedIds` for the ignored-reference +check — to also flag any id that is not a UUID, is not present in the +corresponding state section, and has no matching local resource file, as a +blocking validation finding. The ignored-reference check and a +dangling-reference check would share the same walk, just different match +conditions; it doesn't require picking one runtime behavior (filter vs. +defer vs. 400) for every field, since it stops the push before any of those +three behaviors gets a chance to run. + +### Status + +**Open.** + +--- + ## Out of scope (intentionally not improvements) - **State file is identity-only and not git-ignored.** It's intentionally From 9ec42876904b11eccc40294c911a53cc93623c1a Mon Sep 17 00:00:00 2001 From: Vitali Korezki Date: Mon, 3 Aug 2026 15:30:23 -0700 Subject: [PATCH 6/7] fix: strip yaml comments from destination refs before compare and patch isUnresolvedDestination compared an uncleaned resolved assistantId against a cleaned original, so a slug authored with a trailing `## comment` that failed to resolve was misclassified as resolved and reached the API raw. The tool-linking pass's PATCH body had the same gap for untracked raw UUID destinations, which pass the slug-shaped unresolvedDestinationSlugs guard but still carry the comment. Clean both sides before comparing, and clean every destination's assistantId before it goes out in the linking PATCH. Co-Authored-By: Claude Fable 5 --- src/push.ts | 42 +++++++++-- tests/so-assistant-omit.test.ts | 7 +- tests/tool-assistant-cycle.test.ts | 109 +++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 5 deletions(-) diff --git a/src/push.ts b/src/push.ts index 9b8f018..3270e9e 100644 --- a/src/push.ts +++ b/src/push.ts @@ -615,7 +615,12 @@ export async function applyTool( // A destination is unresolved when reference resolution left the assistantId // exactly as the file wrote it — i.e. the slug is not in state yet, so no UUID -// could be substituted. +// could be substituted. Both sides are cleaned of a trailing `## comment` +// before comparing: an unresolved reference authored with a comment comes +// back from `resolveReferences` with the comment still attached (resolution +// failed, so the raw string is untouched), while the original is compared +// clean — leaving the resolved side uncleaned made every commented-but- +// unresolved reference compare unequal and misclassify as resolved. function isUnresolvedDestination( resolvedDest: Record | undefined, originalDest: Record | undefined, @@ -624,8 +629,11 @@ function isUnresolvedDestination( return false; if (!originalDest || typeof originalDest.assistantId !== "string") return false; - const originalId = (originalDest.assistantId as string).split("##")[0]?.trim(); - return resolvedDest.assistantId === originalId; + const resolvedId = + (resolvedDest.assistantId as string).split("##")[0]?.trim() ?? ""; + const originalId = + (originalDest.assistantId as string).split("##")[0]?.trim() ?? ""; + return resolvedId === originalId; } // Strip destinations with unresolved assistantIds (where original equals resolved = not found in state) @@ -705,6 +713,32 @@ export function unresolvedDestinationSlugs(destinations: unknown): string[] { return slugs; } +// Strip any trailing `## comment` from every destination's `assistantId` +// before it goes out in a PATCH body. An untracked raw UUID authored with a +// comment (`8f14…4e5f ## billing agent (unmanaged)`) fails resolution — it's +// left exactly as authored — and passes `unresolvedDestinationSlugs` (which +// cleans before checking the UUID shape), so without this the linking pass +// would PATCH the comment straight to the API and 400. A no-op for entries +// that already resolved to a bare UUID or never had a comment. +export function cleanDestinationAssistantIds(destinations: unknown): unknown { + if (!Array.isArray(destinations)) return destinations; + + return destinations.map((dest) => { + if ( + !dest || + typeof dest !== "object" || + typeof (dest as Record).assistantId !== "string" + ) { + return dest; + } + const assistantId = + ((dest as Record).assistantId as string) + .split("##")[0] + ?.trim() ?? ""; + return { ...(dest as Record), assistantId }; + }); +} + // Count of authored `assistant_ids` entries — strings, cleaned of any // trailing `## comment`, non-empty after trim. Shared counting rule between // `omitUnresolvedAssistantIds` and `updateStructuredOutputAssistantRefs`. @@ -977,7 +1011,7 @@ export async function updateToolAssistantRefs( console.log(` 🔗 Linking tool ${resourceId} to assistant destinations`); const result = await vapiRequest("PATCH", `/tool/${uuid}`, { - destinations: resolved.destinations, + destinations: cleanDestinationAssistantIds(resolved.destinations), }); // This PATCH mutates the platform AFTER the main upsert wrote its // baseline — refresh it from the linking response, or the next push would diff --git a/tests/so-assistant-omit.test.ts b/tests/so-assistant-omit.test.ts index 820320b..a8f74b9 100644 --- a/tests/so-assistant-omit.test.ts +++ b/tests/so-assistant-omit.test.ts @@ -209,9 +209,14 @@ test("updateStructuredOutputAssistantRefs: a partial resolution (one of two) ski typeof arg === "string" && arg.includes("intake-schema") && arg.includes("clinical-stage-1") && - !arg.includes("clinical-stage-1, front-desk"), + !arg.includes("front-desk"), ), ), + // Checks that the resolved slug never appears in the warning at all — + // not just that it doesn't appear in one particular ordering. The + // implementation preserves authored order, so a buggy message naming + // both refs would read "front-desk, clinical-stage-1" (authored order) + // and slip past a check that only excluded "clinical-stage-1, front-desk". "the warning should name the unresolved ref but not the resolved one", ); }); diff --git a/tests/tool-assistant-cycle.test.ts b/tests/tool-assistant-cycle.test.ts index 1868f02..c4fe8c2 100644 --- a/tests/tool-assistant-cycle.test.ts +++ b/tests/tool-assistant-cycle.test.ts @@ -20,6 +20,7 @@ process.argv = ["node", "test", "test-fixture-org"]; process.env.VAPI_TOKEN = process.env.VAPI_TOKEN || "test-token-not-used"; const { + cleanDestinationAssistantIds, omitUnresolvedDestinations, unresolvedDestinationSlugs, updateToolAssistantRefs, @@ -81,6 +82,28 @@ test("a trailing YAML comment on the reference still counts as resolved", async assert.ok("destinations" in result, "a resolved reference is still sent"); }); +test("an unresolved slug authored with a trailing YAML comment still drops the whole destinations key", async () => { + // Resolution failure leaves the assistantId exactly as authored — comment + // included — in the resolved payload, same as the original. The comparison + // must clean both sides before checking equality, or the commented, + // uncleaned resolved value never matches the cleaned original and the + // destination is wrongly classified as resolved. + const commented = "clinical-stage-1 ## stage one"; + const original = { + destinations: [{ type: "assistant", assistantId: commented }], + }; + const payload = { + destinations: [{ type: "assistant", assistantId: commented }], + }; + + const result = omitUnresolvedDestinations(payload, original); + + assert.ok( + !("destinations" in result), + "the key must be absent so PATCH leaves the platform value untouched", + ); +}); + test("one unresolved destination among several omits the key, not just that entry", async () => { // Sending a filtered array would PATCH-replace `destinations` and drop the // resolved sibling from the dashboard. The linking pass rewrites the whole @@ -172,6 +195,42 @@ test("unresolvedDestinationSlugs: non-array input returns an empty list", () => assert.deepEqual(unresolvedDestinationSlugs("not-an-array"), []); }); +// ───────────────────────────────────────────────────────────────────────────── +// `cleanDestinationAssistantIds` is the last line of defense before the +// linking-pass PATCH body goes out: an untracked-but-valid raw UUID destination +// authored with a trailing `## comment` fails resolution (left exactly as +// authored) and passes `unresolvedDestinationSlugs` (which cleans before the +// UUID check), so without this the comment would reach the API and 400. +// ───────────────────────────────────────────────────────────────────────────── + +test("cleanDestinationAssistantIds: strips a trailing YAML comment from every destination's assistantId", () => { + const untrackedUuid = "1a2b3c4d-5e6f-7a8b-9c0d-1e2f3a4b5c6d"; + const result = cleanDestinationAssistantIds([ + { + type: "assistant", + assistantId: `${untrackedUuid} ## billing agent (unmanaged)`, + }, + { type: "number", number: "+15550000000" }, + ]); + + assert.deepEqual(result, [ + { type: "assistant", assistantId: untrackedUuid }, + { type: "number", number: "+15550000000" }, + ]); +}); + +test("cleanDestinationAssistantIds: a destination with no comment is a no-op", () => { + const result = cleanDestinationAssistantIds([ + { type: "assistant", assistantId: UUID }, + ]); + assert.deepEqual(result, [{ type: "assistant", assistantId: UUID }]); +}); + +test("cleanDestinationAssistantIds: non-array input is returned unchanged", () => { + assert.equal(cleanDestinationAssistantIds(undefined), undefined); + assert.equal(cleanDestinationAssistantIds(null), null); +}); + function emptyState(): StateFile { return { credentials: {}, @@ -243,3 +302,53 @@ test("updateToolAssistantRefs: skips the PATCH and warns when the referenced ass console.warn = originalWarn; } }); + +test("updateToolAssistantRefs: PATCH body strips a trailing YAML comment from an untracked-but-valid raw UUID destination", async () => { + // The UUID is untracked (no state.assistants entry) but still a valid UUID + // shape, so it passes unresolvedDestinationSlugs and the linking pass + // proceeds to PATCH — the comment must not ride along in the request body. + const untrackedUuid = "1a2b3c4d-5e6f-7a8b-9c0d-1e2f3a4b5c6d"; + const state = emptyState(); + state.tools["router"] = { uuid: UUID }; + + const tool: ResourceFile = { + resourceId: "router", + filePath: "/fake/tools/router.yml", + data: { + type: "transferCall", + destinations: [ + { + type: "assistant", + assistantId: `${untrackedUuid} ## billing agent (unmanaged)`, + }, + ], + }, + }; + + const fetchCalls: unknown[] = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (...args: unknown[]) => { + fetchCalls.push(args); + return { + ok: true, + status: 200, + json: async () => ({ id: UUID }), + text: async () => "{}", + } as unknown as Response; + }) as typeof globalThis.fetch; + + try { + await updateToolAssistantRefs([tool], state); + + assert.equal(fetchCalls.length, 1, "the PATCH should still be sent"); + const [, init] = fetchCalls[0] as [string, RequestInit]; + const body = JSON.parse(init.body as string); + assert.deepEqual( + body.destinations, + [{ type: "assistant", assistantId: untrackedUuid }], + "the PATCH body's assistantId must be the bare UUID, no trailing comment", + ); + } finally { + globalThis.fetch = originalFetch; + } +}); From fca76aa34582619764fb60229371a51aadcf0b2e Mon Sep 17 00:00:00 2001 From: Vitali Korezki Date: Mon, 3 Aug 2026 15:34:31 -0700 Subject: [PATCH 7/7] docs: refresh push.ts line citations after comment-stripping fix Co-Authored-By: Claude Fable 5 --- improvements.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/improvements.md b/improvements.md index a421b3a..1e8fa53 100644 --- a/improvements.md +++ b/improvements.md @@ -1451,17 +1451,17 @@ that anything had changed. ### Current behavior (Verified) -- `omitUnresolvedAssistantIds` (`src/push.ts:725-739`) compares the resolved +- `omitUnresolvedAssistantIds` (`src/push.ts:759-773`) compares the resolved `assistantIds` length against `countAuthoredAssistantRefs` - (`src/push.ts:711-716`, counts non-empty authored `assistant_ids` entries) + (`src/push.ts:745-750`, counts non-empty authored `assistant_ids` entries) and omits the `assistantIds` key from the update payload entirely when the resolved array is shorter. Wired into `applyStructuredOutput` at - `src/push.ts:761-764`. -- `updateStructuredOutputAssistantRefs` (`src/push.ts:1004-1061`) applies the - same length check at `src/push.ts:1034-1049`: when `resolveAssistantIds` + `src/push.ts:795-798`. +- `updateStructuredOutputAssistantRefs` (`src/push.ts:1038-1095`) applies the + same length check at `src/push.ts:1068-1083`: when `resolveAssistantIds` returns fewer entries than the cleaned, non-empty authored refs, it skips the PATCH for that structured output and logs a warning naming the - unresolved reference(s), via `assistantRefIsTracked` (`src/push.ts:997-1002`): + unresolved reference(s), via `assistantRefIsTracked` (`src/push.ts:1031-1036`): ``` ⚠️ Structured output "" still references unresolved assistant(s): . Leaving this structured output's assistant links untouched on the platform — they will link on a future push once those assistants exist. @@ -1509,7 +1509,7 @@ been applied, had the identical gap for the same reason. ### Problem -`updateToolAssistantRefs` (`src/push.ts:939-987`) resolves a tool's +`updateToolAssistantRefs` (`src/push.ts:973-1021`) resolves a tool's `destinations` again once all assistants in the push have been applied, then PATCHed `{ destinations: resolved.destinations }` unconditionally. If a destination's `assistantId` is genuinely absent — not in state and not in the @@ -1520,9 +1520,9 @@ linking pass sent that raw slug straight to the API. ### Current behavior (Verified) -`unresolvedDestinationSlugs` (`src/push.ts:687-706`) scans +`unresolvedDestinationSlugs` (`src/push.ts:695-714`) scans `resolved.destinations` for any `assistantId` that still isn't a UUID after -resolution, and `updateToolAssistantRefs` (`src/push.ts:970-976`) skips that +resolution, and `updateToolAssistantRefs` (`src/push.ts:1004-1010`) skips that tool's PATCH entirely when any are found, logging: ```