feat(U6): delete composition runner + skill_catalog.execution/mode#542
Merged
Conversation
Adds hand-rolled migration 0029 to drop the legacy execution + mode columns (plus idx_skill_catalog_execution) from skill_catalog. Removes the Drizzle schema fields so every consumer sees the new shape. Extends the db-migrate-manual drift reporter with a `drops-column:` marker vocabulary so the CI gate can probe for column absence. Migration applies after the runtime cutover ships. The columns are no longer read by any TS or Python code (enforced by U6's grep-for-zero).
…position → invokeSkillRun (U6) * Removes every TS reference to skillCatalog.execution / skillCatalog.mode — the catalog no longer ships those columns after 0029. * Renames invokeComposition → invokeSkillRun (plus CompositionInvokePayload → SkillRunInvokePayload). The function is now generic over any kind=run_skill envelope; the old name tied it to the retired composition path. * Simplifies workspace-map-generator to drop the mode:tool / mode:agent table column — the catalog stopped tracking that. * Updates the three webhook / skill-runs tests that mocked the old function name, plus the integration harness README + helper. * Refreshes composition-era prose in cancelSkillRun, startSkillRun, webhooks/_shared, chat-intent.test, job-trigger so descriptions match the unified dispatch path.
Deletes the parallel orchestrator + its input-parser sibling (~1,783 LOC across 6 files: composition_runner.py, skill_inputs.py, four test files). Collapses run_skill_dispatch.py into a thin rejector that POSTs the canonical unsupported-runtime failure so scheduled / webhook / catalog envelopes still transition the row out of `running` — just with a structured `failed` status instead of executing. Collapses skill_runner to a single register_skill_tools entry point (drops load_composition_skills and the old register_skill_tools variant). Removes the PRD-40 always-on context-body loop from server.py: post-U5 every SKILL.md body loads on demand via the Skill meta-tool. Keeps shadow_dispatch — the harness is still useful for future cutovers. Rewrites test_server_run_skill to cover the new failure-path contract. Per the plan's "nothing in prod, downtime OK" directive, scheduled / webhook / catalog skill_runs are temporarily offline until a replacement out-of-band dispatcher lands.
* Deletes smoke-package-only/ — the probe existed solely to exercise the retired runtime. * Deletes tests/test_seed_compositions.py + test_reconciler_composition.py — both asserted against the composition DSL's YAML shape. * Rewrites scripts/u8-status.ts around a `done | regressed | unknown` axis (drops SMOKE_PROBES) and refreshes its vitest suite. * Rewrites README + characterization/README to describe the post-U6 shapes (script / context) only. * Renames the skill-dispatcher tool pair to start_skill_run / skill_run_status (+ updated SKILL.md, skill.yaml, test suite). * Normalises frame / synthesize / gather metadata: drops the retired `invocable_from: composition` flag in favour of `sub_skill`, prunes composition-* tags, simplifies migration-note prose so the mandatory grep patterns return zero. * Drops the composition / declarative enum values from the census script + updates the fixtures in its vitest suite. * Refreshes SKILL.md migration notes on sales-prep / renewal-prep / account-health-review / customer-onboarding-reconciler.
…ng (U6)
* Deletes scripts/smoke/complete-smoke.sh — it dispatched the
smoke-package-only probe that U6 just removed.
* Rewrites validate-skill-catalog.sh to enforce execution ∈ {script,
context} instead of validating the retired composition Pydantic
schema. Drops the no-blocking-sleep lint (it targeted composition
sub-skills that no longer exist).
* Updates CHECKS.md / README.md / chat-smoke.sh / catalog-smoke.sh
prose to describe the post-U6 contract: every kind=run_skill
envelope currently terminates with the canonical unsupported-
runtime failure; the smokes validate the row lifecycle, not runtime
execution.
6 tasks
ericodom
added a commit
that referenced
this pull request
Apr 24, 2026
…n primitives (#547) * fix(skill-catalog): bootstrap sync hardening + stale-builtin cleanup Carries forward the fixes from the still-open #544 and adds a post-upsert cleanup pass so retired builtin slugs disappear from skill_catalog on every deploy (needed because the next commit retires the composition-era primitives and leaving stale rows would let them resurface in the admin Capabilities page). * Defensive `toStringArray` helper so one authoring mistake (like the `triggers: {}` empty-map that took down #540/#542's Bootstrap) can no longer blow up the entire sync loop. * Accept `slug:` OR `id:` as the catalog key. Deliverable-shape skills use `id:` per the census convention; without this coercion sales-prep / account-health-review / renewal-prep / customer-onboarding-reconciler silently skipped the sync and never landed in skill_catalog. * Drop the dead `execution` + `mode` fields from the insert row (removed by #542's migration 0029). * After upserts, DELETE from skill_catalog WHERE source='builtin' AND slug NOT IN (<active slugs>). Tenant-uploaded rows are untouched. * feat(db): migration 0030 — retire composition-era primitive skills Deletes agent_skills, tenant_skills, and skill_catalog rows for the four composition-era primitives (frame, synthesize, gather, compound) that the pure-skill-spec rewrite retires. Adds a marker view so the db-migrate-manual drift reporter can confirm the migration applied. The YAML directories + S3 artifacts for the retired slugs are removed in the matching commits. sync-catalog-db.ts also scrubs stale builtin rows on every deploy as defense-in-depth, but this migration cleans the first-apply stage explicitly. * refactor(skill-catalog): delete composition-era primitive skills * frame — prompt/frame.md starts with "You are the frame step of a composition" * synthesize — prompt/synthesize.md starts with "You are the synthesize step of a composition" * gather — declarative-to-context stub that never actually executed anything * compound — "Learnings loop for compositions" — redundant with the container's native Hindsight recall/reflect tools All four were orchestration primitives the retired composition_runner invoked between deliverable steps. With the runner deleted in plan §U6 their SKILL.md bodies described a paradigm that no longer exists and the prompts/*.md templates were orphaned (nothing renders them post-U6). The deliverable skills that used to call Skill("frame", ...), etc. now inline the equivalent framing + synthesis guidance directly in their own SKILL.md bodies — a pure Claude-spec skill pattern with no harness-specific indirection. See the next commit. Net: 4 directories, ~1,600 LOC removed. * refactor(skill-catalog): rewrite deliverable SKILL.md as pure Claude-spec Rewrites the four deliverable-shape skill bodies (sales-prep, account-health-review, renewal-prep, customer-onboarding-reconciler) as pure Claude-spec Agent Skills — a markdown instruction doc the model reads and executes, with: * Proper frontmatter (name, description, license, metadata, allowed-tools). * Inline framing + synthesis guidance (no more Skill("frame") / Skill("synthesize") / Skill("gather") hops — the model does those natively as part of its turn). * Direct `recall` / `reflect` calls for the memory loop (Hindsight's native tools are already on the agent; no wrapper skill needed). * Only genuine tool calls remain: `Skill("package", ...)` for the deterministic template render, and the real connector Skills (crm_account_summary, web_search, etc.) for data gathering. Each deliverable's skill.yaml.requires_skills is narrowed to the actual tool-call dependencies (package + connectors) — the retired primitives drop off the session allowlist automatically. Also deletes the orphan customer-onboarding-reconciler/sub-skills/act/ sub-module; task-creation logic lives inline in the reconciler's rewritten SKILL.md now. * feat(bootstrap): S3 cleanup for retired slugs + regen every agent's workspace map Two things the deploy pipeline needs to do cleanly when a skill is retired from the catalog, neither of which it did before: 1. **Remove the retired slug's S3 prefix.** Plain `aws s3 sync` wouldn't delete objects that used to be there. The container's install_skill_from_s3 would still happily sync them onto warm runtimes on cold starts. Fix: per-slug sync is now `--delete` (so stacking stale files on a rewritten SKILL.md stops happening), and an explicit `aws s3 rm --recursive` clears frame/synthesize/gather/ compound prefixes as a belt-and-suspenders pass. 2. **Regenerate AGENTS.md for every existing agent.** The workspace-map- generator only runs on setAgentSkills mutations. When a deploy adds or retires a slug, existing agents' AGENTS.md files keep listing the old set until someone re-saves their skills. Fix: new `packages/api/scripts/regen-all-workspace-maps.ts` loops every agent and invokes regenerateWorkspaceMap. Bootstrap calls it after catalog sync. Per-agent failure is caught + logged; a single bad workspace cannot wedge the deploy. With these two steps running on every deploy, the on-disk workspace of every agent reflects the canonical skill list within one reconcile cycle — no manual intervention needed when slugs change. * test(skill-catalog): adjust u8-status expectations for the cleanup * Lower the `done` floor from 20 → 16 now that frame/synthesize/ gather/compound have been retired. * sales-prep's requires_skills assertion flips from "contains frame/synthesize/package" → "contains package, does NOT contain frame/synthesize/gather" since framing + synthesis happen inline in the rewritten SKILL.md body.
9 tasks
ericodom
added a commit
that referenced
this pull request
Apr 24, 2026
* feat(api): U1 resolveAgentRuntimeConfig helper + /api/agents/runtime-config endpoint
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U1.
Factors the agent-level resolution logic (template + skills + KBs + MCP
configs + guardrail + sandbox template) out of chat-agent-invoke.ts into
a shared helper. Both the chat-invoke Lambda and the new service-auth
REST endpoint call the same resolver, so the run_skill dispatcher and
the chat loop can never drift on "what this agent's runtime looks like."
* packages/api/src/lib/resolve-agent-runtime-config.ts — new helper.
Returns AgentRuntimeConfig (agent, tenant, template, guardrailConfig,
guardrailId, skillsConfig with defaults + built-in tools + blocked-
tools filter, knowledgeBasesConfig, mcpConfigs, sandboxTemplate).
Exports AgentNotFoundError + AgentTemplateNotFoundError for caller
mapping to HTTP responses.
* packages/api/src/handlers/agents-runtime-config.ts — new service-auth
GET /api/agents/runtime-config?tenantId=X&agentId=Y. Bearer
API_AUTH_SECRET per the `docs/solutions/best-practices/service-
endpoint-vs-widening-resolvecaller-auth-2026-04-21.md` precedent.
Optional currentUserId + currentUserEmail query params drive the
CURRENT_USER_EMAIL overlay on default skills.
* packages/api/src/handlers/chat-agent-invoke.ts — replaces the ~300-
line inline resolution block with a single `resolveAgentRuntimeConfig`
call. Per-turn concerns (thread_id, trace_id, message, messages_
history, currentUserId + currentUserEmail derivation, sandbox
preflight) stay in the handler. Behavior-preserving refactor.
* terraform/modules/app/lambda-api/handlers.tf — registers the new
handler and routes GET /api/agents/runtime-config to it.
* Tests: 8 helper-level unit tests (happy path, AgentNotFoundError,
AgentTemplateNotFoundError, template vs tenant-default guardrail,
blocked-tools filter, currentUserEmail overlay, built-in tool
injection, MCP delegation). 13 handler-level tests (method/path/auth/
UUID validation + helper-exception → 404 mapping + currentUser param
pass-through).
Next: U2 — extract `_execute_agent_turn` helper in server.py so the
dispatcher can reuse the chat-loop prologue + `_call_strands_agent`.
* refactor(agentcore-strands): U2 extract _execute_agent_turn
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U2.
Pure refactor — no behavior change. Factors the ~170-line chat-loop
prologue out of AgentCoreHandler.do_POST into a free function
`_execute_agent_turn(payload)` so U3's dispatcher can reuse the same
path without duplicating env setup + skill install + workspace bootstrap
+ eval-span attach + message build + system_prompt build + the
_call_strands_agent call.
What stayed in do_POST (chat-specific):
* invocation_env.apply/cleanup (run_skill branch has its own already).
* OpenAI chat.completion response shape + tool_costs/hindsight_usage
projection.
* _audit_response, auto-retain via api_memory_client, log_agent_invocation.
* Guardrail-exception classification into a blocked-response 200.
* self._respond HTTP writes.
What moved into _execute_agent_turn:
* Payload env unpack (workspace_bucket, thinkwork_api_url/secret,
hindsight_endpoint, _INSTANCE_ID, _ASSISTANT_ID).
* install_skill_from_s3 loop.
* _ensure_workspace_ready.
* _inject_skill_env.
* attach/detach_eval_context.
* messages list build (history + current).
* Router profile resolution + effective_skills filter.
* _build_system_prompt + external-task / workflow-skill / KB context
injection.
* _call_strands_agent.
* tool_costs drain snapshot.
Returns {response_text, strands_usage, duration_ms, invocation_tool_costs}.
Raises on _call_strands_agent failure; caller classifies guardrail vs
non-guardrail.
Covered by existing container tests (234/234 passing). Adds no new
tests — the extraction is a lift-and-shift. U3 will exercise the
helper from the dispatcher path with real assertions.
* feat(agentcore-strands): U3 real kind=run_skill dispatcher
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U3.
Replaces the post-§U6 unsupported-runtime rejector shipped in #542 with
a real dispatcher that:
1. Validates the envelope — null agentId fails fast with a named
reason (webhook-sourced runs deferred to a follow-up per plan).
2. Fetches the agent's runtime config from the new
GET /api/agents/runtime-config endpoint (U1) via a new Python
client api_runtime_config.py. Bounded retry: 5xx + transient
transport errors retry 3× with jittered backoff; 4xx terminal.
404 is a specific AgentConfigNotFoundError subclass.
3. Builds a chat-invoke-shaped synthetic payload (tenant / agent /
skills / MCP / guardrail + a synthetic user message instructing
the agent to run the skill with the given inputs).
4. Runs a headless Strands agent turn via the shared
server._execute_agent_turn helper (U2) — the chat loop and the
dispatcher now share one execution path; no second codebase to
drift.
5. POSTs terminal status back to /api/skills/complete with the
HMAC-signed callback. Successful runs write
deliveredArtifactRef={type: "inline", payload: <markdown>};
empty responses fail with "agent produced no final text";
agent-loop exceptions fail with "agent loop crashed: <msg>".
Also:
* container-sources/_boot_assert.py — adds api_runtime_config to
EXPECTED_CONTAINER_SOURCES so a Dockerfile COPY regression surfaces
loudly at boot instead of silently shipping non-functional.
* test_api_runtime_config.py — 9 tests: happy path, query-string
shape, currentUserId forwarding, missing env, 404 → AgentConfigNot-
FoundError, 401 terminal, 5xx retry, 5xx-then-200 recovery, socket
timeout exhaustion, non-JSON body.
* test_server_run_skill.py — rewritten for the new contract. 16 tests
covering: happy path inline deliverable, null-agentId fast-fail,
missing-field short-circuit, config-404, config-5xx, agent-loop
crash, empty-response-text failure, synthetic-payload shape (both
camelCase + snake_case config keys), HMAC retry semantics carried
forward.
Full container suite: 249 passed (up from 234 pre-U3).
Dockerfile uses the wildcard COPY container-sources/ /app/ so no
explicit entry needed (per the post-#542 Dockerfile shape).
* feat(api,lambda): U4 flip kind=run_skill Lambda invokes to Event
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U4.
RequestResponse with a 28s socket timeout was fundamentally incompatible
with the agent loop the dispatcher now runs — the AgentCore Lambda has
a 900s ceiling and real skill runs routinely take 10-60s+. Every emitter
flips to InvocationType: Event so enqueue acknowledges immediately while
the agent executes for as long as it needs.
The HMAC-signed /api/skills/complete callback is the authoritative
execution-result signal — it writes skill_runs.status on completion
whether success or failure. Enqueue-level errors (IAM, Lambda missing)
still surface synchronously via AWS returning 4xx/5xx on the Invoke
call, which each emitter re-exposes through its ok/error return shape.
This does NOT cross feedback_avoid_fire_and_forget_lambda_invokes —
that rule governs paths with no callback; we have a durable HMAC-signed
one.
Flipped sites:
* packages/api/src/graphql/utils.ts — invokeSkillRun (GraphQL
mutation path + webhooks/_shared.ts both call this).
* packages/api/src/handlers/skills.ts — invokeAgentcoreRunSkill
(POST /api/skills/start service-auth path).
* packages/lambda/job-trigger.ts — scheduled job invoke.
Post-flip error handling checks `res.StatusCode` (Event returns 202 on
success, 4xx/5xx on enqueue failure) rather than `res.FunctionError` +
`res.Payload` (RequestResponse-only). Removed the NodeHttpHandler
socket-timeout override — Event invokes don't hold the connection
open waiting for the Lambda, so the 28s timeout workaround isn't
meaningful.
The chat-agent-invoke path (InvocationType: RequestResponse at
chat-agent-invoke.ts:426) is unaffected — chat turns still need the
synchronous response to land in the thread message stream.
Full TS suite: 1375 passed, 68 passed (no regressions).
* docs(smoke): U5 update smoke scripts for the real dispatcher
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U5.
Replaces the post-#542 "every kind=run_skill envelope terminates with
U6 unsupported-runtime" passing condition with the real contract: the
row reaches `complete` (agent produced the deliverable) or `failed`
with a specific reason (connector missing, agent loop error,
config-fetch failure). Only stuck-running / transport errors fail the
smoke.
* scripts/smoke/CHECKS.md — rewrites the "What passing means today"
section to describe the U4 dispatcher flow (config fetch → headless
agent turn → HMAC callback). Refreshes the "Known gaps" list to
note the remaining webhook null-agentId deferral instead of the
retired U6 placeholder.
* scripts/smoke/README.md — updates the webhook smoke test
expectations; complete or specific-reason failure are both PASS.
Refreshes the "What passes vs fails" bullet list.
* scripts/smoke/catalog-smoke.sh — header docstring + PASS-condition
comment describe the new terminal-states contract.
* scripts/smoke/chat-smoke.sh — same.
* fix(review): apply ce-code-review autofix feedback
- chat-agent-invoke.ts: delete stale 8-line comment describing
currentUserEmail propagation that does not happen (M5).
- scheduled.test.ts + _harness/README.md: update docstrings to
reflect post-U4 Event + HMAC callback semantics, replacing stale
RequestResponse prose (M6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(api,lambda): thread agentId through run_skill envelope
ce-code-review P0-A. SkillRunInvokePayload was missing agentId; all
four TS emitters (startSkillRun.mutation, skills.ts
startSkillRunService, webhooks/_shared.ts, job-trigger.ts) omitted
the field; Python dispatcher rejected every non-webhook envelope with
_MISSING_AGENT_REASON. test_server_run_skill.py hand-injected
agentId="agent-1" which masked the prod bug.
The fix:
- Add `agentId: string | null` to SkillRunInvokePayload.
- Thread `i.agentId ?? null` into the GraphQL mutation emitter.
- Thread `agentId ?? null` into the service-REST startSkillRunService.
- Thread `dispatch.agentId ?? null` into the webhook shared handler.
- Thread `targetAgentId ?? null` into job-trigger's scheduled path.
- Add regression tests in skill-runs-resolvers, webhook-shared, and
job-trigger that assert agentId survives the envelope round-trip.
The job-trigger test also pins InvocationType=Event (§U4 contract).
Without this, PR #552 ships green CI but is inert for
scheduled/chat/catalog runs in production.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(api): tenant-scope users lookups in resolveAgentRuntimeConfig
ce-code-review P0-B. The helper is reachable via the service-auth
REST endpoint `GET /api/agents/runtime-config?currentUserId=...`,
where `currentUserId` is a caller-controlled query parameter. The
users lookup at L225 only filtered on users.id, so any holder of
API_AUTH_SECRET could enumerate a foreign tenant's user emails by
flipping `tenantId` to their own while passing another tenant's
userId.
All three users lookups (currentUserId email, human_pair email
fallback, human_pair name) now AND on users.tenant_id = opts.tenantId.
The human_pair lookups derive from a tenant-scoped agent row so the
predicate is defense-in-depth there; the currentUserId lookup is the
actual leak.
Added a regression test that upgrades the drizzle mock to capture
where() predicates and asserts both `users.id` and `users.tenant_id`
eq-pairs appear for the currentUserId lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tf): MaximumRetryAttempts=0 + DLQ for agentcore-invoke
ce-code-review P0-C. Plan §U4 flipped kind=run_skill dispatch to
InvocationType=Event so the agent loop has the full 900s Lambda
budget. AWS Lambda async-invoke defaults to MaximumRetryAttempts=2,
which on this path means a single transient failure (5xx on the
/api/skills/complete callback, container OOM, etc.) causes the whole
agent turn to run again — re-burning Bedrock tokens and (in
pathological cases) racing the first deliverable's writeback.
The agent turn is not idempotent. Set MaximumRetryAttempts=0 so
dispatch is one-shot. Durable safety net already exists:
/api/skills/complete atomically CAS-updates on status='running' (any
stray retry gets 409), and the skill-runs-reconciler flips rows stuck
in `running` past the 15-min cutoff to `failed`.
Failed invokes now land in thinkwork-<stage>-agentcore-async-dlq (SQS,
14-day retention, SSE-managed) instead of disappearing. Added IAM
policy on the agentcore role for sqs:SendMessage, plus two outputs
for operator inspection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ericodom
added a commit
that referenced
this pull request
May 5, 2026
…+ id-only slugs
Two root causes for the admin Capabilities page rendering "No results"
on dev this afternoon:
1. `gather/skill.yaml` declared `triggers: {}` (empty YAML map) but the
skill_catalog.triggers column is text[]. The sync script naively
cast `(y.triggers as string[]) || []` — `{}` is truthy so the
fallback never fired, and Drizzle's PgArray.mapToDriverValue blew
up with `value.map is not a function` the first time `gather` hit
the sync loop. That tripped the Bootstrap job for #540, #542, and
every other skill-catalog PR whose deploy actually ran Bootstrap
(others skipped it via path filter and masked the regression).
2. The U8 deliverable-shape skills (sales-prep, account-health-review,
renewal-prep, customer-onboarding-reconciler) declare `id:` not
`slug:` as the catalog key, matching the census script's convention.
The sync script only looked at `slug:` and silently dropped those
four skills from skill_catalog — the admin catalog page has been
missing them since the U8 series landed.
Fix:
- `gather/skill.yaml`: `triggers: {}` → `triggers: []`.
- `sync-catalog-db.ts`: defensive `toStringArray` helper coerces
non-array values to [] so one authoring mistake can't blow up the
whole bootstrap again.
- `sync-catalog-db.ts`: accept `slug:` OR `id:` as the catalog key,
matching census.ts.
- Removes the dead `execution` + `mode` fields from the insert row
(those columns were dropped in #542's migration 0029; Drizzle was
silently ignoring them post-deploy but the comment was misleading).
ericodom
added a commit
that referenced
this pull request
May 5, 2026
) * feat(db): drop skill_catalog.execution + mode (U6 migration 0029) Adds hand-rolled migration 0029 to drop the legacy execution + mode columns (plus idx_skill_catalog_execution) from skill_catalog. Removes the Drizzle schema fields so every consumer sees the new shape. Extends the db-migrate-manual drift reporter with a `drops-column:` marker vocabulary so the CI gate can probe for column absence. Migration applies after the runtime cutover ships. The columns are no longer read by any TS or Python code (enforced by U6's grep-for-zero). * refactor(api): drop skill_catalog.execution / mode + rename invokeComposition → invokeSkillRun (U6) * Removes every TS reference to skillCatalog.execution / skillCatalog.mode — the catalog no longer ships those columns after 0029. * Renames invokeComposition → invokeSkillRun (plus CompositionInvokePayload → SkillRunInvokePayload). The function is now generic over any kind=run_skill envelope; the old name tied it to the retired composition path. * Simplifies workspace-map-generator to drop the mode:tool / mode:agent table column — the catalog stopped tracking that. * Updates the three webhook / skill-runs tests that mocked the old function name, plus the integration harness README + helper. * Refreshes composition-era prose in cancelSkillRun, startSkillRun, webhooks/_shared, chat-intent.test, job-trigger so descriptions match the unified dispatch path. * feat(agentcore-strands): delete composition runner + input parser (U6) Deletes the parallel orchestrator + its input-parser sibling (~1,783 LOC across 6 files: composition_runner.py, skill_inputs.py, four test files). Collapses run_skill_dispatch.py into a thin rejector that POSTs the canonical unsupported-runtime failure so scheduled / webhook / catalog envelopes still transition the row out of `running` — just with a structured `failed` status instead of executing. Collapses skill_runner to a single register_skill_tools entry point (drops load_composition_skills and the old register_skill_tools variant). Removes the PRD-40 always-on context-body loop from server.py: post-U5 every SKILL.md body loads on demand via the Skill meta-tool. Keeps shadow_dispatch — the harness is still useful for future cutovers. Rewrites test_server_run_skill to cover the new failure-path contract. Per the plan's "nothing in prod, downtime OK" directive, scheduled / webhook / catalog skill_runs are temporarily offline until a replacement out-of-band dispatcher lands. * refactor(skill-catalog): retire composition DSL + smoke-probe slug (U6) * Deletes smoke-package-only/ — the probe existed solely to exercise the retired runtime. * Deletes tests/test_seed_compositions.py + test_reconciler_composition.py — both asserted against the composition DSL's YAML shape. * Rewrites scripts/u8-status.ts around a `done | regressed | unknown` axis (drops SMOKE_PROBES) and refreshes its vitest suite. * Rewrites README + characterization/README to describe the post-U6 shapes (script / context) only. * Renames the skill-dispatcher tool pair to start_skill_run / skill_run_status (+ updated SKILL.md, skill.yaml, test suite). * Normalises frame / synthesize / gather metadata: drops the retired `invocable_from: composition` flag in favour of `sub_skill`, prunes composition-* tags, simplifies migration-note prose so the mandatory grep patterns return zero. * Drops the composition / declarative enum values from the census script + updates the fixtures in its vitest suite. * Refreshes SKILL.md migration notes on sales-prep / renewal-prep / account-health-review / customer-onboarding-reconciler. * chore(smoke): retire composition probes + unsupported-runtime messaging (U6) * Deletes scripts/smoke/complete-smoke.sh — it dispatched the smoke-package-only probe that U6 just removed. * Rewrites validate-skill-catalog.sh to enforce execution ∈ {script, context} instead of validating the retired composition Pydantic schema. Drops the no-blocking-sleep lint (it targeted composition sub-skills that no longer exist). * Updates CHECKS.md / README.md / chat-smoke.sh / catalog-smoke.sh prose to describe the post-U6 contract: every kind=run_skill envelope currently terminates with the canonical unsupported- runtime failure; the smokes validate the row lifecycle, not runtime execution.
ericodom
added a commit
that referenced
this pull request
May 5, 2026
…n primitives (#547) * fix(skill-catalog): bootstrap sync hardening + stale-builtin cleanup Carries forward the fixes from the still-open #544 and adds a post-upsert cleanup pass so retired builtin slugs disappear from skill_catalog on every deploy (needed because the next commit retires the composition-era primitives and leaving stale rows would let them resurface in the admin Capabilities page). * Defensive `toStringArray` helper so one authoring mistake (like the `triggers: {}` empty-map that took down #540/#542's Bootstrap) can no longer blow up the entire sync loop. * Accept `slug:` OR `id:` as the catalog key. Deliverable-shape skills use `id:` per the census convention; without this coercion sales-prep / account-health-review / renewal-prep / customer-onboarding-reconciler silently skipped the sync and never landed in skill_catalog. * Drop the dead `execution` + `mode` fields from the insert row (removed by #542's migration 0029). * After upserts, DELETE from skill_catalog WHERE source='builtin' AND slug NOT IN (<active slugs>). Tenant-uploaded rows are untouched. * feat(db): migration 0030 — retire composition-era primitive skills Deletes agent_skills, tenant_skills, and skill_catalog rows for the four composition-era primitives (frame, synthesize, gather, compound) that the pure-skill-spec rewrite retires. Adds a marker view so the db-migrate-manual drift reporter can confirm the migration applied. The YAML directories + S3 artifacts for the retired slugs are removed in the matching commits. sync-catalog-db.ts also scrubs stale builtin rows on every deploy as defense-in-depth, but this migration cleans the first-apply stage explicitly. * refactor(skill-catalog): delete composition-era primitive skills * frame — prompt/frame.md starts with "You are the frame step of a composition" * synthesize — prompt/synthesize.md starts with "You are the synthesize step of a composition" * gather — declarative-to-context stub that never actually executed anything * compound — "Learnings loop for compositions" — redundant with the container's native Hindsight recall/reflect tools All four were orchestration primitives the retired composition_runner invoked between deliverable steps. With the runner deleted in plan §U6 their SKILL.md bodies described a paradigm that no longer exists and the prompts/*.md templates were orphaned (nothing renders them post-U6). The deliverable skills that used to call Skill("frame", ...), etc. now inline the equivalent framing + synthesis guidance directly in their own SKILL.md bodies — a pure Claude-spec skill pattern with no harness-specific indirection. See the next commit. Net: 4 directories, ~1,600 LOC removed. * refactor(skill-catalog): rewrite deliverable SKILL.md as pure Claude-spec Rewrites the four deliverable-shape skill bodies (sales-prep, account-health-review, renewal-prep, customer-onboarding-reconciler) as pure Claude-spec Agent Skills — a markdown instruction doc the model reads and executes, with: * Proper frontmatter (name, description, license, metadata, allowed-tools). * Inline framing + synthesis guidance (no more Skill("frame") / Skill("synthesize") / Skill("gather") hops — the model does those natively as part of its turn). * Direct `recall` / `reflect` calls for the memory loop (Hindsight's native tools are already on the agent; no wrapper skill needed). * Only genuine tool calls remain: `Skill("package", ...)` for the deterministic template render, and the real connector Skills (crm_account_summary, web_search, etc.) for data gathering. Each deliverable's skill.yaml.requires_skills is narrowed to the actual tool-call dependencies (package + connectors) — the retired primitives drop off the session allowlist automatically. Also deletes the orphan customer-onboarding-reconciler/sub-skills/act/ sub-module; task-creation logic lives inline in the reconciler's rewritten SKILL.md now. * feat(bootstrap): S3 cleanup for retired slugs + regen every agent's workspace map Two things the deploy pipeline needs to do cleanly when a skill is retired from the catalog, neither of which it did before: 1. **Remove the retired slug's S3 prefix.** Plain `aws s3 sync` wouldn't delete objects that used to be there. The container's install_skill_from_s3 would still happily sync them onto warm runtimes on cold starts. Fix: per-slug sync is now `--delete` (so stacking stale files on a rewritten SKILL.md stops happening), and an explicit `aws s3 rm --recursive` clears frame/synthesize/gather/ compound prefixes as a belt-and-suspenders pass. 2. **Regenerate AGENTS.md for every existing agent.** The workspace-map- generator only runs on setAgentSkills mutations. When a deploy adds or retires a slug, existing agents' AGENTS.md files keep listing the old set until someone re-saves their skills. Fix: new `packages/api/scripts/regen-all-workspace-maps.ts` loops every agent and invokes regenerateWorkspaceMap. Bootstrap calls it after catalog sync. Per-agent failure is caught + logged; a single bad workspace cannot wedge the deploy. With these two steps running on every deploy, the on-disk workspace of every agent reflects the canonical skill list within one reconcile cycle — no manual intervention needed when slugs change. * test(skill-catalog): adjust u8-status expectations for the cleanup * Lower the `done` floor from 20 → 16 now that frame/synthesize/ gather/compound have been retired. * sales-prep's requires_skills assertion flips from "contains frame/synthesize/package" → "contains package, does NOT contain frame/synthesize/gather" since framing + synthesis happen inline in the rewritten SKILL.md body.
ericodom
added a commit
that referenced
this pull request
May 5, 2026
* feat(api): U1 resolveAgentRuntimeConfig helper + /api/agents/runtime-config endpoint
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U1.
Factors the agent-level resolution logic (template + skills + KBs + MCP
configs + guardrail + sandbox template) out of chat-agent-invoke.ts into
a shared helper. Both the chat-invoke Lambda and the new service-auth
REST endpoint call the same resolver, so the run_skill dispatcher and
the chat loop can never drift on "what this agent's runtime looks like."
* packages/api/src/lib/resolve-agent-runtime-config.ts — new helper.
Returns AgentRuntimeConfig (agent, tenant, template, guardrailConfig,
guardrailId, skillsConfig with defaults + built-in tools + blocked-
tools filter, knowledgeBasesConfig, mcpConfigs, sandboxTemplate).
Exports AgentNotFoundError + AgentTemplateNotFoundError for caller
mapping to HTTP responses.
* packages/api/src/handlers/agents-runtime-config.ts — new service-auth
GET /api/agents/runtime-config?tenantId=X&agentId=Y. Bearer
API_AUTH_SECRET per the `docs/solutions/best-practices/service-
endpoint-vs-widening-resolvecaller-auth-2026-04-21.md` precedent.
Optional currentUserId + currentUserEmail query params drive the
CURRENT_USER_EMAIL overlay on default skills.
* packages/api/src/handlers/chat-agent-invoke.ts — replaces the ~300-
line inline resolution block with a single `resolveAgentRuntimeConfig`
call. Per-turn concerns (thread_id, trace_id, message, messages_
history, currentUserId + currentUserEmail derivation, sandbox
preflight) stay in the handler. Behavior-preserving refactor.
* terraform/modules/app/lambda-api/handlers.tf — registers the new
handler and routes GET /api/agents/runtime-config to it.
* Tests: 8 helper-level unit tests (happy path, AgentNotFoundError,
AgentTemplateNotFoundError, template vs tenant-default guardrail,
blocked-tools filter, currentUserEmail overlay, built-in tool
injection, MCP delegation). 13 handler-level tests (method/path/auth/
UUID validation + helper-exception → 404 mapping + currentUser param
pass-through).
Next: U2 — extract `_execute_agent_turn` helper in server.py so the
dispatcher can reuse the chat-loop prologue + `_call_strands_agent`.
* refactor(agentcore-strands): U2 extract _execute_agent_turn
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U2.
Pure refactor — no behavior change. Factors the ~170-line chat-loop
prologue out of AgentCoreHandler.do_POST into a free function
`_execute_agent_turn(payload)` so U3's dispatcher can reuse the same
path without duplicating env setup + skill install + workspace bootstrap
+ eval-span attach + message build + system_prompt build + the
_call_strands_agent call.
What stayed in do_POST (chat-specific):
* invocation_env.apply/cleanup (run_skill branch has its own already).
* OpenAI chat.completion response shape + tool_costs/hindsight_usage
projection.
* _audit_response, auto-retain via api_memory_client, log_agent_invocation.
* Guardrail-exception classification into a blocked-response 200.
* self._respond HTTP writes.
What moved into _execute_agent_turn:
* Payload env unpack (workspace_bucket, thinkwork_api_url/secret,
hindsight_endpoint, _INSTANCE_ID, _ASSISTANT_ID).
* install_skill_from_s3 loop.
* _ensure_workspace_ready.
* _inject_skill_env.
* attach/detach_eval_context.
* messages list build (history + current).
* Router profile resolution + effective_skills filter.
* _build_system_prompt + external-task / workflow-skill / KB context
injection.
* _call_strands_agent.
* tool_costs drain snapshot.
Returns {response_text, strands_usage, duration_ms, invocation_tool_costs}.
Raises on _call_strands_agent failure; caller classifies guardrail vs
non-guardrail.
Covered by existing container tests (234/234 passing). Adds no new
tests — the extraction is a lift-and-shift. U3 will exercise the
helper from the dispatcher path with real assertions.
* feat(agentcore-strands): U3 real kind=run_skill dispatcher
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U3.
Replaces the post-§U6 unsupported-runtime rejector shipped in #542 with
a real dispatcher that:
1. Validates the envelope — null agentId fails fast with a named
reason (webhook-sourced runs deferred to a follow-up per plan).
2. Fetches the agent's runtime config from the new
GET /api/agents/runtime-config endpoint (U1) via a new Python
client api_runtime_config.py. Bounded retry: 5xx + transient
transport errors retry 3× with jittered backoff; 4xx terminal.
404 is a specific AgentConfigNotFoundError subclass.
3. Builds a chat-invoke-shaped synthetic payload (tenant / agent /
skills / MCP / guardrail + a synthetic user message instructing
the agent to run the skill with the given inputs).
4. Runs a headless Strands agent turn via the shared
server._execute_agent_turn helper (U2) — the chat loop and the
dispatcher now share one execution path; no second codebase to
drift.
5. POSTs terminal status back to /api/skills/complete with the
HMAC-signed callback. Successful runs write
deliveredArtifactRef={type: "inline", payload: <markdown>};
empty responses fail with "agent produced no final text";
agent-loop exceptions fail with "agent loop crashed: <msg>".
Also:
* container-sources/_boot_assert.py — adds api_runtime_config to
EXPECTED_CONTAINER_SOURCES so a Dockerfile COPY regression surfaces
loudly at boot instead of silently shipping non-functional.
* test_api_runtime_config.py — 9 tests: happy path, query-string
shape, currentUserId forwarding, missing env, 404 → AgentConfigNot-
FoundError, 401 terminal, 5xx retry, 5xx-then-200 recovery, socket
timeout exhaustion, non-JSON body.
* test_server_run_skill.py — rewritten for the new contract. 16 tests
covering: happy path inline deliverable, null-agentId fast-fail,
missing-field short-circuit, config-404, config-5xx, agent-loop
crash, empty-response-text failure, synthetic-payload shape (both
camelCase + snake_case config keys), HMAC retry semantics carried
forward.
Full container suite: 249 passed (up from 234 pre-U3).
Dockerfile uses the wildcard COPY container-sources/ /app/ so no
explicit entry needed (per the post-#542 Dockerfile shape).
* feat(api,lambda): U4 flip kind=run_skill Lambda invokes to Event
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U4.
RequestResponse with a 28s socket timeout was fundamentally incompatible
with the agent loop the dispatcher now runs — the AgentCore Lambda has
a 900s ceiling and real skill runs routinely take 10-60s+. Every emitter
flips to InvocationType: Event so enqueue acknowledges immediately while
the agent executes for as long as it needs.
The HMAC-signed /api/skills/complete callback is the authoritative
execution-result signal — it writes skill_runs.status on completion
whether success or failure. Enqueue-level errors (IAM, Lambda missing)
still surface synchronously via AWS returning 4xx/5xx on the Invoke
call, which each emitter re-exposes through its ok/error return shape.
This does NOT cross feedback_avoid_fire_and_forget_lambda_invokes —
that rule governs paths with no callback; we have a durable HMAC-signed
one.
Flipped sites:
* packages/api/src/graphql/utils.ts — invokeSkillRun (GraphQL
mutation path + webhooks/_shared.ts both call this).
* packages/api/src/handlers/skills.ts — invokeAgentcoreRunSkill
(POST /api/skills/start service-auth path).
* packages/lambda/job-trigger.ts — scheduled job invoke.
Post-flip error handling checks `res.StatusCode` (Event returns 202 on
success, 4xx/5xx on enqueue failure) rather than `res.FunctionError` +
`res.Payload` (RequestResponse-only). Removed the NodeHttpHandler
socket-timeout override — Event invokes don't hold the connection
open waiting for the Lambda, so the 28s timeout workaround isn't
meaningful.
The chat-agent-invoke path (InvocationType: RequestResponse at
chat-agent-invoke.ts:426) is unaffected — chat turns still need the
synchronous response to land in the thread message stream.
Full TS suite: 1375 passed, 68 passed (no regressions).
* docs(smoke): U5 update smoke scripts for the real dispatcher
Plan docs/plans/2026-04-24-008-feat-skill-run-dispatcher-plan.md §U5.
Replaces the post-#542 "every kind=run_skill envelope terminates with
U6 unsupported-runtime" passing condition with the real contract: the
row reaches `complete` (agent produced the deliverable) or `failed`
with a specific reason (connector missing, agent loop error,
config-fetch failure). Only stuck-running / transport errors fail the
smoke.
* scripts/smoke/CHECKS.md — rewrites the "What passing means today"
section to describe the U4 dispatcher flow (config fetch → headless
agent turn → HMAC callback). Refreshes the "Known gaps" list to
note the remaining webhook null-agentId deferral instead of the
retired U6 placeholder.
* scripts/smoke/README.md — updates the webhook smoke test
expectations; complete or specific-reason failure are both PASS.
Refreshes the "What passes vs fails" bullet list.
* scripts/smoke/catalog-smoke.sh — header docstring + PASS-condition
comment describe the new terminal-states contract.
* scripts/smoke/chat-smoke.sh — same.
* fix(review): apply ce-code-review autofix feedback
- chat-agent-invoke.ts: delete stale 8-line comment describing
currentUserEmail propagation that does not happen (M5).
- scheduled.test.ts + _harness/README.md: update docstrings to
reflect post-U4 Event + HMAC callback semantics, replacing stale
RequestResponse prose (M6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(api,lambda): thread agentId through run_skill envelope
ce-code-review P0-A. SkillRunInvokePayload was missing agentId; all
four TS emitters (startSkillRun.mutation, skills.ts
startSkillRunService, webhooks/_shared.ts, job-trigger.ts) omitted
the field; Python dispatcher rejected every non-webhook envelope with
_MISSING_AGENT_REASON. test_server_run_skill.py hand-injected
agentId="agent-1" which masked the prod bug.
The fix:
- Add `agentId: string | null` to SkillRunInvokePayload.
- Thread `i.agentId ?? null` into the GraphQL mutation emitter.
- Thread `agentId ?? null` into the service-REST startSkillRunService.
- Thread `dispatch.agentId ?? null` into the webhook shared handler.
- Thread `targetAgentId ?? null` into job-trigger's scheduled path.
- Add regression tests in skill-runs-resolvers, webhook-shared, and
job-trigger that assert agentId survives the envelope round-trip.
The job-trigger test also pins InvocationType=Event (§U4 contract).
Without this, PR #552 ships green CI but is inert for
scheduled/chat/catalog runs in production.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(api): tenant-scope users lookups in resolveAgentRuntimeConfig
ce-code-review P0-B. The helper is reachable via the service-auth
REST endpoint `GET /api/agents/runtime-config?currentUserId=...`,
where `currentUserId` is a caller-controlled query parameter. The
users lookup at L225 only filtered on users.id, so any holder of
API_AUTH_SECRET could enumerate a foreign tenant's user emails by
flipping `tenantId` to their own while passing another tenant's
userId.
All three users lookups (currentUserId email, human_pair email
fallback, human_pair name) now AND on users.tenant_id = opts.tenantId.
The human_pair lookups derive from a tenant-scoped agent row so the
predicate is defense-in-depth there; the currentUserId lookup is the
actual leak.
Added a regression test that upgrades the drizzle mock to capture
where() predicates and asserts both `users.id` and `users.tenant_id`
eq-pairs appear for the currentUserId lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tf): MaximumRetryAttempts=0 + DLQ for agentcore-invoke
ce-code-review P0-C. Plan §U4 flipped kind=run_skill dispatch to
InvocationType=Event so the agent loop has the full 900s Lambda
budget. AWS Lambda async-invoke defaults to MaximumRetryAttempts=2,
which on this path means a single transient failure (5xx on the
/api/skills/complete callback, container OOM, etc.) causes the whole
agent turn to run again — re-burning Bedrock tokens and (in
pathological cases) racing the first deliverable's writeback.
The agent turn is not idempotent. Set MaximumRetryAttempts=0 so
dispatch is one-shot. Durable safety net already exists:
/api/skills/complete atomically CAS-updates on status='running' (any
stray retry gets 409), and the skill-runs-reconciler flips rows stuck
in `running` past the 15-min cutoff to `failed`.
Failed invokes now land in thinkwork-<stage>-agentcore-async-dlq (SQS,
14-day retention, SSE-managed) instead of disappearing. Added IAM
policy on the agentcore role for sqs:SendMessage, plus two outputs
for operator inspection.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the plan #7 §U6 arc: the parallel composition orchestrator is
deleted, every TS / Python / SQL surface stops reading
skill_catalog.execution/.mode, and the mandatory grep (plan §U6Approach) returns zero hits.
skill_inputs, 4 tests) + the smoke-package-only probe directory +
scripts/smoke/complete-smoke.sh+ 2 composition-era catalog tests.kind=run_skillenvelopes now terminate with acanonical "unsupported in this runtime" failure so scheduled / webhook
/ catalog paths still transition the row out of
running. Per theuser directive ("nothing in prod, downtime OK") out-of-band skill_run
execution is offline until a replacement dispatcher lands.
execution+modecolumnsand their index.
scripts/db-migrate-manual.shlearned a newdrops-column:vocabulary so the drift reporter can probe forcolumn absence.
invokeComposition→invokeSkillRun(+CompositionInvokePayload → SkillRunInvokePayload);
start_composition/composition_statustools →start_skill_run/skill_run_status.u8-status.tsnow tracks adone | regressed | unknownaxis; the census script drops the retired execution enumvalues;
validate-skill-catalog.shnow refuses any slug whoseexecution is outside
script | context.Mandatory grep result:
Test plan
pnpm --filter='!@thinkwork/agent-tools' -r --if-present typecheck— greenpnpm --filter='!@thinkwork/agent-tools' -r --if-present test— green (100 api + 18 cli + 2 skill-catalog + 5 lambda + 3 admin-ops + 1 db + 1 pricing + 1 workspace)uv run --with pytest --with pytest-asyncio --with strands-agents --with PyYAML --with pydantic pytest packages/agentcore-strands/agent-container/— 234 passedpnpm --filter @thinkwork/admin build— greenpnpm exec prettier --checkon the touched extensions — cleanbash scripts/db-migrate-manual.sh --dry-runrecognises 0029 + itsdrops-column:markerspsql "$DATABASE_URL" -f packages/database-pg/drizzle/0029_collapse_execution_types.sqlon each stageskill_catalog.execution/.modeare gone viapnpm db:migrate-manual🤖 Generated with Claude Code