3523dcf93e
plans/ held 58 documents and exactly one said whether it was open. The rest
mixed finished work, reviews of shipped work, parked specs and genuinely
pending ones, with nothing distinguishing them, so "how many plans are in
the queue" had no answer short of reading all 58.
Now `grep -H '^Status:' plans/*.md` is the answer:
50 done 3 in progress 2 planned 2 reference 1 parked
Statuses were derived rather than guessed: CLAUDE.md's own built list and
"What's NOT built yet" section, plus checking the subject exists in the
code. A review of work that shipped counts as done -- it records what was
found, it is not a request for anything. `reference` separates the two docs
that are conventions rather than work items (admin-design-standards,
admin-work-framework), which otherwise read as permanently-open plans.
The vocabulary is deliberately five words. A larger one invites "mostly
done" and "blocked-ish", which is how the directory became unreadable.
test_plans_declare_status.py keeps it from rotting: a new plan without a
marker fails, as does an unknown status, one buried below the eighth line,
or an open status with no reason -- "planned" alone is the state that rots,
since nobody can tell later whether it waits on a decision, a dependency,
or just nobody's turn.
Also updates the sweep plan with what landed and what did not, including
that #9 was not a defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U
8.6 KiB
Admin profile CRUD should write to the overlay
Status: done -- profile CRUD writes config.local.yaml
Status: FINAL — decision-complete. Written 2026-09-04 against
feat/config-local-overlay at 0570123 (PR #26). Depends on that PR landing.
Why this exists
plans/config-local-overlay.md states the rule without qualification:
The admin portal writes to
config/config.local.yaml. It never writesconfig/config.yaml.
The implementation splits it. Allowlisted scalars go to the overlay; profile
CRUD writes the base file directly, documented at src/admin.py:670.
This is not a considered exception. It is chronological accident: profile CRUD
shipped in PR #25 (340453e), and the overlay decision was made afterwards
(e5ee92b). Plan 8 then implemented the new rule for the scalar path it
touched and left the profile path where it was.
The reasoning is the same one the user already endorsed
Plan 8's justification for the scalar path:
An operator changing a knob in a loopback-only admin portal is making a local operational decision, not a project decision. Someone changing a project default edits
config/config.yamlin the repo and commits it, deliberately, through git.
Creating onlycheaps in the portal is that same act. Nothing about the
argument depends on the value being a scalar rather than a nested object — the
only reason the code diverges is the ordering above.
What it costs to leave it
The tariff no longer lives in config/config.yaml, so the blast radius is far
smaller than the six clobberings Plan 8 was written for. The residual harm is
real but narrower:
- Using the profiles UI re-dirties a tracked file.
git pull --rebaserefuses on a dirty tree — observed, and listed in Plan 8's problem statement. - A
git switchorgit checkoutsilently discards a profile the operator just created, with nothing indicating why. commit -amsweeps profiles into unrelated commits.
The sharpest version: a partial guarantee is worse than none. An operator
who has internalized "the portal writes to the overlay, config.yaml stays
clean" stops checking. The failure then arrives against an expectation this
project's own fix created.
Two defects that exist TODAY, independent of the rule
Both were found by reading the code on 2026-09-04, and the first was confirmed live. They are the reason this is a bug fix and not only a consistency tidy.
1. An overlay-defined profile is live in routing and invisible to the portal
_persisted_profiles() (src/admin.py:~789) reads base only:
store = load_config_store_safe(config_path) or {}
return store.get("profiles") or {}
But load_config deep-merges the overlay, so a profiles: block in
config/config.local.yaml is live in cfg.profiles. Confirmed by writing
a temporary ghosttest profile into the overlay:
| reader | sees |
|---|---|
load_config (what routing uses) |
['ghosttest'] |
_persisted_profiles() (what the portal lists) |
[] |
So the portal's profile list can already disagree with what the router will
actually accept as auto:<name>. That is the same class of failure as the
vision-ceiling incident — a real state, computed correctly somewhere, never
surfaced.
Note this is reachable today by hand-editing the overlay, before any of this plan lands. It is not created by the redirect; the redirect is what fixes it, because the portal will finally be reading the file it writes.
2. Delete reads merged config but writes the base file
admin_profile_delete deliberately reads the merged store so the
default_profile guard is correct regardless of which file set it:
merged = _load_merged_config_store(config_path, config_local_path)
persisted_profiles = merged.get("profiles") or {}
then deletes from base:
_persist_config_block(config_path, ("profiles", name), None, delete=True)
For an overlay-defined profile that is found-then-not-deletable: the existence
check passes on merged, the write raises KeyError, and the operator gets a
404 for a profile the portal just confirmed exists. The endpoint is already
half-overlay-aware, which is a good sign the split was never intentional.
The rule
| operation | destination |
|---|---|
| create | config/config.local.yaml |
| update | config/config.local.yaml |
| delete | overlay-defined profiles only |
| base-defined profile | read-only, alongside built-ins |
The deletion wrinkle, and why it resolves rather than blocks
Deep merge can add and override. It cannot express remove. A profile defined
in base config/config.yaml cannot be deleted from the overlay without a
tombstone, and tombstones are exactly the config-framework machinery Plan 8's
non-goals rule out ("resist the config-framework instinct").
Resolve it by classification, not mechanism. A profile someone committed to
config/config.yaml is a project artifact — the same category as a built-in.
Render it read-only, refuse edit and delete, and say why in a message that
points at the repo.
This is cheap because the refusal path already exists at three sites, each
guarding on name in BUILTIN_PROFILES:
src/admin.py:902— createsrc/admin.py:936— updatesrc/admin.py:960— delete
The change is a widened predicate and a distinct message, not new machinery. Keep the two refusals distinguishable: a built-in cannot be changed at all, while a base-defined profile can be changed by editing the repo. Collapsing both into "read-only" would tell the operator less than the code knows.
It also gives the provenance display Plan 8 already built a real job: base vs overlay is precisely what explains why a given profile is not editable.
Interactions to get right
routing.default_profilemay name a profile from either file. The existing delete guard already reads merged config for this; keep it. Deleting the current default stays refused (422), and that check must run against the merged view, not the overlay alone.- A name collision between base and overlay must resolve to the overlay (standard merge precedence) and be visible as such in the listing, not silently deduplicated. Two definitions for one name is the ambiguity Plan 7 refused to accept for built-ins; the same reasoning applies here.
- Built-in name collisions stay rejected at config load, unchanged.
_profile_probe/_probe_candidate_zero_admitdo not change. Admission is still computed viarouting.select_candidates; this plan moves a write destination and a visibility source, not any routing logic.- The zero-admit warning still fires on save, unchanged.
Non-goals
- No tombstone or delete-marker syntax in the overlay. If a base-defined profile must go, it goes through git.
- Do not mirror profile writes into both files. Plan 8 settled that: one value, one home.
- Do not migrate existing base-defined profiles into the overlay automatically. Same reasoning as Plan 8's Migration section — this plan does not decide where someone else's committed config lives.
- No changes to
RoutingProfile's fields, to built-in definitions, or to howauto:<name>resolves. - Do not widen
_CONFIG_ALLOWLIST.
Success criteria
- Creating a profile through the portal writes
config/config.local.yamland leavesconfig/config.yamlbyte-identical — asserted directly, since the existingtest_profile_create_preserves_comments_and_does_not_create_overlayasserts the opposite and must be inverted rather than deleted. - Updating an overlay-defined profile writes the overlay only.
- Deleting an overlay-defined profile removes it from the overlay only.
- A base-defined profile returns 403 on update and delete, with a message
distinct from the built-in message and naming
config/config.yaml. - The portal lists overlay-defined profiles. A test writes a profile into
the overlay and asserts
GET /admin/api/profiles/returns it — this is the regression test for defect #1 and must fail on currentmain. - A base/overlay name collision resolves to the overlay and is labelled with its source in the listing.
- Deleting the profile named by
routing.default_profileis still refused when that setting comes from either file. - The overlay is created on first profile write when absent, with the same header comment the scalar path writes.
- Backup-before-write and validation-of-merged-config still hold on the profile path.
- Full suite green with
local_energy.enabledboth true and false. - The user's
config/config.local.yamlvalues are untouched: tariff0.159andenabled: trueverbatim, verified after the test run, not before.