fix(engine): let a user remove the bad layer quarantine only showed them - #101
Merged
siracusa5 merged 2 commits intoAug 7, 2026
Merged
Conversation
Quarantine made a hand-edited layer visible without making it fixable. Every mutation route reads through mutateContextManifest, which reads STRICTLY, so with an invalid layer present POST/PATCH/DELETE /api/sources and PATCH /api/settings all threw before doing anything — including the one removal that would have made the manifest valid again. The app could show you the problem and nothing else. repairContextManifest is the door that opens: the single mutation allowed to READ a manifest holding an invalid layer, because it is the one that takes one out. Nothing about what may be PERSISTED moves. The write is still writeContextManifest, so the result must pass validateContextManifest in full; a repair that does not leave the manifest valid is refused and the file on disk is untouched. Both tolerant readers now share one private reader, so the door can never accept a manifest the read path would reject — a duplicate layer name, a second live layer, a broken profiles block all stay fatal. And a repair may only remove: no layers array may come out of the callback longer than it went in. The callback gets the RAW manifest, not the quarantined one. Handing it the cleaned manifest would delete every OTHER broken layer from the user's file as a side effect of removing the one they asked about. For the same reason quarantine records now carry `index` and removeSourceApi removes by index rather than by name: a broken layer that also reuses a healthy layer's name is shown as "seed (2)", and a name filter would have taken the healthy `seed` with it. ?name= repeats, and that is not a convenience. Only a valid manifest may be written, so with two invalid entries, removing either alone is refused — the other still fails validation. Removing them in one transaction is the only shape that both repairs the file and keeps the write strict; measured against three bad layers, the single-name form is a dead end from which the app can never recover. A request that asks for too little answers 409 naming what blocked it, never a 500. Console: rows carry `quarantined`, distinct from a source that failed to READ (createErrorSource now takes the flag explicitly — the two were indistinguishable from status alone, and only one of them can be acted on). On an invalid row Rename and Sync are gone, since both could only fail; the panel says the entry is not a working source, and Remove names every other invalid entry before the click rather than quietly sweeping rows nobody selected. PATCH /api/settings stays a strict write — quietly rewriting a manifest read around a bad layer is how a hand-edited layer gets dropped without being asked about — but it answered 500 with a layer validation error on the Settings screen, which tells the user nothing about where to go. It now answers 409 pointing at Sources, where the repair lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Siracusa <siracusa5@users.noreply.github.com>
Review of the parent commit found the repair half-built. The write rewrites the whole manifest and only a valid one is ever saved, so an invalid entry refuses the removal of a perfectly healthy source exactly as it refuses the removal of another invalid one. The engine already answered that 409 with the remedy — name them in the same request — but the console only offered the sweep on an invalid row, so a user with one bad layer could not remove anything at all and was left staring at a message about a row that has nothing wrong with it. Every removal now carries the invalid entries with it, named in the confirm panel before the click. Same informed-consent shape as before, one row wider. The service test gains the mixed case directly: a healthy source refused on its own, then coming out together with the invalid entry beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Siracusa <siracusa5@users.noreply.github.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
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
Stacked on #100, which added quarantine. Quarantine made a hand-edited layer
visible without making it fixable: every mutation route reads through
mutateContextManifest, which reads strictly, so with an invalid layer presentPOST/PATCH/DELETE /api/sourcesandPATCH /api/settingsall threw before doinganything — including the one removal that would have made the manifest valid
again. The app could show you the problem and nothing else.
repairContextManifestis the door that opens: the single mutation allowed toREAD a manifest holding an invalid layer, because it is the one that takes one
out. Nothing about what may be persisted moves. The write is still
writeContextManifest, so the result must passvalidateContextManifestinfull; a repair that does not leave the manifest valid is refused and the file on
disk is untouched.
Three things keep the door narrow:
never accept a manifest the read path would reject — a duplicate layer name, a
second
livelayer, a broken profiles block all stay fatal.went in.
cleaned manifest would delete every other broken layer from the user's file
as a side effect of removing the one they asked about.
Quarantine records now carry
index, andremoveSourceApiremoves by indexrather than by name. That matters: a broken layer that also reuses a healthy
layer's name shows as
seed (2), and a name filter would have taken the healthyseedwith it.?name=now repeats, and that is not a convenienceOnly a valid manifest may be written, so with two invalid entries, removing
either alone is refused — the other still fails validation. Measured against
three bad layers, the single-name form is a dead end the app can never recover
from. Removing them in one transaction is the only shape that both repairs the
file and keeps the write strict. A request that asks for too little answers 409
naming what blocked it, never a 500.
Console
Rows carry
quarantined, distinct from a source that failed to read(
createErrorSourcenow takes the flag explicitly — the two wereindistinguishable from
statusalone, and only one of them can be acted on). Onan invalid row, Rename and Sync are gone since both could only fail, and the
panel says the entry is not a working source.
The second commit closes a gap review found in the first: because the write
rewrites the whole file, an invalid entry refuses the removal of a healthy
source just as surely as another invalid one, and the console only offered the
sweep on invalid rows — so a user with one bad layer could remove nothing at
all. Every removal now carries the invalid entries with it, named in the confirm
panel before the click.
PATCH /api/settingsStays a strict write — quietly rewriting a manifest read around a bad layer is
how a hand-edited layer gets dropped without being asked about. But it answered
500 with a layer validation error on the Settings screen, which tells the user
nothing about where to go. It now answers 409 pointing at Sources, where the
repair lives.
Deliberately not in scope:
POST /api/sources(add) andPATCH /api/sources(rename) still surface the raw validation error as a 500 while an invalid layer
exists. That is unchanged pre-existing behavior, not a regression here; the same
409 treatment would suit them and is a clean follow-up.
Affected area
Validation
npm test(exit 0, zero FAILs, on the rebased base)npm --prefix apps/console run typechecknpm --prefix apps/console test(234 passed)npm --prefix apps/console run buildnpm --prefix apps/site run buildNew coverage:
manifest.test.mjspins the repair semantics — raw manifest in, a refusedwrite leaves the file byte-identical, the add-guard, index-based removal of a
name-shadowing layer, and v2 profiles.
service-test.shadds a host with three invalid layers: one-of-three refusedwith a message naming the blockers, settings blocked then saving, all-three
succeeding, the healthy same-named layer surviving, and a healthy source
refused alone then removed together with an invalid entry.
Sources.test.tsxcovers the invalid-row panel, the single and multi-entryremoves, a healthy source carrying an invalid entry along, an ordinary
removal when nothing is invalid, and the verbatim engine refusal.
index-stability-test.shis untouched; assertion 3 stays green.Compatibility
DELETE /api/sources?name=xwith a single name behaves exactly as before; theplayground's own client still sends one name.
removedis still the first name;removedNamesis additive.Sign-off
git commit -s) per theDeveloper Certificate of Origin.