Move the triplet to implemented/architecture/, rewrite Proposal into a
present-tense Decision, fold Acceptance criteria and Risks into
Consequences (recording what pins the result), retitle the migration
section to what the move actually touched, and repoint the seven
inbound FIXME/README links.
The layered-env reader gained an unreadable-layer path, a default
reporter, and two absent-layer arms with no cases; the credential store
gained two error paths that must not be mistaken for an absent file.
The platform arms and the `linePos` guard cannot be reached from a POSIX
test run — the first is covered by the native Windows job, the second
only satisfies an optional type that `prettyErrors` always fills — so
both carry a v8 ignore naming why.
The Windows case-folding in the lookup was implemented without a
user-facing contract. Name matching follows the platform, and the
reason it must is the layer ranking it would otherwise invert.
Two had real security consequences:
The bootstrap rejection ran on npm dotenv's parser while process.loadEnvFile
applied the file with Node's own. Two independently maintained dialects meant
the check and the thing it guards could disagree: a name Node accepts but the
checker misses would reach process.env unchecked, and BASH_ENV there runs a
file of the project's choosing on every `bash -c` the bash tool issues. Parse
once with node:util's parseEnv — the same engine loadEnvFile uses — and assign
the entries already checked, which also drops the dotenv dependency.
llm-pi-ai still returned a literal profile.apiKey ahead of everything, and it
registers a settings namespace, so the defect removed from llm-deepseek
survived intact in its design twin. The field is gone from the profile schema,
the resolution path, and the tests.
The rest are consistency and documentation defects the review named:
- verify-config-source-ownership did not scan the Python runtime's bundled
cordis.yml, which still inlined apiKey and baseURL. Both are covered now, and
the line-anchored INLINE_DENY documents that it is a tripwire, not a parser.
- The deny list missed NODE_TLS_REJECT_UNAUTHORIZED, the askpass hooks, the
GIT_CONFIG_* redirections, and PYTHONHOME — all implied by its own stated
rule about what a variable does.
- Snapshot lookups folded case on Windows, where environment names are
case-insensitive and an exact-match Map could miss a higher-ranked layer.
- The credentials note claimed a read-time permission check was "not taken"
while this PR implemented it; the credentials-local README still described
two layers, live process.env reads, dotenv-era limitations, and a renamed
anchor; the llm-deepseek README still advertised the removed literal apiKey;
and web.ts and base.cordis.yml kept personal-overlay wording.
- The ownership note's literal-apiKey claim now names its scope: the
web-search providers keep a literal field but register no settings
namespace, so nothing can shadow a stored credential through them.
Review found five real defects in the configuration-source work, all confirmed
against the code rather than argued:
1. The note claimed --config outranks settings.yaml. It does not: the settings
seam registers a plugin's cordis entry config as the `base` layer and the
user section layers over it, and the seam cannot tell a shipped value from a
--config one. The note now states shipped reality and names --config-replace
as the lever for a deployment that must win. Separately, a literal `apiKey`
in settings outranked both the environment and .credentials.yaml — the field
is removed, so configuration carries a reference and nothing else.
2. DEEPSEEK_SEARCH_BASE_URL was functionally deleted: the shipped inline went
away without the provider learning to read it. It now resolves from the
environment snapshot, as the README always claimed.
3. The bootstrap deny list missed the interpreter start-up hooks. BASH_ENV is
the sharpest: `bash -c` sources it on every bash tool call, so a project
.env could run a file of its choosing before every command. The list now
covers BASH_ENV and its per-language siblings, the Git hook commands, and
the remaining preload and CA variables, organised by what a variable does
rather than which runtime owns it.
4. YAML parse errors quoted the offending source line — which in a credentials
document is the secret — into boot stderr and the watcher's logger. Only the
error code and position are reported now, in credentials-local and
settings-local alike, pinned by a test that asserts the secret is absent.
5. 0600 governed only files the harness wrote. A hand-created 0644 document was
read normally. POSIX now checks the mode before reading contents, at boot
and on every reload; Windows has no mode to inspect and is skipped rather
than faked.
The project a session is launched in is trusted by default, with no prompt and
no stored trust record: it may supply its own endpoint, ordinary variables, and
a key ranked below the managed store. Trust stops at the harness itself — a
discovered file still cannot set DSH_PERMISSION_MODE, PATH, BASH_ENV, or the
rest, because those take effect with no user action, before any turn, outside
the permission policy and the sandbox.
$DSH_HOME/.env had just become an ordinary environment layer, which left the
harness resolving user-facing values from a flattened process.env that could
no longer say where a value came from. A key stored through the web page
stayed shadowed by an older key in the user's own .env. An endpoint could be
redirected by the project: the invoking directory's .env is materialized like
every other layer, and a base URL decides where a resolved API key is sent, so
a DEEPSEEK_BASE_URL written into a model-editable workspace would send the
user's credential — and the prompts carrying their code — to whatever host
that file named.
Give every user-facing value one ordering, with four kinds of source:
explicit for this run per-operation override, CLI argument
> authored by deployment --config / --config-replace
> this launch's shell inherited process environment
> product-managed store settings.yaml, .credentials.yaml
> discovered file $DSH_HOME/.env
> defaults schema default, shipped base, public default
The domains differ only in which tiers exist. The earlier split — credentials
ranking the environment over the managed file while settings ranked over the
environment — was inconsistent: the distinguishing fact is who authored the
source, not the domain.
packages/util/environment owns an immutable snapshot with per-layer
provenance. getFrom(name, sources) searches only the layers a caller names,
and omitting one is a refusal rather than a demotion: the adapters ask for
['process', 'user-env'], so no reordering can let a project file back into a
decision it was excluded from.
isBootstrapOnly rejects, before anything is materialized, any .env setting a
variable that governs how a process launches (PATH, SHELL, NODE_OPTIONS,
LD_PRELOAD), where code or model-visible instructions load from (the whole
DSH_* namespace, HOME, XDG_*), or how the network is reached (proxy and CA
variables). The namespace is denied wholesale so a switch added later cannot
become settable by being forgotten, and there is no opt-out.
verify-config-source-ownership keeps both rules: no unregistered process.env
read under packages/*/*/src (26 allowlisted with reasons), and no apiKey,
baseURL, or headers inlined from the environment in shipped Cordis config —
removing those inlines is what makes the deployment tier meaningful.
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.
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.
New credentials data-structure page (type-equiv manifested), group README,
rewritten llm-deepseek/llm-pi-ai READMEs (dynamic configuration, dict
profiles, credential chain), capability-seams/service-role registration,
Agent Note (bilingual), demo compositions mounting settings-local +
credentials-local with no inline key plumbing, installSettingsSection
consumer helper on the settings seam (deduplicating both adapters' wiring),
jscpd symmetry markers for the provider twins, runtime-closure additions for
python/sdk-runtime, and doc-budget ceilings AGENTS.md 1750→1755 /
packages/README.md 850→865 for the structural one-line group rows.
writeFileAtomic: exclusive-create random-suffix temp + rename carrying the
caller-stated mode; settings-local persistSection now consumes it. The
credentials-local store shares it next.
ui-workspace's two trigger surfaces each declare a single-kind directory-flow
hole (conversation.hero.workspace.directoryFlow / sidebar.workspaces.directoryFlow,
same owner contract) and keep only the trigger and the adoption: the Open-local-
folder entry renders while the surface's hole is occupied, and the occupant
reports one picked path per open through the hole's owner conversation
(open/busy/onPicked/onCancel/onError).
directory-picker-native becomes dual-face: its browser half fills both holes
with a renderless occupant driving host.pickDirectory, so the cordis.yml row
that mounts the backend also composes the client interaction — a mismatch is
impossible and a second flow package fails at client load.
With composition wiring both sides, the host.describe.directoryPicker
advertisement and the client's kind branching lose their last consumer:
the field, WorkspacesService.directoryPickerKind(), the DirectoryPickerKind
wire type, and the picker's per-open describe read are deleted. The connection
fixture now serves a deterministic pickDirectory path so the keyless snapshot
drives the full pick-then-adopt flow. ui-workspace's hand-rolled declaration
deferral is replaced by the deferRegistration helper it duplicated.
master's toolcall-open extracted runNativeCommand inside apiproxy for the
openPath opener while the picker seam had moved the native chooser (its other
consumer) into directory-picker-native; after the merge the two packages each
carried a verbatim copy. The runner now lives in packages/util/native-command
(zero-dependency library, per the util-group contract) and both native
integrations depend on it.
Review fixes for #462:
- resolveDshHome now treats an empty or whitespace-only $DSH_HOME as
unset, so a blank override never resolves the home to cwd via
resolve(''). Restores the guard telemetry's old resolver carried.
- The default-env telemetry test asserts only that globalConfigDir()
returns an absolute path, so a machine DSH_HOME without a .dsh suffix
cannot break it.
Delete @deepseek-ai/dsh-home and make dsh-paths the sole owner of the
single-root harness home ($DSH_HOME || ~/.dsh). Migrate tool-bash,
skill-local, and agent-spine-demo off dsh-home, and fold telemetry's
divergent globalConfigDir onto the shared resolver, dropping its second
XDG/APPDATA policy and the deepseek-harness namespace so the anonymous
id lives under the harness home. Add dshHomeDisplay() for symbolic
user-facing paths, replacing workspace-context's bespoke check.
Tianyi Cui
Yichen Jiang
Turtle
xjt
imccyu
j-xiang
creatixchu