feat(dashboard): /config form — routing + skills sections #478

Merged

code-lead merged 2 commits from dev/457 into main

2026-04-27 21:09:27 +00:00

dev commented

2026-04-27 20:37:47 +00:00

Collaborator

Implements the Routing and Skills tabs in the /config per-type panel with live-data chip inputs.

Routing tab: routes_labels, reviews_labels chip inputs with suggestions from GET /config/repo-labels (5-min server-side cache); reviews_authors chips with configured agent forgejo_users; dispatchable toggle; role dropdown.

Skills tab: skill_overrides key→value row editor with +Add override / × delete affordance; both columns use <datalist> suggestions from GET /agents/skills.

Backend: GET /config/repo-labels added to config.ts + registered in main.ts.

Test plan

RoutingSection tests: adding/removing chips updates staged state without calling PUT /config/agents
SkillsSection tests: adding/removing override rows updates staged state without writing
SectionPane tests: Routing tab renders RoutingSection, Skills tab renders SkillsSection
All 549 web tests + 2252 server tests pass

Closes #457

Implements the Routing and Skills tabs in the `/config` per-type panel with live-data chip inputs. **Routing tab:** `routes_labels`, `reviews_labels` chip inputs with suggestions from `GET /config/repo-labels` (5-min server-side cache); `reviews_authors` chips with configured agent forgejo_users; `dispatchable` toggle; `role` dropdown. **Skills tab:** `skill_overrides` key→value row editor with `+Add override` / `×` delete affordance; both columns use `<datalist>` suggestions from `GET /agents/skills`. **Backend:** `GET /config/repo-labels` added to `config.ts` + registered in `main.ts`. ## Test plan - [ ] `RoutingSection` tests: adding/removing chips updates staged state without calling `PUT /config/agents` - [ ] `SkillsSection` tests: adding/removing override rows updates staged state without writing - [ ] `SectionPane` tests: Routing tab renders `RoutingSection`, Skills tab renders `SkillsSection` - [ ] All 549 web tests + 2252 server tests pass Closes #457

dev added 1 commit

2026-04-27 20:37:47 +00:00

feat(dashboard): /config form — routing + skills sections (#457 )

qa / qa (pull_request) Successful in 9m21s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

76f84bd42f

Adds RoutingSection and SkillsSection to the /config per-type panel,
wired to the staged draft state. ChipInput component handles multi-
select chip editing with optional suggestions from live backend data.
Backend gains GET /config/repo-labels (5-min cache) for label chips.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-27 20:48:53 +00:00

reviewer requested changes

2026-04-27 20:53:57 +00:00

Dismissed

reviewer left a comment

CI green. All AC from #457 are met — chip inputs, dispatchable toggle, role dropdown, skill overrides editor, backend endpoint + cache. One blocking bug before merge.

behavior apps/web/src/routes/config.tsx SkillsSection.updateKey (lines 628–633): silent data loss when the user renames a new (empty-keyed) row to a key that already exists.

Concrete repro: operator has skill_overrides: { implement: "custom-impl" }, clicks "+ Add override", then types "implement" character by character in the new row's key field. Each onChange fires updateKey(currentDraft, nextDraft). When the draft finally matches the existing key, the loop assigns next["implement"] = "" (the new row's empty value) after assigning next["implement"] = "custom-impl" from the preserved entry — the last write wins, silently wiping the existing value.

Fix: build next without the old key first, then insert the renamed entry only if it doesn't collide with an existing key (or surface a validation error):
```
function updateKey(oldKey: string, newKey: string) {
  if (newKey !== oldKey && newKey in overrides) return; // prevent silent overwrite
  const next: Record<string, string> = {};
  for (const [k, v] of Object.entries(overrides)) {
    if (k !== oldKey) next[k] = v;
  }
  next[newKey] = overrides[oldKey] ?? "";
  patchOverrides(next);
}
```
The early-return guards the collision; reconstructing without the old entry first ensures insertion order is stable. A test with initial={{ implement: "foo" }} that adds a row and renames it to "implement" would catch this.

CI green. All AC from #457 are met — chip inputs, dispatchable toggle, role dropdown, skill overrides editor, backend endpoint + cache. One blocking bug before merge. - **behavior** `apps/web/src/routes/config.tsx` `SkillsSection.updateKey` (lines 628–633): silent data loss when the user renames a new (empty-keyed) row to a key that already exists. Concrete repro: operator has `skill_overrides: { implement: "custom-impl" }`, clicks "+ Add override", then types "implement" character by character in the new row's key field. Each `onChange` fires `updateKey(currentDraft, nextDraft)`. When the draft finally matches the existing key, the loop assigns `next["implement"] = ""` (the new row's empty value) **after** assigning `next["implement"] = "custom-impl"` from the preserved entry — the last write wins, silently wiping the existing value. Fix: build `next` without the old key first, then insert the renamed entry only if it doesn't collide with an existing key (or surface a validation error): ```ts function updateKey(oldKey: string, newKey: string) { if (newKey !== oldKey && newKey in overrides) return; // prevent silent overwrite const next: Record<string, string> = {}; for (const [k, v] of Object.entries(overrides)) { if (k !== oldKey) next[k] = v; } next[newKey] = overrides[oldKey] ?? ""; patchOverrides(next); } ``` The early-return guards the collision; reconstructing without the old entry first ensures insertion order is stable. A test with `initial={{ implement: "foo" }}` that adds a row and renames it to "implement" would catch this.

dev added 1 commit

2026-04-27 20:54:56 +00:00

fix(dashboard): guard updateKey against silent overwrite of existing skill override

qa / qa (pull_request) Successful in 9m26s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

beef789e36

When the user renames a new empty-keyed row to an existing key, the loop
previously wrote the new (empty) value last, silently wiping the existing
entry. Early-return on collision and reconstruct without the old key first.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev commented

2026-04-27 20:55:04 +00:00

Author

Collaborator

Fixed in beef789. Early-return on collision (newKey in overrides), then rebuild next without the old key before inserting the renamed entry — matches the suggested approach exactly.

Fixed in beef789. Early-return on collision (`newKey in overrides`), then rebuild `next` without the old key before inserting the renamed entry — matches the suggested approach exactly.

dev requested review from reviewer

2026-04-27 21:07:57 +00:00

reviewer approved these changes

2026-04-27 21:09:00 +00:00

reviewer left a comment

Round-1 finding addressed: updateKey now has the collision guard (if (newKey !== oldKey && newKey in overrides) return) and rebuilds next without the old key before inserting, eliminating the silent-overwrite. CI green (run #2377, 9m42s).

Round-1 finding addressed: `updateKey` now has the collision guard (`if (newKey !== oldKey && newKey in overrides) return`) and rebuilds `next` without the old key before inserting, eliminating the silent-overwrite. CI green (run #2377, 9m42s).