diff --git a/docs/agent-definition-status-lifecycle.md b/docs/agent-definition-status-lifecycle.md new file mode 100644 index 0000000000..fa7be6e146 --- /dev/null +++ b/docs/agent-definition-status-lifecycle.md @@ -0,0 +1,34 @@ +# Agent definition status lifecycle + +Notes on `@corbits/agent-directory`'s `routes.ts`, `PUT /:definitionId/status`. + +## What it does + +Archive and restore is the whole lifecycle a person controls from the agent +detail page. `stopped` drops a definition out of every launchable listing +while leaving the row, its asset, and its git history untouched — which is +what makes the same route, with `deployed`, a restore rather than a +re-create. + +## The invariant this route relies on + +Two other writers of `workflow_definition.status` exist in this build: + +1. Row creation. `ensureWorkflowDefinitionForAsset` inserts with the column + default (`deployed`) under `onConflictDoNothing` on `(assetId, + wireHash)`, so re-deploying the same definition body over an archived + row is a no-op and can never silently un-archive it. Editing an agent + through this package's own routes only repopulates the asset and never + re-projects a definition row at all. +2. `apps/hub`'s `undeployAgentDefinition`, which writes `stopped` — the + same direction as archiving, so the two can't fight. + +## A known hole + +The one way an archived agent reappears as launchable is a deploy of a +changed body over the same asset: a new `wireHash` misses the unique +constraint and inserts a second definition row, `deployed` by default, +beside the archived one. Nothing in this build takes that path for a +hand-authored agent — only `createAgentDefinitionCore` and Interchange's own +selector deploys call the ensure — but it's a real hole in the +status-as-lifecycle model, not a hypothetical one. diff --git a/docs/agent-session-provisioning.md b/docs/agent-session-provisioning.md new file mode 100644 index 0000000000..fd12b9e945 --- /dev/null +++ b/docs/agent-session-provisioning.md @@ -0,0 +1,37 @@ +# Agent session provisioning + +Notes on `@corbits/workflows`'s `launch/agent-session.ts`. + +## Why this module exists + +Every Workbench launch path provisions through Interchange's +`prepareProvisionedDeployment` with a `sessionId` it mints itself, but +nothing writes that id into `agent_session` — the table +`resolveRunSessionId` reads to route a run's outbound mail. Every launcher +must do this itself, through this one shared helper, so mail and spans +persist against a real session from the run's first turn. + +## Why timing forces a two-step write + +`workflow_run.principal_id` (the FK `agent_session` keys on) is still null +the instant `prepareProvisionedDeployment` returns — a provisioned anchor is +born "deployed" with no principal, and only the run's first trigger +reconciles one onto it. Pre-creating the run's principal to dodge this isn't +an option either: Interchange's own grant materialization inserts the +principal row `onConflictDoNothing` and treats a conflict as "grants already +committed," throwing rather than re-materializing — so nothing upstream of +that first trigger may write the principal first. + +`recordAgentSessionAtProvision` writes the row immediately after +`prepareProvisionedDeployment` returns, keyed on the deploying principal +since the run principal doesn't exist yet. `ensureRunSession` re-keys onto +the run's own principal once one is anchored, called from every seam that +might be a run's first mail-routable moment, and is a true upsert so a run +deployed straight through Interchange's own deployments route (which skips +the eager write) still gets a row. + +## `endAgentSessionFor*` + +Called only from places that already know a run died — a chat relaunch or a +one-shot prompt's teardown. No sweeper; a run nobody reacts to stays +`active` until something does. diff --git a/docs/inference-concepts.md b/docs/inference-concepts.md new file mode 100644 index 0000000000..d8c2d19479 --- /dev/null +++ b/docs/inference-concepts.md @@ -0,0 +1,28 @@ +# Inference concept vocabulary + +Design notes for `@corbits/inference-catalog`'s `concepts.ts`, the words an +agent uses to ask for a model. + +## Why plain data, not a table + +Pure data with no branching or network — the `CATALOG_SEEDS` idiom — so it's +importable on its own. It's deliberately not a table: this is product +vocabulary that ships with the build and needs to be reviewable in a diff. A +bench deviates through its policy row's concept ceilings and allow/deny +lists, never by forking the vocabulary. + +## Ceilings + +USD per million tokens, input and output kept separate — a blended number +would hide which axis a workload actually spends on. Ceilings are soft by +default: a model over ceiling is flagged and sorted last, not dropped, so a +bench whose only provider is expensive still gets an answer. + +## Capabilities vs. reference mix + +Capabilities are `@intx/types`' vocabulary — narrower than the pinned +catalog's, since `long-context` and `prompt-caching` are baked onto catalog +offerings but aren't storable capability values. Where a concept wants +"handles a huge input," its reference mix carries that meaning instead: a +10M-token input mix ranks the model that's cheap on enormous inputs first, +which is the decision the capability would have stood in for. diff --git a/docs/offering-capabilities.md b/docs/offering-capabilities.md new file mode 100644 index 0000000000..915b0778e5 --- /dev/null +++ b/docs/offering-capabilities.md @@ -0,0 +1,32 @@ +# Offering capability resolution + +How `@corbits/inference-catalog`'s `offering-capabilities.ts` decides what a +deployment can do. + +## Why this exists + +`model_offering.capabilities` is the column every capability filter reads — +this package's chain resolution and the platform's own source resolution +alike. Nothing populated it before, so it was empty everywhere. This module +is the source of truth, and it only ever reports capabilities the pinned +catalog observed on the wire: + +- `exact-deployment` — this exact (baseURL, model) was probed; use its list. +- `same-model-wire` — the model was probed on other deployments speaking the + same wire, and this one relays it; use the intersection across those + deployments, so a relay never claims more than every probed deployment + demonstrated. +- `unknown` — no probe covers it; empty list, said plainly. + +An empty list is deliberately not softened with a per-adapter baseline: "an +OpenAI-compatible endpoint serves plain text" is false for the embedding +models such endpoints also serve, and a wrong capability tag routes real +work to a model that cannot do it. + +## Storable vocabulary + +The pinned catalog's vocabulary is a superset of what the platform can +store: it bakes in `long-context` and `prompt-caching`, which `@intx/types`' +`Capability` does not accept. Everything this module reports is filtered +down to the storable vocabulary, so a seed never posts a value the hub will +reject. diff --git a/docs/ollama-context-defaults.md b/docs/ollama-context-defaults.md new file mode 100644 index 0000000000..3a0062fe9b --- /dev/null +++ b/docs/ollama-context-defaults.md @@ -0,0 +1,39 @@ +# Ollama context defaults + +Why `@corbits/inference-catalog`'s `ollama-context-defaults.ts` exists and +how it plugs into inference. + +## The problem + +`@intx/inference`'s built-in OpenAI-shaped adapter defaults output tokens to +`options.maxTokens ?? 4096` when a caller sets none, and Ollama's own +OpenAI-compatible endpoint silently defaults `options.num_ctx` (its real +context window) to a small built-in size when nothing sets it either — +truncating real conversations for every locally served chat agent, with no +error to say so. + +## Where the fix lands + +`@corbits/ollama-adapter`'s `OllamaAdapterConfig` is where an override +actually reaches the built request body (`resolveOverride` and +`createOllamaAdapter`). This module supplies the real per-model values, +stored as a `model_offering`'s `quirks` column so it reaches +`InferenceSource.quirks` — and from there the adapter — through the +platform's existing resolution, with no parallel override path. + +## Table entries are native windows, not extended ceilings + +Each table entry is the model's advertised *native* context window, not a +YaRN/rope-extended ceiling: requesting a `num_ctx` past what a model (and +the host's memory) can back can fail allocation or force heavy swap, a real +operational risk rather than a convention to enforce. A model absent from +the table gets no override — Ollama's own default stands rather than this +module guessing at a ceiling it cannot back up. + +## `default` vs. `perModel` + +Each offering is already scoped to one exact model, so `quirksForDeployment` +wraps the resolved override in `default` — the field `resolveOverride` +applies when no `perModel` entry is present — rather than `perModel`, which +exists for a single shared connection serving several models through one +quirks bag. diff --git a/docs/run-outcome-status.md b/docs/run-outcome-status.md new file mode 100644 index 0000000000..1a1176d67d --- /dev/null +++ b/docs/run-outcome-status.md @@ -0,0 +1,29 @@ +# Run outcome status + +Notes on `@corbits/workflows`'s `schedule/run-outcome.ts`: how a platform +run's raw `status` column becomes what a surface should show. + +## Why this exists + +Warm-keep leaves a fire's delivery agent deployed after it replies, so +`workflow_run.status` never settles out of `running` on its own. Insights, +Mission Control, and the shell activity feed all read a listing-shaped +payload through this module rather than badging the raw column, so a +lingering `running` status past `FIRE_RUNNING_WINDOW_MS` reads as completed +instead of still in flight. This logic was originally +`@corbits/routines`'s `health.ts`/`run-language.ts`, carried over verbatim +when routines cut over to native `ScheduleTrigger` definitions. + +## In-flight signal and abandonment + +A finished fire is supposed to land `completed`/`failed`/`cancelled` plus +`endedAt` via `markTerminal`. `FIRE_RUNNING_WINDOW_MS` is a last resort for a +fire already known abandoned that never got that write — it is never +applied to a live in-flight fire, since a tool loop can outlast ten minutes +before persist catches up. + +Absent `turns`/`hasInFlightTurn` is treated as unknown, never as "no +in-flight turn." Treating an omission as empty would revert the +live-tool-loop false-complete this module exists to prevent, so a listing +row is abandoned only when the producer explicitly said there is no +in-flight turn, and the fire is older than the window. diff --git a/docs/workflow-authoring-registry.md b/docs/workflow-authoring-registry.md new file mode 100644 index 0000000000..cc24029384 --- /dev/null +++ b/docs/workflow-authoring-registry.md @@ -0,0 +1,33 @@ +# Workflow authoring registry + +Notes on `@corbits/workflows`'s `authoring/registry.ts`. + +## What it is + +An agent's in-tenant surface for publishing a workflow codebase as a native +`kind:"workflow"` hub asset, republishing it, and reading it back. Every +write is gated by own-tenant scoping plus an explicit grant-store +authorization call, and by `validateWorkflowSourceTree` before anything +reaches `RepoStore`. Deploying is not this registry's job — an agent deploys +a committed asset through the stock deployments route with the run bearer. + +## Why `populateAsset` uses the `hub` principal + +`workflowKindHandler.workflowAuthorize` only recognizes three principal +kinds for a workflow-asset write: `hub` (full access), `sidecar` +(read-only), and `user` (gated by git-token-shaped bearer claims a +sidecar-authenticated caller never carries). There's no fourth +"workflow-run" principal kind the substrate understands, so the real +per-write authorization decision has to be made here, by this registry, +against the grant store and the resolved caller identity — before the +already-authorized write is handed to the substrate as a hub-mediated +commit under `hub` (the same principal `@corbits/skills`'s `writeSkillMd` +uses). + +## Why head-sha reads go through `RepoStore` directly + +`AssetService` exposes blob and directory reads pinned to a ref but never +the sha that ref resolves to, and `listAssetBlobs` lists blobs only (no +subtrees), so a full tree walk needs `RepoStore.openCommittedReads`. The +repo id is the asset id under the `workflow` kind, exactly as `AssetService` +composes it internally. diff --git a/docs/workflow-capability-authorization.md b/docs/workflow-capability-authorization.md new file mode 100644 index 0000000000..c80ad69092 --- /dev/null +++ b/docs/workflow-capability-authorization.md @@ -0,0 +1,36 @@ +# Workflow capability route authorization + +Why `@corbits/agent-directory`'s `workflow-capability-routes.ts` skips a +grant-store check. + +## The gap + +The vendored grant-materialization path never seeds a `kind: "workflow"` +run's own principal a `workflow-definition: /update` grant, so +`requireGrant` would 403 every self-update call a run makes for its own +definition until that vendor gap closes. + +## The interim rule + +Rather than block `request_capability` on an unpublished vendor change, +this route skips the grant-store check entirely for the one case it +accepts: a call whose authenticated run targets its own definitionId. That +narrow case is already gated by a stronger control than a grant row — +`@corbits/capability-tools`'s `request_capability` tool declares `approval: +"ask"`, so the reactor suspends every call as a pending approval and +renders it in-chat before this route ever runs. A human already had to +approve the specific addition; the human is the authorizer here, not a +grant row. + +This route still enforces, unconditionally: + +1. The caller's run must resolve to a live tenant/principal/run via the + sidecar-token + run-address check. +2. The path `definitionId` must equal the resolved run's own definitionId + (rejected 403 otherwise). +3. The addition must fail closed against the tenant's live capability + inventory. + +Once the real self-update grant is seeded in vendor grant materialization, +this route can route through `requireGrant` like every other +definition-mutating surface instead of carrying this interim rule. diff --git a/docs/workflow-definition-projection.md b/docs/workflow-definition-projection.md new file mode 100644 index 0000000000..375be6c532 --- /dev/null +++ b/docs/workflow-definition-projection.md @@ -0,0 +1,34 @@ +# Workflow definition projection + +Notes on `@corbits/workflows`'s `definition-projection.ts`, the pure reader +that turns an already-resolved inert wire projection into a `FoldedBody`. + +## Why this exists + +There's no hub-side row to read a launch body back off of. This module is +the pure half: the schema-validated reader for anything holding a +projection value from elsewhere (chat, in particular). Nothing here queries +a database. + +`grantRequirements` is deliberately excluded from the projection: it does +not survive the live-to-inert projector and lives instead on the +`workflow_definition.grant_requirements` column, so callers pass it in +alongside the projection rather than reading it off. + +## Step shapes + +An inert projection's launch step comes in two shapes: a bare `step` (the +conversational case) or an `onTrigger` section whose inline body carries the +one agent-bearing step that answers each turn. `extractAgentBearingStep` +reads through either shape with one reader, so neither call site duplicates +the other's parsing. + +## Why multi-step definitions are rejected + +The launch target (`@corbits/agent-runtime`'s `AgentRuntimeConfig`) renders +exactly one `systemPrompt` into one mailbox-triggered turn — it has no +notion of step order. Reading past `stepOrder[0]` would silently drop every +later step's behavior rather than run it, so `MultiStepFoldUnsupportedError` +is thrown instead. Genuine multi-step launch needs a different deploy front +(Interchange's native workflow-run trigger, `@intx/workflow-host`'s DAG +supervisor) than this module provides. diff --git a/packages/agent-directory/src/agent-definition-draft-routes.ts b/packages/agent-directory/src/agent-definition-draft-routes.ts index ff5230dce0..8868d72957 100644 --- a/packages/agent-directory/src/agent-definition-draft-routes.ts +++ b/packages/agent-directory/src/agent-definition-draft-routes.ts @@ -1,15 +1,5 @@ -// The HTTP surface for Myra-backed agent-definition drafting -//: tenant-scoped, `requireGrant`-gated, personal to the -// requesting principal, request parsing via arktype at the boundary, -// route registration only. Error copy at this boundary is plain -// language for the person who typed the description; the technical -// detail every fail-closed error class carries goes to the server log -// instead. -// -// Relocated out of `@corbits/task-planner` (deleted along with the -// rest of the tasks primitive) because this drafting flow was never a -// task concept — it backs the "Describe" step of agent creation -// (`CreateAgentPanel` and onboarding), which stays. +// The HTTP surface for Myra-backed agent-definition drafting. Error copy is +// plain language for the person who typed the description; detail is logged. import { type } from "arktype"; import { Hono } from "hono"; @@ -37,14 +27,8 @@ const CreateAgentDefinitionDraftBody = type({ export type CreateAgentDefinitionDraftRoutesDeps = { requireGrant: RequireGrant; - /** - * The agent-definition drafting port — omitted entirely on - * a host that hasn't wired Myra drafting up yet, in which case this - * route 404s rather than pretending to draft and always failing. - * The route never touches `./agent-definition-drafting.ts`'s - * runner/inventory machinery directly, so it stays testable with a - * plain stub — no database, no one-shot runner. - */ + /** Omitted on a host that hasn't wired Myra drafting up yet, in which + * case the route answers 503 instead of pretending to draft. */ draftAgentDefinition?(input: { readonly tenantId: string; readonly principalId: string; @@ -53,13 +37,9 @@ export type CreateAgentDefinitionDraftRoutesDeps = { }): Promise; }; -/** Every fail-closed error the agent-definition drafting path can throw - * — Myra unresolvable, the run timing out or failing, an unparseable - * reply, or an out-of-inventory model/tool package/skill reference — - * reads as the same honest "couldn't draft" 422 to the person who typed - * the description: from their point of view it is still "Myra's draft - * didn't work out," never a REST-shaped bad request they authored - * themselves. */ +/** Every fail-closed error the drafting path can throw reads as the same + * honest "couldn't draft" 422, never a REST-shaped bad request the person + * authored themselves. */ function isDraftingFailure(err: unknown): boolean { return ( err instanceof MyraAgentDefinitionDraftingUnavailableError || diff --git a/packages/agent-directory/src/agent-definition-drafting.ts b/packages/agent-directory/src/agent-definition-drafting.ts index 634929cb68..04a2c65c4c 100644 --- a/packages/agent-directory/src/agent-definition-drafting.ts +++ b/packages/agent-directory/src/agent-definition-drafting.ts @@ -1,24 +1,6 @@ -// Myra-backed agent-definition drafting: turns a name and a -// plain-language "what should this agent do?" description into a -// machine-checked draft via one one-shot Myra call — the third consumer -// of the inventory-assembly + strict-reply-schema + -// fail-closed-inventory-validation pattern this package's own -// `planner-run.ts` already establishes. Every failure mode — Myra -// unresolvable, the run timing out or failing, an unparseable reply, an -// out-of-inventory model/tool package/skill pick — propagates as its -// own honest, specific error; nothing here fabricates a draft or falls -// back to a template. -// -// This module only proposes — it never deploys. The caller (the create- -// agent panel's `POST .../planner/agent-definitions/draft` client call) -// takes the validated draft and submits it through -// `@corbits/agent-directory`'s own sanctioned REST create path, exactly -// like a hand-authored agent. `toolPackagePins` rides in the draft's -// validated shape, bounded the same way `./create-bounds.ts` bounds a -// planner `{create}` step's pins, even though that REST boundary has no -// field for it yet (see `@corbits/agent-directory`'s own `validation.ts` -// comment) — a caller with a deploy path that does accept pins (this -// package's own `{create}` branch) can use the draft unchanged. +// Myra-backed agent-definition drafting: turns a name and a plain-language +// description into a machine-checked draft via one one-shot Myra call. +// This module only proposes; the caller submits it through the create path. import { type } from "arktype"; @@ -32,27 +14,13 @@ const MAX_REPLY_EXCERPT = 400; const MAX_SYSTEM_PROMPT_LENGTH = 8000; const MAX_DESCRIPTION_LENGTH = 500; -/** `@corbits/capability-tools`' package name — the - * `request_capability` bundle every drafted agent gets pinned by - * default (see `validateAgentDefinitionDraftReplyAgainstInventory`'s - * default-pin step below), the same way a definition that pins skills - * always gets `@corbits/skills-tools` alongside them - * (`@corbits/agent-directory`'s `reindexPinnedSkills`) — self-service - * capability requests are a baseline capability every drafted agent - * should carry, not something Myra has to remember to choose. Only - * added when the tenant's own inventory actually offers it (a tenant - * whose hub build has it un-pinnable, or which never seeded it, gets - * no dangling pin). Kept a literal here (rather than importing - * `@corbits/capability-tools`) the same way this module already treats - * every inventory entry as a bare name string, never a package - * dependency. */ +/** The `request_capability` bundle every drafted agent gets pinned by + * default, so self-service capability requests aren't something Myra has + * to remember to choose. Only added when the tenant's inventory offers it. */ const CAPABILITY_REQUEST_TOOL_PACKAGE = "@corbits/capability-tools"; -/** Adds `CAPABILITY_REQUEST_TOOL_PACKAGE` to a draft's resolved - * `toolPackagePins` whenever the tenant's inventory offers it and it - * isn't already there. Applied to every draft unconditionally — Myra's - * own `toolPackagePins` choice never has to include it for the drafted - * agent to carry it. */ +/** Adds `CAPABILITY_REQUEST_TOOL_PACKAGE` when the inventory offers it and + * it isn't already there. Applied to every draft unconditionally. */ function withDefaultCapabilityRequestPin( toolPackagePins: readonly string[], inventory: PlannerInventory, @@ -80,14 +48,9 @@ const BoundedDescription = type("string > 0").narrow((value, ctx) => : ctx.mustBe(`at most ${MAX_DESCRIPTION_LENGTH} characters`), ); -/** - * Myra's reply shape: the system prompt to deploy the agent with, plus - * everything optional — a refined one-line description, a model pick - * from the tenant's catalog, up to `BoundedDedupedToolPackageNameArray`'s - * cardinality of tool package pins, and skill names to attach. Nothing - * here is deployed until `validateAgentDefinitionDraftReplyAgainstInventory` - * proves every reference was actually offered. - */ +/** Myra's reply shape. Nothing here is deployed until + * `validateAgentDefinitionDraftReplyAgainstInventory` proves every + * reference was actually offered. */ export const AgentDefinitionDraftReply = type({ systemPrompt: BoundedSystemPrompt, "description?": BoundedDescription, @@ -97,9 +60,7 @@ export const AgentDefinitionDraftReply = type({ }); export type AgentDefinitionDraftReply = typeof AgentDefinitionDraftReply.infer; -/** The validated, deploy-ready shape `propose` resolves with — every - * optional collection defaulted to `[]` so a caller never has to - * distinguish "Myra proposed none" from "the field was omitted". */ +/** The validated, deploy-ready shape `propose` resolves with. */ export type AgentDefinitionDraft = { readonly systemPrompt: string; readonly description?: string; @@ -139,11 +100,8 @@ export class MyraAgentDefinitionDraftingUnavailableError extends Error { } } -/** Parses `raw` as an `AgentDefinitionDraftReply`, throwing - * `AgentDefinitionDraftReplyUnparseableError` on malformed JSON, a shape - * that doesn't match, an empty/oversized system prompt or description, - * or a `toolPackagePins` array over cardinality/with a duplicate. Never - * partially trusts a near-miss. */ +/** Parses `raw` as an `AgentDefinitionDraftReply`, throwing on malformed + * JSON, a bad shape, or an over-cardinality/duplicate pins array. */ export function parseAgentDefinitionDraftReply(raw: string): AgentDefinitionDraftReply { let json: unknown; try { @@ -164,15 +122,8 @@ export function parseAgentDefinitionDraftReply(raw: string): AgentDefinitionDraf return parsed; } -/** - * Asserts every reference a validated-shape `AgentDefinitionDraftReply` - * makes actually appears in `inventory` — the inventory that was - * actually offered to Myra — then returns the deploy-ready - * `AgentDefinitionDraft` (optional collections defaulted to `[]`). - * Throws `AgentDefinitionDraftReferenceOutOfInventoryError` on the first - * violation found: an out-of-catalog `modelPreference`, an - * out-of-inventory tool package pin, or an out-of-inventory skill name. - */ +/** Asserts every reference the reply makes actually appears in the + * inventory offered to Myra, then returns the deploy-ready draft. */ export function validateAgentDefinitionDraftReplyAgainstInventory( reply: AgentDefinitionDraftReply, inventory: PlannerInventory, @@ -238,17 +189,15 @@ export type AgentDefinitionDraftingPort = { readonly tenantId: string; readonly principalId: string; readonly name: string; - /** Omitted entirely for a name-only "Get started" — the drafting - * flow still runs (see `buildAgentDefinitionDraftPrompt`), never a - * template fallback. */ + /** Omitted for a name-only "Get started" — drafting still runs, + * never a template fallback. */ readonly purpose?: string; }): Promise; }; -/** `purpose` is optional — "Get started" with just a name is a supported - * happy path (the person teaches the agent in the conversation once it's - * created), so this still asks Myra for a real draft rather than a - * template: a different framing, not a different code path. */ +/** `purpose` is optional — a name-only "Get started" is a real happy path, + * so this still asks Myra for a real draft: a different framing, not a + * different code path. */ function buildAgentDefinitionDraftPrompt( name: string, purpose: string | undefined, @@ -333,15 +282,9 @@ function buildAgentDefinitionDraftPrompt( ].join("\n"); } -/** - * Builds an `AgentDefinitionDraftingPort` backed by one one-shot Myra - * call: resolve Myra's definition for the tenant, assemble the - * inventory she may reference, ask her to turn the name + purpose into - * an `AgentDefinitionDraftReply`, and never trust that reply beyond what - * `parseAgentDefinitionDraftReply` and - * `validateAgentDefinitionDraftReplyAgainstInventory` can prove about - * it. - */ +/** Builds an `AgentDefinitionDraftingPort` backed by one one-shot Myra + * call, never trusting the reply beyond what the parse/validate pair + * proves about it. */ export function createMyraAgentDefinitionDrafting( deps: AgentDefinitionDraftingRunnerDeps, ): AgentDefinitionDraftingPort { diff --git a/packages/agent-directory/src/agent-workflow.ts b/packages/agent-directory/src/agent-workflow.ts index f8b01bedff..77c75bf2b0 100644 --- a/packages/agent-directory/src/agent-workflow.ts +++ b/packages/agent-directory/src/agent-workflow.ts @@ -1,15 +1,5 @@ -// Builds the single-step, folded workflow definition a hand-authored -// agent materializes as: exactly the shape `@corbits/chat`'s own -// `buildWorkbenchHostWorkflow`/`@corbits/myra`'s -// `buildAssistantWorkflow` produce, but with the system prompt and -// model left to the caller instead of fixed at build time — this is -// the one difference that makes a defined-by-a-person agent possible -// alongside the platform's own fixed starter agents. -// -// This package is installable data, exactly like `@corbits/chat`'s -// workbench-host builder: nothing imports it statically, and a host -// publishes the serialized definition as a workflow asset before -// deploying or launching it. +// Builds the single-step, folded workflow definition a hand-authored agent +// materializes as, with system prompt and model left to the caller. import { defineAgent } from "@intx/agent"; import { defineWorkflow, step } from "@intx/workflow"; @@ -35,24 +25,16 @@ import { createPinnedVersionResolver } from "./tool-package-version"; export const AGENT_DEFINITION_STEP_ID = "agent"; -/** - * The tool package that turns a name in the `` index - * into an actual skill body at run time. A definition that pins skills - * must pin this too, or its prompt would tell the model to call a - * `skills_load` tool that does not exist. - */ +/** Turns a name in the `` index into an actual skill body + * at run time. A definition that pins skills must pin this too. */ export const SKILLS_TOOL_PACKAGE_PIN = { name: "@corbits/skills-tools", version: "0.0.9", } as const; -/** - * The parts of a serialized definition the pinned-skills reindex - * rewrites: every step agent's system prompt and its tool-package pins. - * Undeclared keys pass through, so re-serializing a validated definition - * preserves the trigger, the step timeouts, the inference sources, and - * everything else the builder put there. - */ +/** The parts of a serialized definition the pinned-skills reindex rewrites. + * Undeclared keys pass through, so re-serializing preserves everything else + * the builder put there. */ const DefinitionWithAgentSteps = type({ steps: { "[string]": type({ @@ -91,12 +73,8 @@ function withSkillsToolPin( return pinsSkills ? [...others, { ...SKILLS_TOOL_PACKAGE_PIN }] : others; } -/** - * Rewrites every step agent so it advertises exactly `entries`: an - * `` index in the system prompt, and the skills tool - * bundle among its tool-package pins. Replaces whatever a previous push - * left, so re-pinning is idempotent and unpinning removes both. - */ +/** Rewrites every step agent to advertise exactly `entries`. Replaces + * whatever a previous push left, so re-pinning is idempotent. */ export function reindexPinnedSkills( workflowJson: string, entries: readonly PinnedSkillIndexEntry[], @@ -118,14 +96,9 @@ export function reindexPinnedSkills( return JSON.stringify(definition); } -/** Reads a definition's pinned skill names back out of its serialized - * `workflow.json` — the `` stanza `reindexPinnedSkills` - * writes into the step agent's system prompt. The asset is the source of - * truth for pins: every writer of pins (create, skills edit, capability - * add, run pin) reindexes the stanza in the same commit it deploys, so a - * reader never needs side state beside the definition row. Every builder - * in this codebase produces exactly one step, so the definition's one - * step is unambiguous regardless of the step's own key. */ +/** Reads pinned skill names back out of the `` stanza + * `reindexPinnedSkills` writes. The asset is the source of truth — every + * pin writer reindexes in the same commit it deploys. */ export function readPinnedSkillNames(workflowJson: string): readonly string[] { const raw: unknown = JSON.parse(workflowJson); const definition = DefinitionWithAgentSteps(raw); @@ -157,12 +130,8 @@ export function readPinnedSkillNames(workflowJson: string): readonly string[] { return names; } -/** Reads a definition's system prompt back out of its serialized - * `workflow.json` — the raw text a person edits in the Assistant - * settings section, before `reindexPinnedSkills` appends the - * `` index on top of it at save time. Every builder - * in this codebase produces exactly one step, so the definition's one - * step is unambiguous regardless of the step's own key. */ +/** Reads the raw system prompt a person edits, before `reindexPinnedSkills` + * appends the `` index at save time. */ export function readAgentSystemPrompt(workflowJson: string): string { const raw: unknown = JSON.parse(workflowJson); const definition = DefinitionWithAgentSteps(raw); @@ -197,19 +166,15 @@ export function withAgentSystemPrompt(workflowJson: string, systemPrompt: string return JSON.stringify(definition); } -/** A definition's guided-capability-add surface: the tool packages it - * pins directly (beyond whatever `reindexPinnedSkills` pins for - * skills — see `SKILLS_TOOL_PACKAGE_PIN`) and the model it resolves - * against, read back out of its serialized `workflow.json`. */ +/** A definition's guided-capability-add surface: its directly pinned tool + * packages and the model it resolves against. */ export type AgentDefinitionCapabilities = { readonly toolPackagePins: readonly ToolPackagePin[]; readonly model?: string; }; -/** Reads a definition's current guided-capability state out of its - * serialized `workflow.json` — the same fields `withAgentToolPackagePin`/ - * `withAgentModel` write, read back for the settings surface's - * "Capabilities" list and for merging an additive pin. */ +/** Reads the same fields `withAgentToolPackagePin`/`withAgentModel` write, + * for the settings surface's "Capabilities" list. */ export function readAgentCapabilities(workflowJson: string): AgentDefinitionCapabilities { const raw: unknown = JSON.parse(workflowJson); const definition = DefinitionWithAgentSteps(raw); @@ -228,19 +193,9 @@ export function readAgentCapabilities(workflowJson: string): AgentDefinitionCapa : { toolPackagePins: step.agent.toolPackagePins ?? [] }; } -/** - * A tool-package pin resolved to a concrete, published version — never - * the npm "any version" range `*`. `ToolPackagePin` itself (the wire - * type `@intx/types/tool-packages` declares) still accepts `*`, because - * an operator hand-authoring a workflow source file is free to write - * one; but every RUNTIME site this package writes a pin from - * (`create_agent`'s tool-package pins, guided capability-add) must name - * a version the resolver actually offers today — a `*` pin - * means a later tarball landing in the registry silently changes what - * an already-deployed specialist runs, with no record of the change. - * `./tool-package-version.ts`'s `resolvePinnedVersion` is how a caller - * that only has a package name gets one of these. - */ +/** A tool-package pin resolved to a concrete, published version — never + * `*`. Every runtime pin site must name a version the resolver actually + * offers, or a later tarball would silently change what runs. */ export const NonWildcardToolPackagePin = type({ name: "string", version: "string", @@ -253,12 +208,8 @@ export const NonWildcardToolPackagePin = type({ ); export type NonWildcardToolPackagePin = typeof NonWildcardToolPackagePin.infer; -/** Adds or replaces one tool-package pin by name, leaving every other - * pin — including the skills bundle `reindexPinnedSkills` manages — - * untouched. Mirrors `withSkillsToolPin`'s replace-by-name shape, - * generalized to a caller-supplied pin rather than the fixed skills - * bundle. Rejects a `"*"` version outright (see `NonWildcardToolPackagePin`) — - * every runtime caller must supply a concrete, resolved version. */ +/** Adds or replaces one tool-package pin by name, leaving every other pin + * untouched. Rejects a `"*"` version outright. */ export function withAgentToolPackagePin( workflowJson: string, pin: NonWildcardToolPackagePin, @@ -283,10 +234,8 @@ export function withAgentToolPackagePin( return JSON.stringify(definition); } -/** Sets a definition's model preference, leaving every other inference - * field (and every other step field) untouched. Mirrors the placeholder - * `buildAgentDefinitionWorkflow` sets at create time (`provider: - * "catalog"` — resolved fresh at launch, never baked in). */ +/** Sets a definition's model preference, leaving every other field + * untouched. `provider: "catalog"` resolves fresh at launch, never baked in. */ export function withAgentModel(workflowJson: string, model: string): string { const raw: unknown = JSON.parse(workflowJson); const definition = DefinitionWithAgentSteps(raw); @@ -301,11 +250,8 @@ export function withAgentModel(workflowJson: string, model: string): string { return JSON.stringify(definition); } -/** Clears a definition's model preference, leaving every other step field - * untouched: with no inference source, launch-time resolution falls to - * whatever catalog default the tenant has seeded — exactly the state a - * definition created without a model lands in. The inverse of - * `withAgentModel`, so a person who pinned a model can un-pin it. */ +/** Clears a definition's model preference; launch-time resolution then + * falls to the tenant's catalog default. The inverse of `withAgentModel`. */ export function withoutAgentModel(workflowJson: string): string { const raw: unknown = JSON.parse(workflowJson); const definition = DefinitionWithAgentSteps(raw); @@ -336,39 +282,18 @@ export interface AgentDefinitionWorkflowInput { * against the live catalog (see `resolveDefinitionSources`), not * baked into the definition. */ readonly model?: string; - /** - * Tool packages pinned directly on this definition — connector tool - * bundles (e.g. `@corbits/example-tools`) a planner-created agent - * needs beyond what skills reindexing pins. Additive: undeclared or - * empty behaves exactly like a definition built before this field - * existed. `defineAgent`'s own `DefineAgentConfig` has no field for - * this (only `AgentDefinition` itself carries `toolPackagePins`, as - * a passthrough for the sidecar's tool-materialization step — see - * `@intx/agent`'s `definition.ts`), so it is set directly on the - * definition `defineAgent` returns rather than threaded through the - * config, mirroring how `reindexPinnedSkills` sets the same field - * post-hoc for skills. - */ + /** Tool packages pinned directly beyond what skills reindexing pins. + * Additive: undeclared or empty behaves as before this field existed. */ readonly toolPackagePins?: readonly ToolPackagePin[]; - /** - * Credential bindings the deployed definition carries at the workflow - * level — the same `CredentialBinding[]` shape and the same - * `defineWorkflow({ credentialBindings, ... })` field - * `workflows/granola-call` pins through (pattern). Additive: - * undeclared or empty behaves exactly like a definition built before - * this field existed. Required for a `toolPackagePins` entry whose - * tool needs a live credential to do anything at runtime — a pin with - * no matching binding is inert. - */ + /** Credential bindings at the workflow level. Required for a + * `toolPackagePins` entry whose tool needs a live credential — a pin + * with no matching binding is inert. */ readonly credentialBindings?: readonly CredentialBinding[]; } -/** - * Builds the definition. Exactly one step, on purpose — the same - * contract every other folded builder in this codebase holds to: a - * second step would trade away the conversational, warm-agent memory - * a folded launch depends on. - */ +/** Builds the definition. Exactly one step, on purpose — a second step + * would trade away the conversational, warm-agent memory a folded launch + * depends on. */ export function buildAgentDefinitionWorkflow( input: AgentDefinitionWorkflowInput, ): WorkflowDefinition { @@ -385,11 +310,8 @@ export function buildAgentDefinitionWorkflow( tools: [], capabilities: [], inference: { - // `provider` only participates in deploy-hash bookkeeping — - // launch-time resolution reads `model` alone and resolves a - // provider fresh against the tenant catalog (see - // `resolveDefinitionSources`), so a placeholder here costs - // nothing real. + // `provider` only participates in deploy-hash bookkeeping; + // launch-time resolution reads `model` alone. sources: input.model !== undefined ? [{ provider: "catalog", model: input.model }] : [], }, }); @@ -423,14 +345,9 @@ export function buildAgentDefinitionWorkflow( const AGENT_DEFINITION_TURN_TIMEOUT_MS = 2 * 60 * 1000; -/** - * Serializes a definition to the JSON a workflow asset carries. - * Re-implemented rather than shared: `assertJsonPortable` is - * module-private in every builder package that carries a copy of it, - * by design (see `@corbits/chat`'s `workbench-workflow.ts`), so this - * copy stays consistent with that convention rather than reaching - * into another package's internals. - */ +/** Serializes a definition to the JSON a workflow asset carries. Its own + * `assertJsonPortable`, not shared, since that helper is module-private by + * convention in every builder package that carries a copy. */ export function serializeAgentDefinitionWorkflow(definition: WorkflowDefinition): string { assertJsonPortable(definition, "definition"); return JSON.stringify(definition); @@ -440,10 +357,7 @@ export type CreateAgentDefinitionCoreDeps = { readonly db: DB["db"]; readonly assetService: AssetService; /** Deploys the definition's commit through the native source pipeline - * (install -> sidecar probe -> gate -> freeze) at create; the - * composition root injects the SAME `WorkflowDeployer` - * `@corbits/workflows`'s `./authoring`'s registry calls, wrapping - * `sessionService.deployWorkflowFromSource`. */ + * (install -> sidecar probe -> gate -> freeze) at create. */ readonly deployer: AgentDefinitionDeployer; readonly skillIndex: { resolve( @@ -452,33 +366,17 @@ export type CreateAgentDefinitionCoreDeps = { names: readonly string[], ): Promise; }; - /** - * Resolves the tenant's current catalog default model — the same - * first-connected-provider model `@corbits/chat`'s - * `workbenchHostInferencePreferences` derives for a fresh workbench host - * (see `createWorkbenchHostInferencePreferencesResolver`) — for a - * `create_agent`/`POST /agent-definitions` call that supplies no - * `model` of its own. Without this, such a definition's `inference.sources` - * stays empty and a later invite launch 409s as `not_launchable` - * ("declares no model requirements"); baking a real model in here - * makes the definition launchable on its own, no fallback needed at - * launch time. Omitted, or a tenant with no connected provider, - * leaves the definition exactly as empty as before this dep existed. - */ + /** Resolves the tenant's catalog default model for a create call that + * supplies none. Without this, the definition's `inference.sources` + * stays empty and a later invite launch 409s as `not_launchable`. */ readonly tenantDefaultModel?: (tenantId: string) => Promise; }; export type CreateAgentDefinitionCoreInput = { readonly tenantId: string; readonly principalId: string; - /** The tenant's mail domain, which the definition's placeholder mail - * trigger is addressed under (`buildAgentDefinitionWorkflow`'s - * `tenantDomain`). Supplied by the caller rather than looked up here: - * `./routes.ts`'s tenant-session route already has it on the - * request's resolved `tenant` context object, and re-querying it here - * would be a redundant round trip on every create; `./workflow-create-routes.ts` - * resolves it itself from the authenticated run's `tenantId` before - * calling in. */ + /** The tenant's mail domain the placeholder mail trigger addresses + * under. Supplied by the caller to avoid a redundant lookup here. */ readonly tenantDomain: string; readonly handle: string; readonly name: string; @@ -486,11 +384,8 @@ export type CreateAgentDefinitionCoreInput = { readonly systemPrompt: string; readonly model?: string; readonly skills: readonly string[]; - /** Tool packages pinned directly by name (e.g. `@corbits/memory-tools`), - * applied via `withAgentToolPackagePin` once per name after the - * skills reindex — the one addition beyond what `CreateAgentDefinitionInput`'s - * REST boundary accepts (see that type's own comment for why the - * person-facing form has no field for this). */ + /** Tool packages pinned directly by name, applied once per name after + * the skills reindex. */ readonly toolPackagePins?: readonly string[]; }; @@ -498,12 +393,8 @@ export type CreateAgentDefinitionCoreResult = { readonly row: typeof workflowDefinition.$inferSelect; }; -/** Thrown when `input.handle` already names a definition in this - * tenant — the same "conflict" case `./routes.ts`'s `POST /` answers - * with a 409, surfaced here as a typed error so both HTTP callers - * (the tenant-session route and the workflow-run route) can translate - * it into their own response shape without duplicating the asset- - * recovery logic that detects it. */ +/** Thrown when `input.handle` already names a definition in this tenant, + * so each HTTP caller can translate it into its own response shape. */ export class DuplicateAgentHandleError extends Error { constructor(handle: string) { super(`An agent with the handle "${handle}" already exists`); @@ -511,18 +402,10 @@ export class DuplicateAgentHandleError extends Error { } } -/** - * The full create-agent-definition sequence: resolve the tenant's mail - * domain, build and pin the definition's serialized workflow, materialize - * it as a `workflow`-kind asset carrying its pinned-skills index in its - * own stanza, and project it onto a first-class `workflow_definition` - * row. Factored out - * of `./routes.ts`'s `POST /` handler so `./workflow-create-routes.ts` - * (a workflow-run-authenticated surface a tool call reaches, never a - * person through a form) can create a definition through the exact - * same materialization the tenant-session route uses — never a second, - * drifting implementation. - */ +/** The full create-agent-definition sequence: build and pin the definition, + * materialize it as a `workflow`-kind asset, and project it onto a + * `workflow_definition` row. Factored out so every caller uses the exact + * same materialization, never a second, drifting implementation. */ export async function createAgentDefinitionCore( deps: CreateAgentDefinitionCoreDeps, input: CreateAgentDefinitionCoreInput, @@ -537,18 +420,12 @@ export async function createAgentDefinitionCore( const definition = buildAgentDefinitionWorkflow( model !== undefined ? { ...baseDefinitionInput, model } : baseDefinitionInput, ); - // The definition's own system prompt is what the caller supplied; the - // pinned-skills index and any directly-named tool-package pins are - // appended on the way to the asset, so the stored prompt always - // describes exactly what the definition currently carries. let workflowJson = reindexPinnedSkills( serializeAgentDefinitionWorkflow(definition), await deps.skillIndex.resolve(input.tenantId, input.principalId, input.skills), ); - // One resolver shared across every named pin: it loads the tenant's - // registry asset and tarball listing at most once, so a five-pin - // create still costs one ancestor walk and one listing, not five - //. + // One resolver shared across every named pin, so a five-pin create + // still costs one ancestor walk and one listing, not five. const resolvePin = createPinnedVersionResolver( { db: deps.db, assetService: deps.assetService }, input.tenantId, @@ -570,10 +447,8 @@ export async function createAgentDefinitionCore( assetId = created.id; } catch (cause) { if (cause instanceof AssetServiceError && cause.reason === "duplicate_asset") { - // A previous attempt may have created the asset row but failed - // before populateAsset wrote its source tree — an empty shell that - // blocks retries with a misleading conflict. Recover: look up the - // existing asset and reuse it only if it has no definition yet. + // Recover from a prior attempt's empty-shell asset: reuse it only + // if it has no definition yet. const existing = await deps.db.query.asset.findFirst({ where: and( eq(asset.tenantId, input.tenantId), diff --git a/packages/agent-directory/src/asset-write.ts b/packages/agent-directory/src/asset-write.ts index c273545cd8..90bc57d7b3 100644 --- a/packages/agent-directory/src/asset-write.ts +++ b/packages/agent-directory/src/asset-write.ts @@ -1,12 +1,6 @@ -// Same-asset definition edits are read-modify-write: two concurrent -// routes that snapshot the same workflow.json and then each write their -// own field would last-write-wins clobber the other. This -// module serializes those writers and retries the loser against the -// latest snapshot. -// -// The lock is an in-process promise chain keyed by asset id. Hub is a -// single replica, so that chain is the lock — not a Redis or Postgres -// advisory lock, and not safe across multiple hub replicas. +// Same-asset definition edits are read-modify-write; this module serializes +// writers with an in-process lock keyed by asset id — correct only for a +// single hub replica. import type { AssetService } from "@intx/hub-sessions"; @@ -48,12 +42,8 @@ export type CommitLatestAgentAssetSnapshotArgs = { write: (prepared: { workflowJson: string; message: string }) => Promise; }; -/** - * Applies one mutation against the definition's current asset, retrying - * when a concurrent writer moved the snapshot between this call's read - * and its write. The per-asset lock makes that stale check atomic so - * the loser reapplies on the winner's tree instead of clobbering it. - */ +/** Applies one mutation against the definition's current asset, retrying + * when a concurrent writer moved the snapshot between read and write. */ export async function commitLatestAgentAssetSnapshot( args: CommitLatestAgentAssetSnapshotArgs, ): Promise { diff --git a/packages/agent-directory/src/capability-add.ts b/packages/agent-directory/src/capability-add.ts index bdfcc2fb30..07f860002e 100644 --- a/packages/agent-directory/src/capability-add.ts +++ b/packages/agent-directory/src/capability-add.ts @@ -1,10 +1,5 @@ -// Capability-add is a read-modify-write of the definition's asset: two -// concurrent POSTs that both snapshot the same workflow.json and then -// each write their own pin would last-write-wins clobber the other -//. This module owns that mutation so both the tenant-session -// route and the workflow-run route retry the loser against the latest -// snapshot instead of silently dropping an add. The lock itself lives -// in `./asset-write.ts` so sibling RMW routes share it. +// Capability-add is a read-modify-write of the definition's asset; the lock +// lives in `./asset-write.ts` so sibling RMW routes share it. import type { PinnedSkillIndexEntry } from "@corbits/skills-tools"; import type { DB } from "@intx/db"; @@ -46,13 +41,8 @@ export type CommitAgentCapabilityAddResult = { model?: string; }; -/** - * Applies one capability add against the definition's current asset, - * retrying when a concurrent writer moved the snapshot between this - * call's read and its write. The per-asset lock makes that stale check - * atomic so the loser reapplies on the winner's tree instead of - * clobbering it. - */ +/** Applies one capability add, retrying when a concurrent writer moved the + * snapshot between this call's read and its write. */ export async function commitAgentCapabilityAdd( args: CommitAgentCapabilityAddArgs, ): Promise { @@ -95,20 +85,14 @@ async function prepareCapabilityAdd( ): Promise { let nextWorkflowJson: string; let message: string; - // Pins read out of the snapshot itself — the asset's stanza is the - // source of truth, so a concurrent writer's pins survive a retry - // against the latest snapshot instead of being clobbered by a stale - // side-table read. + // Read out of the snapshot itself, so a concurrent writer's pins survive + // a retry instead of being clobbered by a stale side-table read. let skills = readPinnedSkillNames(workflowJson); switch (args.body.kind) { case "toolPackage": { - // A package already pinned keeps its stored version: re-adding it - // (e.g. a person re-clicking "add" on something already listed) is - // a no-op on the version, never a silent bump to whatever the - // registry's newest tarball happens to be today. Only a name with - // no existing pin resolves fresh against the registry. An explicit - // bump is a distinct, explicit input this does not add. + // A package already pinned keeps its stored version — re-adding it + // is a no-op, never a silent bump to the registry's newest tarball. const packageName = args.body.name; const existingPin = readAgentCapabilities(workflowJson).toolPackagePins.find( (pin) => pin.name === packageName, diff --git a/packages/agent-directory/src/capability-inventory.ts b/packages/agent-directory/src/capability-inventory.ts index 6b2c0615e2..2c53dad65a 100644 --- a/packages/agent-directory/src/capability-inventory.ts +++ b/packages/agent-directory/src/capability-inventory.ts @@ -1,27 +1,13 @@ -// The guided-capability-add fail-closed check: an addition to a -// definition's tools/skills/model is only ever accepted if it names -// something the tenant's live inventory actually offers. This mirrors -// this codebase's other inventory-validation ports (parse the -// addition, then check its one reference against a `Set` built from the -// inventory that was actually offered) exactly — but the caller -// deploys through this package's own builder, so importing its -// inventory type back here would cycle. `CapabilityInventory` below is -// the same shape stripped to what a single addition needs to check -// against; the composition root (`apps/hub`) wires it from the exact -// same listers the drafting inventory uses, so the two inventories one -// tenant sees are never allowed to drift apart even though the types -// are declared twice. +// The guided-capability-add fail-closed check: an addition is only +// accepted if it names something the tenant's live inventory offers. import { type } from "arktype"; export type CapabilityToolPackageEntry = { readonly name: string }; export type CapabilitySkillEntry = { readonly name: string }; export type CapabilityModelEntry = { readonly canonicalName: string }; -/** - * The pins every created specialist carries unless the caller names its - * own: firm memory and the ask_user interaction card. A specialist - * without these is a name with a prompt. - */ +/** The pins every created specialist carries unless the caller names its + * own. A specialist without these is a name with a prompt. */ export const BASELINE_AGENT_TOOL_PINS = ["@corbits/memory", "@corbits/interaction-tools"] as const; /** The baseline pins this tenant can actually resolve — a package the @@ -59,11 +45,7 @@ export const AddCapabilityInput = type({ .or(type({ kind: "'model'", canonicalName: "string > 0" })); export type AddCapabilityInput = typeof AddCapabilityInput.infer; -/** - * Asserts `addition` names something `inventory` actually offers. Throws - * `CapabilityOutOfInventoryError` on the first (only) violation — never - * partially trusted, exactly like the other inventory checks this mirrors. - */ +/** Asserts `addition` names something `inventory` actually offers. */ export function assertCapabilityInInventory( addition: AddCapabilityInput, inventory: CapabilityInventory, diff --git a/packages/agent-directory/src/client.ts b/packages/agent-directory/src/client.ts index 9c07a4e4be..7ba531bd6a 100644 --- a/packages/agent-directory/src/client.ts +++ b/packages/agent-directory/src/client.ts @@ -1,11 +1,6 @@ // Browser-safe "what counts as a user-facing agent" logic: filtering out -// the chat anchor machinery's workbench hosts (they are plumbing, not an -// agent a person created), full-text search across the fields a person -// actually reads (never an id), and flagging an instance whose definition -// has since gone missing from the tenant's own listing. Depends directly on -// `@corbits/chat/workbench-host-naming` — a domain package, not app state — -// to identify that plumbing; a host injects only its raw definition/ -// instance lists, already fetched from wherever it gets them. +// workbench-host plumbing, full-text search across fields a person actually +// reads, and flagging an instance whose definition has gone missing. import { deriveDisplayName, @@ -16,15 +11,8 @@ import { } from "./display-name"; import { isConversationalWorkflowName } from "@corbits/workflows/catalog"; -// `deriveDisplayName`/`humanizeSlug` live in `@corbits/chat` -// itself, not here: this package already depends on `@corbits/chat` (for -// `isWorkbenchHostDefinitionName` below), so a copy defined here could -// never be imported back by `@corbits/chat`'s own call sites without a -// circular dependency — exactly the gap that traces the "Run -// 737a058d…" leak to (the chat participant invite/greeting path never -// migrated onto this derivation because it couldn't). Re-exported here so -// every existing caller of `@corbits/agent-directory/client`'s -// `deriveDisplayName`/`humanizeSlug` keeps working unchanged. +// Re-exported from `./display-name` so every existing caller of this +// module's `deriveDisplayName`/`humanizeSlug` keeps working unchanged. export { deriveDisplayName, humanizeSlug, withDisplayName, withDisplayNames }; export type { WithDisplayName }; @@ -41,33 +29,17 @@ export type UserFacingAgentInstance = { readonly address?: string; }; -/** Every definition, minus the chat anchor machinery's workbench hosts — - * those are internal plumbing, never a user-facing agent — and minus every - * non-conversational workflow-catalog utility (`workbench-digest`/"Daily - * digest", `last-30-days-research`/"Last 30 days research", …): those are - * mail-triggered automations a routine schedules, never something a person - * opens a chat with, so they belong on the Routines page only. The one - * distinguishing property is `isConversationalWorkflowName` — "can this be - * DMed" — the same test `listVisibleAgentDefinitions` already applies to the - * sidebar's DM list; `automatable` (schedulable as a routine) is orthogonal - * and never decides this. Definitions never need the run-id filter - * `purposeAgentInstances` below takes: definition rows aren't run rows, so a - * chat-plumbing run's own id can never match here — an invited agent's real - * `definitionId` stays a legitimate, reusable template even though its - * *instance* is chat plumbing. */ +/** Every definition minus workbench-host plumbing and minus every + * non-conversational workflow-catalog utility. `isConversationalWorkflowName` + * is the one distinguishing property; `automatable` is orthogonal. */ export function purposeAgentDefinitions( definitions: readonly T[], ): readonly T[] { return definitions.filter((d) => isConversationalWorkflowName(d.name)); } -/** - * `excludeRunIds` additionally drops chat-plumbing runs (invited agents): - * they self-anchor like a real deployment and launch under a real, - * user-authored `definitionId` that `isWorkbenchHostDefinitionName` never - * catches, so the host names them by id instead. Defaults to an empty - * set so callers without a run-id source keep the name-only filter. - */ +/** `excludeRunIds` additionally drops chat-plumbing runs (invited agents), + * which a name-based filter never catches. Defaults to empty. */ export function purposeAgentInstances( instances: readonly T[], excludeRunIds: ReadonlySet = new Set(), @@ -96,15 +68,8 @@ export function filterInstances( return instances.filter((i) => i.definitionName.toLowerCase().includes(needle)); } -/** - * An instance is orphaned when the tenant's own definitions listing no - * longer carries its `definitionId` — the definition was deleted or, - * more commonly, has scrolled past the page's fetch window. A - * definition row's own FK to the run means this can never mean "no - * definition ever existed"; it means "not resolvable from here", which - * is exactly the distinction the UI floor cares about: never hide an - * instance the page cannot fully explain, mark it instead. - */ +/** An instance is orphaned when the tenant's own definitions listing no + * longer carries its `definitionId` — never hide it, mark it instead. */ export function isOrphanedInstance( instance: Pick, definitionsById: ReadonlyMap, diff --git a/packages/agent-directory/src/create-bounds.ts b/packages/agent-directory/src/create-bounds.ts index 72d554d29a..e8af10aea2 100644 --- a/packages/agent-directory/src/create-bounds.ts +++ b/packages/agent-directory/src/create-bounds.ts @@ -1,20 +1,12 @@ -// Bounds the `{create}` branch's `toolPackagePins` the same way -// `@corbits/agent-directory`'s `validation.ts` bounds every other -// free-text/list field a create-agent request carries. That REST -// boundary's own `CreateAgentDefinitionInput` has no `toolPackagePins` -// field at all (see its own comment: no UI affordance ever sends one -// through that route today), so this is the one bound this package -// must own itself rather than reuse — everything else `spawnFromTaskSpec` -// validates before deploying reuses `CreateAgentDefinitionInput` -// directly. +// Bounds `toolPackagePins` the way `./validation.ts` bounds every other +// list field a create-agent request carries. `CreateAgentDefinitionInput` +// has no field for this, so this package owns the bound itself. import { type } from "arktype"; const MAX_TOOL_PACKAGE_PINS = 8; -/** A deduped array of at most `MAX_TOOL_PACKAGE_PINS` tool package - * names, mirroring `@corbits/agent-directory`'s `validation.ts` - * `SkillNameArray`'s own dedup-check style exactly. */ +/** A deduped array of at most `MAX_TOOL_PACKAGE_PINS` tool package names. */ export const BoundedDedupedToolPackageNameArray = type("string[]").narrow((names, ctx) => { if (names.length > MAX_TOOL_PACKAGE_PINS) { return ctx.mustBe(`at most ${MAX_TOOL_PACKAGE_PINS} tool package pins`); diff --git a/packages/agent-directory/src/definition-asset.ts b/packages/agent-directory/src/definition-asset.ts index 228e64e2cf..6098fc7670 100644 --- a/packages/agent-directory/src/definition-asset.ts +++ b/packages/agent-directory/src/definition-asset.ts @@ -1,14 +1,5 @@ // Every read and write of an agent definition's asset tree goes through -// here, so this lineage has exactly one notion of what a definition's -// asset holds: the source codebase `@corbits/workflows`'s `./source` -// renders, never the retired `workflow.json` envelope the push validator -// now refuses. -// -// The rendered package's name never leaves the asset — the tree is a -// standalone codebase the sidecar evaluates, not something anyone -// installs — so it only has to be a valid, stable npm name. A -// definition's handle is already lowercase-kebab (see `./validation.ts`), -// which makes it one. +// here: the source codebase, never the retired `workflow.json` envelope. import { parseWorkflowSourceEntry, @@ -57,25 +48,15 @@ export function parseAgentDefinitionEntry(entryModule: Uint8Array, assetId: stri return parseWorkflowSourceEntry(new TextDecoder().decode(entryModule), assetId); } -/** The `WorkflowDeployer` seam this package needs — never the whole - * registry surface, just the one call that deploys a commit through the - * native source pipeline (install -> sidecar probe -> gate -> freeze). - * The composition root (`apps/hub`) injects the SAME deployer - * `@corbits/workflows`'s `./authoring`'s own registry calls; this - * package never reimplements install/probe/gate/freeze itself. */ +/** The `WorkflowDeployer` seam this package needs — just the one deploy + * call, never the whole registry surface; this package never reimplements + * install/probe/gate/freeze itself. */ export type AgentDefinitionDeployer = Pick; -/** - * Writes a definition's serialized workflow into its asset tree, then +/** Writes a definition's serialized workflow into its asset tree, then * deploys the resulting commit through the native source pipeline — the - * one sequence every content-mutating route in this package needs - * (create, restore, capability add, instructions edit, skills edit). - * Replaces the old write-then-`DefinitionFreezer.freeze`/`refreeze` - * pair: a deploy IS a freeze, plus the install/probe/gate a bare freeze - * skipped. Throws `WorkflowAuthorError` on rejection — `not_found`, - * `invalid` (rejected package/definition), or `unavailable` (sidecar - * unreachable) — for the caller's route to translate into its response. - */ + * one sequence every content-mutating route in this package needs. Throws + * `WorkflowAuthorError` for the caller's route to translate. */ export async function writeAndDeployAgentDefinition(args: { assetService: AssetService; deployer: AgentDefinitionDeployer; @@ -110,10 +91,7 @@ export async function writeAndDeployAgentDefinition(args: { } /** The HTTP status a `WorkflowAuthorError` from `writeAndDeployAgentDefinition` - * should surface as — the same mapping `@corbits/workflows`'s `./authoring`'s - * own `workflow-routes.ts` uses for the native deploy surface, reused here - * so a sidecar-unavailable deploy reads as the same 502 envelope shape - * everywhere a deploy can fail. */ + * should surface as, matching the native deploy surface's own mapping. */ export function statusForAgentDefinitionDeployError( reason: WorkflowAuthorError["reason"], ): 400 | 403 | 404 | 409 | 502 { diff --git a/packages/agent-directory/src/definition-history.ts b/packages/agent-directory/src/definition-history.ts index ebfe00a6c8..7456518bbd 100644 --- a/packages/agent-directory/src/definition-history.ts +++ b/packages/agent-directory/src/definition-history.ts @@ -1,7 +1,5 @@ -// A definition's `workflow`-kind asset carries its own git history. The -// git history IS the version store here: no separate versions table, and -// restoring a version re-commits an older commit's blobs onto the default -// ref rather than rewriting history. +// A definition's `workflow`-kind asset carries its own git history as the +// version store; restoring re-commits an older commit's blobs onto the ref. import fs from "node:fs"; import git from "isomorphic-git"; diff --git a/packages/agent-directory/src/display-name.ts b/packages/agent-directory/src/display-name.ts index 99cc035b73..cc843de057 100644 --- a/packages/agent-directory/src/display-name.ts +++ b/packages/agent-directory/src/display-name.ts @@ -1,14 +1,6 @@ -// A workflow definition's person-facing display name, derived once here -// so every caller that decides "what does this agent look like to a -// person" reads it the same way — never a scattered `description ?? name` -// (or worse, a raw address/run id) reimplemented per call site. -// -// Lives in `@corbits/chat` rather than `@corbits/agent-directory` (which -// originated this logic) because `@corbits/agent-directory` -// itself depends on `@corbits/chat` (`workbench-host-naming`); a reverse -// dependency would be circular. `@corbits/agent-directory/client.ts` -// re-exports `deriveDisplayName`/`humanizeSlug` from here for its existing -// callers rather than keeping a second copy that could drift. +// A workflow definition's person-facing display name, derived once here so +// every caller reads it the same way — never a scattered `description ?? +// name` reimplemented per call site. import { type } from "arktype"; import { ID_LEAK_PATTERN } from "./id-leak-guard"; @@ -17,13 +9,8 @@ const DisplayNameSource = type({ "description?": "string | null", }); -/** kebab-case identifier -> Title Case words: `"research-analyst"` -> - * `"Research Analyst"`. Words that aren't hyphen-separated (a name that - * already reads as prose) pass through with only their case fixed up, so - * this is safe to run over a definition's raw `name` unconditionally — - * PROVIDED that `name` is never an internal id in disguise; see - * `deriveDisplayName`'s own guard for why the raw run-id case never - * reaches this function at all. */ +/** kebab-case identifier -> Title Case words. Safe over a definition's raw + * `name` only because `deriveDisplayName` guards out an internal id first. */ export function humanizeSlug(slug: string): string { return slug .split(/[-_\s]+/) @@ -32,21 +19,10 @@ export function humanizeSlug(slug: string): string { .join(" "); } -/** - * The display name a definition should render as: its own description - * when one was set at creation, otherwise a humanized reading of its - * immutable slug. A whitespace-only description reads as absent — never - * a blank display name — since it carries nothing a person actually - * typed. Throws on a shape that isn't at least `{ name }` — this is a - * trust boundary, not a formatting helper, so a malformed record fails - * loudly rather than rendering "undefined". - * - * Also throws when `name` is itself an internal-id shape (`run_…`, - * `wfd_…`, …, see `./id-leak-guard`) — the product rule is that a person - * never sees an internal identifier, so a caller that reaches this - * function with a run id where a definition's slug belongs gets a loud - * failure instead of a Title-Cased leak like "Run 737a058d…". - */ +/** The display name a definition should render as: its description when + * set, otherwise a humanized slug. Throws on a malformed shape or when + * `name`/`description` is itself an internal id, rather than rendering + * a leak like "Run 737a058d…". */ export function deriveDisplayName(definition: { readonly name: string; readonly description?: string | null; @@ -85,8 +61,7 @@ export type UserFacingAgentDefinition = { export type WithDisplayName = T & { readonly displayName: string }; /** Projects `deriveDisplayName` onto a definition, keeping every other - * field untouched — the read-boundary derivation done once here rather - * than as scattered `??` fallbacks in UI code. */ + * field untouched. */ export function withDisplayName( definition: T, ): WithDisplayName { diff --git a/packages/agent-directory/src/id-leak-guard.ts b/packages/agent-directory/src/id-leak-guard.ts index 6c1d65d0bc..284642c58c 100644 --- a/packages/agent-directory/src/id-leak-guard.ts +++ b/packages/agent-directory/src/id-leak-guard.ts @@ -1,22 +1,5 @@ -// The systemic guard this module enforces: a person never sees an internal -// identifier — not as a participant's name, not in a join/system line, -// not in a greeting, not in a chat title. This has recurred repeatedly -// (a raw definitionId/runId in insights, a `writing_systems` id in a -// Myra reply, and its own "Run 737a058d…" / "I'm run_737a…") each -// time as a one-off display-time patch; this module is the one place -// every id-generating prefix is named, so a new leak is a missed test -// run rather than a missed grep. -// -// Prefix words mirror `@intx/hub-common`'s `generateId` (vendored; -// upstream source: `packages/hub-common/src/ids.ts`'s `PREFIXES`) for -// the six kinds this guard names explicitly — a workflow run, a workflow -// definition, a tenant, a principal, an asset, and a git token — each -// normally followed by `_` and 32 lowercase hex characters (`run_737a…`). -// -// Matched with either `_`, a space, or `-` as the separator, not just -// `_`: `humanizeSlug`'s Title Case reading of a leaked id (its own -// "Run 737a058d48006e2bde12559576f422e0") replaces the underscore with a -// space, so the raw and the humanized-leak forms both need to trip this. +// The one place every id-generating prefix is named, so a display-time leak +// is a missed test run rather than a missed grep. const ID_PREFIX_WORDS = ["run", "wfd", "tnt", "prn", "ast", "gtk"] as const; export const ID_LEAK_PATTERN = new RegExp( @@ -24,15 +7,9 @@ export const ID_LEAK_PATTERN = new RegExp( "i", ); -/** - * Throws when `value` carries an internal identifier — a raw id, or a - * humanized reading of one (`"Run 737a058d…"`, produced by title-casing - * `run_737a058d…`'s underscore split). The humanized case is why this - * checks the ORIGINAL prefixes case-insensitively rather than requiring - * the literal lowercase prefix: `humanizeSlug` capitalizes the first - * letter of the leading word, so a leaked run id renders as "Run …", not - * "run …", by the time it would reach a person. - */ +/** Throws when `value` carries an internal identifier — a raw id, or a + * humanized reading of one, matched case-insensitively since + * `humanizeSlug` capitalizes the leading word. */ export function assertNoLeakedInternalId(value: string, context: string): void { if (ID_LEAK_PATTERN.test(value)) { throw new Error( diff --git a/packages/agent-directory/src/inventory.ts b/packages/agent-directory/src/inventory.ts index 94ceb53840..2bd052f5fc 100644 --- a/packages/agent-directory/src/inventory.ts +++ b/packages/agent-directory/src/inventory.ts @@ -1,22 +1,6 @@ -// Assembles the compact "here is everything you may reference" fact -// sheet that rides inside the planner prompt. Every source is -// host-injected (mirrors `@corbits/tasks`' `TaskLauncherDeps.isTaskableDefinition` -// seam): this package owns the shape and the assembly, never the -// listing logic itself — a tenant's usable agents, tool packages, -// skills, and models are each already owned by another package -// (`@corbits/tasks`' definition list, the stock credentials route, -// `@corbits/skills-tools`' registry, the tenant model catalog route), so -// task-planner only defines the seam those listers plug into. -// -// Memory folds into `listUsableToolPackages` rather than getting its -// own lister: memory is consumed by an agent as a tool package pin -// (the memory tool package), exactly like any connector-backed tool -// package, so a host's implementation only includes that entry when -// `memoryAvailable` is true and nothing else in this module needs to -// know how memory is wired. `memoryAvailable` is still surfaced as -// its own top-level fact so the planner prompt can reason about it -// directly ("memory is available" is a fact worth stating plainly, -// not left for the model to infer from a tool package name). +// Assembles the compact "here is everything you may reference" fact sheet +// for the planner prompt. Every source is host-injected — this package owns +// only the shape and assembly, never the listing logic. import type { CredentialBinding } from "@intx/types"; import { sanitizeInventoryText } from "./sanitize-inventory-text"; @@ -33,12 +17,8 @@ export type InventoryAgent = { export type InventoryToolPackage = { readonly name: string; readonly connectorId: string; - /** The credential a `{create}` definition pinning this tool package - * must be granted for the pin to work at runtime — the same - * `CredentialBinding` shape `workflows/granola-call`'s - * `GRANOLA_CALL_CREDENTIAL_BINDINGS` uses. `null` for a - * tool package that needs no per-tenant credential (e.g. the memory - * tool package). */ + /** The credential a definition pinning this tool package must be + * granted; `null` when no per-tenant credential is needed. */ readonly credentialBinding: CredentialBinding | null; }; @@ -64,19 +44,14 @@ export type InventorySources = { listConversationalAgents(tenantId: string): Promise; listUsableToolPackages(tenantId: string): Promise; listSkills(caller: { tenantId: string; principalId: string }): Promise; - /** A process-level fact (`mountMemory() !== undefined` at hub boot), - * not a per-call async lookup — memory is either compiled into this - * process's deployment or it isn't, for the process's whole - * lifetime. */ + /** A process-level fact, not a per-call lookup — memory is either + * compiled into this deployment or it isn't. */ readonly memoryAvailable: boolean; listModels(tenantId: string): Promise; }; -/** Builds the inventory Myra is offered for one planning call. Kept - * compact and JSON-serializable — this rides inside an LLM prompt, so - * no redundant or verbose fields belong here beyond what - * `validateTaskSpecAgainstInventory` needs to check references - * against and what the prompt needs to describe each option. */ +/** Builds the inventory Myra is offered for one planning call. Kept compact + * and JSON-serializable, since this rides inside an LLM prompt. */ export async function assembleInventory( sources: InventorySources, caller: { tenantId: string; principalId: string }, diff --git a/packages/agent-directory/src/one-shot-prompt.ts b/packages/agent-directory/src/one-shot-prompt.ts index 670cbdab09..9d5e27ac2a 100644 --- a/packages/agent-directory/src/one-shot-prompt.ts +++ b/packages/agent-directory/src/one-shot-prompt.ts @@ -1,8 +1,6 @@ -// A synchronous wrapper around one provisioned run's opening turn: -// prepare, send the prompt, wait for exactly one reply, tear down the -// subscription AND the launched run itself. Other launchers return as -// soon as the run starts; this module turns the event stream into an -// awaitable promise for a caller that has no later-delivery surface. +// A synchronous wrapper around one provisioned run's opening turn: send, +// await exactly one reply, tear down the run — for a caller with no +// later-delivery surface. import { and, eq } from "drizzle-orm"; import { connectorReplyContent, messageRunEnded } from "./agent-events"; import { reportError } from "@corbits/error-sink"; @@ -60,28 +58,18 @@ export type OneShotRunnerDeps = { >; readonly sidecarRouter: Pick; readonly eventCollectors: EventCollectorPort; - /** - * Reads the hub's live sidecar routing table (the same source the - * webhook launch path uses). A freshly provisioned run's sidecar - * takes several seconds to register, so the opening send fires once; - * if it fails as unreachable, this is polled — bounded — until the - * address is routable and the send is retried exactly once. - */ + /** Reads the hub's live sidecar routing table so a retry can wait for a + * freshly provisioned run's sidecar to register. */ readonly isRoutable: (address: string) => boolean; - /** - * Test seam only. Overrides `deliverWhenRoutable`'s default wait - * budget/poll so the expiry path stays fast under test; production - * never sets it and the defaults apply. - */ + /** Test seam only. Overrides the default wait budget/poll so the expiry + * path stays fast under test. */ readonly deliverWait?: { readonly deadlineMs?: number; readonly pollIntervalMs?: number; readonly sleep?: (ms: number) => Promise; }; - /** - * Test seam only. Production never sets these; they default to - * Interchange `prepareProvisionedDeployment` and `SidecarRouter.routeMail`. - */ + /** Test seam only. Production defaults to Interchange's own + * provisioning and routing. */ readonly provision?: (input: { readonly tenantId: string; readonly principalId: string; @@ -201,16 +189,9 @@ async function provisionOnAsset( }; } -/** - * Provisions a run against `input.definitionId`, sends `input.prompt` - * as its opening mail, and resolves with the run's accumulated - * `connector.reply` content once its opening turn's `message.run.ended` - * bracket closes. - * - * No owning workbench_launch row and no Inbox delivery. The event - * subscription unsubscribes exactly once, and `deps.undeploy` tears the - * run down exactly once, on every exit path. - */ +/** Provisions a run, sends the opening mail, and resolves with the reply + * once the turn's bracket closes. Unsubscribes and tears the run down + * exactly once, on every exit path. */ export async function runOneShotPrompt( deps: OneShotRunnerDeps, input: OneShotPromptInput, diff --git a/packages/agent-directory/src/resolve-myra-definition-id.ts b/packages/agent-directory/src/resolve-myra-definition-id.ts index 780c664245..2c1ea1531e 100644 --- a/packages/agent-directory/src/resolve-myra-definition-id.ts +++ b/packages/agent-directory/src/resolve-myra-definition-id.ts @@ -1,9 +1,6 @@ -// Resolves Myra's own workflow-definition id for a tenant — the one -// piece of DB wiring every one-shot Myra caller in this codebase needs -// (this package's own `agent-definition-drafting.ts`), each of which -// takes it as an injected `resolveMyraDefinitionId` port rather than -// importing this module directly, so a test can stub it without -// touching a database. +// Resolves Myra's own workflow-definition id for a tenant. Callers take it +// as an injected port rather than importing this module directly, so a +// test can stub it without touching a database. import { and, eq } from "drizzle-orm"; import type { DB } from "@intx/db"; import { workflowDefinition } from "@intx/db/schema"; @@ -21,11 +18,8 @@ export class MyraDefinitionUnresolvableError extends Error { } } -/** - * Queries `workflowDefinition` by Myra's seeded asset name and - * `tenantId` — there is no "current tenant's Myra" foreign key - * anywhere else to join through. - */ +/** Queries `workflowDefinition` by Myra's seeded asset name and `tenantId` + * — there is no "current tenant's Myra" FK to join through. */ export async function resolveMyraDefinitionIdFromDb( db: DB["db"], tenantId: string, diff --git a/packages/agent-directory/src/routes.ts b/packages/agent-directory/src/routes.ts index 482c34c965..eec4641fe2 100644 --- a/packages/agent-directory/src/routes.ts +++ b/packages/agent-directory/src/routes.ts @@ -1,18 +1,6 @@ -// The create-agent-definition surface: a tenant member submits a -// name/handle/description/system-prompt/model, and this route -// materializes it exactly the way the platform's own starter agents -// (`@corbits/myra`, `@corbits/chat`'s workbench host) are -// materialized — a `workflow`-kind asset carrying a single-step -// definition as a source codebase (see `./definition-asset.ts`), -// projected onto a first-class `workflow_definition` row. No git subprocess: `AssetService.populateAsset` writes the -// commit in-process, the same seam `createAsset` used to hydrate a -// workbench host's asset lives beside. -// -// The definition lands with the schema's own default status -// ("deployed") and a non-null assetId, which is exactly what -// `ChatPlatform.listInvitableDefinitions`/`launchInvite` require to -// treat it as launchable — a freshly created agent is invitable and -// launchable the moment this route returns, no separate "deploy" step. +// The create-agent-definition surface: materializes a submitted definition +// as a `workflow`-kind asset. Lands `deployed` with a non-null assetId, so +// a freshly created agent is invitable and launchable immediately. import { type } from "arktype"; import { and, eq } from "drizzle-orm"; @@ -70,12 +58,8 @@ import { listVisibleAgentDefinitions } from "./visible-definitions"; import { reportError } from "@corbits/error-sink"; import { makeErrorEnvelope } from "@corbits/error-sink"; -/** - * Resolves the pinned skill names a definition carries into the - * name-and-description index its system prompt advertises. Required, not - * optional: a definition pushed without a resolved index would carry - * pins its agent has no way to discover. - */ +/** Resolves pinned skill names into the name-and-description index the + * system prompt advertises. Required, not optional. */ export type PinnedSkillIndexResolver = { resolve( tenantId: string, @@ -102,17 +86,13 @@ export type CreateAgentDefinitionRoutesDeps = { capabilityInventory: CapabilityInventoryProvider; requireGrant: RequireGrant; /** Deploys the definition's commit through the native source pipeline - * on every content write; the composition root injects the SAME - * `WorkflowDeployer` `@corbits/workflows`'s `./authoring`'s registry - * calls. */ + * on every content write. */ deployer: AgentDefinitionDeployer; tenantDefaultModel?: CreateAgentDefinitionCoreDeps["tenantDefaultModel"]; }; -/** The same 404 shape a missing definition gets — deliberately reused - * for a workbench host's definition too (see `hostGuardedRow`), so a - * caller cannot distinguish "no such definition" from "that id names a - * workbench host" by response shape alone. */ +/** The same 404 shape reused for a workbench host's definition (see + * `hostGuardedRow`), so response shape can't distinguish the two. */ function definitionNotFound(definitionId: string) { return makeErrorEnvelope({ code: "not_found", @@ -120,14 +100,8 @@ function definitionNotFound(definitionId: string) { }); } -/** - * A workbench host is a single-step workflow definition exactly like a - * hand-authored agent, but it is the workbench's own silent anchor, never - * a participant a person edits through this surface — rewriting its - * system prompt would turn a silent anchor into a responder. Refused - * the same way a missing definition is: 404, not 403, so the row's - * existence isn't leaked either. - */ +/** A workbench host is never editable through this surface. Refused the + * same way a missing definition is: 404, not 403. */ function hostGuardedRow( row: { readonly name: string; readonly assetId: string | null } | undefined, ): row is { readonly name: string; readonly assetId: string } { @@ -185,11 +159,8 @@ export function createAgentDefinitionRoutes({ const principal = c.get("principal"); const skills = body.skills ?? []; - // `CreateAgentDefinitionCoreInput`'s optional fields are declared - // under `exactOptionalPropertyTypes`, so an absent `description`/ - // `model` must be an absent key, not a key set to `undefined` — - // built up mutably rather than as one literal, mirroring - // `apps/hub`'s own `deployAgentDefinition` caller of this same core. + // Built up mutably, not as one literal: `exactOptionalPropertyTypes` + // needs an absent `description`/`model` to be an absent key. const coreInput: { -readonly [K in keyof CreateAgentDefinitionCoreInput]: CreateAgentDefinitionCoreInput[K]; } = { @@ -264,12 +235,8 @@ export function createAgentDefinitionRoutes({ ), }); if (row === undefined || row.assetId === null) return null; - // Pins read out of the asset's own stanza: the bulk read - // survives the side table's deletion by going to the same - // source `GET /:definitionId` reads. One unreadable asset - // (a pre-cutover retired envelope, a missing blob) must not - // fail the whole batch — skip that id, report it, serve the - // healthy ones. + // One unreadable asset must not fail the whole batch — skip, + // report, serve the healthy ones. const workflowJson = await readAgentDefinitionWorkflowJson(assetService, row.assetId); const skills = readPinnedSkillNames(workflowJson); return [definitionId, skills] as const; @@ -291,10 +258,8 @@ export function createAgentDefinitionRoutes({ return c.json({ skills }); }); - // Feeds the settings surface's guided capability-add picker with only - // what this tenant actually has — the same source `POST - // /:definitionId/capabilities` re-checks fail-closed on the add itself, - // so a name this call doesn't list can never be added either. + // Feeds the guided capability-add picker with only what this tenant has; + // the add route re-checks fail-closed against the same source. app.get("/capabilities/inventory", requireGrant("workflow-definition:*", "read"), async (c) => { const tenant = c.get("tenant"); const principal = c.get("principal"); @@ -305,23 +270,16 @@ export function createAgentDefinitionRoutes({ return c.json(inventory); }); - // Every agent this tenant can open a direct chat with — its own agent - // definitions plus every ancestor's: the sidebar's unified - // recency-sorted stream reads this list, keyed by `tenantId` per row - // so a click can mint the DM in the agent's actual owning tenant, not - // the caller's. + // Every agent this tenant can open a direct chat with, keyed by + // `tenantId` per row so a click mints the DM in the right tenant. app.get("/visible", requireGrant("workflow-definition:*", "read"), async (c) => { const tenant = c.get("tenant"); const definitions = await listVisibleAgentDefinitions(db, tenant.id); return c.json({ definitions }); }); - // A definition's kebab `name` is its immutable, URL-facing slug - //, so a slug-addressed detail screen resolves through this - // route rather than scanning a page of the definitions listing — an - // agent past the listing's pagination ceiling still answers on its own - // URL. Grant-checked tenant-wide because there is no definition id to - // scope to until the lookup itself has run. + // Resolves the immutable slug directly rather than scanning the + // listing, so an agent past its pagination ceiling still resolves. app.get("/by-name/:name", requireGrant("workflow-definition:*", "read"), async (c) => { const tenant = c.get("tenant"); const name = c.req.param("name"); @@ -622,13 +580,8 @@ export function createAgentDefinitionRoutes({ }, ); - // Un-pinning a model is its own verb, not `POST /capabilities` with an - // empty name: that route's whole contract is "a name from this tenant's - // live inventory", and "no model at all" is not a name. Clearing returns - // the definition to resolving whatever catalog default the tenant has - // seeded, which is where a definition created without a model already - // sits — so "Bench default" is a state a person can get back to, not a - // one-way door. + // Its own verb, not `POST /capabilities` with an empty name: clearing + // returns to the tenant's catalog default, a real reachable state. app.delete( "/:definitionId/capabilities/model", requireGrant(idResource("workflow-definition", "definitionId"), "update"), @@ -684,34 +637,8 @@ export function createAgentDefinitionRoutes({ }, ); - // Archive and restore: a definition's status is the whole lifecycle a - // person controls from the agent detail page. `stopped` drops it out of - // every launchable listing (`listVisibleAgentDefinitions`, - // `listInvitableDefinitions`) while leaving the row, its asset, and its - // git history untouched — which is what makes the same route, with - // `deployed`, a restore rather than a re-create. - // - // The invariant this route relies on, and the two other writers of - // `workflow_definition.status` in this build: - // - // 1. Row creation. `@intx/hub-sessions`' `ensureWorkflowDefinitionForAsset` - // inserts with the column default (`deployed`) under - // `onConflictDoNothing` on `(assetId, wireHash)` — so re-deploying - // the same definition body over an archived row is a no-op and can - // never silently un-archive it. Editing an agent through this - // package's own routes only repopulates the asset and never - // re-projects a definition row at all. - // 2. `apps/hub`'s `undeployAgentDefinition`, which writes `stopped` — - // the same direction as archiving, so the two cannot fight. - // - // The one way an archived agent reappears as launchable is a deploy of a - // CHANGED body over the same asset: a new `wireHash` misses the unique - // constraint and inserts a SECOND definition row, `deployed` by default, - // beside the archived one. Nothing in this build takes that path for a - // hand-authored agent (only `createAgentDefinitionCore` and Interchange's - // own selector deploys call the ensure), but it is a real hole in the - // status-as-lifecycle model rather than a hypothetical one, and archiving - // by row status is what makes it reachable. + // Archive and restore. See docs/agent-definition-status-lifecycle.md for + // the invariant this relies on and a known hole in the model. app.put( "/:definitionId/status", requireGrant(idResource("workflow-definition", "definitionId"), "update"), diff --git a/packages/agent-directory/src/sanitize-inventory-text.ts b/packages/agent-directory/src/sanitize-inventory-text.ts index d7136e3576..bee2768369 100644 --- a/packages/agent-directory/src/sanitize-inventory-text.ts +++ b/packages/agent-directory/src/sanitize-inventory-text.ts @@ -1,18 +1,8 @@ -// Defense-in-depth against a hostile or careless free-text field riding -// inside the planner prompt. `buildPlannerPrompt` (`./planner-run.ts`) -// sends the whole inventory through `JSON.stringify`, which already -// delimits/escapes every string as a JSON value — the primary, -// already-sound defense against a description breaking OUT of the JSON -// structure it rides in. The residual risk this module blunts is -// narrower: a long block of imperative-sounding text sitting inside a -// legitimately-quoted JSON string field, trying to socially-engineer -// the model reading it. Truncation and newline-stripping meaningfully -// reduce that surface without pretending to eliminate it. +// Defense-in-depth against a free-text field riding inside the planner +// prompt: a quoted string can still carry an imperative block. -/** Strips newlines/control characters (replaced with a single space, - * whitespace runs collapsed) and truncates to `maxLen`, so an - * untrusted description can't pad the prompt with an oversized, - * multi-line block of imperative text. */ +/** Strips newlines/control characters to a single space and truncates to + * `maxLen`. */ export function sanitizeInventoryText(raw: string, maxLen: number): string { const singleLine = raw .replace(/[\r\n\t\p{Cc}]+/gu, " ") diff --git a/packages/agent-directory/src/stale-task-agent-naming.ts b/packages/agent-directory/src/stale-task-agent-naming.ts index 6c001ea724..37bcf1566e 100644 --- a/packages/agent-directory/src/stale-task-agent-naming.ts +++ b/packages/agent-directory/src/stale-task-agent-naming.ts @@ -1,11 +1,6 @@ -// Recognizes an agent definition minted by the now-deleted tasks -// primitive's planner `{create}` branch, each of which -// existed for exactly one task. The minting side (what used to be -// `@corbits/task-planner`'s `plannerCreatedDefinitionHandle`) is gone -// along with that primitive — nothing creates a definition with this -// prefix anymore — but a tenant seeded before that deletion can still -// carry lingering rows, and any picker meant for agents a person -// deliberately keeps around must keep excluding them. +// Recognizes an agent definition minted by the now-deleted task-planner +// primitive. Nothing mints this prefix anymore, but a tenant seeded before +// that deletion can still carry lingering rows a picker must exclude. const PLANNER_CREATED_HANDLE_PREFIX = "myra-task-"; export function isPlannerCreatedDefinitionName(definitionName: string): boolean { diff --git a/packages/agent-directory/src/tool-package-version.ts b/packages/agent-directory/src/tool-package-version.ts index c94f8f598c..26011e4638 100644 --- a/packages/agent-directory/src/tool-package-version.ts +++ b/packages/agent-directory/src/tool-package-version.ts @@ -1,22 +1,5 @@ -// A runtime tool-package pin must resolve to a concrete, -// published version — never the npm "any version" range `*` — so a new -// tarball landing in the tenant's registry never silently changes what -// an already-deployed specialist runs. `withAgentToolPackagePin` (see -// `./agent-workflow.ts`) rejects `"*"` at its own boundary; this module -// is the one place a caller that only has a package *name* (guided -// capability-add, `create_agent`'s own tool-package pins) resolves it to -// the version to pin instead. -// -// The tenant's usable tool packages are published as npm-style tarballs -// into its (possibly inherited) `corbits-tools` `package-registry` -// asset — the exact same asset `@intx/tool-packaging`'s -// `AssetRegistrySource` reads from at deploy-assembly time -// (`vendor/intx/hub-sessions/src/session-service.ts`'s `buildAndResolve`). -// Reading the tarball listing directly here, rather than re-deriving a -// packument, keeps this resolver a plain filename scan: exactly the -// same `tarballCoversPackage` filter the web app's own registry publisher -// (`apps/web/src/tools/registry-publish.ts`) uses to decide whether a -// registry carries a given package. +// A runtime tool-package pin must resolve to a concrete, published version +// — never `*` — so a new tarball never silently changes a deployed pin. import semver from "semver"; import type { DB } from "@intx/db"; @@ -41,11 +24,9 @@ export type ResolvePinnedVersionDeps = { readonly assetService: AssetService; }; -/** The `-.tgz` filename convention the web app's registry - * publisher writes into a registry's `tarballs/` tree. Returns `null` - * for a filename that does not cover `packageName` or does not parse as - * a valid semver version — a defensively-shaped filename is treated as - * absent rather than crashing the resolution. */ +/** Parses the `-.tgz` filename convention. Returns `null` + * for a filename that doesn't cover `packageName` or parse as semver, + * treated as absent rather than crashing resolution. */ function versionFromTarballFilename(filename: string, packageName: string): string | null { if (!tarballCoversPackage(filename, packageName)) return null; const prefix = `${packageName.replace(/^@/, "").replace("/", "-")}-`; @@ -53,26 +34,16 @@ function versionFromTarballFilename(filename: string, packageName: string): stri return semver.valid(version) !== null ? version : null; } -/** The highest version in `versions`, preferring a stable (non-prerelease) - * version whenever one exists — a prerelease only wins when the registry - * carries no stable version for the package at all. Without this, a - * `2.0.0-rc.1` tarball would outrank an already-published `1.9.0` purely - * because prerelease identifiers still sort after a lower release under - * plain semver comparison for a higher major/minor/patch, silently - * pinning a pre-release build a person never asked for. */ +/** The highest version, preferring stable over prerelease — without this a + * `2.0.0-rc.1` could silently outrank a published `1.9.0`. */ function highestPreferringStable(versions: readonly string[]): string | undefined { const stable = versions.filter((version) => semver.prerelease(version) === null); const candidates = stable.length > 0 ? stable : versions; return candidates.slice().sort(semver.compare).at(-1); } -/** - * Resolves `packageName` against the tenant's (possibly inherited) - * `corbits-tools` registry against the given, already-loaded tarball - * `filenames` listing (see `createPinnedVersionResolver` for the case - * where the caller wants that listing loaded once and reused across - * several package names). - */ +/** Resolves `packageName` against an already-loaded tarball `filenames` + * listing. See `createPinnedVersionResolver` to reuse the listing. */ function resolveFromFilenames( filenames: readonly string[], packageName: string, @@ -87,17 +58,9 @@ function resolveFromFilenames( return { name: packageName, version: highest }; } -/** - * Builds a resolver against the tenant's (possibly inherited) - * `corbits-tools` registry that loads the registry asset and its tarball - * listing at most once — the first `resolve` call pays for the ancestor - * walk and the listing round trip, every subsequent call against the - * same resolver reuses that listing. `resolvePinnedVersion` below is the - * one-name convenience wrapper; a caller resolving several names in one - * request (e.g. `create_agent`'s own `toolPackagePins`) should build one - * resolver and call it once per name instead, so a five-pin create still - * costs one ancestor walk and one listing, not five. - */ +/** Builds a resolver that loads the registry's tarball listing at most + * once, reused across every call. Prefer this over `resolvePinnedVersion` + * when resolving several names in one request. */ export function createPinnedVersionResolver( deps: ResolvePinnedVersionDeps, tenantId: string, @@ -129,27 +92,10 @@ export function createPinnedVersionResolver( }; } -/** - * Resolves `packageName` against the tenant's (possibly inherited) - * `corbits-tools` registry to `{ name, version }`, `version` always the - * highest published STABLE semver among the registry's tarballs for that - * package (a prerelease wins only when the registry carries no stable - * version at all) — never `*`, so a runtime pin is reproducible: a later - * tarball landing in the registry changes what a *new* pin resolves to, - * never what an already-deployed definition's stored pin resolves to. - * - * Throws `CapabilityOutOfInventoryError` — the same fail-closed error - * `assertCapabilityInInventory` throws for any other out-of-inventory - * addition, so every caller's existing `CapabilityOutOfInventoryError` - * -> 4xx mapping (`./routes.ts`, `./workflow-capability-routes.ts`) - * covers this case with no new wiring — when the tenant has no visible - * `corbits-tools` registry, or that registry carries no tarball for - * `packageName`. - * - * Resolving several names for one tenant in the same request? Build a - * `createPinnedVersionResolver` once instead — this wrapper always pays - * for its own ancestor walk and listing round trip. - */ +/** Resolves `packageName` to `{ name, version }`, the highest published + * stable semver, never `*`. Throws `CapabilityOutOfInventoryError` when + * unresolvable. Build a `createPinnedVersionResolver` instead for several + * names in one request. */ export async function resolvePinnedVersion( deps: ResolvePinnedVersionDeps, tenantId: string, diff --git a/packages/agent-directory/src/tool-registry.ts b/packages/agent-directory/src/tool-registry.ts index 3f2502fcf9..deae046717 100644 --- a/packages/agent-directory/src/tool-registry.ts +++ b/packages/agent-directory/src/tool-registry.ts @@ -1,13 +1,7 @@ // The one site naming the tenant-scoped package-registry asset that -// resolves `@corbits/*` tool-package pins, and the filename filter that -// decides whether a registry's tarball listing carries a given package. -// A rename changes one file instead of chasing string literals across -// `apps/hub` (scope routing) and every publish call site. +// resolves `@corbits/*` tool-package pins, so a rename touches one file. -/** - * Asset name the hub's `@corbits` scope routing resolves tool-package - * pins against (`apps/hub/src/index.ts`'s `CORBITS_TOOLS_REGISTRY`). - */ +/** Asset name the hub's `@corbits` scope routing resolves pins against. */ export const CORBITS_TOOLS_REGISTRY = "corbits-tools"; /** True when `filename` is an npm-style tarball for `packageName` (any version). */ diff --git a/packages/agent-directory/src/validation.ts b/packages/agent-directory/src/validation.ts index 301dcab980..d73c5e7169 100644 --- a/packages/agent-directory/src/validation.ts +++ b/packages/agent-directory/src/validation.ts @@ -4,18 +4,13 @@ import { type } from "arktype"; import { skillNameSchema } from "@corbits/skills-tools"; -// Mirrors `@intx/hub-sessions`' `ASSET_NAME_PATTERN` exactly (not -// imported: the constant is internal to that package). A definition's -// handle becomes its workflow asset's name, so it is bound by the same -// lowercase-kebab rule the asset service enforces at creation — failing -// here gives a specific, field-scoped error instead of a generic -// asset-service rejection three calls deeper. +// Mirrors `@intx/hub-sessions`' internal asset-name pattern, so a bad +// handle fails here with a field-scoped error, not a generic rejection +// three calls deeper. const HANDLE_PATTERN = type(/^[a-z0-9]+(?:-[a-z0-9]+)*$/); /** A string that is non-blank once trimmed and at most `max` characters - * untrimmed. Used for every free-text field a person types into the - * create-agent form, so a whitespace-only submission reads as the same - * "required" error a truly empty one would. */ + * untrimmed, so a whitespace-only submission reads as required too. */ function boundedNonBlankString(max: number) { return type("string").narrow((value, ctx) => { if (value.trim() === "") return ctx.mustBe("a non-blank string"); @@ -24,12 +19,8 @@ function boundedNonBlankString(max: number) { }); } -// A pinned skill names a row in the tenant's skill registry -// (`@corbits/skills-tools`), so it is bound by exactly the registry's own name -// rule — the same kebab-case, `<=64`-char shape the hub's `skill` kind -// handler requires of a SKILL.md's frontmatter. A name outside it could -// never resolve to a real skill, so rejecting it here beats storing a -// pin that silently indexes nothing. +// A pinned skill name must match the registry's own naming rule — a name +// outside it could never resolve to a real skill. const SkillNameArray = skillNameSchema.array().narrow((skills, ctx) => { const seen = new Set(); for (const name of skills) { @@ -60,13 +51,8 @@ export const CreateAgentDefinitionInput = type({ systemPrompt: boundedNonBlankString(8000), "model?": boundedNonBlankString(200), "skills?": SkillNameArray, - // `toolPackagePins` names tool packages by name only (no version — the - // core resolves each to `*`, matching `./workflow-create-routes.ts`'s - // own handling of the same field). Absent for a person hand-authoring - // an agent through a form, which has no affordance for typing one; - // callers that supply it install an agent whose tools ship as pinned - // packages rather than inline capabilities (a native `workflows/*` - // agent-directory create). + // Names tool packages by name only — the core resolves each to `*`. + // Absent for a hand-authored form submission. "toolPackagePins?": ToolPackagePinArray, }); export type CreateAgentDefinitionInput = typeof CreateAgentDefinitionInput.infer; @@ -89,21 +75,13 @@ export const UpdateAgentInstructionsInput = type({ }); export type UpdateAgentInstructionsInput = typeof UpdateAgentInstructionsInput.infer; -/** The body of a request that moves a definition between the two - * lifecycle states the schema knows: `deployed` (launchable, listed - * everywhere a person can start a chat) and `stopped` — what the agent - * detail page's "Archive" writes. Never a delete: the definition, its - * asset, and its history all stay intact, so restoring it is one write - * back to `deployed`. */ +/** Moves a definition between `deployed` and `stopped`. Never a delete — + * restoring is one write back to `deployed`. */ export const UpdateDefinitionStatusInput = type({ status: "'deployed' | 'stopped'", }); export type UpdateDefinitionStatusInput = typeof UpdateDefinitionStatusInput.infer; -/** The body of a request that restores a definition to an earlier - * commit on its own asset history — the same `commitSha` shape - * `@corbits/skills-tools`' restore route takes, kept plain-text ("history", - * never "commit SHA") everywhere a person reads it; the sha only - * appears in a tooltip. */ +/** Restores a definition to an earlier commit on its own asset history. */ export const RestoreDefinitionInput = type({ commitSha: "string > 0" }); export type RestoreDefinitionInput = typeof RestoreDefinitionInput.infer; diff --git a/packages/agent-directory/src/visible-definitions.ts b/packages/agent-directory/src/visible-definitions.ts index 9b050379ec..ba243fd47d 100644 --- a/packages/agent-directory/src/visible-definitions.ts +++ b/packages/agent-directory/src/visible-definitions.ts @@ -1,12 +1,6 @@ -// Every agent definition this tenant can open a direct chat with: -// its own, plus every ancestor's, walking the tenant ancestor chain the -// same way `@intx/db`'s own `resolveProviderByName`/`listAssetsForTenant` -// do and `packages/connections/src/mcp-server-store.ts`'s -// `listMcpServerConnections` already does for MCP server connections -// — a child tenant can reach anything a parent tenant made -// available, and a same-name definition made at the child shadows the -// ancestor's. Reads inherit up the chain; creating/editing a definition -// stays own-tenant only (`./routes.ts`). +// Every agent definition this tenant can open a direct chat with: its own, +// plus every ancestor's. A same-name definition at the child shadows the +// ancestor's. Reads inherit up the chain; create/edit stays own-tenant only. import { and, eq } from "drizzle-orm"; import type { DB } from "@intx/db"; import { getAncestorChain, schema } from "@intx/db"; @@ -58,17 +52,11 @@ export async function listVisibleAgentDefinitions( } for (const row of rows) { - // A definition with no materialized asset isn't launchable yet; a - // workbench host is a silent per-workbench anchor, never a DM target; - // a seeded workflow-catalog utility (Echo, Workbench digest, - // Recurring task, Last 30 days research, …) is a mail-triggered - // automation, not a conversational agent — DMing it produces - // nonsense, so only a genuinely conversational definition is listed. + // An unmaterialized asset isn't launchable; a workbench host or a + // mail-triggered catalog utility isn't a DM target. if (row.assetId === null) continue; if (!isConversationalWorkflowName(row.name)) continue; - // Leaf-to-root order means the closer tenant's definition for this - // name was already recorded — an ancestor's same-name row never - // overwrites it. + // Leaf-to-root order: an ancestor's same-name row never overwrites. if (byName.has(row.name)) continue; byName.set(row.name, { id: row.id, diff --git a/packages/agent-directory/src/workflow-capability-routes.ts b/packages/agent-directory/src/workflow-capability-routes.ts index a6b99dabf4..b7bb968667 100644 --- a/packages/agent-directory/src/workflow-capability-routes.ts +++ b/packages/agent-directory/src/workflow-capability-routes.ts @@ -1,43 +1,6 @@ // The sanctioned path for a workflow-process child to add itself a -// capability (`@corbits/capability-tools`'s `request_capability`) -// — the execution half of `POST /:definitionId/capabilities` -// / `GET /capabilities/inventory` in `./routes.ts`, mirroring -// `@corbits/skills-tools`' `createWorkflowSkillRoutes` and -// `@corbits/artifacts-hub`'s workflow-artifacts routes: a workflow child -// has no browser session, only its sidecar bearer token and its own run -// address, so it authenticates through a `WorkflowRunAuthenticator` -// rather than the tenant-session pipeline `./routes.ts` uses. -// -// Mounted OUTSIDE the tenant prefix for that reason, at -// `/api/workflow-capabilities`. Identity NEVER rides in a request body -// or path beyond the definitionId itself: the tenant and principal -// every write is scoped to come from the authenticated run alone. -// -// Authorization decision (deliberate): -// the vendored grant-materialization path never seeds a `kind: -// "workflow"` run's own principal a `workflow-definition: /update` grant — `requireGrant` would 403 every self-update call a -// run makes for its own definition until that vendor gap closes. Rather -// than block `request_capability` on an unpublished vendor change, this -// route skips a grant-store check entirely for the ONE case it accepts: -// a call whose authenticated run targets its OWN definitionId. That -// narrow case is already gated by a stronger control than a grant row — -// `@corbits/capability-tools`' `request_capability` tool declares -// `approval: "ask"` (`@intx/agent`'s native per-invocation gate), so the -// reactor suspends every call as a pending approval and renders it -// in-chat BEFORE this route ever runs; a human already had to approve -// the specific addition. The human is the authorizer here, not a grant -// row. This route still enforces, unconditionally: (1) the caller's run -// must resolve to a live tenant/principal/run via the sidecar-token + -// run-address check below, (2) the path `definitionId` must equal the -// resolved run's OWN definitionId (rejected 403 otherwise — a run can -// never touch another definition through this surface), and (3) the -// addition must fail closed against the tenant's live capability -// inventory (`assertCapabilityInInventory`, unchanged from the -// tenant-session route). Once the real self-update grant is seeded in -// vendor grant materialization, this route can -// route through `requireGrant` like every other definition-mutating -// surface instead of carrying this interim rule. +// capability, authenticated through `WorkflowRunAuthenticator`. No +// grant-store check — see docs/workflow-capability-authorization.md. import { type } from "arktype"; import { and, eq } from "drizzle-orm"; import { Hono } from "hono"; @@ -62,15 +25,9 @@ import { import type { PinnedSkillIndexResolver } from "./routes"; import { makeErrorEnvelope } from "@corbits/error-sink"; -/** - * The tenant + principal + run a presented sidecar token and run - * address resolve to. Declared structurally (mirroring - * `@corbits/skills-tools`' `WorkflowRunScope`) rather than importing - * `@corbits/artifacts-hub`'s concrete type, so this package carries no - * dependency on the artifacts plane; `apps/hub` supplies - * `@corbits/artifacts-hub`'s `createWorkflowRunAuthenticator`, which - * satisfies this shape exactly (it resolves a superset: `runId` too). - */ +/** The tenant + principal + run a sidecar token and run address resolve + * to. Declared structurally so this package carries no dependency on the + * artifacts plane. */ export type WorkflowCapabilityRunScope = { readonly tenantId: string; readonly principalId: string; @@ -107,9 +64,7 @@ export type CreateWorkflowCapabilityRoutesDeps = { capabilityInventory: CapabilityInventoryProvider; authenticator: WorkflowRunAuthenticator; /** Deploys the definition's commit through the native source pipeline - * after the rewrite; the composition root injects the SAME - * `WorkflowDeployer` `@corbits/workflows`'s `./authoring`'s registry - * calls. */ + * after the rewrite. */ deployer: AgentDefinitionDeployer; }; @@ -165,11 +120,7 @@ export function createWorkflowCapabilityRoutes( const scope = c.get("workflowCapabilityScope"); const definitionId = c.req.param("definitionId"); - // Resolve the calling run's OWN definitionId and reject outright if - // the path names anything else — a run may only ever touch its own - // definition through this surface (see the file-level "why" comment - // for the full authorization decision this enforces in place of a - // grant-store check). + // A run may only ever touch its own definition through this surface. const run = await deps.db.query.workflowRun.findFirst({ where: eq(workflowRun.id, scope.runId), }); @@ -208,10 +159,7 @@ export function createWorkflowCapabilityRoutes( tenantId: scope.tenantId, principalId: scope.principalId, }); - // Throws `CapabilityOutOfInventoryError`, caught by `app.onError` - // above — fail closed against exactly the inventory this call just - // fetched, never a stale or wider one. Unchanged from the - // tenant-session route's own fail-closed check. + // Throws `CapabilityOutOfInventoryError`, caught by `app.onError`. assertCapabilityInInventory(body, inventory); const added = await commitAgentCapabilityAdd({ diff --git a/packages/agent-directory/src/workflow-create-routes.ts b/packages/agent-directory/src/workflow-create-routes.ts index d329c8f6f7..9ccfa8fc05 100644 --- a/packages/agent-directory/src/workflow-create-routes.ts +++ b/packages/agent-directory/src/workflow-create-routes.ts @@ -1,36 +1,6 @@ -// The sanctioned path for a workflow-process child to create a NEW -// agent definition in its own tenant and list the tenant's taskable -// agents — the execution half of `@corbits/agent-directory-tools`' -// `create_agent`/`list_agents` (Myra's manager tools), mirroring -// `./workflow-capability-routes.ts`: a workflow child has no browser -// session, only its sidecar bearer token and its own run address, so -// it authenticates through a `WorkflowRunAuthenticator` rather than the -// tenant-session pipeline `./routes.ts` uses. -// -// Mounted OUTSIDE the tenant prefix for that reason. Identity NEVER -// rides in a request body or path: the tenant and principal every -// write is scoped to come from the authenticated run alone. -// -// Scope, deliberately different from `./workflow-capability-routes.ts`'s -// "own definition only" rule: creating a definition has no existing -// row to scope self-ness against, so this surface is scoped to the -// caller's own TENANT instead — a run may create any number of new -// definitions in its own tenant, never in another. -// -// Authorization decision (same shape as `./workflow-capability-routes.ts`'s, -// see that file's own comment for the full reasoning this mirrors): -// this route carries no `requireGrant` check. The calling tool -// (`@corbits/agent-directory-tools`' `create_agent`) declares -// `approval: "ask"` (`@intx/agent`'s native per-invocation gate), so -// the reactor suspends the call as a pending approval and renders it -// in-chat BEFORE this route ever runs — a human already had to approve -// the specific agent being created. This route still enforces, -// unconditionally: (1) the caller's run must resolve to a live -// tenant/principal/run via the sidecar-token + run-address check -// below, and (2) any `toolPackagePins` named in the create body must -// fail closed against the tenant's live capability inventory -// (`assertCapabilityInInventory`, unchanged from the tenant-session -// route's own guided-capability-add check). +// The sanctioned path for a workflow-process child to create a new agent +// definition. No `requireGrant` check: the tool declares `approval: "ask"`, +// so a human already approved the agent before this route runs. import { type } from "arktype"; import { and, eq } from "drizzle-orm"; import { Hono } from "hono"; @@ -64,9 +34,7 @@ export type WorkflowAgentCreateEnv = { Variables: { workflowCapabilityScope: WorkflowCapabilityRunScope }; }; -// Mirrors `./validation.ts`'s `HANDLE_PATTERN` exactly — this route's -// handle is bound by the same lowercase-kebab rule the asset service -// enforces at creation, same as the tenant-session route's own field. +// Mirrors `./validation.ts`'s `HANDLE_PATTERN` exactly. const HANDLE_PATTERN = type(/^[a-z0-9]+(?:-[a-z0-9]+)*$/); const CreateWorkflowAgentDefinitionInput = type({ @@ -78,16 +46,9 @@ const CreateWorkflowAgentDefinitionInput = type({ "toolPackagePins?": type("string > 0").array(), }); -/** - * A definition this listing offers as a taskable/pickable conversational - * agent for Myra to know about — the same shape `apps/hub`'s - * `listMyraConversationalAgents` builds, replicated here directly - * against `workflowDefinition` rather than imported: `apps/hub` is the - * composition root and depends on this package, so importing its - * listing helper back would invert that dependency. Deployed, not an - * automatable catalog workflow, and not a workbench host's own silent - * anchor definition. - */ +/** A definition this listing offers as a taskable conversational agent. + * Replicated here rather than imported, since importing `apps/hub`'s + * listing helper back would invert the dependency. */ function isConversationalAgentDefinition(definition: { readonly name: string }): boolean { return !isAutomatableWorkflowName(definition.name); } @@ -148,19 +109,14 @@ export function createWorkflowAgentCreateRoutes( ); } - // Resolved once, unconditionally: every branch below already needed - // it (the pin check when pins are named, the baseline lookup when - // they aren't), and it is now also the source of truth for `model` - // validation just below. + // Resolved once: also the source of truth for `model` validation below. const inventory = await deps.capabilityInventory.resolve({ tenantId: scope.tenantId, principalId: scope.principalId, }); if (body.toolPackagePins !== undefined && body.toolPackagePins.length > 0) { - // Throws `CapabilityOutOfInventoryError`, caught by `app.onError` - // above — fail closed against exactly the inventory this call - // just fetched, never a stale or wider one, for every named pin. + // Throws `CapabilityOutOfInventoryError`, caught by `app.onError`. for (const name of body.toolPackagePins) { assertCapabilityInInventory({ kind: "toolPackage", name }, inventory); } @@ -174,9 +130,8 @@ export function createWorkflowAgentCreateRoutes( } const skills = body.skills ?? []; - // Mutably built for the same `exactOptionalPropertyTypes` reason - // `./routes.ts`'s `POST /` caller builds its own core input this - // way: an absent `model`/`toolPackagePins` must be an absent key. + // Mutably built: `exactOptionalPropertyTypes` needs an absent + // `model`/`toolPackagePins` to be an absent key. const coreInput: { -readonly [K in keyof CreateAgentDefinitionCoreInput]: CreateAgentDefinitionCoreInput[K]; } = { @@ -189,13 +144,9 @@ export function createWorkflowAgentCreateRoutes( skills, }; - // `body.model` is free text a language-model tool call supplied — - // untrusted input at a trust boundary (AGENTS.md). A name the - // tenant's own catalog doesn't offer can never resolve at launch, - // so it is never baked in verbatim: fall back to the tenant's - // catalog default and say so, rather than creating a dead agent - //. A name the catalog does offer is used exactly as - // asked, no fallback consulted. + // `body.model` is untrusted tool-call input. A name outside the + // catalog falls back to the tenant default and says so, rather than + // creating a dead agent. let modelNote: string | null = null; if (body.model !== undefined) { const knownModel = inventory.models.some((entry) => entry.canonicalName === body.model); @@ -214,9 +165,7 @@ export function createWorkflowAgentCreateRoutes( if (body.toolPackagePins !== undefined && body.toolPackagePins.length > 0) { coreInput.toolPackagePins = body.toolPackagePins; } else { - // No pins named: the specialist still gets the baseline set this - // tenant can resolve, so a created "research agent" can actually - // search, remember, and ask. + // No pins named: still gets the baseline set this tenant can resolve. const baseline = baselineAgentToolPins(inventory); if (baseline.length > 0) coreInput.toolPackagePins = baseline; } diff --git a/packages/agent-directory/src/workflow-skill-pin-routes.ts b/packages/agent-directory/src/workflow-skill-pin-routes.ts index 7e56134c23..f437337415 100644 --- a/packages/agent-directory/src/workflow-skill-pin-routes.ts +++ b/packages/agent-directory/src/workflow-skill-pin-routes.ts @@ -1,26 +1,6 @@ -// Gives a running workflow (Myra) a way to pin a skill onto ANY -// definition in its own tenant — the execution half of a `pin_skill` -// tool the workbench-tools side wires up, mirroring this package's own -// `createWorkflowCapabilityRoutes`: a workflow child has -// no browser session, only its sidecar bearer token and its own run -// address, so it authenticates through a `WorkflowRunAuthenticator` -// rather than the tenant-session pipeline `./routes.ts` uses. Mounted -// OUTSIDE tenant-session middleware. -// -// Unlike `workflow-capability-routes.ts` (self-DEFINITION scoped: a run -// may only touch its own agent definition), this surface is -// self-TENANT scoped: the caller may pin a skill onto any definition -// that belongs to its own tenant, including a definition someone else -// authored. That is deliberately wider, for the same reason -// `workflow-dispatch-routes.ts`'s file header gives for skipping -// `requireGrant`: `@corbits/skills-tools`' `pin_skill` tool declares -// `approval: "ask"`, so the reactor suspends every call as a pending -// approval and renders it in-chat before this route ever runs — a -// human already approved this exact pin. The human is the authorizer -// here, not a grant row; this route still enforces, unconditionally, -// that the target definition belongs to the authenticated run's own -// tenant and is never a workbench host (the same host guard -// `workflow-capability-routes.ts` applies). +// Gives a running workflow (Myra) a way to pin a skill onto any definition +// in its own tenant. `pin_skill` declares `approval: "ask"`, so a human +// already approved this exact pin before the route runs. import { type } from "arktype"; import { and, eq } from "drizzle-orm"; import { Hono } from "hono"; @@ -45,11 +25,8 @@ import type { WorkflowRunAuthenticator as WorkflowCapabilityRunAuthenticator, } from "./workflow-capability-routes"; -/** Structurally the same run scope `workflow-capability-routes.ts` - * resolves — reused by type alias rather than a fresh declaration, since - * this route lives in the same package and there is no cycle risk to - * avoid (contrast `workflow-dispatch-routes.ts`, which is in a different - * package and redeclares the shape for that reason). */ +/** The same run scope `workflow-capability-routes.ts` resolves, reused by + * type alias since both live in this package. */ export type WorkflowSkillPinRunScope = WorkflowCapabilityRunScope; export type WorkflowRunAuthenticator = WorkflowCapabilityRunAuthenticator; @@ -64,10 +41,8 @@ function definitionNotFound(definitionId: string) { }); } -/** Same host-guard `./routes.ts`/`./workflow-capability-routes.ts` - * apply: a workbench host is never a target a workflow run may mutate - * through this surface either. Duplicated rather than imported since - * `workflow-capability-routes.ts` does not export its own copy. */ +/** Same host-guard as `./routes.ts`, duplicated since + * `workflow-capability-routes.ts` doesn't export its own copy. */ function hostGuardedRow( row: { readonly name: string; readonly assetId: string | null } | undefined, ): row is { readonly name: string; readonly assetId: string } { @@ -85,9 +60,7 @@ export type CreateWorkflowSkillPinRoutesDeps = { skillIndex: PinnedSkillIndexResolver; authenticator: WorkflowRunAuthenticator; /** Deploys the definition's commit through the native source pipeline - * after the rewrite; the composition root injects the SAME - * `WorkflowDeployer` `@corbits/workflows`'s `./authoring`'s registry - * calls. */ + * after the rewrite. */ deployer: AgentDefinitionDeployer; }; diff --git a/packages/approvals/src/allowance.ts b/packages/approvals/src/allowance.ts index eade7dabec..be59efaa02 100644 --- a/packages/approvals/src/allowance.ts +++ b/packages/approvals/src/allowance.ts @@ -1,24 +1,9 @@ -// Read-only tool calls ride the grant allowance instead of -// per-call approval. An `approval: "ask"` tool call parks as a pending -// approval row the moment the sidecar registers its suspension -// (`registerSignalCorrelation`); this module is the allowance gate that -// runs right after that registration. A call whose tool carries a -// declarative allowance annotation, classifies as read-only for THESE -// arguments, and whose resolved resource (`repo:`, -// `room:`, `mcp:`, ...) an existing `allow` grant in the -// tenant's grant store covers is auto-approved through the native -// resolve machinery — the approval row still exists and flips to -// "approved", so the decision is ledgered, but no card ever needs a -// human. Every other call — unannotated tools, write classifications, -// uncovered resources, classification failures — keeps today's parked -// behavior unchanged. -// -// Classification is registry-declarative: an annotation is keyed by the -// qualified tool name the approval row carries and supplies its own -// `classify` (which may verify a downstream claim live, e.g. an MCP -// server's `readOnlyHint`). Nothing here matches tool names ad hoc at -// the call site, and coverage is always resolved through the real grant -// rows (`@intx/authz`'s `evaluateGrants`), never a parallel store. +// Read-only tool calls ride the grant allowance instead of per-call +// approval: a call whose tool declares an allowance, classifies read-only +// for these arguments, and whose resource an existing grant covers is +// auto-approved through the native resolve machinery — the approval row +// still exists and flips to "approved", so the decision is ledgered, but no +// card needs a human. Everything else keeps today's parked behavior. import { evaluateGrants } from "@intx/authz"; import type { GrantRule } from "@intx/types/authz"; @@ -26,13 +11,9 @@ export type AllowanceClassification = | { readonly readOnly: true; readonly resource: string } | { readonly readOnly: false }; -/** - * One tool's declarative allowance annotation. `classify` receives the - * parked call's arguments and answers whether THIS invocation is - * read-only and which resource it touches; `grantAction` names the - * action the covering grant must allow on that resource (the action the - * standing grant was minted with, e.g. `"read"`). - */ +/** One tool's declarative allowance: `classify` answers whether this call is + * read-only and which resource it touches; `grantAction` names the action a + * covering grant must allow on that resource. */ export type ToolAllowance = { /** Qualified tool name exactly as the approval row records it. */ readonly tool: string; @@ -76,12 +57,8 @@ export type AllowanceDecision = | "classification_failed"; }; -/** - * The pure allowance decision: park unless the tool is annotated, the - * call classifies read-only, and an `allow` grant in `grants` covers - * the classified resource under the annotation's action. Fails closed — - * a throwing classifier parks rather than rides. - */ +/** The pure allowance decision. Fails closed — a throwing classifier parks + * rather than rides. */ export async function evaluateToolAllowance(args: { registry: ToolAllowanceRegistry; tenantId: string; @@ -132,11 +109,8 @@ export type GrantAllowanceGateDeps = { findRegisteredApproval(correlationId: string): Promise; /** The tenant's live grant rows — role- and principal-scoped alike. */ listTenantGrants(tenantId: string): Promise; - /** - * Resolves the approval "approved" through the native resolve - * machinery with allowance (null-principal) authority. Returns whether - * the resolution actually landed — a false is logged, never thrown. - */ + /** Resolves the approval "approved" with allowance (null-principal) + * authority; a false is logged, never thrown. */ autoApprove(args: { approvalId: string; tenantId: string; @@ -154,13 +128,8 @@ type RegisterApprovalArgs = { }; }; -/** - * Wraps a `registerSignalCorrelation` lookup: after the base - * registration co-writes the pending approval, evaluates the allowance - * and auto-approves a riding call. Never throws into the registration - * path — an allowance failure leaves the approval parked, which is the - * safe (status quo) outcome. - */ +/** Wraps a registration to evaluate the allowance and auto-approve a riding + * call. Never throws into the registration path. */ export function withGrantAllowance( base: (args: Args) => Promise, deps: GrantAllowanceGateDeps, diff --git a/packages/approvals/src/headline.ts b/packages/approvals/src/headline.ts index 1536504c9e..7baa558d31 100644 --- a/packages/approvals/src/headline.ts +++ b/packages/approvals/src/headline.ts @@ -1,9 +1,6 @@ -// The one display concern this package owns: the human-readable line an -// approval is described by. Pure string work over the tool snapshot an -// approval already carries, with no database or server dependency, so the -// browser can compose the same headline the hub-side tools do — hence its -// own module and `./headline` export rather than living beside the -// grant-allowance gate. +// The human-readable line an approval is described by. Pure string work, no +// database dependency, so the browser can compose the same headline the +// hub-side tools do. function stringField(source: object, field: string): string | undefined { if (!(field in source)) return undefined; @@ -27,17 +24,9 @@ function toolPackagePinsField( ); } -/** - * `workflow_deploy` (`@corbits/workflow-authoring-tools`) parks an - * approval whose arguments carry the packageName/toolPackagePins a prior - * `wf_deploy_preview` call (a static read of the committed source) - * reported, passed through. This renders that directly rather than - * falling back to the tool's generic description, so the approval card - * names the real package and tools instead of a bare asset id. It does - * NOT show grants/capabilities: those are stamped by the native - * install+probe+gate `workflow_deploy` itself runs, which has no - * no-freeze preview yet. - */ +/** Renders the packageName/toolPackagePins a prior `wf_deploy_preview` call + * reported, so the card names the real package instead of a bare asset id. + * Does not show grants/capabilities: those have no preview yet. */ function workflowDeployHeadline(toolArguments: object): string | undefined { const commitSha = stringField(toolArguments, "commitSha"); if (commitSha === undefined) return undefined; @@ -51,31 +40,17 @@ function workflowDeployHeadline(toolArguments: object): string | undefined { return `Deploy workflow ${packageName} @ ${sha7} — tools: ${toolsText}`; } -/** - * Per-tool headline renderers, keyed by tool name — a tool kind that wants - * its approval card to name what it will actually do, rather than fall - * back to its generic description, registers itself here instead of - * growing another `if (toolName === ...)` branch in `headlineFor`. - * Returning `undefined` (missing required args) falls through to the - * generic description/title rendering below. - */ +/** Per-tool headline renderers, keyed by tool name — registering here avoids + * another `if (toolName === ...)` branch in `headlineFor`. */ const TOOL_HEADLINE_RENDERERS: Readonly< Record string | undefined> > = { workflow_deploy: workflowDeployHeadline, }; -/** - * Builds the headline for an approval. A registered per-tool renderer - * (`TOOL_HEADLINE_RENDERERS`) wins when the tool call's own arguments - * carry what it needs. Otherwise prefers the tool's own `description` — - * written by the tool's author to be human-readable — over its bare - * `name`, which is a machine identifier. When the live call's arguments - * carry a `title` (a tool author's own convention for per-invocation - * context, e.g. "finalize this piece of collateral titled X"), it is - * appended so the headline reflects what THIS approval is actually - * about, not just which tool is asking. - */ +/** Builds the headline for an approval: a registered per-tool renderer wins + * when it can, else the tool's `description` (falling back to its bare + * `name`), with any call-supplied `title` appended. */ export function headlineFor(toolDefinition: unknown, toolArguments: unknown): string { const toolName = typeof toolDefinition === "object" && toolDefinition !== null diff --git a/packages/cron/src/cron.ts b/packages/cron/src/cron.ts index f7d27dc407..58c85f9f7d 100644 --- a/packages/cron/src/cron.ts +++ b/packages/cron/src/cron.ts @@ -1,21 +1,7 @@ // The single 5-field cron grammar `@corbits/workflows` speaks: one parser -// shared by validation (does this expression make sense at save time?) -// and execution (does this expression match this minute?). Before this -// module existed, `trigger.ts` and the hub's scheduler each hand-rolled -// their own field parser — format-only, no range checking, incompatible -// clause orderings — so an expression could validate as saveable and -// then never fire, or fire on one parser's reading and not the other's. -// One parser closes that gap: whatever validates here is exactly what -// matches here. -// -// Semantics match Vixie/POSIX cron on the cases that matter: -// - `*/N` steps from the field minimum (so `*/2` on day-of-month is -// 1,3,5… not 2,4,6…). -// - When both day-of-month and day-of-week are restricted, they OR -// (`0 0 13 * 5` = the 13th or any Friday). -// - Day-of-week accepts both 0 and 7 as Sunday. -// - Matching can be evaluated in an IANA timezone; `nextFireAt` is still -// stored as a UTC instant. +// shared by validation and execution, so whatever validates here is +// exactly what matches here (previously two hand-rolled parsers could +// disagree). Semantics match Vixie/POSIX cron; see the functions below. export type CronField = "minute" | "hour" | "dayOfMonth" | "month" | "dayOfWeek"; /** Field order in a 5-field cron expression, paired with its valid range. */ @@ -43,10 +29,8 @@ type CronClause = { readonly step?: number; }; -// Standard cron clause order is base, then an optional range, then an -// optional step: `5`, `5-10`, `*/2`, `5-10/2`. Only this order is -// accepted — the reversed `5/2-10` idiom neither cron nor either of -// this repo's previous hand-rolled parsers meaningfully supported. +// Only base-range-step order is accepted (`5`, `5-10`, `*/2`, `5-10/2`) — +// the reversed `5/2-10` idiom is not supported. const CLAUSE_PATTERN = /^(\*|[0-9]+)(?:-([0-9]+))?(?:\/([0-9]+))?$/; function parseCronClause(raw: string): CronClause | undefined { @@ -59,13 +43,8 @@ function parseCronClause(raw: string): CronClause | undefined { return step !== undefined ? { ...withRangeEnd, step: Number(step) } : withRangeEnd; } -/** - * True when `clause` is meaningful for a field whose valid values span - * `[min, max]`: every literal value in range, and — the case the old - * format-only validators missed — a reversed range (`10-5`) rejected - * rather than accepted as an expression that is syntactically fine and - * unconditionally never true. - */ +/** True when `clause` is meaningful for `[min, max]`: values in range, and a + * reversed range (`10-5`) rejected rather than silently never-true. */ function clauseInRange(clause: CronClause, [min, max]: readonly [number, number]): boolean { if (clause.step !== undefined && clause.step <= 0) return false; if (clause.base === "*") return true; @@ -75,11 +54,8 @@ function clauseInRange(clause: CronClause, [min, max]: readonly [number, number] return clause.rangeEnd >= clause.base; } -/** - * Does `clause` match `value` for a field whose minimum is `min`? - * Star-with-step (asterisk-slash-N) steps from `min`, not from zero — so on a - * 1-based day-of-month, that pattern yields 1,3,5… rather than 2,4,6…. - */ +/** Does `clause` match `value`? Star-with-step steps from `min`, not zero, + * so on a 1-based day-of-month it yields 1,3,5… not 2,4,6…. */ function clauseMatches(clause: CronClause, value: number, min: number): boolean { if (clause.base === "*") { return clause.step === undefined ? true : (value - min) % clause.step === 0; @@ -106,16 +82,8 @@ function someClause(field: string, test: (clause: CronClause) => boolean): boole }); } -/** - * Loud, eager validation for a raw 5-field cron expression - * (minute hour day-of-month month day-of-week): every field's syntax - * AND every field's values must be sane for that position — a minute - * of 60, a month of 13, or a reversed range all fail here, never - * silently accepted only to fail at fire-time. - * - * This is syntactic + range only. Whether the expression can ever - * actually fire (e.g. `0 0 31 2 *` — Feb 31) is `cronExpressionCanFire`. - */ +/** Loud, eager syntax + range validation (never a fire-time surprise like a + * minute of 60). Whether it can ever actually fire is `cronExpressionCanFire`. */ export function isValidCronExpression(expression: string): boolean { const fields = expression.trim().split(/\s+/); if (fields.length !== 5) return false; @@ -130,10 +98,7 @@ function fieldMatches(field: string, value: number, min: number): boolean { return someClause(field, (clause) => clauseMatches(clause, value, min)); } -/** - * Day-of-week match with 0/7 both meaning Sunday. A clause of `7` matches - * a Date whose day is 0, and a clause of `0` matches the same. - */ +/** Day-of-week match with 0/7 both meaning Sunday. */ function dayOfWeekMatches(field: string, dayOfWeek: number): boolean { const [min] = CRON_FIELD_RANGES.dayOfWeek; if (fieldMatches(field, dayOfWeek, min)) return true; @@ -142,11 +107,8 @@ function dayOfWeekMatches(field: string, dayOfWeek: number): boolean { return false; } -/** - * True when the day-of-month field is restricted (not a bare `*`, and not - * only `*` with a step that still covers every day). Vixie OR-semantics - * for DOM/DOW only apply when *both* fields are restricted. - */ +// True when the field is restricted (not a bare wildcard or step-1). Vixie +// OR-semantics for DOM/DOW only apply when both fields are restricted. function isDayFieldRestricted(field: string): boolean { const trimmed = field.trim(); if (trimmed === "*") return false; @@ -165,12 +127,8 @@ export type ZonedParts = { readonly dayOfWeek: number; }; -/** - * Wall-clock parts of `at` in `timeZone` (IANA). Falls back to UTC when - * `timeZone` is omitted or `"UTC"`. Throws if `timeZone` is not a valid - * IANA name — call sites that accept user input must validate first via - * `isValidTimeZone`. - */ +/** Wall-clock parts of `at` in `timeZone`. Throws on an invalid IANA name — + * user input must validate first via `isValidTimeZone`. */ export function zonedParts(at: Date, timeZone: string = "UTC"): ZonedParts { if (timeZone === "UTC") { return { @@ -237,11 +195,8 @@ export function isValidTimeZone(timeZone: string): boolean { } } -/** - * True when `expression`'s fields match the wall-clock minute of `at` - * in `timeZone` (default UTC). DOM and DOW OR when both are restricted - * (Vixie/POSIX); otherwise AND. - */ +/** True when `expression` matches the wall-clock minute of `at` in + * `timeZone`. DOM and DOW OR when both are restricted; otherwise AND. */ export function cronMatchesMinute(expression: string, at: Date, timeZone: string = "UTC"): boolean { const fields = expression.trim().split(/\s+/); const [minute, hour, dayOfMonth, month, dayOfWeek] = fields; @@ -277,23 +232,12 @@ function minuteKey(at: Date): number { return Math.floor(at.getTime() / 60_000); } -/** - * Bounds how far ahead `nextCronFireAfter` will search before giving up. - * One leap year of minutes is enough for any expression that fires at - * least annually; impossible expressions (Feb 31, etc.) fail here — and - * at save time via `cronExpressionCanFire` — never inside a claim - * transaction that would otherwise spin for millions of iterations. - */ +/** Bounds how far ahead `nextCronFireAfter` searches — one leap year is + * enough for any expression that fires at least annually. */ export const MAX_LOOKAHEAD_MINUTES = 366 * 24 * 60; -/** - * The next minute at or after `after` (exclusive) that `expression` - * matches in `timeZone` — the closed-form "when does this actually fire - * next" calculation, used both to persist a routine's `nextFireAt` and - * to render a UI's next-run estimate against the exact semantics that - * fire it. `nextFireAt` is always a UTC instant even when matching is - * zoned. - */ +/** The next minute at or after `after` (exclusive) that `expression` + * matches. Always returns a UTC instant, even when matching is zoned. */ export function nextCronFireAfter(expression: string, after: Date, timeZone: string = "UTC"): Date { const start = minuteKey(after) + 1; for (let minute = start; minute - start <= MAX_LOOKAHEAD_MINUTES; minute++) { @@ -306,12 +250,8 @@ export function nextCronFireAfter(expression: string, after: Date, timeZone: str ); } -/** - * True when `expression` has at least one fire inside the lookahead - * window from `from` (default: Unix epoch). Used at save time so an - * impossible expression (`0 0 31 2 *`) is rejected before it can ever - * reach the scheduler's claim path. - */ +/** True when `expression` has at least one fire inside the lookahead window + * from `from`. Used at save time to reject an impossible expression. */ export function cronExpressionCanFire( expression: string, timeZone: string = "UTC", diff --git a/packages/cron/src/mount.ts b/packages/cron/src/mount.ts index 6d491d6992..9491da4939 100644 --- a/packages/cron/src/mount.ts +++ b/packages/cron/src/mount.ts @@ -1,8 +1,5 @@ -// CRUD over a tenant's saved cron schedules. Mirrors `@corbits/mailbox`'s -// `mountMailbox` shape: absolute routes registered directly on the host's -// app (never a sub-router mounted under a prefix), a caller-supplied -// resolver for "is the caller allowed to act on this tenant," and arktype -// parsing at the one trust boundary that matters (the request body). +// CRUD over a tenant's saved cron schedules. Absolute routes registered +// directly on the host's app, never a sub-router under a prefix. import { randomUUID } from "node:crypto"; import { type } from "arktype"; import { eq, and } from "drizzle-orm"; diff --git a/packages/cron/src/schema.ts b/packages/cron/src/schema.ts index bbd1242941..2048b06283 100644 --- a/packages/cron/src/schema.ts +++ b/packages/cron/src/schema.ts @@ -25,14 +25,8 @@ function quoteIdentifier(name: string): string { return `"${name.replace(/"/g, '""')}"`; } -/** - * The package's one idempotent migration, built against - * `tenantSchema.tenant` — `"public"` in every real deployment; a test - * harness that runs the platform migrations into a scratch schema - * passes that schema instead so the FK targets its own `tenant` row, - * not the real one. Every statement is `IF NOT EXISTS`, so no ledger - * table is needed to know what has already run. - */ +/** Builds the FK against `tenantSchema.tenant` — `"public"` in every real + * deployment, a scratch schema in tests. */ function cronMigrationSql(tenantSchema: string): string { const tenantTable = `${quoteIdentifier(tenantSchema)}."tenant"`; return ` @@ -51,11 +45,8 @@ function cronMigrationSql(tenantSchema: string): string { `; } -/** - * Apply the package's migration against `databaseUrl`, idempotently - * (every statement is `IF NOT EXISTS`) and inside one advisory-locked - * transaction so concurrent hub replicas cannot race the same DDL. - */ +/** Applies the migration idempotently, inside one advisory-locked + * transaction so concurrent hub replicas cannot race the same DDL. */ export async function applyCronMigrations( databaseUrl: string, options?: { tenantSchema?: string }, diff --git a/packages/cron/src/ticker.ts b/packages/cron/src/ticker.ts index 25c7e6881f..5a36579154 100644 --- a/packages/cron/src/ticker.ts +++ b/packages/cron/src/ticker.ts @@ -1,10 +1,7 @@ -// Turns due cron schedules into mail. Every tick opens one transaction, -// claims whatever rows are due with `SELECT ... FOR UPDATE SKIP LOCKED` (so -// two tickers racing the same table never double-fire a row), delivers -// each, and stamps `last_fired_at`. A schedule that missed several ticks -// (the process was down, the poll was slow) fires once for the most recent -// due minute, never once per missed tick — `last_fired_at` is only ever -// "did this fire since its last fire," not a queue to drain. +// Turns due cron schedules into mail. `SELECT ... FOR UPDATE SKIP LOCKED` +// means two tickers racing the same table split due rows rather than +// double-fire; a schedule that missed several ticks fires once for the +// most recent due minute, never once per missed tick. import { eq } from "drizzle-orm"; import type { PostgresJsDatabase } from "drizzle-orm/postgres-js"; @@ -40,12 +37,8 @@ export type CronTicker = { stop(): void; }; -/** - * A schedule's next fire time: `nextCronFireAfter` from its `last_fired_at` - * when it has fired before, or from `created_at` when it never has — a - * freshly saved schedule is due at its first matching minute after - * creation, not retroactively for every minute since the epoch. - */ +/** A freshly saved schedule is due at its first matching minute after + * creation, not retroactively for every minute since the epoch. */ function isDue( row: { expression: string; lastFiredAt: Date | null; createdAt: Date }, now: Date, diff --git a/packages/error-sink/src/context.ts b/packages/error-sink/src/context.ts index fba46f4963..04394afbd9 100644 --- a/packages/error-sink/src/context.ts +++ b/packages/error-sink/src/context.ts @@ -1,9 +1,4 @@ -// The structured shape every `reportError` call carries: the -// operation that failed, optional tenant/room/agent identifiers scoping -// it, and a `refId` a person can quote back to support -- the same -// `refId` pattern `packages/onboarding/src/routes.ts`'s -// `reportOnboardingError` already establishes (see this package's own -// `generateRefId` / `makeErrorEnvelope`). Free-form detail +// The structured shape every `reportError` call carries. Free-form detail // beyond these named fields belongs in `extra`, never inlined into a // string message. import { type } from "arktype"; diff --git a/packages/error-sink/src/error-envelope.ts b/packages/error-sink/src/error-envelope.ts index 56b252f424..deaf6d7f2a 100644 --- a/packages/error-sink/src/error-envelope.ts +++ b/packages/error-sink/src/error-envelope.ts @@ -1,10 +1,6 @@ -// The one shape every hub route uses to answer a failure a user can see: -// a stable `code` a client can branch on, a `userMessage` written in -// consumer language, and a `refId` a person can quote in a support -// message or a bug report. The raw failure — stack text, file paths, -// upstream error prose — never crosses the wire; it belongs in the -// hub's own logger, keyed by the same `refId`, so an operator can find -// it without the client ever having seen it. +// The one shape every hub route uses to answer a user-visible failure. The +// raw failure never crosses the wire — it stays in the hub's own logger, +// keyed by the same `refId`. import { type } from "arktype"; import { generateRefId } from "./ref-id"; diff --git a/packages/error-sink/src/index.ts b/packages/error-sink/src/index.ts index c5500f798b..3021c99b6a 100644 --- a/packages/error-sink/src/index.ts +++ b/packages/error-sink/src/index.ts @@ -1,24 +1,7 @@ // The one entry point every catch block calls instead of swallowing a -// failure (the owner ruling this package exists to enforce). -// -// `@intx/log` (LogTape underneath) is already the repo's one logging -// concept -- 65+ `getLogger`/`log.error` call sites across apps/hub and a -// dozen packages -- and it already has a pluggable-sink seam and -// universal runtime support (Node, Bun, browsers). This package adds -// only what that seam is missing: a fixed structured-error convention -// (operation, optional tenant/room/agent identifiers, and a `refId` a -// person can quote to support -- the same pattern -// `packages/onboarding/src/routes.ts`'s `reportOnboardingError` already -// establishes) plus a redaction pass, so no call site hand-rolls that -// shape or leaks a secret into a log line. Reaching OTEL/Sentry later is -// a LogTape sink registered once via `@intx/log`'s `configureSync`/ -// `setup` -- nothing here changes when that happens. -// -// The guarantee that makes this safe to sprinkle into any catch block: -// `reportError` itself never throws. A malformed context degrades to -// `operation: "unknown"` rather than rejecting the report outright, and -// a throwing sink is LogTape's own concern to isolate -- this function's -// own try/catch is the last line of defense either way. +// failure. Builds on `@intx/log` (the repo's one logging concept) with a +// fixed structured-error convention and a redaction pass, so no call site +// hand-rolls that shape or leaks a secret into a log line. import { getLogger } from "@intx/log"; import { type } from "arktype"; import { ErrorContext, type ErrorContext as ErrorContextInput } from "./context"; diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index 9622bda512..b486ccd7fc 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -1,9 +1,6 @@ -// Strips obvious secrets before a report reaches any transport: -// a key that looks like a credential is replaced outright, and every -// string value -- messages, stack traces, anything nested in `extra` -- -// is scanned for bearer tokens and authorization-header fragments. -// Heuristic, not a full secret-scanning engine: the bar is "never ships -// an obvious secret", not "catches every possible one". +// Strips obvious secrets before a report reaches any transport. Heuristic, +// not a full secret-scanning engine: the bar is "never ships an obvious +// secret", not "catches every possible one". const SECRET_KEY_PATTERN = /token|secret|password|passwd|api[-_]?key|apikey|credential|authorization|cookie/i; @@ -24,22 +21,15 @@ const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ // closing paren, a stack-frame's `:12:5)`, ...). const ASSIGNMENT_VALUE = "[\\w.+/=%-]+"; -// `token=`, `access_token=`, ... assignments: covers both a raw OAuth -// callback URL's query string (`?access_token=...`) and the same shape -// typed into a free-text error message. Only the value is replaced so the -// param name -- and the rest of the URL/message -- stays readable for -// debugging. These names are unambiguous: outside of a credential they -// don't ordinarily show up as a bare `name=value` assignment at all. +// Covers `token=`/`access_token=`/... assignments in a URL or free-text +// message. Only the value is replaced so the rest stays readable. const SENSITIVE_ASSIGNMENT_PATTERN = new RegExp( `\\b(access_token|refresh_token|id_token|api[-_]?key|apikey|secret|password|passwd|token|credential)(\\s*=\\s*)(${ASSIGNMENT_VALUE})`, "gi", ); -// `code` and `key` are NOT unambiguous -- `code=404`, logfmt's -// `code=DB_TIMEOUT`, and `key=user:1234:profile` are everyday non-secret -// shapes. The only place they reliably mean "secret" is a URL query string -// (an OAuth `code`, a `key=` API credential passed as a param), so these -// two are scoped to right after a literal `?` or `&`. +// `code`/`key` are ambiguous elsewhere (`code=404`, `key=user:1234:...`), so +// these two are scoped to right after a literal `?` or `&`. const QUERY_PARAM_SENSITIVE_ASSIGNMENT_PATTERN = new RegExp( `([?&])(code|key)(\\s*=\\s*)(${ASSIGNMENT_VALUE})`, "gi", diff --git a/packages/error-sink/src/ref-id.ts b/packages/error-sink/src/ref-id.ts index 6574a08d8b..a35188ae85 100644 --- a/packages/error-sink/src/ref-id.ts +++ b/packages/error-sink/src/ref-id.ts @@ -1,8 +1,5 @@ -// The sole implementation of the short, quotable, grep-able refId shape -//, and the one this package's own `makeErrorEnvelope` calls. -// This package stays free of the workflow-catalog and inference stack, -// which this package's browser callers (and its own server-side ones) -// have no reason to carry just to mint an id. +// The short, quotable, grep-able refId shape — dependency-free so browser +// callers don't carry unrelated stacks just to mint an id. export function generateRefId(): string { const random = Math.random().toString(36).slice(2, 8); return `${Date.now().toString(36)}-${random}`; diff --git a/packages/inference-catalog/src/capabilities.ts b/packages/inference-catalog/src/capabilities.ts index 8c5c66beb3..216dad2bc4 100644 --- a/packages/inference-catalog/src/capabilities.ts +++ b/packages/inference-catalog/src/capabilities.ts @@ -1,6 +1,3 @@ -// The capability vocabulary is the platform's, verbatim. `@intx/types`' -// `WIRE_CAPABILITIES` is the canonical list and its arktype `Capability` is -// built from it, so re-exporting both here gives this package one import -// point without declaring a second vocabulary that could drift from the one -// source resolution filters against. +// Re-exported verbatim so this package has one import point without +// declaring a second vocabulary that could drift from the platform's. export { Capability, WIRE_CAPABILITIES } from "@intx/types"; diff --git a/packages/inference-catalog/src/catalog.ts b/packages/inference-catalog/src/catalog.ts index 09d92f08fe..f2305374ec 100644 --- a/packages/inference-catalog/src/catalog.ts +++ b/packages/inference-catalog/src/catalog.ts @@ -1,16 +1,5 @@ -// The catalog shape chain resolution works on: the platform's own model -// discovery response (`GET /api/tenants/:tenantId/models`), parsed. -// -// That route already does the parts that belong to the platform — the -// ancestor walk, shadowing, disable suppression, and picking each -// offering's active price per currency — so this package restates none of -// them. It declares the wire shape once, as an arktype schema a caller -// parses an untrusted response with, and flattens it into the per-offering -// rows the resolver ranks. -// -// Nothing here reaches a database. The types are the platform's response, -// not its tables, which is what lets this package ship to a workflow child -// with no server dependency at all. +// The catalog shape chain resolution works on: the platform's model +// discovery response, parsed. Nothing here reaches a database. import { type } from "arktype"; import { Capability } from "./capabilities"; diff --git a/packages/inference-catalog/src/concepts.ts b/packages/inference-catalog/src/concepts.ts index 6cf922a671..c64307c3d3 100644 --- a/packages/inference-catalog/src/concepts.ts +++ b/packages/inference-catalog/src/concepts.ts @@ -1,25 +1,5 @@ // The concept vocabulary: the words an agent uses to ask for a model. -// -// Pure data, no branching and no network, so it is importable on its own — -// the `CATALOG_SEEDS` idiom. It is deliberately not a table: this is product -// vocabulary that ships with the build and has to be reviewable in a diff, -// and a table would be a second store for something the bench policy row -// already parameterizes. A bench deviates through its policy's concept -// ceilings and allow/deny lists, never by forking the vocabulary. -// -// Ceilings are USD per million tokens, input and output kept separate — a -// blended number would hide which axis a workload actually spends on. They -// are soft by default: a model over ceiling is flagged and sorted last, not -// dropped, so a bench whose only provider is expensive still gets an answer. -// -// Capabilities are `@intx/types`' vocabulary, the one `model_offering` -// stores and source resolution filters on. It is narrower than the pinned -// catalog's: `long-context` and `prompt-caching` are baked onto catalog -// offerings but are not storable capability values, so no concept can filter -// on them. Where a concept wanted "handles a huge input", its reference mix -// carries that meaning instead — a 10M-token input mix ranks the model that -// is cheap on enormous inputs first, which is the decision the capability -// was standing in for. +// Design rationale: docs/inference-concepts.md. import type { Capability } from "@intx/types"; /** The token mix that defines "cheapest" for a concept, in millions of diff --git a/packages/inference-catalog/src/offering-capabilities.ts b/packages/inference-catalog/src/offering-capabilities.ts index 9a03a7ec8b..7331235f3f 100644 --- a/packages/inference-catalog/src/offering-capabilities.ts +++ b/packages/inference-catalog/src/offering-capabilities.ts @@ -1,31 +1,5 @@ // What a deployment can actually do, resolved from the pinned catalog at the -// moment an offering is created. -// -// `model_offering.capabilities` is the column every capability filter reads — -// this package's chain resolution and the platform's own source resolution -// alike. Nothing populated it before, so it was empty everywhere and no -// capability question had an answer. This module is where the answer comes -// from, and it only ever reports capabilities the pinned catalog observed on -// the wire: -// -// exact-deployment — this exact (baseURL, model) was probed. Use its list. -// same-model-wire — the model was probed on other deployments that speak -// the same wire, and this one is a relay of it. Use the -// INTERSECTION across those deployments, so a relay -// never claims more than every probed deployment of -// that model demonstrated. -// unknown — no probe covers it. Empty list, said plainly. -// -// An empty list is the honest answer for a local or open-weight deployment -// nobody has probed, and it is deliberately not softened with a per-adapter -// baseline: "an OpenAI-compatible endpoint serves plain text" is false for -// the embedding models such endpoints also serve, and a wrong capability tag -// routes real work to a model that cannot do it. -// The pinned catalog's own vocabulary is a superset of what the platform can -// store: it bakes `long-context` and `prompt-caching`, which `@intx/types`' -// `Capability` — the arktype guarding the offerings API and the column — does -// not accept. Everything this module reports is filtered down to the storable -// vocabulary, so a seed never posts a value the hub will reject. +// moment an offering is created. Full rationale: docs/offering-capabilities.md. import { catalogProviders } from "@intx/inference-catalog"; import { WIRE_CAPABILITIES, type Capability } from "@intx/types"; @@ -43,11 +17,8 @@ export type DeploymentIdentity = { readonly canonicalName: string; }; -/** OpenAI Direct and every openai-compatible relay speak one wire, so a model - * probed on one is speaking the same protocol on the other. The Anthropic and - * Google adapters each speak their own wire: a relay serving a Claude model - * over the OpenAI wire genuinely offers less than the native adapter does, so - * their probes never carry across. */ +/** OpenAI Direct and every openai-compatible relay speak one wire; other + * adapters speak their own, since a relay genuinely offers less. */ function wireFamily(plugin: string): string { return plugin === "openai" || plugin === "openai-compatible" ? "openai-wire" : plugin; } diff --git a/packages/inference-catalog/src/ollama-context-defaults.ts b/packages/inference-catalog/src/ollama-context-defaults.ts index 71276417af..1fe1643352 100644 --- a/packages/inference-catalog/src/ollama-context-defaults.ts +++ b/packages/inference-catalog/src/ollama-context-defaults.ts @@ -1,28 +1,5 @@ // Per-model context-window and output-token defaults for locally served -// Ollama models, resolved the same way `capabilitiesForDeployment` resolves -// capabilities: from the deployment's identity, at the moment an offering -// is created. -// -// `@intx/inference`'s built-in OpenAI-shaped adapter defaults output tokens -// to `options.maxTokens ?? 4096` when a caller sets none, and Ollama's own -// openai-compatible endpoint silently defaults `options.num_ctx` (its real -// context window) to a small built-in size when nothing sets it either -- -// truncating real conversations for every locally served chat agent, with -// no error to say so. `@corbits/ollama-adapter`'s `OllamaAdapterConfig` is -// where that override actually lands on the built request body (see its -// `resolveOverride` and `createOllamaAdapter`); this module supplies the -// real per-model values it should carry, stored as a `model_offering`'s -// `quirks` column so it reaches `InferenceSource.quirks` -- and from there -// the adapter -- through the platform's existing resolution, no parallel -// override path of its own. -// -// Each table entry is the model's advertised *native* context window, not a -// YaRN/rope-extended ceiling: requesting a `num_ctx` past what a model (and -// the host's memory) can actually back can fail allocation or force heavy -// swap on the inference host, a real operational risk rather than a -// convention to enforce. A model absent from this table gets no override at -// all -- Ollama's own model-specific default stands, rather than this -// module guessing at a ceiling it cannot back up. +// Ollama models. See docs/ollama-context-defaults.md. import type { OllamaAdapterConfig, OllamaAdapterOverride } from "@corbits/ollama-adapter"; export type OllamaModelDefaults = Readonly>; @@ -44,25 +21,9 @@ export type OllamaDeploymentIdentity = { readonly canonicalName: string; }; -/** - * The `quirks` value to store on a newly created Ollama offering. Each - * offering is already scoped to one exact model, so the resolved override - * is wrapped in `default` -- the field `resolveOverride` applies whenever - * no `perModel` entry is present -- rather than `perModel`, which exists - * for a single shared connection serving several models through one quirks - * bag. - * - * `overrides` lets a caller's own probed or operator-configured ceilings - * win over this table (the "maximum flexibility, sane by default" seam); - * unset resolves to this module's built-in table alone. - * - * Returns `undefined` for any provider other than `ollama` (this - * mechanism's whole scope -- other providers' built-in adapters handle - * their own request shape unmodified) and for any model neither `overrides` - * nor the built-in table names, so an unvetted model is left exactly as - * the built-in adapter already handles it rather than getting a guessed - * ceiling. - */ +/** The `quirks` value to store on a newly created Ollama offering; undefined + * for a non-Ollama provider or an unvetted model. See + * docs/ollama-context-defaults.md#default-vs-permodel. */ export function quirksForDeployment( deployment: OllamaDeploymentIdentity, overrides: OllamaModelDefaults = {}, diff --git a/packages/inference-catalog/src/policy.ts b/packages/inference-catalog/src/policy.ts index 78cf303050..e371077885 100644 --- a/packages/inference-catalog/src/policy.ts +++ b/packages/inference-catalog/src/policy.ts @@ -1,17 +1,5 @@ -// A bench's model policy: what this bench is willing to spend, and on -// which models. Everything else the package answers is derived at read time -// from the platform's own catalog and price history. -// -// The policy lives in the bench's own tenant `config` blob, under -// `corbits.modelPolicy` — read with the stock `GET /api/tenants/:tenantId`, -// written with the stock `PATCH`. A bench is a plain Interchange tenant and -// its config blob is the platform's own place for per-tenant settings, so -// there is no product table and no Workbench-only route in the path: a -// workflow child reads its bench's policy through the same stock route -// everything else reads a tenant through. -// -// A bench with no key uses EMPTY_POLICY, which constrains nothing — that is -// what makes a freshly connected bench work with no configuration at all. +// A bench's model policy: what it is willing to spend, and on which models. +// Lives in tenant `config` under `corbits.modelPolicy` — no product table. import { type } from "arktype"; import { ProviderPreference } from "@intx/types"; @@ -77,15 +65,9 @@ const CorbitsTenantConfig = type({ * spells it. */ export const MODEL_POLICY_CONFIG_PATH = ["corbits", "modelPolicy"] as const; -/** - * This bench's model policy, out of its tenant `config` blob. - * - * Anything the schema rejects — a missing key, a config blob shaped some - * other way, a half-written policy — reads as the parts that do parse over - * EMPTY_POLICY. A bench is never left unable to pick a model because its - * settings blob is malformed; it is left unconstrained, which is the same - * place a brand-new bench starts. - */ +/** This bench's model policy. Anything the schema rejects reads as the parts + * that do parse over EMPTY_POLICY — a malformed blob leaves the bench + * unconstrained, never unable to pick a model. */ export function readModelPolicy(tenantConfig: unknown): BenchModelPolicy { const parsed = CorbitsTenantConfig(tenantConfig); if (parsed instanceof type.errors) return EMPTY_POLICY; diff --git a/packages/inference-catalog/src/price.ts b/packages/inference-catalog/src/price.ts index 69ef294401..de1f719acb 100644 --- a/packages/inference-catalog/src/price.ts +++ b/packages/inference-catalog/src/price.ts @@ -1,15 +1,5 @@ -// Price normalization. The catalog stores per-token prices as decimal -// strings; every human number in this package — ceilings, estimates, the -// copy an agent reads — is USD per million tokens. That conversion happens -// here and nowhere else. -// -// As-of selection is not restated here: the platform's model discovery -// route already returns the row in effect per currency, so this module only -// picks the currency and converts. -// -// A missing row, a missing currency, or a null price component yields -// `known: false` with null numbers. It never yields zero: a fabricated zero -// would read as "free" and quietly win every cheapest-first sort. +// Converts decimal-string per-token prices to USD/million. A missing row +// yields `known: false`, never a fabricated zero that wins cheapest-first. import type { CatalogPricingRow } from "./catalog"; export const DEFAULT_CURRENCY = "USD"; diff --git a/packages/inference-catalog/src/resolve-chain.ts b/packages/inference-catalog/src/resolve-chain.ts index 8fd4671dc6..9f06ba852b 100644 --- a/packages/inference-catalog/src/resolve-chain.ts +++ b/packages/inference-catalog/src/resolve-chain.ts @@ -1,17 +1,5 @@ -// Chain resolution: what this bench can reach for a given kind of work, -// cheapest first, with fallbacks. -// -// Pure — every read is done by the caller against the platform's own model -// discovery route and handed in, so the whole algorithm is exercised with -// plain literals and no database anywhere near it. It answers -// with an ordered chain, never a single model: one model is not a fallback -// plan, and the platform's own source resolution already consumes chains. -// -// It also never resolves credentials or builds an InferenceSource. Its -// output projects to `ModelRequirement[]`, which `resolveModelSources` -// turns into runnable sources — the capability predicate and the priority -// tiebreakers here are deliberately the same ones it uses, so a chain this -// package returns can never be rejected downstream by upstream's own rules. +// Chain resolution: what this bench can reach for a kind of work, cheapest +// first, with fallbacks. Pure — never touches credentials or a database. import type { Capability, ProviderPreference } from "@intx/types"; import type { CatalogOffering, CatalogPricingRow } from "./catalog"; diff --git a/packages/url-path/src/decoded-or-null.ts b/packages/url-path/src/decoded-or-null.ts index b21acd4fef..f69de3ec5e 100644 --- a/packages/url-path/src/decoded-or-null.ts +++ b/packages/url-path/src/decoded-or-null.ts @@ -1,8 +1,5 @@ -/** A URL path segment or param carries percent-escapes an id needs - * decoded, and a hand-typed or truncated URL can carry a malformed one — - * which `decodeURIComponent` answers with a throw. A segment that cannot - * be decoded names no entity, so it reads as no selection at all rather - * than taking the render — or the request — down with it. */ +/** Decodes a URL path segment, reading a malformed one as no selection + * rather than throwing. */ export function decodedOrNull(segment: string): string | null { try { return decodeURIComponent(segment); diff --git a/packages/workflows/src/authoring/registry.ts b/packages/workflows/src/authoring/registry.ts index 4638153895..14b98d6b22 100644 --- a/packages/workflows/src/authoring/registry.ts +++ b/packages/workflows/src/authoring/registry.ts @@ -1,35 +1,6 @@ // The workflow-authoring registry: an agent's in-tenant surface for -// publishing a workflow codebase as a native `kind:"workflow"` hub asset, -// republishing it, and reading it back. Every write is gated by two -// independent checks — own-tenant scoping (resolved from the DB row, -// mirroring `@corbits/skills`' `requireOwnTenant`) and an explicit -// grant-store authorization call (`asset:*`/create for a new asset, -// `asset:`/write for a republish, `asset:`/read for a source read) -// — and by `validateWorkflowSourceTree` before anything reaches -// `RepoStore`. Deploying is not this registry's job: an agent deploys a -// committed workflow asset through stock `POST -// /api/tenants/:tenantId/workflows/deployments` with the run bearer -//. -// -// `populateAsset` is called with `principal: { kind: "hub" }`, the same -// principal `@corbits/skills`' `writeSkillMd` uses. This is deliberate, -// not a shortcut: `workflowKindHandler.workflowAuthorize` -// (`@intx/hub-sessions`) only recognizes three principal kinds for a -// workflow-asset write — `hub` (full access), `sidecar` (read-only, -// explicitly denied `writeTree`), and `user` (gated by git-token-shaped -// bearer claims this sidecar-authenticated caller never carries). There is -// no fourth "workflow-run" principal kind the substrate understands, so a -// real per-write authorization decision has to be made HERE, by this -// registry, against the grant store and the resolved caller identity — -// exactly what the checks below do — before the already-authorized write -// is handed to the substrate as a hub-mediated commit. -// -// Head-sha reads (`expectedHeadSha`, `readSource`) go through `RepoStore` -// directly: `AssetService` exposes blob and directory reads pinned to a -// ref but never the sha that ref resolves to, and `listAssetBlobs` lists -// blobs only (no subtrees), so a full tree walk needs -// `RepoStore.openCommittedReads`. The repo id is the asset id under the -// `workflow` kind, exactly as `AssetService` composes it internally. +// publishing, republishing, and reading back a `kind:"workflow"` hub asset. +// See docs/workflow-authoring-registry.md. import { authorize } from "@intx/authz"; import type { ConditionRegistry, GrantStore } from "@intx/types/authz"; import { @@ -95,14 +66,9 @@ export type WorkflowDeployResult = { readonly status: "deployed" | "pending"; }; -/** - * The apps/hub-supplied seam onto the same operation the native `POST - * /workflows/deployments` route drives (`prepareProvisionedDeployment`), - * with catalog offering ids resolved server-side. An agent authoring a - * workflow does not use it — it calls the stock route directly with the - * run bearer; the hub's own on-demand catalog-block deploy - * does. - */ +/** The apps/hub-supplied seam onto `prepareProvisionedDeployment`, used by + * the hub's own on-demand catalog-block deploy, not by an authoring agent + * (which calls the stock route with the run bearer). */ export type WorkflowDeployer = { deploy(params: { tenantId: string; @@ -247,17 +213,9 @@ async function collectTree( } } -/** - * A best-effort, read-only render of an inert `export default - * {...}` object literal in an entry module — the shape a folded/single-step - * workflow package's entry commonly takes. Deliberately NOT a JS parser or - * evaluator (the source is untrusted agent output and must never be - * executed): strips the `export default` prefix and a trailing `;`, then - * accepts the remainder only if `JSON.parse` on it (after quoting bare - * object keys, the one common non-JSON literal shape) succeeds. Any import, - * function call, or other executable construct fails this and the caller - * falls back to listing files only. - */ +/** Best-effort, read-only render of an inert `export default {...}` object + * literal. Deliberately not a JS parser or evaluator — the source is + * untrusted agent output and must never be executed. */ function tryReadInertDefaultExport(source: string): unknown { const trimmed = source.trim(); const match = /^export\s+default\s+([\s\S]*?);?\s*$/.exec(trimmed); diff --git a/packages/workflows/src/authoring/run-client.ts b/packages/workflows/src/authoring/run-client.ts index 30a76eacda..4649766d6a 100644 --- a/packages/workflows/src/authoring/run-client.ts +++ b/packages/workflows/src/authoring/run-client.ts @@ -1,16 +1,6 @@ -// Shared fetch plumbing for a run-authenticated tool client — every tool -// bundle that calls a hub route mounted behind `WorkflowRunAuthenticator` -// (sidecar bearer token + run address, never a human session) needs the -// same two headers and the same `{ error: { code, userMessage } }` -// envelope parse. Before this module existed, `@corbits/workflow- -// authoring-tools`' and `@corbits/capability-tools`' own `client.ts` -// files each reimplemented both slightly differently (arktype-validated -// vs. ad hoc field access). This is that one shared seam. -// -// Browser-safe: no `@intx/*`, `drizzle-orm`, `hono`, or `postgres` — pure -// fetch/arktype, exported from `@corbits/workflows/client` alongside the -// rest of this package's browser-safe surface, because a tool bundle runs -// inside the sidecar's workflow-host, not the hub server. +// Shared fetch plumbing for a run-authenticated tool client, so every tool +// bundle behind `WorkflowRunAuthenticator` shares the same headers and +// error-envelope parse instead of reimplementing both. import { type } from "arktype"; export interface RunBearerClientConfig { diff --git a/packages/workflows/src/authoring/source-tree.ts b/packages/workflows/src/authoring/source-tree.ts index c9614da630..3f115279a5 100644 --- a/packages/workflows/src/authoring/source-tree.ts +++ b/packages/workflows/src/authoring/source-tree.ts @@ -1,9 +1,6 @@ // The trust-boundary validator for a workflow source tree an agent hands -// the authoring registry. The substrate's `workflowKindHandler.validatePush` -// already checks the manifest's shape once bytes are staged; this module -// runs BEFORE any write so a traversal path, a secret-looking filename, an -// oversize tree, or an entry that names a file the tree does not carry is -// rejected without touching git — and with a message the model can act on. +// the authoring registry. Runs before any write so a traversal path, an +// oversize tree, or a dangling entry is rejected without touching git. import path from "node:path"; import { type } from "arktype"; import { isContainedEntryPath, PackageJSON } from "@intx/types/package-json"; diff --git a/packages/workflows/src/authoring/workflow-routes.ts b/packages/workflows/src/authoring/workflow-routes.ts index 8b523b1eef..dfdccfe33a 100644 --- a/packages/workflows/src/authoring/workflow-routes.ts +++ b/packages/workflows/src/authoring/workflow-routes.ts @@ -1,21 +1,6 @@ -// The sanctioned path for a workflow-process child to author, republish, -// or read back a workflow-kind asset, mirroring `@corbits/skills`' own -// `createWorkflowSkillRoutes`: a workflow child has no browser session, -// only its sidecar bearer token and the run's address, so it -// authenticates through a `WorkflowRunAuthenticator` rather than the -// tenant-session pipeline a human-facing route would use. -// -// Mounted OUTSIDE the tenant prefix for that reason. Identity NEVER rides -// in a request body: the tenant and principal every write is scoped to -// come from the authenticated run alone, so a model's tool arguments can -// never name a different tenant or write into another principal's -// workflow asset. -// -// Deployment is NOT here: `@corbits/workflow-authoring-tools` calls stock -// `POST /api/tenants/:tenantId/workflows/deployments` with the run bearer -// instead. What remains is the git half — reading and writing the asset -// repo's trees — which no run-bearer credential can do against stock git -// smart-HTTP today. +// The sanctioned path for a workflow-process child to author, republish, or +// read back a workflow-kind asset, authenticated through +// `WorkflowRunAuthenticator`. Deployment goes through the stock route instead. import { type } from "arktype"; import { Hono } from "hono"; import { makeErrorEnvelope } from "@corbits/error-sink"; @@ -153,11 +138,8 @@ export function createWorkflowAuthorRoutes( return c.json({ data: snapshot }); }); - // A preview of `/:assetId/deploy` — a STATIC read of the - // already-committed source at `commitSha` (package name, entry, file - // list, any statically-declared tool pins). Never calls install/probe/ - // gate/freeze, so it cannot deploy anything; a human approves the real - // `workflow_deploy` call with this committed source already visible. + // A static read of the already-committed source at `commitSha`. Never + // calls install/probe/gate/freeze, so it cannot deploy anything. app.post("/:assetId/deploy/preview", async (c) => { const body = DeployPreviewBody(await c.req.json().catch(() => undefined)); if (body instanceof type.errors) { diff --git a/packages/workflows/src/catalog.ts b/packages/workflows/src/catalog.ts index a6f2f7b924..ccdbb36b75 100644 --- a/packages/workflows/src/catalog.ts +++ b/packages/workflows/src/catalog.ts @@ -1,19 +1,6 @@ -// Deploy-layer metadata for every seeded workflow package: display name, -// whether it's schedulable as a routine, where its result actually lands -// (a channel vs. the creator's Inbox only), required connections, and -// optional trigger-input fields. Interchange's `defineWorkflow` has no -// automatable/display-name concept of its own. -// -// `assetName`/`displayName`/`automatable` are read straight off each -// `workflows/*/package.json`'s `corbits.workflow` block via a static JSON -// import below — resolved at build time (Vite, tsc, and Bun all support -// `resolveJsonModule`), never at runtime, so this stays importable from -// the browser bundle. There is nothing left to keep "in lockstep": the -// package.json block IS the value this module reads, not a copy of it. -// The remaining fields (`conversational`, `deliveryMode`, `whatItDoes`, -// `requiredConnections`, `exampleOutput`, `typicalDuration`, -// `triggerFields`) have no npm-visible home of their own — they exist -// only here, so there is no second copy for them to drift from. +// Deploy-layer metadata for every seeded workflow package, since +// Interchange's `defineWorkflow` has no automatable/display-name concept. +// Read straight off each package's `corbits.workflow` block, no second copy. import { type } from "arktype"; import assistantPkg from "../../../agents/myra/package.json"; @@ -38,13 +25,8 @@ function workflowBlock(pkg: { return parsed; } -/** - * One named field a mail trigger reads by name — the create-time UI's only - * source of truth for what a workflow's trigger actually expects (see a - * workflow's own system prompt / intake tool for the underlying contract - * this mirrors). `key` is the exact field name a trigger payload carries — - * never relabeled or humanized before it reaches the workflow. - */ +/** One named field a mail trigger reads by name — the create-time UI's only + * source of truth for what a workflow's trigger expects. */ export const WorkflowTriggerField = type({ key: "/^[a-zA-Z][a-zA-Z0-9]*$/", // Explicit, never defaulted: `"text"` renders a plain input and @@ -72,16 +54,8 @@ export type WorkflowCatalogEntry = { * `assistant`/Myra definition is `true`. */ readonly conversational: boolean; - /** - * Where a run's result actually lands — the honest end-to-end - * contract a routine's "Deliver results to" step depends on. Every - * entry states this plainly, whether or not it is automatable: - * `"workbench"` posts into the picked delivery workbench's thread — - * every catalog entry today; `"inbox"` never posts to a workbench at - * all — its result reaches only the creator's Inbox, so a create/run - * flow for it must never collect or require a deliveryWorkbenchId that - * would otherwise be silently discarded. - */ + /** Where a run's result lands: `"workbench"` posts into the picked + * delivery workbench's thread; `"inbox"` reaches only the creator's Inbox. */ readonly deliveryMode: "workbench" | "inbox"; /** One honest sentence: what this workflow actually does. No metrics, no hype. */ readonly whatItDoes: string; @@ -96,20 +70,14 @@ export type WorkflowCatalogEntry = { readonly exampleOutput: string; /** A short, honestly-hedged hint — never fabricated precision. */ readonly typicalDuration: string; - /** - * Named trigger inputs a person can fill in at create time (the routine - * create stepper) or on a manual run — omitted entirely for workflows - * whose trigger carries no human-supplied content (see each entry's own - * comment for why). Order is display order. - */ + /** Named trigger inputs a person can fill in at create time or on a + * manual run, in display order. Omitted when there's nothing to fill in. */ readonly triggerFields?: readonly WorkflowTriggerField[]; }; -/** - * Every known workbench workflow package, keyed by the asset name seed - * deploys under. Agent definitions created at runtime are never listed - * here, so they cannot pass the automatable filter by accident. - */ +/** Every known workbench workflow package. Agent definitions created at + * runtime are never listed here, so they cannot pass the automatable + * filter by accident. */ export const WORKFLOW_CATALOG: readonly WorkflowCatalogEntry[] = [ { ...workflowBlock(assistantPkg), @@ -140,27 +108,16 @@ export function isAutomatableWorkflowName(name: string): boolean { return byAssetName.get(name)?.automatable === true; } -/** - * Whether a workflow definition name is fit to offer as a DM/chat target - * (the sidebar's agent rows, a taskable-agent picker, …) rather than a - * triggered automation utility. A name absent from this catalog is an - * agent-directory-created definition — "Agent definitions created at - * runtime are never listed here" (see `WORKFLOW_CATALOG`'s own comment) — - * and is always conversational; a name present here is conversational - * only when its entry says so (`assistant`/Myra, the only entry today). - */ +/** Whether a workflow name is fit to offer as a DM/chat target. A name + * absent from the catalog is agent-directory-created and always + * conversational; a catalog entry is conversational only when it says so. */ export function isConversationalWorkflowName(name: string): boolean { const entry = byAssetName.get(name); return entry === undefined || entry.conversational; } -/** - * Whether a routine on this workflow needs a `deliveryWorkbenchId` at - * all — `false` only for a known `"inbox"`-delivering entry (see - * `WorkflowCatalogEntry.deliveryMode`). An unknown name defaults `true` - * (workbench required): the safe, prior-behavior default when a workflow - * isn't catalog-known at all. - */ +/** Whether a routine on this workflow needs a `deliveryWorkbenchId`. An + * unknown name defaults to `true` (workbench required). */ export function deliveryWorkbenchRequiredForWorkflowName(name: string): boolean { return byAssetName.get(name)?.deliveryMode !== "inbox"; } @@ -199,22 +156,10 @@ export type TriggerFieldsValidation = | { readonly ok: true } | { readonly ok: false; readonly message: string }; -/** - * The create-time boundary check for a routine's stored `input` - *: inputs bind at USE, never at creation, so a required - * field with no value at all in `input` is never a create-time - * rejection — a workflow with a required trigger field can still be - * created with it left open until someone actually runs it. Only a - * value the caller explicitly provided gets checked, - * and only for basic shape (a non-empty string) — a key present but - * blank is a caller bug, not an open input, and still rejected. - * `kind: "agent"` resolution (does a provided value name a real - * taskable definition) is a separate, still-eager check a host layers - * on top (`apps/hub/src/index.ts`'s `routineInputValid`) since it - * needs a tenant DB lookup this function can't do. Fire-time - * validation (a host's own launcher definition checks) remains the - * authoritative required-field gate. - */ +/** Create-time boundary check for a routine's stored `input`. Inputs bind at + * use, never at creation, so a missing required field is not rejected here + * — only a caller-provided but blank value is. Fire-time validation remains + * the authoritative required-field gate. */ export function validateTriggerFieldsAtCreate( fields: readonly WorkflowTriggerField[], input: Record, diff --git a/packages/workflows/src/client.ts b/packages/workflows/src/client.ts index 4cd50dcede..70db810f4d 100644 --- a/packages/workflows/src/client.ts +++ b/packages/workflows/src/client.ts @@ -1,8 +1,5 @@ // @corbits/workflows browser-safe entry — no `@intx/*`, no `drizzle-orm`, -// no `hono`: the workflow source-tree constants, the definition-detail -// wire schema, and the pure lifecycle-copy helper a workflow's own page -// reads directly. `check:browser-safe-subpaths` walks the real import -// graph from here and fails if anything server-only leaks in. +// no `hono`. A structural check walks the import graph from here. export * from "./source"; export { runBearerHeaders, diff --git a/packages/workflows/src/definition-projection.ts b/packages/workflows/src/definition-projection.ts index 40a4057ded..8648cdd612 100644 --- a/packages/workflows/src/definition-projection.ts +++ b/packages/workflows/src/definition-projection.ts @@ -1,30 +1,12 @@ // Reads a `WorkflowDefinition`'s launch body back out of an already-resolved -// inert wire projection. -// -// There is no hub-side row to read a launch body back off of — this -// module is the pure half: the named errors and the schema-validated -// reader that turns an already-in-hand inert projection into a -// `FoldedBody`, needed by chat and anything else holding a projection -// value from elsewhere. Nothing in this file queries a database. -// -// One field of the launch body is deliberately NOT in the projection: -// `grantRequirements` does not survive the live->inert projector and is -// therefore outside the wire hash. Its hub-side home is the -// `workflow_definition.grant_requirements` column, so it is passed in -// alongside the projection rather than read off it. +// inert wire projection. See docs/workflow-definition-projection.md. import type { FoldedBody } from "@intx/workflow-deploy"; import { GrantRequirement, CredentialBinding } from "@intx/types"; import { ToolPackagePin } from "@intx/types/tool-packages"; import { type } from "arktype"; -/** - * Thrown when a definition carries no frozen wire projection — a row - * persisted before the projection was stored, or one whose approval - * never completed. Carries consumer language so an HTTP boundary can - * answer with a named 4xx instead of an unhandled 500, mirroring - * `InferenceResolutionError`'s split between the human `message` and the - * `guidance` a caller surfaces verbatim. - */ +/** Thrown when a definition carries no frozen wire projection. Carries + * consumer-facing `guidance` so an HTTP boundary can answer 4xx, not 500. */ export class DefinitionProjectionMissingError extends Error { readonly definitionName: string; readonly guidance: string; @@ -46,19 +28,9 @@ export type DefinitionCandidate = { readonly name: string; }; -/** - * Thrown when a definition's frozen projection carries more than one - * step. The launch target (`@corbits/agent-runtime`'s - * `AgentRuntimeConfig`) renders exactly one `systemPrompt` into one - * mailbox-triggered turn — it has no notion of step order at all, so - * there is nothing here a length check could safely relax: reading past - * `stepOrder[0]` would silently drop every later step's behavior rather - * than run it. Genuine multi-step launch needs a different deploy - * front (Interchange's native workflow-run trigger, `@intx/workflow-host`'s - * DAG supervisor) than this module provides. Carries consumer language - * so an HTTP boundary can answer with a named 4xx instead of an - * unhandled 500, mirroring `DefinitionProjectionMissingError`. - */ +/** Thrown when a definition's frozen projection carries more than one step — + * this launch target has no notion of step order. See + * docs/workflow-definition-projection.md#why-multi-step-definitions-are-rejected. */ export class MultiStepFoldUnsupportedError extends Error { readonly definitionId: string; readonly stepCount: number; @@ -78,14 +50,8 @@ export class MultiStepFoldUnsupportedError extends Error { } } -/** - * The launch-relevant subset of an inert projection's single `step` - * primitive. The projector reifies the live `AgentDefinition` into plain - * data and FLATTENS its inference chain: `agent.inference.sources` - * becomes a top-level `modelSources: { provider, model }[]` and the - * function-bearing `toolFactories` become descriptors. This schema - * validates exactly the reified fields `readFoldedBody` below reads. - */ +/** The launch-relevant subset of an inert projection's `step` primitive, + * after the projector flattens the live `AgentDefinition`'s inference chain. */ const InertWorkflowStepSchema = type({ kind: "'step'", agent: { @@ -95,14 +61,8 @@ const InertWorkflowStepSchema = type({ }, }); -/** - * The launch-relevant subset of an inert projection's `onTrigger` - * primitive (per-turn section shape, - * `@corbits/agent-runtime`'s `buildSectionWorkflow`): the agent-bearing - * step lives one level down, inside the section's inline body, not on - * the section step itself. `readFoldedBody` below reads through this - * shape the same way it reads a bare `step` primitive. - */ +/** The launch-relevant subset of an inert projection's `onTrigger` + * primitive — the agent-bearing step lives inside the section's inline body. */ const InertOnTriggerStepSchema = type({ kind: "'onTrigger'", body: { @@ -113,14 +73,8 @@ const InertOnTriggerStepSchema = type({ }, }); -/** - * Extracts the agent-bearing step primitive `readFoldedBody` needs, - * whichever of the two shapes a projection's launch step takes — a - * bare `step` (the conversational shape) or an `onTrigger` - * section whose inline body carries the one step that answers each - * turn. One reader for both shapes: neither call site duplicates the - * other's parsing. - */ +/** Extracts the agent-bearing step, whichever of the two launch-step shapes + * a projection takes. See docs/workflow-definition-projection.md#step-shapes. */ function extractAgentBearingStep( rawStep: unknown, definitionId: string, @@ -170,14 +124,9 @@ export const FoldedBodySchema = type({ model: "string | null", }); -/** - * Reads the launch body back out of a definition's frozen inert - * projection — the same fields `@intx/workflow-deploy`'s - * `extractFoldedBody` reads off a live `WorkflowDefinition`, read here - * off the projected plain data instead. `grantRequirements` comes from - * the definition row because the projector drops it (see the module - * header). - */ +/** Reads the launch body off a frozen inert projection instead of a live + * `WorkflowDefinition`. `grantRequirements` comes from the caller since the + * projector drops it. */ export function readFoldedBody(projection: unknown, grantRequirements: unknown): FoldedBody { const definition = InertWorkflowDefinitionSchema(projection); if (definition instanceof type.errors) { diff --git a/packages/workflows/src/deliver-when-routable.ts b/packages/workflows/src/deliver-when-routable.ts index d846212018..f278320df0 100644 --- a/packages/workflows/src/deliver-when-routable.ts +++ b/packages/workflows/src/deliver-when-routable.ts @@ -1,10 +1,6 @@ -// Since the process provisioner started spawning one sidecar per -// allocation, a freshly launched run's first mail can arrive before that -// sidecar has finished booting and registering with the hub, so the send -// fails with "agent is unreachable" even though nothing is actually wrong -// — the address just isn't routable yet. The webhook trigger's ingress -// delivery goes through this helper; chat's `sendRunMailWithReclaimRetry` -// shares this same budget rather than declaring its own. +// A freshly launched run's first mail can arrive before its sidecar +// finishes booting and registering, failing "agent is unreachable" even +// though nothing is wrong. This helper retries once the address routes. export const DEFAULT_ROUTABLE_DEADLINE_MS = 20_000; const DEFAULT_POLL_INTERVAL_MS = 250; @@ -31,13 +27,8 @@ function defaultSleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -/** - * Attempts `send`; if it fails with "agent is unreachable", polls - * `isRoutable` until the address is routable (or the deadline passes) - * and sends exactly once more. Any other failure, or an unreachable - * failure that never resolves before the deadline, is rethrown as-is — - * this never swallows the original error or retries indefinitely. - */ +/** Attempts `send`; on "agent is unreachable", polls until routable (or the + * deadline passes) and sends once more. Never swallows the original error. */ export async function deliverWhenRoutable(opts: DeliverWhenRoutableOptions): Promise { const isUnreachable = opts.isUnreachable ?? isAgentUnreachableError; try { diff --git a/packages/workflows/src/detail/definition-detail.ts b/packages/workflows/src/detail/definition-detail.ts index c64e4b9bc3..0fc24fdefc 100644 --- a/packages/workflows/src/detail/definition-detail.ts +++ b/packages/workflows/src/detail/definition-detail.ts @@ -1,18 +1,5 @@ -// The wire shape a workflow's own page (`apps/web/src/pages/ -// workflow-detail-page.tsx`) reads. Pure/browser-safe: no `@intx/*`, no -// `drizzle-orm`, no `hono` — `@corbits/workflows/client` re-exports this -// directly so `apps/web` can import it. -// -// The hub mounts no Workbench-composed detail route for this — the -// client renders directly off `@intx/hub-api`'s stock -// `GET /api/tenants/:tenantId/workflows/definitions` list -// (`vendor/intx/hub-api/src/routes/workflow-definitions.ts`, -// `WorkflowDefinitionResponse` in `vendor/intx/types/src/workflows.ts`), -// which exposes only `id`, `name`, `description`, `currentVersion`, -// `status` (`deployed` | `stopped`), `createdAt`, `updatedAt` — no asset -// display name, no manifest/package metadata, no wire projection (so no -// steps, no schedule trigger), and no grant/credential-binding read. -// This is a real gap, not a design choice made here. +// The wire shape a workflow's own page reads. Pure/browser-safe. Mirrors +// `@intx/hub-api`'s stock definitions list, which exposes only these fields. import { type } from "arktype"; export const WorkflowDefinitionDetail = type({ diff --git a/packages/workflows/src/detail/index.ts b/packages/workflows/src/detail/index.ts index f3f7ca9143..d02a9eeffa 100644 --- a/packages/workflows/src/detail/index.ts +++ b/packages/workflows/src/detail/index.ts @@ -1,8 +1,5 @@ -// The detail symbols are the browser-safe `../client` barrel's canonical -// export site — re-exported here rather than redeclared, so there is -// exactly one place that owns this list and the two barrels cannot drift -// apart. There is no hub route here — see -// `./definition-detail.ts`'s header for why. +// Re-exported from `../client`, not redeclared, so the two barrels can't +// drift apart. export { workflowNotLaunchableReason, workflowDetailPath, diff --git a/packages/workflows/src/index.ts b/packages/workflows/src/index.ts index 3d59aab1da..10a8528c86 100644 --- a/packages/workflows/src/index.ts +++ b/packages/workflows/src/index.ts @@ -1,8 +1,4 @@ -// @corbits/workflows server entry — everything: the source-tree -// renderer/reader and agent-authored-workflow authoring. There are no -// hub-mounted definition detail or scheduled-workflow routes — the -// client reads stock `@intx/hub-api` routes directly and this package -// only exposes pure, browser-safe shaping. Browser code imports +// @corbits/workflows server entry. Browser code imports // `@corbits/workflows/client` instead (see ./client.ts). export * from "./source"; export * from "./detail/index"; diff --git a/packages/workflows/src/launch/agent-session.ts b/packages/workflows/src/launch/agent-session.ts index 22862fa8e9..b41d8c3ff7 100644 --- a/packages/workflows/src/launch/agent-session.ts +++ b/packages/workflows/src/launch/agent-session.ts @@ -1,68 +1,17 @@ -// Every Workbench launch path provisions through Interchange's -// `prepareProvisionedDeployment` with a `sessionId` it mints itself, but -// nothing writes that id into `agent_session` (vendor-owned, -// `vendor/intx/db/src/schema/sessions.ts`) — the table -// `resolveRunSessionId` (`vendor/intx/hub-sessions/src/hub-session-lookups.ts`) -// reads to route a run's outbound mail. Every launcher must do this -// itself, through this one shared helper, so mail and spans persist -// against a real session from the run's first turn. -// -// Timing matters: `workflow_run.principal_id` (the FK `agent_session` -// keys on) is still null the instant `prepareProvisionedDeployment` -// returns — a provisioned anchor is born "deployed" with no principal, -// and only the run's first trigger reconciles one onto it (Interchange's -// `anchorWithPrincipal`, `vendor/intx/db/src/workflow-run-store.ts`). -// Pre-creating the run's principal to dodge this is not an option either: -// Interchange's own grant materialization -// (`vendor/intx/hub-api/src/run-grant-materialization.ts`) inserts the -// principal row `onConflictDoNothing` and treats a conflict as "grants -// already committed", throwing rather than re-materializing — so nothing -// upstream of that first trigger may write the principal first. -// -// `recordAgentSessionAtProvision` writes the row immediately after -// `prepareProvisionedDeployment` returns, keyed on the deploying -// principal (`sourceAuthorityPrincipalId`) since the run principal does -// not exist yet — every other field the launcher already knows or can -// read straight off the fresh run row. `ensureRunSession` re-keys onto -// the run's own principal once one is anchored, moving `principal_id` -// without ever touching the session id; it is called from every seam -// that might be a run's first mail-routable moment (chat's `sendMail`, -// the vendored `persistMail` a heartbeat run's first outbound mail -// hits), and is a true upsert so a missing row — a run deployed -// straight through Interchange's own -// `POST /api/tenants/:id/workflows/deployments` route never gets the -// eager write above — is created rather than treated as a bug. +// Every native launcher writes the `sessionId` it mints here, so mail and +// spans persist from a run's first turn. See docs/agent-session-provisioning.md. import { eq } from "drizzle-orm"; import type { DB } from "@intx/db"; import { agentSession, workflowRun, workflowRunLaunchSpec } from "@intx/db/schema"; import type { EventCollectorRegistry } from "@intx/hub-sessions"; -/** - * The one port every launcher threads the same wrapped - * `EventCollectorRegistry` through (`apps/hub/src/index.ts`'s - * `eventCollectors`) — never a second registry construction. `create` - * is what actually makes `inference_turn`/`turn_part` rows exist for a - * run; `abandon` tears the collector down when a run's session - * ends; `has` lets a caller check whether a collector already exists - * before creating a second one. - */ +/** The one port every launcher threads the same wrapped + * `EventCollectorRegistry` through, never a second registry construction. */ export type EventCollectorPort = Pick; -/** - * The one shared write every native launcher calls right after its own - * `prepareProvisionedDeployment` returns — a run row exists - * the instant that call resolves, so the session it will use is known - * then too: the launch-spec `sessionId` it just minted, `tenantId` and - * `definitionId` read fresh off the new `workflow_run` row (Interchange - * resolves the asset id the caller passed to a real `workflow_definition` - * row internally; callers never see that id themselves), and the - * deploying principal (`sourceAuthorityPrincipalId`) standing in for the - * run's own principal, which does not exist yet. `onConflictDoNothing` - * makes a second call for the same run id a no-op. Every launcher threads - * the same wrapped `EventCollectorRegistry` through here so the run's - * `inference_turn`/`turn_part` rows have somewhere to land from its very - * first turn. - */ +/** The one shared write every native launcher calls right after + * `prepareProvisionedDeployment` returns. See + * docs/agent-session-provisioning.md#why-timing-forces-a-two-step-write. */ export async function recordAgentSessionAtProvision(params: { readonly db: DB["db"]; readonly eventCollectors: Pick; @@ -99,26 +48,8 @@ export async function recordAgentSessionAtProvision(params: { } } -/** - * True upsert of a run's `agent_session`, keyed by the run's own - * principal once Interchange anchors one (`anchorWithPrincipal`, the - * run's first trigger). Before that, `recordAgentSessionAtProvision` - * records the row under the deploying principal for a Workbench-launched - * run — but a run deployed straight through Interchange's own - * `POST /api/tenants/:id/workflows/deployments` route (the e2e suites, - * or any other API client) never passes through a Workbench launcher, so - * that eager write never runs for it, and no `agent_session` row exists - * yet when this seam is first reached. By the time persist/dispatch call - * this, though, the run is anchored (`runRow.principalId` is set), so - * everything needed to create the row is on hand: this inserts it rather - * than treating the missing row as a bug. The launch-spec row is still - * the source of truth for which session id belongs to this run — its - * absence means a launcher provisioned this run without going through - * the shared path, which is a bug, not a state to paper over. Also - * creates the run's event collector if none is live for its address (a - * fresh process has no in-memory collectors regardless of what is on - * disk). - */ +/** True upsert of a run's `agent_session`, keyed by the run's own principal + * once Interchange anchors one. See docs/agent-session-provisioning.md. */ export async function ensureRunSession(params: { readonly db: DB["db"]; readonly eventCollectors: Pick; @@ -174,12 +105,8 @@ export async function ensureRunSession(params: { return sessionId; } -/** - * Marks a run's `agent_session` ended — called only from the places that - * already know the run died: a chat relaunch (the old run's session) or - * a one-shot prompt's teardown. No sweeper; a run nobody reacts to stays - * `active` until something does. - */ +/** Marks a run's `agent_session` ended. No sweeper; a run nobody reacts to + * stays `active` until something does. */ export async function endAgentSessionForPrincipal( db: DB["db"], principalId: string, @@ -191,17 +118,9 @@ export async function endAgentSessionForPrincipal( .where(eq(agentSession.principalId, principalId)); } -/** - * Same as `endAgentSessionForPrincipal`, keyed by the run's own id - * instead — for a caller (e.g. a one-shot prompt's teardown, or a chat - * relaunch replacing a terminal run) that only ever held the run id, - * never minted or looked up its principal. Also abandons the outgoing - * run's event collector, through the same shared - * `EventCollectorPort` every launcher threads — one mechanism for - * ending a run's session. No-ops for a run row already gone, or one - * never triggered (no principal, hence no session was ever recorded - * for it). - */ +/** Same as `endAgentSessionForPrincipal`, keyed by run id, plus abandons the + * run's event collector. No-ops for a run row already gone or never + * triggered. */ export async function endAgentSessionForRun( db: DB["db"], anchorRunId: string, diff --git a/packages/workflows/src/launchable/target-rule.ts b/packages/workflows/src/launchable/target-rule.ts index eef8273e8f..97cb1d810c 100644 --- a/packages/workflows/src/launchable/target-rule.ts +++ b/packages/workflows/src/launchable/target-rule.ts @@ -1,8 +1,5 @@ -// The follow-latest rule, pure — no `drizzle-orm`, no `@intx/db`, so this -// half of `./target.ts` is safe on `@corbits/workflows/client` and -// testable without a database. `./target.ts` is the DB-touching half -// (`resolveLaunchableDefinition`) that queries rows and hands them to -// `pickLaunchableDefinition` below. +// The follow-latest rule, pure — no `drizzle-orm`, no `@intx/db` — so this +// half is browser-safe and testable without a database. export type LaunchableDefinitionRejection = | "not_found" | "unfrozen" @@ -22,14 +19,8 @@ export type LaunchableDefinitionResolver = ( definitionAssetId: string, ) => Promise; -/** - * The columns the pick needs from one `workflow_definition` row joined - * to its current `workflow_definition_version` — the row the deploy - * freeze stamps `approved_wire_hash`, `grant_snapshot`, and - * `wire_projection` onto (`@intx/db`'s `loadFrozenGrantSnapshot` reads - * the same row). Declared as a plain shape so the ordering rule below - * is testable without a database. - */ +/** The columns the pick needs from one `workflow_definition` row, declared + * as a plain shape so the ordering rule is testable without a database. */ export type LaunchableDefinitionCandidate = { readonly id: string; readonly tenantId: string; @@ -40,11 +31,8 @@ export type LaunchableDefinitionCandidate = { readonly createdAt: Date; }; -/** A deploy freeze stamps `approvedWireHash`, `grantSnapshot`, and - * `wireProjection` onto a definition's current version atomically — this - * is the one predicate for "did that freeze land," shared by the - * follow-latest rule below and `../detail/definition-lifecycle.ts`'s - * lifecycle derivation. */ +/** The one predicate for "did a deploy freeze land," shared by the + * follow-latest rule and the definition-lifecycle derivation. */ export function isFrozen(candidate: { readonly approvedWireHash: string | null; readonly grantSnapshot: unknown; @@ -59,16 +47,9 @@ export function isFrozen(candidate: { ); } -/** - * The follow-latest rule, pure: among every definition row minted for - * one asset (across tenants — the caller passes them all so a - * cross-tenant reference can be named as such rather than read as - * "missing"), the newest row in `tenantId` that is `deployed` AND - * frozen wins. The rejection reason is the most specific one the rows - * support: no rows at all → `not_found`; rows, none in this tenant → - * `cross_tenant`; in-tenant rows, none deployed → `not_deployed`; - * deployed rows, none frozen → `unfrozen`. - */ +/** The follow-latest rule: the newest row in `tenantId` that is `deployed` + * and frozen wins. The rejection reason is the most specific the rows + * support, so a cross-tenant reference is named as such, not "missing." */ export function pickLaunchableDefinition( candidates: readonly LaunchableDefinitionCandidate[], tenantId: string, @@ -80,10 +61,7 @@ export function pickLaunchableDefinition( if (deployed.length === 0) return { ok: false, reason: "not_deployed" }; const frozen = deployed.filter(isFrozen); if (frozen.length === 0) return { ok: false, reason: "unfrozen" }; - // Newest wins; a `createdAt` tie (redeploys minted in the same request, - // at timestamp granularity that doesn't separate them) breaks on `id` - // desc so the pick is a deterministic total order, never array-input - // order — the one tiebreak this rule uses, everywhere it's used. + // A `createdAt` tie breaks on `id` desc for a deterministic total order. const newest = [...frozen].sort((a, b) => { const byCreatedAt = b.createdAt.getTime() - a.createdAt.getTime(); if (byCreatedAt !== 0) return byCreatedAt; @@ -99,13 +77,8 @@ export function pickLaunchableDefinition( }; } -/** - * The typed refusal a route answers with when a routine's target does - * not resolve — one code per reason so a UI or Myra can branch on it, - * and a sentence a person can act on. A cross-tenant asset is reported - * as not found: naming another tenant's asset must not confirm it - * exists. - */ +/** The typed refusal a route answers with when a routine's target does not + * resolve. A cross-tenant asset reports as not found. */ export function routineTargetRejection(reason: LaunchableDefinitionRejection): { readonly status: 404 | 409; readonly code: string; @@ -136,12 +109,8 @@ export function routineTargetRejection(reason: LaunchableDefinitionRejection): { } } -/** - * Thrown by a launcher when a routine fires and its target no longer - * resolves: the fire fails closed (recorded as a failed run by the - * scheduler's own bookkeeping) instead of running whatever row happens - * to exist. - */ +/** Thrown by a launcher when a routine fires and its target no longer + * resolves — fails closed rather than running whatever row exists. */ export class RoutineTargetUnresolvableError extends Error { readonly reason: LaunchableDefinitionRejection; readonly definitionAssetId: string; diff --git a/packages/workflows/src/schedule/cron-sentence.ts b/packages/workflows/src/schedule/cron-sentence.ts index cbfc64dbdb..837d3a8a74 100644 --- a/packages/workflows/src/schedule/cron-sentence.ts +++ b/packages/workflows/src/schedule/cron-sentence.ts @@ -1,10 +1,6 @@ -// Every schedule a person reads, as a sentence — DESIGN.md Copy: "cron -// expressions render as human sentences ('every weekday at 9am'), never as -// the raw expression, in any surface a person reads them". -// -// `cronstrue` (MIT, zero runtime dependencies, browser-safe) covers the -// same 5-field grammar `./cron.ts` validates. The timezone is named only -// when the schedule has a wall clock to read in that zone. +// Every schedule renders as a human sentence, never a raw cron expression +// (DESIGN.md Copy). Timezone is named only when there's a wall clock to +// read it in. import { toString as describeCronExpression } from "cronstrue"; /** A cron field is unpinned when it matches every value in its range — @@ -14,25 +10,16 @@ function fieldIsUnpinned(field: string): boolean { return /^\*(\/\d+)?$/.test(field); } -/** - * True when the expression names a time of day — an hour or a minute - * someone could point at on a clock. A step-every-15-minutes expression - * does not; `0 9 * * *` does, and only then does the zone it is read in - * mean anything. - */ +/** True when the expression names a time of day, e.g. `0 9 * * *`, not a + * step cadence like every 15 minutes. */ export function cronHasWallClock(expression: string): boolean { const [minute, hour] = expression.trim().split(/\s+/); if (minute === undefined || hour === undefined) return false; return !fieldIsUnpinned(minute) || !fieldIsUnpinned(hour); } -/** - * A raw 5-field cron expression as an English sentence, naming its - * timezone when the schedule has a wall clock to read in it — `null` when - * the expression is not describable, so a caller can show the person - * their own invalid input instead of a confident sentence about a - * schedule that will never fire. - */ +/** A cron expression as an English sentence, or `null` when not + * describable, so a caller can show the raw invalid input instead. */ export function cronSentence(expression: string, timezone: string = "UTC"): string | null { let described: string; try { diff --git a/packages/workflows/src/schedule/run-outcome.ts b/packages/workflows/src/schedule/run-outcome.ts index 8c7eca4b32..ed345d3e53 100644 --- a/packages/workflows/src/schedule/run-outcome.ts +++ b/packages/workflows/src/schedule/run-outcome.ts @@ -1,14 +1,5 @@ // A platform run's status the way surfaces should show it, and its words. -// Warm-keep leaves a fire's delivery agent deployed after it -// replies, so `workflow_run.status` never settles out of `running` on its -// own. Insights, Mission Control, and the shell activity feed all read a -// listing-shaped payload through here rather than badging the raw column, -// so a lingering `running` status past `FIRE_RUNNING_WINDOW_MS` reads as -// completed instead of still in flight. Originally -// `@corbits/routines`' `health.ts`/`run-language.ts`; carried over verbatim -// when routines were cut over to native `ScheduleTrigger` definitions -// — every surface that reads a run's displayed status still -// needs the same reading. +// See docs/run-outcome-status.md. /** One turn on a listing-shaped payload — enough to tell in-flight from settled. */ export type ListingTurn = { @@ -40,15 +31,8 @@ function nestedTurns(run: Record | undefined): readonly unknown return Array.isArray(turns) ? turns : undefined; } -/** - * How long an *abandoned* fire may linger as `running` with no `endedAt` - * before it is read as completed (warm-keep). A finished - * fire is supposed to land `completed`/`failed`/`cancelled` plus `endedAt` - * via `markTerminal`; this window is last-resort for a fire already known - * abandoned that never got that write. It is never applied to a live - * in-flight fire — a tool loop can outlast ten minutes, and persist has - * not settled yet. - */ +/** How long an abandoned fire may linger as `running` with no `endedAt` + * before it reads as completed. See docs/run-outcome-status.md. */ export const FIRE_RUNNING_WINDOW_MS = 10 * 60 * 1000; type InFlightSignal = "yes" | "no" | "unknown"; @@ -61,10 +45,7 @@ function runHasNoInFlightTurn(run: Record | undefined): boolean return run?.hasInFlightTurn === false; } -/** - * Absent `turns` / `hasInFlightTurn` is unknown, not "no in-flight turn". - * Treating omit as empty reverts the live-tool-loop 10-minute false-complete. - */ +/** Absent `turns`/`hasInFlightTurn` is unknown, not "no in-flight turn". */ function listingInFlightSignal(listing: ListingRun): InFlightSignal { if (listing.hasInFlightTurn === true || runHasInFlightTurn(listing.run)) { return "yes"; @@ -80,23 +61,14 @@ function listingInFlightSignal(listing: ListingRun): InFlightSignal { return "unknown"; } -/** - * A listing row has an in-flight turn when the producer said so - * (`hasInFlightTurn`) or attached a running turn (top-level `turns` or - * nested on `run`). A finished turn (`endedAt` set, or status other than - * `running`) does not count. Omitted fields are unknown, not false. - */ +/** A listing row has an in-flight turn when the producer said so or attached + * a running turn. Omitted fields are unknown, not false. */ export function listingHasInFlightTurn(listing: ListingRun): boolean { return listingInFlightSignal(listing) === "yes"; } -/** - * Abandoned only when the producer explicitly said there is no in-flight - * turn — an empty `turns` array after a real query, or `hasInFlightTurn: - * false` — and the fire is older than `FIRE_RUNNING_WINDOW_MS`. Omitting - * those fields is not a no; a live tool-loop listing that has not attached - * them must stay running. - */ +/** Abandoned only when the producer explicitly said there is no in-flight + * turn and the fire is older than `FIRE_RUNNING_WINDOW_MS`. */ export function listingAbandoned(listing: ListingRun, now: number): boolean { if (listingInFlightSignal(listing) !== "no") return false; const startedAt = Date.parse(listing.createdAt); @@ -110,16 +82,8 @@ export function withListingAbandoned(listing: T, now: numb return { ...listing, abandoned: true }; } -/** - * `runOutcomeStatus` for a platform run whose status lives at the top - * level (`workflow_run.status`). Insights, Mission Control, and the shell - * activity feed all see that shape. `endedAt` is the persist-path signal - * that the fire already finished. A listing-shaped payload with an - * in-flight turn stays `running` however old it is; one that explicitly - * says there is no in-flight turn is abandoned past - * `FIRE_RUNNING_WINDOW_MS` and remapped to `completed`. Omitting those - * fields is not a no. - */ +/** `runOutcomeStatus` for a platform run whose status lives at the top level + * (`workflow_run.status`). See docs/run-outcome-status.md. */ export function runOutcomeStatus( run: { readonly createdAt: string; diff --git a/packages/workflows/src/source.ts b/packages/workflows/src/source.ts index 9f854bb453..0a74153925 100644 --- a/packages/workflows/src/source.ts +++ b/packages/workflows/src/source.ts @@ -1,25 +1,6 @@ -// The one shape a `workflow`-kind asset may carry. -// -// Upstream retired the on-disk `workflow.json` envelope: the push -// validator (`vendor/intx/hub-sessions/src/workflow-kind.ts`, -// `workflowKindHandler.validatePush`) refuses a bare serialized -// definition and accepts only a source codebase — a `package.json` -// declaring `interchange.workflow` plus that entry module, which -// default-exports the definition. Every authoring path in this repo -// writes that tree through `renderWorkflowSourceTree` and recovers the -// definition back out of it through `parseWorkflowSourceEntry`, so the -// bytes on disk have exactly one producer and one consumer. -// -// `validate-push.test.ts` round-trips the rendered tree through the real -// `workflowKindHandler.validatePush`, so a renderer/validator drift fails a -// test here instead of surfacing as a push rejection on a workflow someone -// just created. -// -// The entry is a JSON literal rather than a call into a builder: an -// asset tree is a standalone codebase, so it can declare no workspace -// dependency to evaluate, and the definition it carries is inert data. -// That is what lets the reader below be a strict slice of a known -// prefix and suffix instead of an evaluation. +// The one shape a `workflow`-kind asset may carry: a source codebase, per +// upstream's push validator. `renderWorkflowSourceTree` and +// `parseWorkflowSourceEntry` are its one producer and consumer. /** The entry module's path inside the asset tree. */ export const WORKFLOW_SOURCE_ENTRY_PATH = "workflow.js"; @@ -53,13 +34,8 @@ export function renderWorkflowSourceTree(args: { }; } -/** - * Thrown when an asset does not carry the source form — in practice, an - * asset last written before the retirement, whose tree still holds a - * bare `workflow.json`. Named so every route boundary can answer it as - * a client-visible conflict with re-authoring guidance rather than - * letting it read as a server fault. - */ +/** Thrown when an asset still carries the retired bare `workflow.json` + * envelope, so a route boundary can answer it as a client-visible conflict. */ export class RetiredWorkflowEnvelopeError extends Error { readonly assetId: string; @@ -75,12 +51,8 @@ export class RetiredWorkflowEnvelopeError extends Error { } } -/** - * Recovers the serialized definition from the exact bytes - * `renderWorkflowSourceTree` emits. A strict single-shape slice, never - * an evaluation: anything else is an asset this lineage did not author - * in its current form. - */ +/** Recovers the serialized definition from the exact bytes + * `renderWorkflowSourceTree` emits — a strict slice, never an evaluation. */ export function parseWorkflowSourceEntry(entryModule: string, assetId: string): string { if (!entryModule.startsWith(ENTRY_PREFIX) || !entryModule.endsWith(ENTRY_SUFFIX)) { throw new RetiredWorkflowEnvelopeError(assetId); @@ -88,21 +60,14 @@ export function parseWorkflowSourceEntry(entryModule: string, assetId: string): return entryModule.slice(ENTRY_PREFIX.length, entryModule.length - ENTRY_SUFFIX.length); } -/** - * The blob read a source-form asset needs. Declared structurally so this - * package stays dependency-free; `@intx/hub-sessions`' `AssetService` - * satisfies it exactly. - */ +/** The blob read a source-form asset needs, declared structurally so this + * package stays dependency-free. */ export type WorkflowSourceBlobReader = { readAssetBlob(params: { assetId: string; path: string }): Promise; }; -/** - * Reads a source-form asset's serialized definition. A missing entry - * module is the retirement's own failure mode — an asset written before - * the cutover — so it surfaces as `RetiredWorkflowEnvelopeError` rather - * than the asset service's generic not-found. - */ +/** Reads a source-form asset's serialized definition. A missing entry + * module surfaces as `RetiredWorkflowEnvelopeError`, not a generic not-found. */ export async function readWorkflowSourceDefinition( reader: WorkflowSourceBlobReader, assetId: string,