Finish the contract gaps a downstream host had to patch locally

[sroussey]

Four contracts this repo owns had holes that every consumer worked around in its own tree, and one interaction kind it never had a name for. Follow-up to #923, which landed the fifth (the entitlement approval gate).

Each of these was found the same way: a host reached the failure honestly, patched it downstream, and the patch could not stay downstream — because the thing it compensates for is defined here.

Chat history hygiene — @workglow/ai ChatHistory.ts

normalizeHistoryForModel repairs user, user and tool, user, the sequences that strict chat templates reject outright. HuggingFace's apply_chat_template throws Conversation roles must alternate — this repo ships that template and shipped none of the defence for it. A host reaches the state honestly: a turn stopped while the model was still thinking leaves a trailing user message, and the next thing typed makes two.

trimHistoryForModel caps a message list by characters, not tokens — this package has no tokenizer where it runs, and a wrong tokenizer is worse than an honest approximation. It drops whole turns from the front, cutting only at user boundaries so no tool_result outlives the tool_use it answers (providers reject that orphan, a harder failure than being over budget), and keeps the newest turn even when it alone busts the budget.

Tool-call id durability — @workglow/ai ToolCallIds.ts

Gemini_ToolCalling already documents that ids restart at call_0 every run, and compensates inside its own message conversion. That fixes what reaches the provider, not what a caller keeps: a host holding several rounds in one list collides on round two, and the symptom is not a crash — patches land on the wrong entry, UI keys duplicate, and a pending answer resolves the wrong call.

uniquifyToolCallIds, repairDuplicateToolCallIds and collectToolUseIds make the ids durable caller-side. Ids are opaque to providers, which rebuild their id→name map from the messages each run, so renaming both halves of a pair is invisible downstream.

Bounds on a model-authored JSON Schema — @workglow/ai ModelAuthoredSchema.ts

sanitizeToolArgs already hardens tool-call arguments before validation. The schema those arguments are validated against went through nothing — and in this codebase a format annotation does not style a field, it selects a runtime editor and resolves a live resource ("storage:tabular", "knowledge-base", "credential"). An unbounded format from a model is therefore the model choosing a resource.

validateModelAuthoredSchema bounds property count, nesting depth, and format against a frozen allowlist, returning the reason rather than throwing since the caller's next move is usually handing it back to the model.

This is the one item here with no libs-side caller, and unlike the other four it may never gain one. A guard like this belongs at a trust boundary, and libs owns no boundary that receives a model-authored schema — HumanInputTask.config.contentSchema is the only unvalidated path, and the task cannot validate it because it cannot know its own provenance (every in-code caller builds a trusted schema, and a legitimate format: "knowledge-base" picker is exactly what the allowlist refuses). It ships beside sanitizeToolArgs, the other half of the same surface.

TaskGraphJson shape validation — @workglow/task-graph

createGraphFromGraphJSON throws from inside its own construction, in words written for whoever wrote the deserializer. That is the wrong audience for graph JSON the process did not author — a file, a request body, a model's output — where the caller's next move is handing a reason back to whoever supplied it.

taskGraphJsonShapeError / validateTaskGraphJsonShape name the offending id, recurse into nested subgraphs, and stay structural only: whether a type is runnable is a question about the host's registry, not about the JSON.

kind: "confirm" on IHumanRequest

A confirm is not an elicit with two options. An elicit asks what a value should be and its schema describes fields to fill in; a confirm asks whether something should happen at all, and its schema describes the action, for a person to read. Collapsing them loses the distinction a renderer needs to draw an approval rather than a form.

HumanInputTask expects a response for it (single-turn — a decision has no follow-up round to negotiate). McpElicitationConnector maps it onto elicitation, MCP having no approval primitive. The conformance contract gains a confirm capability, fixture and assertions.

The review round, and what it found

The five above were reviewed after they landed. Two of them did not hold as written, both in the guard whose whole value is its allowlist:

• The format allowlist was bypassable five ways. check() walked only properties and items, so nesting a field under any composition or applicator keyword skipped it. Verified by execution: oneOf, prefixItems, patternProperties, contains, and a $ref into $defs each returned ok: true with format: "credential" inside. The walk now visits every keyword that can carry a subschema, distinguishing those describing the same value from those describing a child, and refuses references outright — a $ref points where the walk cannot follow.
• The "frozen" allowlist was not frozen. Object.freeze does not reach a Set's members, so anything holding the export could allowedFormats.add("credential") and widen the guard globally, while Object.isFrozen stayed true and the test asserting the freeze kept passing. It is a frozen array now. (This changes ModelAuthoredSchemaLimits.allowedFormats from ReadonlySet<string> to readonly string[] — breaking, and free only because the module is unreleased.)

Three more, one per remaining item:

• Branch keywords sit at their parent's depth by design, so maxDepth bounded nothing along a chain of them and a deep oneOf chain overflowed the stack — in the function documented to return a reason rather than throw. Both walks now carry their own ceiling, including the subgraph recursion.
• repairDuplicateToolCallIds counted tool_use and tool_result occurrences in two independent maps, which pairs correctly only when every call has exactly one result. An interrupted round left the surviving result answering the abandoned call. Results now match against the calls of the round they follow.
• Forwarding a confirm's contentSchema as the elicitation's requestedSchema turned the action's description into empty inputs for the person to type — and with required, accept was unreachable. A confirm now puts the details in the message, the only part a client must display, and sends an empty form; its answer is the decision, so nothing comes back as content.

And the conformance suite declared the confirm capability while asserting nothing about it, so a connector could claim the kind and silently auto-accept — the exact failure the capability exists to catch. Three assertions added, with ids so an adapter can mark one known-failing rather than dropping the capability. Reverting the connector's confirm branch fails them.

Verification

• bun scripts/test.ts ai util task-graph graph task entitlement rag unit vitest — 357 files, 4,307 passed, 0 failed
• bun run build — 88/88 turbo tasks
• bun run build:types + bun run typecheck:tests — clean across all seven packages
• bun run format-check, bun scripts/test.ts --check-sections — clean

Run on Node 22.22.2, one below the declared floor of 24; storage-backed sections were left unrun for that reason.

One caveat worth a maintainer's eye: bun run lint reports clean locally while CI, running the identical command, found two type-aware errors (now fixed). Locally oxlint --type-aware walks the tree for ~6s and reports nothing; the base rules do run — a planted debugger is caught — so the type-aware pass is silently inert in this environment. Root cause unknown, and it is not the missing-dist case CLAUDE.md documents: the two errors involved only local types. Worth looking at independently of this PR, since it means a contributor's local lint does not exercise what CI enforces.

Downstream

workglow-dev/builder#461 deletes builder's local copies of all four and is red until this merges and a release follows.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JEFYAGb9D3mWfyhYkmeAvN

---

Generated by Claude Code

[sroussey]

Correcting one thing I wrote in the description

The description carried a caveat saying bun run lint was clean locally while CI found two restrict-template-expressions errors, that the type-aware pass appeared "silently inert" in my environment, that the root cause was unknown, and that it was worth a maintainer's eye because a contributor's local lint would not exercise what CI enforces.

That diagnosis was wrong, and the invitation to investigate should be withdrawn before someone spends time on it.

The cause is 5097240, which turned on every staged-off rule:

this branch's .oxlintrc.json:  "typescript/restrict-template-expressions": "off",  // 56
main's .oxlintrc.json:         "typescript/restrict-template-expressions": "error",

16 rules staged off here, 8 on main. This branch was cut before that commit, and GitHub lints a PR as a merge with the base tip — so CI used main's config while my checkout used the branch's. Both runs behaved correctly on the config each was given; the type-aware pass was never inert.

My evidence didn't discriminate: I confirmed oxlint worked locally by planting a debugger, but no-debugger is a base rule, so it would have been caught under either config. I should have diffed .oxlintrc.json against main before concluding anything.

Nothing about the merged code changes — the two errors were real under the stricter config and the fix (binding the dataflow ids before interpolating them) is right either way. The only false claim was about the tooling.

---

Generated by Claude Code