GitHub created no workflow runs and no check runs for eaaec8173, leaving the
two required contexts waiting for a status that was never going to arrive.
Empty commit, identical tree — this only makes GitHub emit the pull_request
event again.
These goldens were recorded on a machine with personal skills under ~/.dsh and
~/.agents, so every one of them pinned a context-injection row a clean runner
cannot produce — which is why the browser lane failed on CI and passed
locally. master's scaffold fix confines skill-local's host-level roots to the
temp world; re-recording against it removes the row and nothing else.
seeded-history keeps its row: that scenario seeds the event itself.
Two lifecycle holes the registry and the consumer helper left open.
`AdapterRegistrationHandle.replace` had no liveness guard: after the handle's
disposer ran, a replace put routes back into the registry with nothing left to
release them, so the adapter leaked permanently. `owned` being empty cannot
carry that fact, because `replace([])` is the legal empty-section state, so the
disposer records it explicitly.
`installSettingsSection`'s watcher lacked the guard its own disposer carries:
a stored change landing while the consumer unloads reached `onChange`, which
re-registers routes against a fiber whose resources are being released.
Also documents `withFileLock` in the atomic-write README (it claimed one
export), records the age-based lock takeover as a known limitation, and lists
ctx.settings and ctx.credentials in the architecture capability table.
boot() asserts over ctx.loader after awaiting the Loader, but the TUI renders
as soon as its own fiber starts: an /exit typed before the last entry settles
runs disposeRootAndExit, which takes the Loader service with the tree. The
assertions then read undefined and crashed the process with a TypeError over
an app that exited exactly as asked. The keyless personal-overlay PTY smoke
lost this race in roughly two of three runs.
Merging master swapped the plain "▸ 上下文注入" button for the shared
DisclosureRow, which every conversation golden carrying an injected-context
row renders. The only change in all twelve files is that row.
The rename to deepseek-official reached cordis.snapshot.yml but not its
persistent-tools sibling, so the replay catalog no longer owned the route the
agent asks for. The SDK server then mounted the real adapter, which failed the
turn on a missing key. Re-records the six transcripts and the two diagnostics
cards that still carried the old route name.
The mutate seam's root-path ops, its non-array rejection, and the recursion
into an existing nested object were unexercised, as was the whole containment
side of the settings/document-updated fan-out and the before-snapshot repair
of a section a hand edit left non-object. The settings.mutate route had no
round trip at all: neither the client method nor the handler entry ran.
The per-file coverage gate caught three uncovered paths in the error handling
this round added: the page banner for a failed row removal, the editor card's
transport-rejection catch, and `store.fail` itself.
Two of them are one click each — Remove with a rejecting write, Apply with a
rejecting write — so they are covered through the UI rather than by calling
the helpers directly. The third was a duplicated `error instanceof Error ?
error.message : String(error)` in two files; it becomes one exported
`messageOf`, which removes the branch from both call sites and gives the
fallback arm a home a direct unit test can reach (the lint rule forbids
rejecting a promise with a non-Error, so a rejection cannot exercise it).
The master merge moved the TUI composition into `apps/cli/config/base.cordis.yml`
and I carried the `settings-local` / `credentials-local` rows across without
adding them to the resolver manifest. Bare specifiers in an app config resolve
through that manifest's dependencies, so the whole tree failed to boot:
dsh: plugin(s) failed to load: @deepseek-ai/dsh-settings-local,
@deepseek-ai/dsh-credentials-local
which took every TUI PTY smoke with it. `verify-cordis-config` did not catch
it, so the boot smoke was the first signal.
The review named this call site with the other two, and the previous pass
missed it: the editor card's mount-time `credentials.describe` had only a
fulfillment handler, so a transport failure reached the browser as an
unhandled rejection.
The probe is a placeholder hint ("already configured"), never a precondition
for editing, so it now renders without the hint rather than failing. Covered
by a test that fails without the handler.
The remaining P1 from the #939 review, plus the P2 it shares a mechanism with.
Nothing carried a version, so two tabs editing one namespace silently
overwrote each other — reproduced as tab B's `reasoning` lost to tab A's
older draft. The seam's per-namespace write queue orders writes; it cannot
tell a fresh writer from one replaying a snapshot a predecessor superseded.
Each namespace now carries a monotonic `revision` over its RAW section. A
write may send `expectedRevision`, checked at the FRONT of the queue (not at
call time, which would race the very predecessor it guards against); a
mismatch rejects with `SettingsConflictError` → `settings-conflict` on the
wire, carrying both revisions. The editor captures the revision it opened at
and, on conflict, asks the user to reopen rather than replaying its snapshot.
The same counter fixes the missing broadcast. `settings/updated` is gated on
the resolved value — correct for consumers, wrong for configuration surfaces:
storing an override equal to the composition base leaves the resolved value
alone while changing what the document says (the field is now overridden, not
inherited) and moving every open editor's revision. `settings/document-updated
(ns, revision)` fires on any raw-section change, in-process or external, and
`host/settings-changed` now rides it.
That event also closes the stale model picker: editing a provider's `models`
changes no route, so `llm/adapters-updated` never fired and an open picker
kept serving the old catalog. A change to an exposed provider namespace now
emits `host/models-changed` too — that namespace holds the catalog.
Docs: both sides of the five touched README pairs, a type-equiv block for
`SettingsPathOp`, and an Agent Note recording what the plane exposes and who
may overwrite what. The deferred wire-redaction gaps (secrets behind
union/intersection/transform, `.default(...)` in the served envelope, schema
text in rejection messages, `new Function` rehydration, pi-ai's `headers`) are
recorded as TODO(settings-wire-redaction) and in Known Limitations rather than
half-fixed.
Five findings from the #939 review, each reproduced before being fixed.
**Configuration reads are as privileged as writes.** `settings.describe`
returns every exposed namespace's configuration and `credentials.describe`
reports whether an arbitrary environment-variable name is configured and from
where — reconnaissance no anonymous caller should have. Both join
PRIVILEGED_METHODS, so the whole configuration plane is loopback-only until
real authentication exists; `trustedHosts` was never authentication. The model
catalog stays reachable: it carries no endpoints or key state, and a LAN
client's model picker legitimately needs it. Asserted over a real HTTP server,
because the Host header a browser actually sends is what decides this.
**The proxy serves only namespaces a registered model provider addresses.**
The settings seam is general — any plugin may register one — but the Web
configuration plane is the model-provider surface. Without the gate, every
future `settings.register()` would silently become remotely readable and
writable configuration. An unregistered namespace and an unexposed one answer
identically, so no caller can enumerate the registry one probe at a time.
**Path-addressed writes replace the redacted-document rebuild.** The editor
reads the REDACTED descriptor, so rebuilding a section from it and replacing
wholesale deleted every literal secret the wire never returned — reproduced as
`{baseURL, reasoning}` in, stored `apiKey` gone out. `settings.mutate` applies
set/unset ops to the section as it stands at the front of the seam's write
queue, and the client names only fields it can see, so an unseen secret is
untouched by construction rather than by care.
P2s in the same pass: `llm/adapters-updated` now contains async listener
rejections (an uncontained one escaped as unhandledRejection, contradicting
the documented "observer failures are contained"); llm-deepseek's retry-policy
swap uses the atomic `registration.replace` instead of dispose-then-register,
which published `[]` then `["deepseek-official"]` so an observer saw the
provider disappear and come back; and a transport rejection no longer strands
the page in `loading` or a card in `busy`, with removal failures surfaced on
the page banner instead of swallowed.
The `readDenyPaths` policy field shipped in the previous commit broke Linux
confinement outright. bwrap has to create the `/dev/null` bind's mount point
inside a tree its own profile has already made read-only, so it refused the
entire confinement whenever the parent directory was absent — every host that
has not stored a credential yet, including a fresh install:
bwrap: Can't mkdir parents for /home/runner/.dsh/.env: Read-only file system
which the executor correctly classifies as SANDBOX_UNAVAILABLE, so every
confined bash call failed closed. Landlock cannot subtract from its own `/`
read grant, so it reported `partial` enforcement on every confined call for a
file it never hid, with no way to switch the denial off (schemastery fills an
omitted array with `[]`, so empty and omitted were indistinguishable).
A protection that breaks confinement where it works and misreports it where it
does not is worse than a documented absence. Revert the field, both expressible
backends, the enforcement downgrade, and the policy default; state the residue
plainly in the credentials-local READMEs — file mode stops other OS users, not
the model — and keep the OS-keychain provider recorded as the real answer.
The narrower discipline stands: no surface hoists the credential document into
`process.env`, and the model is never handed a resolved path to it.
The personal-config smoke asserted that `$DSH_HOME/.env` feeds a `!!js`
expression in the personal `config.yaml` — the hoist this branch removed so
`credentials-local` can own that document and keep stored keys rotatable.
Seed both layers instead and let one expression separate them: the welcome
prefers the personal variable, so it can only render the invoking directory's
value while the harness home's `.env` stays out of `process.env`. The negative
that made the removal worth doing is now asserted in the assembled
application, not just in the provider's unit tests.
The effort field's existence check was unreachable — EFFORT_FIELD is
total over the two known families — and a coverage exemption was papering
over the branch, which the merged toolchain no longer honored. Taking the
narrowed family as a parameter makes the lookup total at the type level,
so the check and its exemption both disappear. The rendered output is
unchanged: the browser goldens replay byte-identical.
Both provider READMEs state what actually holds: credentials-local now
documents the physical-line editor, the read-modify-write under the
writer lock, and a Security boundary section saying plainly that the file
mode stops other OS users and not the model. sandbox-policy documents
readDenyPaths and its per-backend enforcement. The llm READMEs carry the
registration handle, pi-ai's credential-miss semantics, and DeepSeek's
same-generation snapshot; app-boot and the CLI README stop describing
$DSH_HOME/.env as an environment layer.
A new Agent Note records the round (and the prior seam note cross-links
it); the sandbox and core catalog pages gain readDenyPaths and
AdapterRegistrationHandle with their manifest entries. The headless
missing-credential snapshot re-records for the reworded guidance, pi-ai
gains the Loader-composition guard its twin already had, and the
deliberate provider symmetry is marked for the clone detector.
The policy home's resolve() now stamps readDenyPaths, so every consumer
that pins the resolved shape (bash-sandbox hand-off, tool-fs stamps)
carries it, and three uncovered branches gained real tests: landlock
reporting partial enforcement for a denial it cannot express, the
policy's default under programmatic construction, and both ambient
credential paths in llm-deepseek without a mounted seam.
The credential store is 0600 under a 0700 directory, which stops other OS
users but not the model: tool processes run as the same user, so under
the shipped danger-full-access default they read it like any other file.
SandboxExecutionPolicy grows readDenyPaths, and sandbox-policy defaults
it to $DSH_HOME/.env — the exact file rather than the harness home, so
the model keeps its documented access to its own session log. Seatbelt
appends a trailing deny (last matching rule wins) and bwrap maps
/dev/null over each path after any workspace bind; Landlock grants are a
pure allow-list that cannot subtract from its own / read grant, so
confine() reports partial enforcement there instead of claiming a
boundary the process does not have.
A real-kernel Seatbelt e2e proves the shape: the same read succeeds
unconfined and fails under the denial, while a sibling file in the same
directory stays readable. Both READMEs state the residual boundary
plainly — no confining mode means no boundary — and record the OS
keychain provider as the real answer.
The helper's cleanup ran the same fallback for two different events. A
settings provider detaching leaves the consumer running, so falling back
to the composition entry and re-judging derived facts is right. The
consumer's own unload ran it too — re-registering routes and touching
resources the teardown was releasing. The disposer now checks the
consumer fiber's own state and returns when it is unloading or disposed.
Four review findings across the seam and both adapters.
registerAdapter now returns a handle carrying replace(providers): the
candidate route set is validated in full before anything moves, so a
route another adapter owns leaves the previous registration intact, and
the swap itself is one synchronous section with no observable gap. pi-ai
uses it instead of dispose-then-register — the old shape dropped every
route when the new set conflicted, and its facts cache could then equal
the registry's, so reverting to a working configuration never re-applied.
Its registration facts are also sorted by provider, so a settings
document that merely reorders keys no longer triggers a swap.
DeepSeek's per-request snapshot now carries the credential facts, and
resolveApiKey receives it instead of re-reading the raw config: a
settings generation the resolver rejects can no longer contribute its
literal key to a request the previous generation's endpoint serves.
pi-ai only defers to the SDK's provider-native discovery when a profile
names no credential at all; a configured apiKeyEnv that misses now fails
with MISSING_CREDENTIAL naming the route and the reference, instead of
handing pi-ai undefined and letting it authenticate with an unrelated
ambient key.
The eager boot-time credential probe is gone: it could run before the
credentials service mounted and reported every failure as a missing key.
The route stays registered and browsable; the first request gives the
accurate error, whose guidance now leads with the credential store and
mentions a literal apiKey last.
The shipped surfaces loaded the harness home's .env into the process
environment before cordis booted. credentials-local then saw every stored
key as an ambient launch override: describe reported source 'env' with
writable false, and set/unset rejected as shadowed — so a key the web page
or TUI stored was unrotatable and undeletable from the next run onward,
and the adapter kept using the value captured at launch.
The home's .env is now the credential provider's own store, read by that
provider alone and hot-reloaded by it. The genuine launch environment and
the invoking directory's .env (loaded by the bin) remain the read-only
ambient layer, so a plain composition without the provider still resolves
keys exactly as before.
Proven by a real restart in the loader composition: store a key through
the seam, dispose the tree, re-boot over the same harness home, and the
entry is still file-sourced and writable — rotating it lands on the very
next request.
Review round three, credentials half. dsh-atomic-write grows the
cross-process writer-lock primitive (withFileLock: wx sentinel, bounded
backoff, stale takeover via onStaleBreak, deadline failure) plus a dirMode
option, and settings-local migrates its private copy to it; both providers
now create harness-home directories 0700.
credentials-local reuses the reviewed settings-local shape: watcher
reloads and line edits share one settled operation chain; every write
re-reads the document under the lock and publishes unobserved external
entries before editing, so an edit inside the debounce window (or another
process's write) can never be overwritten; the watcher's ready signal
queues one reconcile closing the startup gap.
The line editor is now physical-line aware: continuation lines of a
quoted multi-line value are never mistaken for assignments, untouched
lines keep their exact bytes (CRLF included), an edited line keeps its
own terminator, and appends use the document's dominant ending. A
multi-line entry reports writable: false, matching what set() would do.
The Credentials base class owns a contained notifyUpdated fan-out:
providers publish only after the commit, every listener runs, sync throws
and async rejections are logged without failing the committed write, and
INVARIANT-coded failures rethrow after the fan-out.
Drop the explicit-undefined base fixture exactOptionalPropertyTypes
forbids (the repository trusts TypeScript at typed same-process seams —
no test for an input the static interface excludes; coverage holds), and
reshape the async-listener containment fixture as an unknown-returning
function: the earlier inline cast was silently stripped by the staged
oxlint fixer, leaving a shape the next lint pass rejects.
The seam README states the JSON-shaped write boundary, watch-disposer
quiescence, async listener containment, and the drained teardown; the
provider README rewrites Behavior around the operation chain,
read-modify-write, writer lock, ready reconcile, and leaf-level YAML
diffs, and updates Known Limitations to the residual guarantees.
A new Agent Note records the round's decisions and supersedes the
original note's deferred-lockfile alternative (cross-linked in place).
Chinese counterparts updated pair-by-pair (three briefed minimal
updates, one whole-document translation); type-equiv, config, cordis,
and module-graph catalogs re-recorded.
Review round three found the provider's write path could destroy state it
never observed:
- Watcher reloads and document writes ran on two independent promise
chains, and a write rendered the whole next document from the cached
text. An external edit still inside the debounce window (or missed
outright) was overwritten, and the follow-up reload no-oped because the
post-rename content matched the cache — the edit vanished without a
trace. Reloads and writes now share one operation chain, and every write
starts by reconciling the on-disk text into the seam before rendering,
so unobserved sibling sections survive and publish first. An unparsable
on-disk document fails the write loud instead of being overwritten.
- The initial load raced the watcher's own setup: a change written between
that read and the watcher becoming active never fired an event. The
watcher's ready signal now queues one reconcile, closing the gap.
- Two processes sharing a harness home rendered from independent caches,
last writer winning. Writes now hold a wx-created <file>.lock sibling
around the read-render-rename cycle with bounded backoff, a crashed-
holder stale takeover, and a deadline failure; readers stay lock-free
because the rename commit is atomic.
- renderYaml replaced the whole namespace node, dropping every comment
inside the section. The next section now lands as a leaf-level diff
(set changed values, delete removed keys), so comments, anchors, and
formatting survive on every untouched node and on the key of every
changed pair; arrays still replace wholesale when unequal.
Review round three found four seam defects:
- A watch() disposer only removed the observer from the set; an invocation
already chained onto the watcher tail still ran after disposal. Watchers
now carry an active flag checked when a queued invocation would start,
and the service dispose drain awaits started invocations (pendingTails)
beside the write queues, so disposal is quiescent.
- The settings/updated manual fan-out caught only synchronous throws; an
async listener rejection escaped as an unhandled rejection. Thenable
returns are now contained through the shared listener diagnostic, and
the event contract documents that the INVARIANT rethrow serves
synchronous listeners only.
- structuredClone admitted Dates, Maps, BigInts, and cycles that YAML/JSON
storage silently distorts on reload (a Date lands as a timestamp string,
a Map as a plain map, a BigInt as a number). The write snapshot is now a
single-pass cloneJsonShaped walk that rejects non-JSON values with their
path before anything persists.
- mergeLayers' per-entry undefined guard became dead code once the clone
strips undefined entries at the boundary; removed, with the sparse-patch
contract restated at its enforcement point.
Both files fit their ceilings on each parent; the merge union of this
branch's settings rows with master's typert row and source-launch rewrite
overflows by 6 and 2 words. Every added row is a fixed-format layout or
package-table entry with nothing to relocate, so the ceilings move to the
union size.
The Models page drops the generic schema renderer and the visible
environment-variable field: each editor is a curated per-family card whose
primary input is one write-only API key stored under a derived
<ROUTE>_API_KEY reference (recorded as apiKeyEnv in the pi-ai profile), an
unkeyed whole-section provider opens as its setup card, and the collapsed
customized-settings fold carries baseURL/reasoningEffort (deepseek) or
reasoning (pi-ai). dsh-client-schema-form reduces to the schema/draft model
layer (no React).
apps/cli/cordis.yml gains settings-local, credentials-local, and the bare
dormant llm-pi-ai row (manifest deps added for the resolver contract);
llm-deepseek drops its !!js apiKey inline for per-request credential
resolution. Both adapters tag apiKeyEnv role('credential-ref') so the
form mounts the credential control. The web e2e scaffold isolates a
harness home per run — an in-process boot must never touch the
developer's real ~/.dsh — and the new models-settings scenario pins the
whole loop through the shipped app: dormant directory as add vocabulary,
schema-driven editor apply landing in settings.yaml, the route
registering live (topology frame), and a write-only key landing in the
temp .env with the configured badge converging. A hermetic test-owned
reference name keeps a developer's real provider keys from flipping the
badge. schema-form joins the platform module table (seed + externals)
so client bundles share one instance.
Yichen Jiang