charles/agent-hooks

Fork 0

feat(shared): roster-assistant proposal contract (#1210) #1217

Merged

reviewer merged 1 commit from code-lead/1210 into main

2026-05-15 18:12:33 +00:00

code-lead commented

2026-05-15 18:04:37 +00:00

Collaborator

Summary

First ticket of the Agents Roster AI Assistant feature — lands the typed shape both halves of the assistant agree on so the rest of the feature can build against a single contract. Closes #1210.

New shared module packages/shared/src/roster-proposal.ts:
- ProposalEnvelope, ProposedAgent, ProposedToolPermission, ProposedMcpServer, CatalogSnapshot types.
- MODEL_TIERS, TOOL_PERMISSION_MODES, DEFAULT_REFINEMENTS_SOFT_CAP consts.
- PROPOSAL_ENVELOPE_JSON_SCHEMA (draft 2020-12) — the same object the server hands to Claude as response_format and re-validates the response against (defence in depth).
File-level comment ships a destination map for the acceptor implementer (name → agent_type.name + agents.name, system_prompt → agent_type.system_prompt_template, skills[] → agent_skill, mcp_servers[] → mcp_server, …) so the next ticket walks one ProposedAgent and writes straight to the existing roster store with zero translation.
name regex on the JSON schema mirrors handleAgentsCreate's ^[a-z][a-z0-9-]*$ so the assistant fails fast in the structured-output validator instead of the acceptor 400-ing after a refinement loop.
30 contract tests in roster-proposal.test.ts:
- JSON round-trip (incl. nullable parent_proposal_id, optional rationale / setup_notes).
- Schema acceptance: canonical sample, refinement, every model tier, every permission mode, empty-array agents.
- Schema rejection: missing required fields, unknown top-level / agent fields (additionalProperties: false), empty agents, invalid tier / mode, name regex violations, non-integer / negative version, malformed created_at, wrong-type parent_proposal_id, missing required fields on tool/mcp entries.
- Write-path acceptance: the proposal's field types align with the existing createAgent() and downstream stores so a future field rename can't silently widen the surface.
ajv + ajv-formats added as devDeps on @claude-hooks/shared (validator only used in tests; the schema literal stays runtime-dep-free).

Test plan

bun test packages/shared/src/roster-proposal.test.ts — 30/30 pass.
just qa — typecheck + lint + format + sql-layer-check + paraglide + i18n + flow-schema all green; 3362-test server suite still passes.

🤖 Generated with Claude Code

## Summary First ticket of the **Agents Roster AI Assistant** feature — lands the typed shape both halves of the assistant agree on so the rest of the feature can build against a single contract. Closes #1210. - New shared module `packages/shared/src/roster-proposal.ts`: - `ProposalEnvelope`, `ProposedAgent`, `ProposedToolPermission`, `ProposedMcpServer`, `CatalogSnapshot` types. - `MODEL_TIERS`, `TOOL_PERMISSION_MODES`, `DEFAULT_REFINEMENTS_SOFT_CAP` consts. - `PROPOSAL_ENVELOPE_JSON_SCHEMA` (draft 2020-12) — the same object the server hands to Claude as `response_format` and re-validates the response against (defence in depth). - File-level comment ships a **destination map** for the acceptor implementer (`name` → `agent_type.name + agents.name`, `system_prompt` → `agent_type.system_prompt_template`, `skills[]` → `agent_skill`, `mcp_servers[]` → `mcp_server`, …) so the next ticket walks one `ProposedAgent` and writes straight to the existing roster store with zero translation. - `name` regex on the JSON schema mirrors `handleAgentsCreate`'s `^[a-z][a-z0-9-]*$` so the assistant fails fast in the structured-output validator instead of the acceptor 400-ing after a refinement loop. - 30 contract tests in `roster-proposal.test.ts`: - JSON round-trip (incl. nullable `parent_proposal_id`, optional `rationale` / `setup_notes`). - Schema acceptance: canonical sample, refinement, every model tier, every permission mode, empty-array agents. - Schema rejection: missing required fields, unknown top-level / agent fields (`additionalProperties: false`), empty `agents`, invalid tier / mode, name regex violations, non-integer / negative `version`, malformed `created_at`, wrong-type `parent_proposal_id`, missing required fields on tool/mcp entries. - Write-path acceptance: the proposal's field types align with the existing `createAgent()` and downstream stores so a future field rename can't silently widen the surface. - `ajv` + `ajv-formats` added as devDeps on `@claude-hooks/shared` (validator only used in tests; the schema literal stays runtime-dep-free). ## Test plan - [x] `bun test packages/shared/src/roster-proposal.test.ts` — 30/30 pass. - [x] `just qa` — typecheck + lint + format + sql-layer-check + paraglide + i18n + flow-schema all green; 3362-test server suite still passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added the

area:agents

area:meta

labels

2026-05-15 18:04:37 +00:00

code-lead added 1 commit

2026-05-15 18:04:37 +00:00

feat(shared): roster-assistant proposal contract (#1210 )

qa / sql-layer-check (pull_request) Successful in 10s

Details

qa / i18n-string-check (pull_request) Successful in 11s

Details

qa / db-schema (pull_request) Successful in 13s

Details

qa / dockerfile (pull_request) Successful in 27s

Details

qa / qa-1 (pull_request) Successful in 3m2s

Details

qa / qa (pull_request) Successful in 0s

Details

121d235aab

Adds the typed shape both halves of the Agents Roster AI Assistant agree
on — server constrains Claude's structured output against
PROPOSAL_ENVELOPE_JSON_SCHEMA and the web preview pane renders the
ProposalEnvelope / ProposedAgent / CatalogSnapshot types verbatim.

The shape is intentionally over-specified on the proposed-agent side
(per-grant tool_permissions, mcp_servers with setup_required) so the
acceptor ticket can write straight to agent_type / agents / agent_skill /
mcp_server rows with zero translation. See the destination map in the
file-level comment of roster-proposal.ts.

Tests cover JSON round-trip, schema acceptance + rejection across every
documented malformed shape (additionalProperties, enums, regex, minimums),
and field-by-field alignment with the existing handleAgentsCreate write
path so a future field rename can't silently widen the surface.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer

2026-05-15 18:10:28 +00:00

reviewer approved these changes

2026-05-15 18:12:22 +00:00

reviewer left a comment

Review — `feat(shared): roster-assistant proposal contract` (#1210)

CI: ✅ green · Mergeable: ✅

Overview

Clean contract-only PR. Lands the typed shape (ProposalEnvelope, ProposedAgent, CatalogSnapshot), the draft 2020-12 JSON Schema, and 30 contract tests covering round-trip, schema accept/reject, and write-path alignment. Every deliverable from issue #1210 is present and the destination map comment is accurate against the live schema.

Verified against:

apps/server/src/main.ts handleAgentsCreate — name regex /^[a-z][a-z0-9-]*$/ matches the schema pattern exactly ✅
infrastructure/database/schema/agents.ts — name, type, prompt_appendix, match_labels, notes, lifecycle, idle_stop_seconds all align with writeArgs in the write-path test ✅
infrastructure/database/schema/agent-type.ts — system_prompt_template direction correct; role column is separate from the prose role field and the destination map calls this out correctly ✅
infrastructure/database/schema/agent-skill.ts — skills[] maps to agent_skill.name (the natural key), consistent with the proposal's string-array shape ✅

Findings

1. Missing format: "uuid" on the three ID fields — suggest adding before follow-on tickets build on this

proposal_id, parent_proposal_id (the string branch), and session_id are documented as UUIDs (issue text: "Stable id (uuid)") but the schema only validates minLength: 1. ajv-formats is already a devDep and the tests already spin up an AJV instance with addFormats. Adding format: "uuid" to all three would:

Close the gap between the issue spec and the runtime validator
Give the structured-output gate a free extra check against Claude hallucinating a non-UUID (e.g. a plain incrementing integer)
Cost zero — the dep is already there

proposal_id: {
  type: "string",
  format: "uuid",          // ← add
  description: "Stable id (uuid) for this proposal.",
},
parent_proposal_id: {
  type: ["string", "null"],
  format: "uuid",          // ← add (AJV applies format checks to the string branch only)
  description: "...",
},
session_id: {
  type: "string",
  format: "uuid",          // ← add
  description: "...",
},

Not a blocker for this ticket since the schema is defence-in-depth over the structured-output gate, but worth landing before the propose-endpoint ticket starts consuming this schema in production.

2. default in refinements_soft_cap schema property has no runtime effect as configured

refinements_soft_cap: {
  type: "integer",
  minimum: 1,
  default: DEFAULT_REFINEMENTS_SOFT_CAP,   // ← AJV ignores this without useDefaults: true
  ...
},

The default is useful documentation for the Claude structured-output call (the model sees it in the schema) but the AJV validator in buildValidator() doesn't set useDefaults: true, so a response that omits the field would fail the required check rather than get the default applied. This is actually the correct behaviour (the field is in required), but the server-side code that eventually calls AJV with this schema should be aware the default is schema-documentation-only, not an AJV coercion trigger. Worth a one-line comment on the constant when the propose-endpoint ticket lands.

3. Validator instantiation inconsistency — minor

The accept-path describe block creates a fresh buildValidator() per test; the reject-path describe block shares a single instance at describe scope. AJV compiled validators are reentrant so there's no bug, but the inconsistency is mildly surprising. Either pattern is fine — just worth making uniform if this file grows.

Summary

All three ACs from #1210 are fully met, the destination map is accurate, the name regex matches the live handler, and the 30 tests exercise both the happy-path and the rejection surface. Finding #1 is the only one worth acting on before the propose-endpoint ticket ships; #2 and #3 are informational.

APPROVED. Landing this unblocks the rest of the feature.

## Review — `feat(shared): roster-assistant proposal contract` (#1210) **CI: ✅ green · Mergeable: ✅** --- ### Overview Clean contract-only PR. Lands the typed shape (`ProposalEnvelope`, `ProposedAgent`, `CatalogSnapshot`), the draft 2020-12 JSON Schema, and 30 contract tests covering round-trip, schema accept/reject, and write-path alignment. Every deliverable from issue #1210 is present and the destination map comment is accurate against the live schema. Verified against: - `apps/server/src/main.ts` `handleAgentsCreate` — name regex `/^[a-z][a-z0-9-]*$/` matches the schema `pattern` exactly ✅ - `infrastructure/database/schema/agents.ts` — `name`, `type`, `prompt_appendix`, `match_labels`, `notes`, `lifecycle`, `idle_stop_seconds` all align with `writeArgs` in the write-path test ✅ - `infrastructure/database/schema/agent-type.ts` — `system_prompt_template` direction correct; `role` column is separate from the prose `role` field and the destination map calls this out correctly ✅ - `infrastructure/database/schema/agent-skill.ts` — `skills[]` maps to `agent_skill.name` (the natural key), consistent with the proposal's string-array shape ✅ --- ### Findings **1. Missing `format: "uuid"` on the three ID fields — suggest adding before follow-on tickets build on this** `proposal_id`, `parent_proposal_id` (the string branch), and `session_id` are documented as UUIDs (issue text: "Stable id (uuid)") but the schema only validates `minLength: 1`. `ajv-formats` is already a devDep and the tests already spin up an AJV instance with `addFormats`. Adding `format: "uuid"` to all three would: - Close the gap between the issue spec and the runtime validator - Give the structured-output gate a free extra check against Claude hallucinating a non-UUID (e.g. a plain incrementing integer) - Cost zero — the dep is already there ```ts proposal_id: { type: "string", format: "uuid", // ← add description: "Stable id (uuid) for this proposal.", }, parent_proposal_id: { type: ["string", "null"], format: "uuid", // ← add (AJV applies format checks to the string branch only) description: "...", }, session_id: { type: "string", format: "uuid", // ← add description: "...", }, ``` Not a blocker for this ticket since the schema is defence-in-depth over the structured-output gate, but worth landing before the propose-endpoint ticket starts consuming this schema in production. **2. `default` in `refinements_soft_cap` schema property has no runtime effect as configured** ```ts refinements_soft_cap: { type: "integer", minimum: 1, default: DEFAULT_REFINEMENTS_SOFT_CAP, // ← AJV ignores this without useDefaults: true ... }, ``` The `default` is useful documentation for the Claude structured-output call (the model sees it in the schema) but the AJV validator in `buildValidator()` doesn't set `useDefaults: true`, so a response that omits the field would fail the `required` check rather than get the default applied. This is actually the correct behaviour (the field is in `required`), but the server-side code that eventually calls AJV with this schema should be aware the `default` is schema-documentation-only, not an AJV coercion trigger. Worth a one-line comment on the constant when the propose-endpoint ticket lands. **3. Validator instantiation inconsistency — minor** The accept-path describe block creates a fresh `buildValidator()` per test; the reject-path describe block shares a single instance at describe scope. AJV compiled validators are reentrant so there's no bug, but the inconsistency is mildly surprising. Either pattern is fine — just worth making uniform if this file grows. --- ### Summary All three ACs from #1210 are fully met, the destination map is accurate, the name regex matches the live handler, and the 30 tests exercise both the happy-path and the rejection surface. Finding #1 is the only one worth acting on before the propose-endpoint ticket ships; #2 and #3 are informational. **APPROVED.** Landing this unblocks the rest of the feature.

reviewer merged commit d6096866c0 into main

2026-05-15 18:12:33 +00:00