fix(engine): let a user remove the bad layer quarantine only showed them

CLAUDE.md

@@ -131,6 +131,8 @@ Key files:
 - Capture tools (`log_capture`/`confirm_capture`) exist only behind `mcp-server.mjs --capture`. The default server exposes 6 read-only tools (the original `search`/`read_file`/`list_concepts`/`get_links` stay byte-identical to the committed `fixtures/mcp-tools-baseline.json`, plus always-on read-only `find_captures`/`whats_new`); `--capture` adds the two write tools for 8. Telemetry (`--telemetry`) records concept ids and enums only — never content. Top-level tool `description` strings are NOT frozen by the baseline (only `name`/`inputSchema`/`annotations` are) — `search`/`read_file`/`get_links` descriptions teach conflict candor (sections may carry `conflicts[]`; weigh `fresherDissent`; `search` hits may carry `contested`/`conflictSections`).
 - **The Mac app adds a `github-rest` source kind** (`POST /api/sources`) alongside the existing `github` clone kind — public repos read over the REST adapter with no clone (`repo`/`ref`/`paths`, default `cache:{ttlSeconds:900}`); private repos still use the clone kind ("uses your existing git credentials or SSH"). The app deliberately does not accept `auth`/`apiBase` from its own UI for `github-rest` in this release — the keychain `auth` alias resolution is real in the engine (`buildSources(manifest, dir, {tokens})`) but no caller in the app injects a `tokens` map yet, so an alias would silently read anonymously; headless users can still hand-write `{"auth":{"tokenEnv":"NAME"}}`.
 - **Manifest reads are profile-view-unified.** `service.mjs` builds one `{...manifest, layers: getManifestProfileLayers(manifest)}` view in `openSources()` and threads it through every read site (index keys, `buildSources`, the manifest watcher, `layerMeta`, sync lookups) plus `layer-files.mjs`'s `layerRootMap` — a manifest migrated to v2 (e.g. by `contextcake profile create`) no longer empties the app's source list.
+- **A bad layer is read around, never written around.** `readContextManifestQuarantined` (the read path, via `readManifestForRead()` in `service.mjs`) lifts a layer that fails validation out of the manifest it returns, so one hand-edited layer no longer 500s every route — it becomes an error row with `quarantined: true` in `/api/graph`. Writes stay strict: `mutateContextManifest` reads through the strict reader, and every write goes through `writeContextManifest`, which validates the whole manifest. The single exception is `repairContextManifest`, used only by `removeSourceApi` — it tolerates an invalid layer on the way IN (same rule as the read path), hands the callback the RAW manifest so a removal drops exactly the entry asked for, refuses any callback that lengthens a layers array, and still validates in full before writing. Do not make `mutateContextManifest` tolerant to make some other route work; the repair door is the one that removes, which is why it is allowed to see the mess.
+- **Removing invalid entries is all-or-nothing.** Because only a valid manifest may be persisted, any removal that leaves an invalid layer behind is refused — including one that only meant to drop a healthy source, since the write rewrites the whole file. `DELETE /api/sources?name=` therefore repeats (`searchParams.getAll`), and the console's Remove on ANY row names every invalid row before it sends them together. A removal that would leave invalid layers behind answers 409 listing what blocked it, never a 500.
 - All git mutations against a live root go through `git-core.mjs` (advisory `.contextcake.lock`, per-repo serialization) — never call git directly against a live layer from engine code.
 - The engine (`packages/core/src/`) is dependency-free — plain Node.js built-ins only. Do not add npm dependencies without discussion. The exceptions are `apps/console/`, `apps/site/`, and `apps/desktop/` — self-contained npm packages. Console and site never import from the engine; the desktop app imports engine modules by path (one-way: app → engine, never the reverse) and must never cause a dependency to leak into `packages/core`.
 - `apps/console/` and `apps/site/` each have their own `package.json`, build, and tests; run their commands from that subdirectory, not the repo root. Web Demo previews are path-filtered to `apps/console/**`; production deploys with the matching Mac app from the single `app-v*` release workflow.

apps/console/src/api.ts

@@ -528,6 +528,7 @@ export function adaptSources(g: GraphSummary): Source[] {
       conceptCount: s.conceptCount,
       origin: s.origin ?? null,
       error: s.error ?? null,
+      ...(s.quarantined === true ? { quarantined: true } : {}),
       // The true count, which the capped message list is not: a source with 40
       // unreadable files sends 40 here and 10 messages.
       warnings: s.warnings ?? (s.warningMessages?.length ?? 0),

apps/console/src/data.ts

@@ -32,6 +32,12 @@ export interface Source {
   /** Git remote a clone-backed layer came from; enables Sync alongside kind 'github'. */
   origin?: string | null
   error?: string | null
+  /**
+   * Not a source at all: a manifest entry the engine could not validate, so
+   * nothing was built for it. Rename and Sync have nothing to act on; removing
+   * the entry is the repair.
+   */
+  quarantined?: boolean
   /** Progress while the background index is reading this source. */
   indexing?: SourceProgress
   /** Content this source indexed around: too big to read, or not readable. */

apps/console/src/types.ts

@@ -78,6 +78,8 @@ export interface SourceStatus {
   /** Ready and re-reading behind a good snapshot. Distinct from `status: indexing`. */
   refreshing: boolean
   error: string | null
+  /** An invalid manifest entry rather than a source — same meaning as GraphSource's. */
+  quarantined?: boolean
 }
 
 /**
@@ -115,6 +117,12 @@ export interface GraphSource {
    */
   status: string
   error: string | null
+  /**
+   * This row is a manifest entry that failed validation, not a source that
+   * failed to read: nothing was built for it, so there is nothing to retry,
+   * rename or sync. Removing the entry is the only action that helps.
+   */
+  quarantined?: boolean
   /**
    * Things this source could not read even though it indexed successfully — a
    * document over the per-file size cap, a subfolder it lacks permission to

apps/console/src/views/Sources.test.tsx

@@ -163,6 +163,95 @@ describe('Sources remove', () => {
   })
 })
 
+describe('Sources with an invalid manifest entry', () => {
+  const broken = (over: Partial<Source> = {}) => src({
+    name: 'bad-kind', status: 'error', quarantined: true, conceptCount: 0, coverage: 0,
+    error: 'Layer bad-kind has unsupported source kind: notarealkind', ...over,
+  })
+
+  it('offers only Remove, and says the entry is not a working source', async () => {
+    await mount([broken()])
+
+    expect(container.textContent).toContain('This entry is not a working source')
+    // Rename writes through the strict manifest path and Sync has nothing to
+    // talk to, so offering either would only produce an error.
+    expect(container.querySelector('button[aria-label^="Rename"]')).toBeNull()
+    expect(container.querySelector('button[aria-label^="Sync"]')).toBeNull()
+    expect(container.textContent).not.toContain('The path, repository, or command is fixed')
+    expect(container.textContent).toContain('unsupported source kind: notarealkind')
+    buttonByAria('Remove bad-kind') // throws if absent
+  })
+
+  it('removes a lone invalid entry on its own', async () => {
+    await mount([src({}), broken()])
+
+    await act(async () => sourceButton('bad-kind').click())
+    await act(async () => buttonByAria('Remove bad-kind').click())
+    expect(container.textContent).toContain('nothing was being read from this entry')
+    await act(async () => button('Remove entry').click())
+
+    expect(mocks.apiFetch).toHaveBeenCalledWith('/api/sources?name=bad-kind', expect.objectContaining({ method: 'DELETE' }))
+  })
+
+  it('names every other invalid entry and removes them in one request', async () => {
+    // Two invalid entries: the engine only persists a manifest that validates,
+    // so removing either alone would be refused. The panel has to say that
+    // before the click rather than removing rows the user never selected.
+    await mount([src({}), broken(), broken({ name: 'layer 4', error: 'Layer in legacy default must have a non-empty name.' })])
+
+    await act(async () => sourceButton('bad-kind').click())
+    await act(async () => buttonByAria('Remove bad-kind').click())
+    expect(container.textContent).toContain('One other entry is also invalid')
+    expect(container.textContent).toContain('layer 4')
+
+    await act(async () => button('Remove 2 entries').click())
+    expect(mocks.apiFetch).toHaveBeenCalledWith('/api/sources?name=bad-kind&name=layer%204', expect.objectContaining({ method: 'DELETE' }))
+    expect(mocks.reload).toHaveBeenCalled()
+  })
+
+  it('carries the invalid entries along when a healthy source is removed', async () => {
+    // The write rewrites the whole manifest, so an invalid entry blocks
+    // removing a working source too. Refusing with an explanation the user
+    // cannot act on would leave them stuck on a row that has nothing wrong.
+    await mount([src({ name: 'notes' }), broken()])
+
+    await act(async () => sourceButton('notes').click())
+    await act(async () => buttonByAria('Remove notes').click())
+    expect(container.textContent).toContain('One other entry is also invalid')
+    expect(container.textContent).toContain('only the cascade entry is removed')
+
+    await act(async () => button('Remove 2 entries').click())
+    expect(mocks.apiFetch).toHaveBeenCalledWith('/api/sources?name=notes&name=bad-kind', expect.objectContaining({ method: 'DELETE' }))
+  })
+
+  it('leaves an ordinary removal alone when nothing is invalid', async () => {
+    await mount([src({ name: 'notes' })])
+
+    await act(async () => buttonByAria('Remove notes').click())
+    await act(async () => button('Remove source').click())
+    expect(mocks.apiFetch).toHaveBeenCalledWith('/api/sources?name=notes', expect.objectContaining({ method: 'DELETE' }))
+  })
+
+  it('renders the engine refusal verbatim when the manifest cannot be repaired', async () => {
+    mocks.apiFetch.mockImplementation(async (_url: string, init?: RequestInit) => {
+      if (init?.method === 'DELETE') {
+        return new Response(
+          JSON.stringify({ error: 'Nothing was removed: the manifest is invalid in a way this app cannot repair. Edit /kb/manifest.json by hand — legacy default contains duplicate layer name: seed' }),
+          { status: 409, headers: { 'content-type': 'application/json' } },
+        )
+      }
+      return ok()
+    })
+    await mount([broken()])
+
+    await act(async () => buttonByAria('Remove bad-kind').click())
+    await act(async () => button('Remove entry').click())
+
+    expect(container.textContent).toContain('Edit /kb/manifest.json by hand')
+    expect(mocks.reload).not.toHaveBeenCalled()
+  })
+})
+
 describe('Sources rename + re-level', () => {
   it('PATCHes only name and level — and says a wrong path means remove + re-add', async () => {
     await mount([src({ name: 'notes', level: 3 })])