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.
The mocked process.send consumed vitest's own fork-pool IPC messages and immediately ran the post callback, whose disconnect() severed the test worker's channel (process.connected is true under forks) — the whole spec's results vanished and win32-dialog-bindings.ts/win32-dialog-worker.ts fell to near-zero coverage on CI. The mock now records without invoking the callback or disconnecting; the real close lifecycle stays with built-worker.e2e.ts. Verified under both the threads and forks pools.
The win32 tier is exactly the koffi IFileOpenDialog child process; any failure surfaces as-is. The pwsh -> Windows PowerShell 5.1 cascade, the shared WinForms script, and the triple-miss AggregateError are deleted: koffi is a packaged dependency whose availability the install guarantees, so no mechanism fallback exists (the browse backend remains the fallback at the composition level). The pwsh-first DPI picker-fix note is consolidated into a new simplification note recording the reversal.
The koffi IFileOpenDialog conversation runs in a spawned child process instead of a worker thread: the dialog is the child's first window, so Windows activates it without a foreground call, and a native fault stays contained to the child. The driver maps the child's message protocol onto a promise and services aborts by posting WM_CLOSE to the dialog thread's windows, killing the child when the close budget is exhausted. The built worker ships as lib/worker.cjs (the ./worker export) under plain node, and win32-dialog.spec.ts returns to the thread-safe pool.
The koffi redesign moved the dialog from a spawned child process (which
inherits a foreground-activation right from the spawning app) onto a worker
thread of the same process, so Windows shows the dialog without activating
it — it opens behind the app with a taskbar flash. The app has no native
HWND to hand the seam, so raise from the driver: on the 'showing' notice
(the worker posts it right before Show, before the dialog window exists),
attach this thread's input queue to the dialog thread's, call
SetForegroundWindow on its top-level window, and detach — retried on the
close cadence until the window appears, stopped on settle/abort/success,
never blocking the pick. Injectable seam mirrors closeThreadWindows; driver
tests pin the raise and its retry; the in-process note records the
mechanism (both languages, pairing re-recorded).
The driver's switch over Win32DialogWorkerMessage handled all three current
kinds but had no default, so a fourth kind added to the worker protocol
would compile cleanly and silently no-op — settle() never called and the
pick dangles until worker exit. Add the local assertNever backstop (the
command-compact shape; this package does not depend on dsh-llm for the
helper) and the return the error case needs to avoid falling into it.
Round-five review finding.
The round-four review's v6 pass found three factual gaps and the v5 pass two
nits. Correct them before merge:
- The in-process note claimed a koffi signature mistake is 'contained to
the worker thread' — worker_threads share the process, so a native access
violation takes down the whole Node process with no PowerShell fallback.
State the real blast radius and record the deferred pkg-VFS worker-spawn
arm in Consequences (both languages, pairing re-recorded).
- The 2026-07-27 picker note claimed unconditional 'per-monitor-v2 DPI';
PMv2-less hosts (Server 2016 / Win10 1607) cascade to per-monitor or
system-aware. Say 'the best thread DPI awareness the host accepts'
(both languages, pairing re-recorded).
- built-worker.e2e.ts was not in any keyless gate (vitest.e2e config is
not part of the default unit run and builtBinSmokeGate's explicit list
missed it), so lib/worker.cjs load regressions passed keyless CI. Add it
to builtBinSmokeGate alongside the workflow-workerthread sibling.
- Remove the dead trailing 'void bindings' in win32-dialog-logic.spec.ts
and give native-picker.spec.ts the sibling module header it lacked.
The @module header still described the Windows adapter as the pre-PR 'STA
PowerShell FolderBrowserDialog' while the README and Agent Notes document
the koffi IFileOpenDialog primary with the PowerShell chain as fallback;
mirror the README's platform summary.
- setThreadDpiAwareness checks SetThreadDpiAwarenessContext's return value
and cascades per-monitor-v2 -> per-monitor -> system-aware; DPI stays a
deliberate cosmetic best-effort - a host accepting none (or lacking the
API, pre-1607) still gets the modern dialog instead of a downgrade to the
legacy fallback chain over a cosmetic concern.
- The mocked-koffi world now uses a distinctive 4-byte pointer width and
rejects mis-sized out-buffers and mis-divided vtable offsets, so a
regression to hardcoded 8s fails the suite (the ia32 bug class).
- A keyless built-worker e2e guard loads lib/worker.cjs under plain
worker_threads on POSIX (the workflow-workerthread shape).
- The 'loaded lazily' module claims are reworded to attribute laziness to
the dynamic import('koffi') calls, and the discarded close-attempt
rejection is named at its catch.
Review round two on the in-process dialog:
- Vtable slots and out-pointers use koffi.sizeof('void *') instead of a
hardcoded 8 - win32-ia32 (which Node and koffi both ship) would have read
method pointers from the wrong address and crashed in-process before any
fallback could run.
- runFolderDialog pairs every successful (incl. S_FALSE) CoInitializeEx
with CoUninitialize in the outermost finally, releasing the dialog first;
a failed init is deliberately unpaired. Pinned across fake-bindings and
mocked-koffi suites.
- The abort close budget starts unconditionally: a worker hung before the
showing notice (koffi import or COM init) now ends in terminate instead
of a dangling promise; WM_CLOSE posting still waits for the thread id.
- A triple miss (dialog + pwsh + 5.1) surfaces an AggregateError carrying
all three causes - the in-process tier's reason was previously
unrecoverable from the final PowerShell error.
- The stray '=>{ ' formatter artifacts are normalized to real blocks.
Both stale note claims from the review are fixed: the DPI note's
Consequences no longer claims an ENOENT classification or zero new
dependencies, and the 2026-07-27 picker note's Windows bullet now names
the in-process primary and keeps the PowerShell chain as fallback (both
languages, pairings re-recorded).
The workspace files constraint keys worker bundles on the ./worker export
(lib/worker.cjs, the workflow-workerthread shape); the descriptive source
entry stays win32-dialog-worker.ts and tsdown renames the bundle.
On a fast host the 1ms close budget can exhaust and reject between waitFor
ticks; a rejection with no listener yet counted as an unhandled error in the
Linux run.
The modern IFileOpenDialog becomes the primary win32 tier: a koffi-driven
COM conversation on a worker_threads worker (the modal Show never blocks
the host event loop), per-monitor-v2 DPI via SetThreadDpiAwarenessContext,
and abort service by re-posting WM_CLOSE to the dialog thread's windows,
with terminate+unref as the last resort (Node cannot interrupt a thread
blocked in native code, and such a worker must never hold the process open).
The PowerShell chain stays as the fallback tier with its trigger widened
from ENOENT to any pwsh failure, closing the review-flagged PowerShell 6
regression (no WinForms: exit 1, not ENOENT, so 5.1 never ran).
Layering keeps per-file coverage honest on every host: pure sequencing and
the driver test against fakes anywhere; the bindings run against a mocked
koffi COM world (the session-persistence-jsonl technique); POSIX hosts
drive the real spawn plumbing to its koffi-load rejection; win32 hosts run
a real open-and-abort-close smoke. The smoke joins processBoundTests: a
worker blocked in a native modal wedges the threads pool's teardown, while
a fork contains it. The worker bundles as its own CJS tsdown entry
(workflow-workerthread's pattern; no TLA), and the host module is imported
statically so the node-half bundle stays chunk-free.
Built-plane and real-COM behavior verified on native Windows: standalone
probes for the source worker, the built CJS worker, and the driver's abort
path all open and close the real dialog.
Agent Notes: new implemented/feature/2026-08-02-win32-in-process-folder-dialog
(bilingual) owns the decision; the DPI note is re-scoped to the fallback tier
it now describes and its AutoUpgradeEnabled attribution corrected (.NET Core
3.0 rewrote FolderBrowserDialog; the opt-out arrived in .NET 6).
.NET 10's modern FolderBrowserDialog renders Description as a bottom
strip above the folder input, and the 5.1 classic dialog as an unthemed
white box; the property is dropped entirely and a regression assertion
pins its absence.
README pairs document the pwsh-preferred adapter and the PowerShell 7
requirement for the modern dialog; the 2026-07-27 picker note's Windows
adapter fact is updated in place, and a new bug-fix note records the
defect, the fallback decision, and the DPI awareness rationale.
The win32 branch now spawns pwsh.exe (PowerShell 7) first and falls back
to powershell.exe (Windows PowerShell 5.1) only when pwsh is missing
(ENOENT), mirroring the Zenity-KDialog fallback. PowerShell 7 renders the
modern IFileDialog folder picker; the 5.1 fallback keeps the legacy tree
functional. Both runtimes execute the identical script, which opts the
process into system DPI awareness (SetProcessDPIAware) before any window
exists, fixing the blurry bitmap-stretched dialog on scaled displays.
Master removed the TUI package, the `meta` and `upgrade` subcommands, and
`--config-replace`, and made raw `dsh` require a `--config` overlay. Resolved
onto that shape:
- Dropped this branch's TUI edits with the surface itself, including
`tui.cordis.yml`, `runTui`, and the TUI keyless PTY smoke.
- Dropped the `--config-replace` plumbing rather than reintroducing a flag
master deliberately removed. The gap this branch fixed remains: `dsh -p`
still could not name its composition, so it keeps `--config`.
- Kept this branch's deletion of the personal `$DSH_HOME/config.yaml` layer,
which master still carried, and provided the environment snapshot in the new
raw `runConfig` surface alongside web and headless.
- Ported the headless shutdown PTY test off the personal overlay onto a named
`--config` file, which is what proves that flag now exists on `-p`.
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.
