refactor(service-config): URL consolidation series — PRs A + B + queued cleanup #827

Merged
reviewer merged 10 commits from refactor/service-config-consolidation into main 2026-05-04 17:36:57 +00:00
Collaborator

Summary

Picks up specs/service-config-url-consolidation.md. Branch carries 7 commits — 5 backlog cleanups already accumulated locally + PR A (rip 5 dead tabs) + PR B (rename Penpot tab → Design, co-present URL/secret).

PR C (relocate forgejo_url into operator_oauth_tokens.base_url) is not in this branch — it ships separately once this lands.

Commits (oldest → newest)

Commit Subject
50a57a8 chore(forge-mcp): rip legacy packages/forge-mcp/
85e19b5 chore: rip stale openapi.yml + agents.json.pre-f1.bak, vendor penpot-mcp-server
7cef3cd docs: update for #823 + #826 secret-table migrations
d4d82b5 fix(config): drop hardcoded /home/charles in clone-template token_file fallback
32652e9 fix(agents): retire legacy /config/agents — wire SPA editor to DB-backed sub-resources
42d677d PR A: refactor(service-config): rip 5 dead tabs + their service_config columns
64f12b0 PR B: refactor(service-config): rename Penpot tab → Design + co-present URL/secret

PR A recap (42d677d)

5 dead tabs ripped: Auth, UI, MCP, Routing, Node flows. Migration 015-drop-service-config-dead-cols.ts drops ui_version, forge_mcp_command, node_flows_json, default_reviewer_type. WebhookConfig + ServiceConfigSettingsRow + handler types trim. Tab bar drops to Forge · Container · Watchdogs · Penpot. Net: −1250 LOC.

PR B recap (64f12b0)

Penpot tab → Design tab. New DesignSection:

  • Penpot connection card co-presents base_url + default_team_id + a PENPOT_TOKEN editor in one card. Save merges the URL/team_id back into penpot_json and POST/PUTs the secret through existing SC-6 routes.
  • Per-repo team mapping card holds team_by_repo JsonField separately (per-repo routing, distinct concern).

Design call (option (b) in the spec): no external_connection table — secret table is name → ciphertext by design (#823) and reference-by-name is the established contract. Building a one-row metadata table for Penpot would be premature abstraction. PR C will reuse the existing operator_oauth_tokens.base_url for the forge URL, so neither PR needs a new schema home.

Test plan

  • just qa — 3135/3135 server tests + web typecheck green
  • just restart — service running on the new SPA bundle
  • Operator smoke — sign in, hit /app/settings/service, verify Design tab renders Penpot connection card + per-repo mapping card
  • Operator smoke — paste a value into the PENPOT_TOKEN field and confirm the secret row updates (last-set timestamp moves)

Follow-up (not in this PR)

PR C — drop service_config.forgejo_url, route every reader through operator_oauth_tokens.base_url (forge_type='forgejo' row already carries it). Service config tab bar collapses to Container · Watchdogs · Design; Forge URL surface moves to settings.repos.tsx / a Connections panel.

🤖 Generated with Claude Code

## Summary Picks up `specs/service-config-url-consolidation.md`. Branch carries 7 commits — 5 backlog cleanups already accumulated locally + PR A (rip 5 dead tabs) + PR B (rename Penpot tab → Design, co-present URL/secret). PR C (relocate `forgejo_url` into `operator_oauth_tokens.base_url`) is **not** in this branch — it ships separately once this lands. ## Commits (oldest → newest) | Commit | Subject | |---|---| | `50a57a8` | chore(forge-mcp): rip legacy `packages/forge-mcp/` | | `85e19b5` | chore: rip stale `openapi.yml` + `agents.json.pre-f1.bak`, vendor penpot-mcp-server | | `7cef3cd` | docs: update for #823 + #826 secret-table migrations | | `d4d82b5` | fix(config): drop hardcoded `/home/charles` in clone-template `token_file` fallback | | `32652e9` | fix(agents): retire legacy `/config/agents` — wire SPA editor to DB-backed sub-resources | | `42d677d` | **PR A:** refactor(service-config): rip 5 dead tabs + their `service_config` columns | | `64f12b0` | **PR B:** refactor(service-config): rename Penpot tab → Design + co-present URL/secret | ## PR A recap (`42d677d`) 5 dead tabs ripped: Auth, UI, MCP, Routing, Node flows. Migration `015-drop-service-config-dead-cols.ts` drops `ui_version`, `forge_mcp_command`, `node_flows_json`, `default_reviewer_type`. WebhookConfig + ServiceConfigSettingsRow + handler types trim. Tab bar drops to **Forge · Container · Watchdogs · Penpot**. Net: **−1250 LOC**. ## PR B recap (`64f12b0`) Penpot tab → **Design** tab. New `DesignSection`: - **Penpot connection** card co-presents `base_url` + `default_team_id` + a `PENPOT_TOKEN` editor in one card. Save merges the URL/team_id back into `penpot_json` and POST/PUTs the secret through existing SC-6 routes. - **Per-repo team mapping** card holds `team_by_repo` JsonField separately (per-repo routing, distinct concern). Design call (option (b) in the spec): no `external_connection` table — secret table is `name → ciphertext` by design (#823) and reference-by-name is the established contract. Building a one-row metadata table for Penpot would be premature abstraction. PR C will reuse the existing `operator_oauth_tokens.base_url` for the forge URL, so neither PR needs a new schema home. ## Test plan - [x] `just qa` — 3135/3135 server tests + web typecheck green - [x] `just restart` — service running on the new SPA bundle - [ ] Operator smoke — sign in, hit `/app/settings/service`, verify Design tab renders Penpot connection card + per-repo mapping card - [ ] Operator smoke — paste a value into the PENPOT_TOKEN field and confirm the secret row updates (last-set timestamp moves) ## Follow-up (not in this PR) PR C — drop `service_config.forgejo_url`, route every reader through `operator_oauth_tokens.base_url` (`forge_type='forgejo'` row already carries it). Service config tab bar collapses to **Container · Watchdogs · Design**; Forge URL surface moves to `settings.repos.tsx` / a Connections panel. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Active forge-mcp lives at apps/forge-mcp/ (Dockerfile + justfile build that one). The packages/ copy was unreachable from any runtime path — only stale doc-comments, an obsolete README, and self-references kept it alive while its tools.ts had already diverged from apps/forge-mcp/src/tools.ts. Drop the dir, retarget stale doc-comments + docs/multi-forge.md to apps/, refresh bun.lock.

mcp-config.test.ts already imported TOOL_NAMES via a relative path that resolves to apps/forge-mcp/ (verified), so no test retarget needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- openapi.yml: 5 paths declared vs 170 live routes; never consumed (zero refs in code/docs/CI). Last touched 2 weeks ago. Stale, kill it.
- config/agents.json.pre-f1.bak: dangling F1-migration backup on this dev machine (gitignored, never tracked). config/ dir is now empty, removed too. Live F1 migration code in webhook-config.ts kept — it still creates the backup if any operator re-runs against an unmigrated agents.json.
- penpot-mcp-server/ → vendor/penpot-mcp-server/: Python project at repo root mixed with the bun TS workspace tree; hatchling, distinct from packages/ (which the bun workspace globs). Move to vendor/ to signal "non-TS, not a workspace member, vendored fork." Dockerfile COPY path, biome.json ignore, docs/penpot.md updated.

just qa green (3195/3195).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- decisions/containers.md (D5/D7): note Forgejo PATs migrated from per-agent token files into the secret table (FORGEJO_TOKEN_<TYPE>, #823) and that sessions.json moved from the container state volume into claude_sdk_sessions on the host. Drop the now-removed sessions.json line from the volume diagram and the inContainerSessionsPath helper from the reference list.
- penpot.md: rewrite the Secret section — penpot-token file → secret table row PENPOT_TOKEN (#826), exposed via ${SECRET:PENPOT_TOKEN} placeholder. Boot migration drains the legacy file and renames to .penpot-token.migrated.bak.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
buildClonedTypeBlock fell back to a literal /home/charles/.config/claude-hooks/tokens/<name> when the template carried no token_file. Operator-specific path bleeds through to any deployment whose runtime user is not "charles". Use homedir() so the fallback matches whatever HOME the service runs under, mirroring agents-default-config.ts:28.

Existing tests cover the template-supplied path branch only; fallback branch was untested. qa green (3195/3195).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The on-disk \`agents.json\` file (and its \`GET/PUT /config/agents\` editor surface) was retired by DOB-5 (#797), but the SPA's \`fetchAgentConfig\`/\`putAgentConfig\` were never migrated to the new \`/api/agent-types/*\` + \`/api/service-config\` endpoints. The /app/agents page bootstrap query failed with 503 \`config not loaded\` and rendered the red "Failed to load config" banner on every load (#823 follow-up — discovered while closing that issue).

**Server**

- \`apps/server/src/http/handlers/config.ts\` — gut the file. Drop \`handleGetAgentsConfig\`, \`handleGetAgentsConfigSchema\`, \`handlePutAgentsConfig\` + their helpers (\`atomicWriteText\`, \`formatZodIssues\`, \`_resetSchemaCacheForTest\`). Keep only \`handleGetRepoLabels\` (the chip-input autocomplete is the sole survivor).
- \`apps/server/src/main.ts\` — drop the three \`/config/agents*\` routes and the now-unused \`getWebhookConfigPath\` import.
- \`apps/server/src/http/handlers/config.test.ts\` — delete (covered the dead endpoints only; \`handleGetRepoLabels\` had no coverage there).
- \`apps/server/src/shared/config/agents-config-schema.ts\` — drop \`agentsConfigJsonSchema()\` Draft-7 generator (only ever wired to the dead \`/config/agents/schema\` endpoint).
- \`apps/server/src/http/handlers/agent-types-settings.ts\` — extend \`GET /api/agent-types/:name\` with a new \`derived\` field exposing the boot-time \`AgentTypeConfig\` knobs the dashboard reads but doesn't store in the per-kind DB tables (\`default_model\`, \`default_plugins\`, \`default_system_prompt\`, \`system_prompt_template\`, \`provider\`, \`provider_chain\`, \`failover\`). Sourced from \`getTypeConfig()\` so the summary chip + prompt-section preview keep working.

**SPA (apps/web)**

- \`apps/web/src/lib/api.ts\` — replace \`fetchAgentConfig()\` with a composer that fans out to \`/api/agent-types\` (list) + N×\`/api/agent-types/:name\` + \`/api/service-config\` and assembles the editor-side \`AgentConfigResponse\`. Replace \`putAgentConfig()\` with a splitter that re-fetches the current server state, diffs against the next snapshot, and PUTs only the changed slices to \`/api/agent-types/:name/identity\`, \`/api/agent-types/:name/container\`, \`/api/agent-types/:name/routing\`, and \`/api/service-config\`. Drop dead exports: \`fetchAgentsConfig\`, \`fetchAgentsConfigSchema\`, \`putAgentsConfig\`, \`AgentsConfigResponse\`, \`AgentsConfigIssue\`, \`AgentsConfigPutError\`.
- \`apps/web/src/features/agents/sections.tsx\` — remove the "Pipeline (global)" editor section and \`patchPipeline\`. \`pipeline.*\` was never wired into \`/api/service-config\` (it's a boot-time builtin-sync-only field per \`service-config-schema.ts:139\`); the previous editor only ever reached the file-backed \`PUT /config/agents\` and so was non-functional after DOB-5 anyway. Watchdogs + container_image_default remain editable via the new endpoint.
- \`apps/web/src/lib/api.ts\` + \`agents.index.tsx\` + \`sections.tsx\` — drop \`GlobalPipelineConfig\` and the \`pipeline?\` field from \`AgentConfigResponse\` / \`globalsDraft\`.
- \`apps/web/src/features/agents/sections.test.tsx\` — drop the two pipeline-section tests (\`stall_threshold_ms\` set + reset).

just qa green (3179 / 3179, down from 3195 — the deleted config.test.ts + two pipeline-section tests).

Net diff: -703 LOC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Service config page gets 5 dead/legacy tabs ripped — Auth, UI, MCP, Routing, Node flows.
The four remaining tabs (Forge, Container, Watchdogs, Penpot) stay; PRs B and C will
relocate Forge URL + Penpot base_url to live with their respective credentials.

Why each tab dies:

- **Auth**: Authelia / Remote-User trust was retired by #822, migration 011 dropped
  the auth_json column. The SPA tab still surfaced "operator_user, trust_proxy,
  session_gate_enabled" but every PUT was rejected as `unknown field: auth`.

- **UI** (ui_version): only "spa" worked. The legacy dashboard.html was sunset
  (M18-9 / #170); the 410 fallback for `ui_version: legacy` was a deliberate
  trap for upgrade-time misconfig that no operator hits anymore. `/` and
  `/dashboard` now unconditionally 302 → /app/monitor.

- **MCP** (forge_mcp_command): the binary always lands at /usr/local/bin/forge-mcp
  via the Dockerfile and resolves through PATH. The override knob was a "what if
  someone ships a custom build" hatch that nobody used. Hardcoded to "forge-mcp".

- **Routing** (default_reviewer_type): placeholder for multi-reviewer-type fleets
  that never landed. Hardcoded to "reviewer" (the existing fallback in webhook-routing.ts).

- **Node flows**: the suppress_legacy field was the per-trigger cutover gate from
  the NF-5 → NF-6 dry-run migration. NF-6 Phase 7 (webhook.ts:278-282) made the
  flow runner the SOLE dispatch path — there is no longer a legacy switch alongside
  to suppress, so the field had zero runtime consumers. Mode is hardcoded "live"
  in flow-dispatch.ts; the divergence-summary background task (a dry-run-era
  observability tool) is no longer scheduled.

Changes:

**Server**

- New migration `015-drop-service-config-dead-cols.ts` drops `ui_version`,
  `forge_mcp_command`, `node_flows_json`, `default_reviewer_type` from
  service_config. Idempotent via PRAGMA table_info lookup.
- `apps/server/src/shared/config/service-config-schema.ts` — deleted (zero
  consumers since DOB-4 dropped the boot-time JSON loader).
- `apps/server/src/shared/config/flow-mode.test.ts` — deleted (whole file tested
  the now-gone node_flows config block).
- WebhookConfig: drop `forgeMcpCommand`, `defaultReviewerType`, `uiVersion`,
  and the entire `node_flows: { mode, summary_interval_ms, divergence_window_ms,
  suppress_legacy }` block. Boot-time parsing for these is gone.
- ServiceConfigRow / ServiceConfigSettingsRow / handler types: trim to the
  surviving fields (forgejo_url, container_image, container_image_default,
  watchdogs, penpot, speech, webhook_secret_ref).
- `default-mcp-registry.ts`, `mcp-config.ts`, `agent-runner.ts` — drop
  `forgeMcpCommand` option, hardcode "forge-mcp" at the spawn site.
- `flow-dispatch.ts`: hardcode `mode = "live"`, drop the "off" branch from
  DispatchOptions.mode (only "dry-run"|"live" left for tests).
- `flows-divergence.ts`: hardcode the 60s window default (was reading
  cfg.node_flows.divergence_window_ms).
- `webhook-routing.ts`: pushRoutingSnapshot uses literal "reviewer".
- main.ts: drop divergence-summary import + scheduler (cutover artifact).
- Tests: every `INSERT INTO service_config` that mentioned a dropped column
  now uses the trimmed shape. flow-dispatch.test.ts loadConfigWithMode →
  loadFleetConfig (no mode column to write).

**SPA**

- settings.service.tsx — drop AuthSection, UISection, McpSection, NodeFlowsSection,
  RoutingSection plus their TabId / TABS entries and {activeTab === "..."} gates.
  Page is now Forge / Container / Watchdogs / Penpot.
- ServiceConfigSettingsRow / Patch — drop the four dead fields + auth.

just qa green (3135 / 3135). Service restarted; /api/service-config returns the
trimmed shape; the page loads with 4 tabs.

Net diff: -1250 LOC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(service-config): rename Penpot tab → Design + co-present URL/secret (PR B of A/B/C)
All checks were successful
qa / dockerfile (pull_request) Successful in 9s
qa / qa-1 (pull_request) Successful in 3m28s
qa / qa (pull_request) Successful in 0s
64f12b0f19
Picks up where 42d677d (PR A) left off in the service-config URL-consolidation
series. PR A ripped 5 dead tabs and dropped 4 dead columns. PR B reshapes the
last loose end on the SPA — the Penpot connection — so the URL lives next to
its credential.

The principle from the spec: "every URL belongs with its credentials." Today
`service_config.penpot_json.base_url` lives in one place and the
`PENPOT_TOKEN` value lives in the encrypted `secret` table; the operator had
to bounce between two pages to set up the design agent. Now they live
side-by-side under a single "Design" tab.

Design call:
- Picked option (b) from the spec (reshape SPA only, no schema churn) over
  option (a) (new `external_connection` table). The secret table is
  `name → ciphertext` by design (#823) and reference-by-name is the
  established contract — building a one-row metadata table for Penpot would
  be premature abstraction. PR C will reuse the existing
  `operator_oauth_tokens.base_url` for the forge URL, so neither PR needs a
  new schema home for "where does this URL live".

SPA changes (apps/web/src/routes/settings.service.tsx):

- TabId `penpot` → `design`, label "Penpot" → "Design".
- New `DesignSection`:
  - "Penpot connection" card: Penpot URL input (`penpot.base_url`) +
    Default team ID input (`penpot.default_team_id`) + inline
    `PENPOT_TOKEN` editor in one card.
  - "Per-repo team mapping" card: `team_by_repo` JsonField stays separate
    (per-repo routing — distinct concern from the connection itself).
  - Save merges the URL/team_id/team_by_repo back into the `penpot` JSON
    patch and clears the override entirely when all three are empty.
- New `PenpotTokenEditor` widget: queries the existing `/secrets` endpoint,
  shows status (set / not configured + last-rotated timestamp), and
  POST/PUTs through the existing `createSecret`/`updateSecret` helpers.
  No deletion / rotation here — the full secrets manager at
  `/settings/agent-config` covers that.
- Page subtitle + file docstring updated; "auth, and more" copy gone.

just qa green (3135/3135 server tests + web typecheck). Service restarted;
SPA bundle rebuilt; the Design tab loads in place of the old Penpot tab.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(service-config): replace team_by_repo JSON textarea with structured row editor (PR B follow-up)
All checks were successful
qa / dockerfile (pull_request) Successful in 21s
qa / qa-1 (pull_request) Successful in 2m57s
qa / qa (pull_request) Successful in 0s
f79ee3bbff
PR B left the per-repo Penpot mapping as a raw JsonField — operator
feedback: that's not a normal form. Replace it with a TeamByRepoEditor
that renders one row per mapping (repo / team_name / team_id inputs +
trash button), plus an "Add mapping" button beneath.

- DesignSection draft state switches from `team_by_repo: Record<string, unknown>`
  to `team_by_repo: TeamByRepoRow[]` so add/remove/reorder don't fight the
  underlying object identity. `rowsFromMap` / `rowsToMap` shuttle between
  the wire shape and the row list; empty rows drop on save.
- Inline validation: `owner/name` regex on the repo input, UUID regex on
  the team_id input. Failing rows mark `aria-invalid` + show the inline
  error but don't block save (server parser also rejects).
- Save still merges the rebuilt `team_by_repo` map back into `penpot_json`
  alongside `base_url` + `default_team_id`; an empty map collapses the
  field out of the patch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(service-config): replace Watchdogs JSON textarea with structured form (PR B follow-up)
All checks were successful
qa / dockerfile (pull_request) Successful in 9s
qa / qa-1 (pull_request) Successful in 3m7s
qa / qa (pull_request) Successful in 0s
d99597d83a
Same operator feedback as the team_by_repo follow-up: raw JSON
textareas aren't a normal form. Watchdogs gets one input per known
knob:

- `tail_pr_rebase` → checkbox (default on).
- `container_interval_sec` → number input (seconds, default 60).
- `tail_pr_rebase_interval_ms` → number input (ms, default 60_000).
- `dead_letter_threshold` → number input (count, default 3).
- `janitor_interval_ms` → number input (ms, default 600_000).

Each field's placeholder shows the builtin default; clearing a field
elides it from the saved JSON. Default-shaped values get stripped on
save so the persisted row only carries explicit overrides — clearing
every field nulls the override entirely.

`JsonField` was the only remaining caller of the textarea helper;
removed alongside.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs: spec out service-config URL consolidation series + Settings sub-routes restructure
All checks were successful
qa / dockerfile (pull_request) Successful in 21s
qa / qa-1 (pull_request) Successful in 3m9s
qa / qa (pull_request) Successful in 0s
a8ccfc5d22
Two new specs land alongside PRs A/B and the in-flight follow-ups:

- specs/service-config-url-consolidation.md — picks up after PR A
  (commit 42d677d). Covers PR B (Penpot URL/secret co-presentation —
  shipped in this PR), PR C (relocate forgejo_url into
  operator_oauth_tokens.base_url), and a new PR D
  (Forge OAuth client/secret moved out of env into the secret table +
  operator_oauth_tokens). Captures the design call between an
  external_connection table (a) vs. reusing the existing
  operator_oauth_tokens row (b) — leans b across the board to avoid a
  two-systems-fighting bug.
- specs/settings-service-subroutes.md — operator feedback on the
  Service-config page after PR B: full-width inputs sprawl, no width
  cap, the page sits orphaned outside the /settings hub. Spec
  restructures the in-page <Tabs> as URL-routed sub-routes
  (/settings/service/{forge,container,watchdogs,design}), adds a
  ServiceConfigContext for shared state, and surfaces the page via a
  card on /settings/. Six-step implementation order with quick wins
  shippable inside this PR.

Both specs cross-link: PR D's UI assumes the sub-routes restructure has
landed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reviewer approved these changes 2026-05-04 17:36:50 +00:00
reviewer left a comment

Migration 015 idempotent and correct; dead-code removal thorough across all 5 tabs, DB columns, schema types, and tests. Design section structured UI + inline PENPOT_TOKEN editor wired correctly to SC-6 secrets API. handleRoot unconditional 302 and putAgentConfig split-write refactor both clean.

Nit (non-blocking): WatchdogsSection and DesignSection connection card pass no disabled={!dirty} to Toolbar — Save appears clickable when nothing has changed; clicking is silently a no-op. Worth a follow-up.

Migration 015 idempotent and correct; dead-code removal thorough across all 5 tabs, DB columns, schema types, and tests. Design section structured UI + inline PENPOT_TOKEN editor wired correctly to SC-6 secrets API. `handleRoot` unconditional 302 and `putAgentConfig` split-write refactor both clean. Nit (non-blocking): `WatchdogsSection` and `DesignSection` connection card pass no `disabled={!dirty}` to `Toolbar` — Save appears clickable when nothing has changed; clicking is silently a no-op. Worth a follow-up.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 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!827
No description provided.