feat(validate): catch broken references and show findings on the PR

[scott-lowe-vapi]

Value

V.A.L.U.E. tier: small — a behavior change: validate, and so apply and the Validate resources check, now fail on configs they used to pass.

• Problem: a reference that names no file fails in one of three ways, depending on the field (improvements.md docs: document orphan-YAML gate + --allow-new-files in README and AGENTS #31):

  • it's silently dropped: model.toolIds, artifactPlan.structuredOutputIds;
  • it's sent raw and rejected partway through a push: squad members, hook tools, personalityId, scenarioId;
  • or it's deferred to a linking pass.

  validate never checked references, so the Validate resources check from ci: validate every org's resources on every pull request #76 couldn't catch a typo'd tool name either. Warnings were also invisible in CI: they don't fail the check, and nobody reads the job log.

• Who it affects: everyone who edits resource files by hand or with a coding agent, and reviewers of their PRs.

• What changes:

  • New src/validate-refs.ts, run by validate (so by apply and CI) and by push:

    Rule	Severity	Catches
    dangling-reference	error	A name with no local file and no state entry. Uses the shared reference walk, plus scenario judges' evaluations[].structuredOutputId.
    override-tool-by-name	error	A tool name in toolIds inside assistantOverrides, membersOverrides or targetOverrides, where push never resolves names.
    unresolved-credential	warning	A credential name missing from the state file. The message names the org's bootstrap pull.
    reference-by-uuid	warning	A UUID reference, which breaks promotion. Stock personalities are exempt.

  • validate now also runs reference-to-ignored, as push already did. It reads the committed state file and stays offline.

  • On GitHub Actions, every finding becomes an annotation, so it shows on the file in the PR, warnings included.

  • push reports the new rules alongside its existing validators: warnings by default, blocking under --strict.

  • Docs:

    • a rule-by-rule table in troubleshooting;
    • the validate row in the commands guide;
    • AGENTS.md: never edit the state file to make a reference resolve;
    • improvements.md docs: document orphan-YAML gate + --allow-new-files in README and AGENTS #31 marked resolved by validation.

Evidence of value

The starter example with two typos, scheduler → schedular in the squad and booking-confirmed → booking-confirmd in a judge:

	Before (#76)	After
npm run validate	0 error(s) — ✅ Validation passed	2 error(s), one dangling-reference per typo, naming the file and the missing name

• New tests/validate-refs.test.ts covers:
  • names that resolve to local files and to state;
  • a typo in each reference field;
  • ignored references;
  • UUIDs and stock personalities;
  • all three override keys;
  • credentials as names, as UUIDs, and known to state.
• Annotation format: escaping of %, newlines, : and , is tested in tests/validate.test.ts.
• End to end: tests/ci-validate-workflow.test.ts runs the CI step with GITHUB_ACTIONS=true on the typo'd squad. The step fails, and the ::error points at resources/clinic/squads/front-desk.yml.
• Mutation: dropping the judge collection or the override walk fails two tests.
• Every example org still passes. The cross-org promotion example (which has no state files) now shows an unresolved-credential warning, which is accurate.

Testing plan

• npm test (523 tests) and npx tsc --noEmit pass.
• Behavior to expect: apply validates before it pulls. So a reference to a resource created in the dashboard and never pulled now stops apply; the message says to pull first. Before, apply went on to pull and push, and the reference resolved only if the pull happened to produce that exact name.
• Not tested: a live apply or push. Neither code path changed except for the added findings.
• Not in this PR: a check for secrets committed in resource files. It goes in its own PR, because a false positive there would block a customer's merge.

Refs TEST-141

🤖 Generated with Claude Code

[scott-lowe-vapi]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat: identify every gitops API request with a User-Agent #78
• feat(validate): catch broken references and show findings on the PR #77 👈 (View in Graphite)
• ci: validate every org's resources on every pull request #76
• ci: lint workflows and test the PR check's key-sharing decision #75
• test(docs): check that doc links, commands and flags exist #74
• fix(cleanup): never delete resources matched by .vapi-ignore #73
• docs(agents): one compact guide for Claude, Codex and Cursor #72
• docs: edit the guides for new readers and fix inaccuracies #71
• docs: restructure the README into a short landing page and topic guides #70
• docs: prepare for public release: scrub internal names, fix examples, add community files #69
• feat(promotion): gate promotion out of an org on a passing check #66
• fix(promotion): commit the files of transitions that applied when a later one fails #65
• test(promotion): pin the exact files promoting an assistant writes to the target #68
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[chris-garber-vapi]

Why are we testing a function that we define in the test file rather than the validateReferences function directly?

[chris-garber-vapi]

Aggressive review pass. Each finding was checked against the code, and most were reproduced by running this branch's validators on the input described in the comment.

Severity: 🔴 blocker · 🟠 fix before merge · 🟡 should fix (a stacked PR is fine where noted) · 🟢 nit

• 🟠 ×3: validate checks .vapi-ignored files that push skips, which blocks CI and apply. The override-tool-by-name fix advice (model.tools) replaces the member's tool set (message and docs row).
• 🟡 ×6:
  • Judge references to ignored structured outputs pass both rules.
  • A non-string toolIds entry crashes validate.
  • Override, toolRefs and inline-assistant references aren't checked.
  • reference-by-uuid fires on every PR for dashboard-owned resources.
  • Annotations have no line.
  • The troubleshooting row tells readers to un-ignore resources.
• 🟢 ×3: prototype-key lookup, duplicated credential walk, #31 status wording.

No 🔴. Tests and tsc pass on the branch as-is.

[chris-garber-vapi]

🟠 validate loads .vapi-ignored files that push never loads, so the new reference rules fail CI and apply on resources that are never deployed.

Push filters ignored ids when it loads (push.ts:1591-1595, loadOpts = { ignorePatterns }), and docs/learnings/yaml-conventions.md:193 says ignored ids are "filtered out before drift detection, validation, or any API call". validate-cmd.ts calls loadResources(type) with no options. An ignored file can still be on disk, for example one pulled before its pattern was added, or one kept for reference. That file is now checked by reference-to-ignored and dangling-reference. Before this PR, validate ran only the content rules on it.

Repro (the starter example copied to org demo):

# resources/demo/.vapi-ignore
assistants/legacy/**
tools/old-*

# resources/demo/assistants/legacy/desk.yml
name: Legacy Desk
model: { provider: openai, model: gpt-4.1, toolIds: [old-crm-lookup] }

npm run validate -- demo prints ❌ [reference-to-ignored] assistants/legacy/desk … and exits 1. CI goes red and apply refuses to deploy, but push would never load that file.

Fix: read the patterns first and pass them to every load, as push does:

const ignorePatterns = loadIgnorePatterns();
const load = (type: ResourceType) => loadResources(type, { ignorePatterns });
const resources: LoadedResources = {
  tools: await load("tools"),
  structuredOutputs: await load("structuredOutputs"),
  // …the other seven types
};

Then delete the later const ignorePatterns = … line and add ResourceType to the type import. Add a case to ci-validate-workflow.test.ts: an ignored file with a broken reference should pass.

[chris-garber-vapi]

🟠 This message tells users to move the tool into the override's model.tools, which replaces the member's whole tool set, so following it can silently remove handoff tools from a live squad.

docs/learnings/squads.md:15-37: "model.tools in overrides REPLACES (through deep merge + tool resolution) … Use model.tools in overrides only when you want to replace the tool set." The repo already gives the safe advice for this same mistake in two places:

• docs/guides/pr-checks.md:71: "Put them inline in tools:append".
• overridesProcess in check-payload-assistant.ts:279: "(model.tools or tools:append)".

Squad members often keep their handoff tools in model.tools. A user or agent who follows this message would drop them, and routing would break without any error.

Fix: point to tools:append here, and in the override-tool-by-name row of troubleshooting.md (suggestion on line 108):

Suggested change

	`the tool inline in the override's model.tools instead`,
	`the tool inline in the override's tools:append instead`,

[chris-garber-vapi]

🟠 This row needs the same model.tools → tools:append correction as the error message (see the comment on src/validate-refs.ts:183).

Suggested change

	| `override-tool-by-name` | error | References inside `assistantOverrides`, `membersOverrides` and `targetOverrides` aren't resolved. Put the tool inline in the override's `model.tools`. |
	| `override-tool-by-name` | error | References inside `assistantOverrides`, `membersOverrides` and `targetOverrides` aren't resolved. Put the tool inline under the override's `tools:append`. `model.tools` there replaces the member's whole tool set; see [squads](../learnings/squads.md). |

[chris-garber-vapi]

🟡 A scenario judge that references an ignored structured output passes both rules, because reference-to-ignored never checks evaluations[].structuredOutputId.

This line skips ignored names because reference-to-ignored is supposed to report them. But that rule (checkResourceRefs, validate.ts:455-482) only walks extractReferencedIds, which doesn't include judges. That's why referencesCollect adds judges separately above. So with this input:

# .vapi-ignore:  structuredOutputs/legacy-*
# simulations/scenarios/s.yml
evaluations: [{ structuredOutputId: legacy-so }]

both validators return [] (I ran them on this input). At push, resolveReferences can't find legacy-so in state, because ignored ids are never tracked. It warns and sends the raw name, which causes the mid-push 400 this PR is meant to prevent.

The test "a reference to an ignored resource is left to the reference-to-ignored rule" only checks that this validator stays silent. It never checks that the other one reports the reference, which is how this gap got through.

Fix: use one collector for both rules.

1. Move referencesCollect into resolver.ts, next to extractReferencedIds, and export it.
2. Have checkResourceRefs loop over it instead of extractReferencedIds.
3. Delete REF_TYPES / RESOURCE_TYPES here. They're exact copies of REF_TYPE_KEYS / RESOURCE_TYPES_WITH_REFS in validate.ts:431-453.
4. Add a test that runs validateNoIgnoredReferences and validateReferences together on the judge case and expects exactly one reference-to-ignored.

[chris-garber-vapi]

🟡 A non-string entry in any reference list crashes validate with an error that names no file, instead of producing a finding.

extractReferencedIds calls id.split("##") on every entry, so toolIds: [~] (an empty - list item left while editing) or toolIds: [{ name: foo }] throws:

❌ Validation failed: Cannot read properties of null (reading 'split')

The output has no file, no rule and no annotation, and the CI step only says which org failed. Before this PR, validate never called extractReferencedIds, so these files passed validation and failed later at push.

Two related edge cases on the same path:

• toolIds: [""] or ["## note"] is removed by .filter(Boolean) (line 71) and passes silently. Push drops it silently too.
• A judge structuredOutputId: "## note" isn't filtered (line 77), so it reports references structuredOutputs/, but there is no such file with an empty name.

Fix:

1. In resolver.ts, make cleanId handle any value: (id: unknown) => typeof id === "string" ? (id.split("##")[0]?.trim() ?? "") : "". This also stops the crash in validateNoIgnoredReferences, which runs first.
2. Here, stop filtering empty names and report them for every field, judges included:

if (id === "") {
  finding("error", "malformed-reference",
    `a ${folder} reference is empty or isn't a name`);
  continue;
}

[chris-garber-vapi]

🟡 Annotations have no line, so most of them won't appear next to the bad line in the PR, which the PR description and docs promise.

findingAnnotation emits only file= and title=. Without a line, GitHub attaches the annotation to the top of the file. In Files changed it then appears inline only when line 1 is part of the diff. For a typo on line 14 of a squad, it appears only in the run summary or under "Unchanged files with check annotations". The reference findings also set no fieldPath, so neither the log line nor the annotation says which field holds the bad name, for example members[2].assistantId versus members[0].assistantDestinations[0].assistantId.

Two limits to know before relying on annotations:

• GitHub keeps at most 10 error and 10 warning annotations per step. This one step validates every org, so orgs late in the loop lose theirs.
• Warnings that already exist are annotated on every PR. The starter example already annotates so-assistant-lockstep on receptionist.md.

Fix:

1. Add an optional line to ValidationFinding, and emit line= in findingAnnotation.
2. For reference findings, store the referenced name on the finding and use the first line of the file that contains it. That's accurate enough here:

function lineOf(filePath: string, needle: string): number | undefined {
  const index = readFileSync(filePath, "utf8")
    .split("\n")
    .findIndex((line) => line.includes(needle));
  return index === -1 ? undefined : index + 1;
}

Optionally, annotate warnings only on files the PR changed, so the 10-annotation cap goes to findings the author can act on.

[chris-garber-vapi]

🟢 state[refType][id] is truthy for constructor, toString and __proto__, so those names pass here while push's resolver drops them.

The section is a plain object, so this lookup reaches Object.prototype. Push uses state.tools[cleanId]?.uuid, which is undefined for those names (checked). They're unlikely names, but the fix is small and makes this check match the resolver exactly:

Suggested change

	if (local.get(refType)!.has(id) || state[refType][id]) continue;
	if (local.get(refType)!.has(id) || state[refType][id]?.uuid) continue;

[chris-garber-vapi]

🟢 The credential map is rebuilt for every resource, and this is the third copy of the credential-name walk.

• credentialForwardMap(state) runs once per resource. Build it once in validateReferences and pass it down.
• credentialNames duplicates collectCredentialNames (push.ts:390) and the key handling in replaceCredentialRefs (credentials.ts). Exporting one walker from credentials.ts keeps them consistent; push's copy already misses credentialIds.
• In push, a missing credential is now reported twice: once by this warning and once by warnUnresolvedCredentials (push.ts:383).

The deduplication can go in a stacked PR.

[chris-garber-vapi]

🟡 "Stop ignoring it" tells readers, including coding agents, to do what yaml-conventions.md calls an anti-pattern.

docs/learnings/yaml-conventions.md:211-213: don't edit .vapi-ignore without explicit user direction. Removing a pattern takes a dashboard-owned resource back under gitops control and "can blow away dashboard-only edits on the next push". AGENTS.md sends agents to this table when the Validate check fails, so this row should lead with the safe fix. If a resource must stay dashboard-owned, a UUID reference is the supported form; pull already writes one (see the reference-by-uuid comment).

Suggested change

	| `reference-to-ignored` | error | The referenced resource matches `.vapi-ignore`, so it's never deployed. Stop ignoring it, or remove the reference. |
	| `reference-to-ignored` | error | The referenced resource matches `.vapi-ignore`, so this repo never deploys it. Remove the reference, or reference it by UUID if it must stay dashboard-owned. Don't edit `.vapi-ignore` to get past this without the resource owner's sign-off: un-ignoring gives the resource to gitops (see [YAML conventions](../learnings/yaml-conventions.md)). |

Consider extending the new AGENTS.md line the same way: never edit the state file or .vapi-ignore to make a reference resolve.

[chris-garber-vapi]

🟢 #31 is mitigated rather than resolved, and its status should include the PR number.

• Plain push, which chore: retire src/eval.ts and npm run eval #24 calls "too easy to use", still only warns. It then filters, defers or sends raw names exactly as before.
• A reference whose file was deleted, while its state entry remains, still passes validation. Under push --force, the orphan pass deletes the target and its state entry (delete.ts:457) before the apply pass runs (push.ts:1900 versus 1931+). The reference then hits the resolver's silent drop, one of the three behaviors docs: document orphan-YAML gate + --allow-new-files in README and AGENTS #31 describes.

The table already has wording for this situation: #24 "Open — mitigated by …" and #27 "RESOLVED … (consumers gap open)". CLAUDE.md asks for (#<PR-number>).

Suggested change

	| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | RESOLVED 2026-10-03 (validation) |
	| 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Mitigated 2026-10-03 (#77): validate/apply/CI block it; plain push only warns |

Apply the same wording to the banner at line 1571 and the Status at line 1666.