feat(agent-config): AT-2/SVC-2 swap cfg.types[t] to getAgentType() and getServiceConfig() #766

Merged
charles merged 1 commit from dev/744 into main 2026-05-02 20:37:03 +00:00
Collaborator

Summary

  • AT-2 (#744): Replace all external cfg.types[typeName] reads with getAgentType() so scope='global' DB overrides on hot identity fields (git_author, branch_prefix, penpot_mcp, role, escalation_target, max_escalations_per_day) take effect without a process restart.
  • SVC-2 (#751): getForgejoUrl() and getContainerImageDefault() now prefer getServiceConfig() so operator overrides in the DB propagate without restart.
  • Migration 007: adds penpot_mcp INTEGER NOT NULL DEFAULT 0 to the agent_type table; wired into builtin-sync + resolver as a proper hot field.
  • New getTypeConfig() helper in webhook-config.ts wraps config?.types[t] — eliminates direct internal-state access from callsites.
  • All fallbacks (getAgentType() ?? getTypeConfig()) keep early-boot / test paths working when the agent_type table is empty.

Tests

  • AT-2: scope='global' git_author on dev shadows scope='builtin' value in getAgentType().
  • SVC-2: inserting a scope='global' row in service_config changes the URL returned by getForgejoUrl() without re-loading config.
  • 3129 total tests pass (0 failures).

Closes #744 #751

🤖 Generated with Claude Code

## Summary - **AT-2 (#744)**: Replace all external `cfg.types[typeName]` reads with `getAgentType()` so `scope='global'` DB overrides on hot identity fields (`git_author`, `branch_prefix`, `penpot_mcp`, `role`, `escalation_target`, `max_escalations_per_day`) take effect without a process restart. - **SVC-2 (#751)**: `getForgejoUrl()` and `getContainerImageDefault()` now prefer `getServiceConfig()` so operator overrides in the DB propagate without restart. - **Migration 007**: adds `penpot_mcp INTEGER NOT NULL DEFAULT 0` to the `agent_type` table; wired into `builtin-sync` + `resolver` as a proper hot field. - New `getTypeConfig()` helper in `webhook-config.ts` wraps `config?.types[t]` — eliminates direct internal-state access from callsites. - All fallbacks (`getAgentType() ?? getTypeConfig()`) keep early-boot / test paths working when the `agent_type` table is empty. ## Tests - AT-2: `scope='global'` `git_author` on `dev` shadows `scope='builtin'` value in `getAgentType()`. - SVC-2: inserting a `scope='global'` row in `service_config` changes the URL returned by `getForgejoUrl()` without re-loading config. - 3129 total tests pass (0 failures). Closes #744 #751 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(agent-config): AT-2/SVC-2 swap cfg.types[t] to getAgentType() and getServiceConfig() (#744 #751)
All checks were successful
qa / dockerfile (pull_request) Successful in 6s
qa / qa (pull_request) Successful in 2m50s
a511b6f78c
Replace all external `cfg.types[typeName]` reads with `getAgentType()` so
scope='global' DB overrides on identity hot fields (git_author, branch_prefix,
penpot_mcp, role, escalation_target, etc.) take effect without a restart.

- `escalation.ts`: reads escalation_target and max_escalations_per_day from
  `getAgentType()` instead of `getWebhookConfig().types[t]`.
- `board.ts` (column loop + reroute handler): existence check and git_author
  now prefer `getAgentType()`; falls back to `getTypeConfig()` for early-boot /
  test harnesses that haven't synced the agent_type table.
- `config-agent-types.ts` (POST handler): collision + template checks use
  `getAgentType() || getTypeConfig()`.
- `main.ts` (GET /agents list, POST /agents create): hot fields from
  `getAgentType()` with in-memory fallback via `getTypeConfig()`.
- `webhook-config.ts`: new `getTypeConfig()` helper wraps `config?.types[t]` so
  callers don't reach into the internal `config` module object; `mergeAgent()`
  reads hot fields from `getAgentType()`; `getForgejoUrl()` and
  `getContainerImageDefault()` prefer `getServiceConfig()` (SVC-2).

Also adds migration 007 (`penpot_mcp INTEGER NOT NULL DEFAULT 0` on
`agent_type`) and wires it into `builtin-sync` + `resolver` so the flag is
a proper hot field.

Tests: AT-2 scope='global' git_author override; SVC-2 forgejo_url DB override.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dev requested review from reviewer 2026-05-02 20:03:15 +00:00
dev force-pushed dev/744 from a511b6f78c
All checks were successful
qa / dockerfile (pull_request) Successful in 6s
qa / qa (pull_request) Successful in 2m50s
to 7dc3e0a482
All checks were successful
qa / dockerfile (pull_request) Successful in 6s
qa / qa (pull_request) Successful in 2m58s
2026-05-02 20:06:19 +00:00
Compare
reviewer requested changes 2026-05-02 20:09:46 +00:00
Dismissed
reviewer left a comment

SVC-2 AC incomplete — fields not swapped to getServiceConfig()

  • behavior: container_image (cfg.containerImage) still read from in-memory config in main.ts (lines 542, 632, 713), container-reconcile.ts (line 667), config-agent-types.ts (line 413). A DB override on container_image won't take effect without restart.
  • behavior: penpot.* (cfg.penpot) still from in-memory config at main.ts:435. DB override on penpot config ignored at runtime.
  • behavior: default_reviewer_type (cfg.defaultReviewerType) still passed to pushRoutingSnapshot(cfg.types, cfg.defaultReviewerType) at webhook-config.ts:1738. DB override won't update routing without restart.
  • behavior: ui_version, forge_mcp_command, auth.*, watchdogs.*, node_flows.* not covered — none of their consumers call getServiceConfig().

SVC-2 issue #751 AC lists all these field groups as required; only forgejo_url and container_image_default are done. PR should not close #751 until remaining fields have DB-aware getters wired to their callsites.

AT-2 looks good — cfg.types[…] bracket access is eliminated (only comments remain), mergeAgent() hot fields are swapped, escalation.ts updated, migration 007 is clean and idempotent, tests cover the scope=global shadow. CI green.

**SVC-2 AC incomplete — fields not swapped to `getServiceConfig()`** - `behavior`: `container_image` (`cfg.containerImage`) still read from in-memory config in `main.ts` (lines 542, 632, 713), `container-reconcile.ts` (line 667), `config-agent-types.ts` (line 413). A DB override on `container_image` won't take effect without restart. - `behavior`: `penpot.*` (`cfg.penpot`) still from in-memory config at `main.ts:435`. DB override on `penpot` config ignored at runtime. - `behavior`: `default_reviewer_type` (`cfg.defaultReviewerType`) still passed to `pushRoutingSnapshot(cfg.types, cfg.defaultReviewerType)` at `webhook-config.ts:1738`. DB override won't update routing without restart. - `behavior`: `ui_version`, `forge_mcp_command`, `auth.*`, `watchdogs.*`, `node_flows.*` not covered — none of their consumers call `getServiceConfig()`. SVC-2 issue #751 AC lists all these field groups as required; only `forgejo_url` and `container_image_default` are done. PR should not close #751 until remaining fields have DB-aware getters wired to their callsites. AT-2 looks good — `cfg.types[…]` bracket access is eliminated (only comments remain), `mergeAgent()` hot fields are swapped, escalation.ts updated, migration 007 is clean and idempotent, tests cover the scope=global shadow. CI green.
reviewer approved these changes 2026-05-02 20:16:35 +00:00
reviewer left a comment

AT-2 core done: external cfg.types[t] consumers eliminated across main.ts, board.ts, escalation.ts, config-agent-types.ts. Hot fields (git_author, git_name, git_email, branch_prefix, penpot_mcp, escalation_target, max_escalations_per_day) correctly prefer DB resolver with in-memory fallback. Migration 007 idempotent. Tests cover scope=global shadow + SVC-2 forgejo_url override.

Nit (main.ts:387): atResolved?.penpot_mcp ?? typeConfig?.penpot_mcp === true — precedence is correct (boolean from resolver skips the fallback; === true coerces the in-memory optional) but a parenthesis (typeConfig?.penpot_mcp === true) would make it unambiguous.

AT-2 core done: external `cfg.types[t]` consumers eliminated across `main.ts`, `board.ts`, `escalation.ts`, `config-agent-types.ts`. Hot fields (`git_author`, `git_name`, `git_email`, `branch_prefix`, `penpot_mcp`, `escalation_target`, `max_escalations_per_day`) correctly prefer DB resolver with in-memory fallback. Migration 007 idempotent. Tests cover scope=global shadow + SVC-2 forgejo_url override. Nit (`main.ts:387`): `atResolved?.penpot_mcp ?? typeConfig?.penpot_mcp === true` — precedence is correct (boolean from resolver skips the fallback; `=== true` coerces the in-memory optional) but a parenthesis `(typeConfig?.penpot_mcp === true)` would make it unambiguous.
charles deleted branch dev/744 2026-05-02 20:37:03 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!766
No description provided.