fix: don't walk the file tree for read-only commands (whoami/orgs/logs hang from large dirs)

[uriva]

Problem

actionHandler (in config.ts) eagerly calls resolve_config, whose recursive downward file walk builds the deploy upload manifest for every command. But read-only commands — whoami, orgs list, apps list, env list, logs, database reads — never use that manifest. Only the upload path (publish.ts:52, configContext.files) does.

When a read-only command is run from a large directory tree (a home directory, or a repo with a big node_modules/cache), the walk dominates and the command effectively hangs, producing no output.

This is especially painful for AI agents and CI, which the CLI explicitly supports (AGENTS.md). The documented way to discover the org for constructing the <app>.<org>.deno.net URL is:

deno deploy whoami --json

…which times out with empty stdout when the cwd is large.

Reproduction

# From a large directory (e.g. $HOME):
cd ~
DENO_DEPLOY_TOKEN=ddo_… timeout 40 deno deploy whoami --json < /dev/null
# exit 124 (timed out), no output

# From an empty dir, the same command returns instantly:
cd "$(mktemp -d)"
DENO_DEPLOY_TOKEN=ddo_… deno deploy whoami --json < /dev/null
# {"authenticated":true,...,"orgs":[{"slug":"…"}]}

The hang scales with the number of files under the working directory — it is the collect_files walk in rs_lib, not a network/tRPC issue.

Fix

Split config handling in config.ts:

• discoverConfig — cheap: locate the nearest deno.json[c] (or an explicit --config) to read deploy.org/deploy.app. No directory walk.
• readDeployFiles — the expensive recursive walk (still via resolve_config), now invoked lazily behind a configContext.files getter, so it runs only when the upload path actually reads the manifest.

No wasm/Rust changes. Behavior is unchanged for:

• deploys — publish.ts reads configContext.files, which triggers the same full walk as before;
• --config <file> — used directly.

Read-only commands are now instant regardless of the working directory's size.

Verification

• deno deploy whoami --json from $HOME: exit 124 (hang) before → exit 0 instantly after, correct org returned.
• Full end-to-end create + --prod deploy with the patched CLI: files uploaded, app served at <app>.<org>.deno.net, clean exit.
• deno fmt --check, deno lint, deno check, and the test suite pass.

Test

Adds a regression test in tests/config.test.ts that builds a large tree and asserts a read-only action (one that never reads .files) is markedly cheaper than one that does. It fails before this change (both walked eagerly) and passes after.

[uriva]

@piscisaureus this is the reason commands hung

[avocet-bot]

Review: don't walk the file tree for read-only commands (#132)

Reviewed head SHA: f147d9b9d5b9e07e61081cdbce1d262f3f5dc4f2
Verdict: 👍 Sound, well-motivated fix with a real regression test. No blockers. A few non-blocking notes below — one minor error-handling regression worth tightening, one test-robustness suggestion, and one workspace-semantics point I traced and cleared (documented for confidence).

Review skill evidence

Invoked via the Task tool from the pr-review-toolkit plugin:

• pr-review-toolkit:code-reviewer — correctness/regression pass over config.ts vs. rs_lib/src/lib.rs, the lazy getter, save path, and the new test.
• pr-review-toolkit:silent-failure-hunter — focused audit of the swallowing catch blocks in discoverConfig.

I did not take the subagents' conclusions at face value: I independently re-traced the Rust specifier-selection logic and downgraded the code-reviewer's "critical" workspace finding to a non-issue (see §3).

---

What the PR does

actionHandler previously called resolve_config eagerly for every command. That triggers the wasm collect_files recursive downward walk to build the upload manifest — work only the upload path needs. From a large cwd ($HOME, a big node_modules), read-only commands (whoami, orgs/apps/env list, logs) effectively hung. The fix splits this in two:

• discoverConfig — cheap upward walk for deno.json[c] (or an explicit --config) to read deploy.org/deploy.app.
• readDeployFiles — the expensive resolve_config walk, now behind a memoized configContext.files getter, paid only by the upload path.

The mechanism is correct and the motivation is real.

---

Findings

1. (Non-blocking, minor regression) discoverConfig swallows all read errors, not just "not found" — config.ts:294-299 (and :283-289)

try {
  const content = await Deno.readTextFile(path);
  return { config: { path, content } };
} catch {
  // Not here; keep looking upward.
}

The bare catch treats every failure as "no config here," but that assumption only holds for Deno.errors.NotFound. A PermissionDenied/NotCapable/IO error on a deno.json that does exist is silently reinterpreted as absent, and the walk continues upward. Worst case: the tool skips the config the user intended and either picks up a different ancestor deno.json or falls through to "no config" — silently wrong org/app, or save() targeting the wrong file.

This is a small behavioral regression: the old readConfig read the resolved config path without a swallowing catch, so such errors surfaced. Impact is low in practice (the CLI generally runs with read permission and unreadable deno.json files are rare), which is why this is non-blocking — but it's a cheap, clearly-correct tightening:

} catch (err) {
  if (!(err instanceof Deno.errors.NotFound)) throw err;
  // NotFound: keep looking upward.
}

The same applies to the Deno.stat catch at :283-289 (comment says "non-existent root" but it also swallows permission errors on a user-supplied positional root). The explicit --config path at :276-280 is handled correctly — it deliberately does not swallow, so a bad --config is a hard, visible error. Good.

2. (Non-blocking) The regression test is timing-based and may flake under CI load — tests/config.test.ts:170-181

assert(readOnlyMs * 3 < withFilesMs, ...)

The intent is right and the 3× margin is defensible: the read-only path does one stat + one file read while the other walks ~9000 files with gitignore handling, so the real-world margin is large. But wall-clock ratios are inherently susceptible to a warm-cache fast machine or a contended CI runner compressing the difference. If this ever flakes, prefer a deterministic signal — e.g. spy/counter on resolve_config/readDeployFiles and assert it was invoked 0 times for the read-only action and once for the .files action. That tests the actual invariant ("the walk is skipped") rather than a proxy for it. Also note assert(files.length >= 9000) is coupled to the 30×30×10 generation loop. Building 9000 files does add real cost to the suite.

3. (Info — traced and cleared, no change needed) Workspace org/app discovery is unchanged

The code-reviewer subagent initially flagged a "critical" workspace regression: that stopping at the first deno.json reads org/app from a member package instead of the workspace root. I traced the old path in rs_lib/src/lib.rs:144-155 and it does not hold. The old specifier is:

member_deno_json().filter(|c| c.to_deploy_config().is_ok()).map(specifier)
  .or_else(|| root_deno_json().filter(...).map(specifier))

to_deploy_config().is_ok() is true even for a member with no deploy block (it returns Ok(None)), so the or_else(root...) fallback never fires when the member deno.json merely lacks deploy. The old code therefore returned the member file's specifier, and getAppFromConfig parsed the member's content → undefined org/app. The new upward walk stops at that same member deno.json → same undefined. Both then rely on --org/DENO_DEPLOY_ORG/interactive selection. The Rust workspace_root_includes_member_files test runs from the root (where both old and new read the root's deploy), so it's unaffected. And Deno's WorkspaceDirectory::discover itself walks up to find a deno.json, so the "stray ancestor config" concern isn't a divergence either. No regression.

---

Verified correct

• Lazy getter memoization (config.ts:199-210): collectedFiles is closure-captured and only computed while undefined. publish.ts reads configContext.files twice (:52, :157); the walk runs exactly once, second read hits cache. Matches old single-walk behavior while making it lazy. ✅
• Save path (config.ts:213-229, writeConfig): still writes to config.config.path (or creates deno.jsonc in cwd). For the single-config case this is unchanged; the workspace case behaves as before per §3. ✅
• Loop termination: dirname(dir) === dir breaks at filesystem root; the walk operates on a resolved path string via dirname (no symlink following), so each iteration strictly shortens the path — no infinite loop. ✅
• Positional file root: discoverConfig normalizes a file root to its parent via Deno.stat().isFile → dirname, matching the Rust path_is_file handling and the positional_file_root_discovers_parent_config test. ✅
• deno.json before deno.jsonc ordering and --config as-is handling match Deno's precedence. ✅
• Type surface: ConfigContext.files: string[] is satisfied by the getter; deno check passes per the PR description. ✅

Bottom line

Solid, correct, well-tested performance fix that removes a genuine hang for AI-agent/CI use of read-only commands. Recommended follow-ups (all optional, none blocking): narrow the discovery catch blocks to Deno.errors.NotFound (§1), and consider a deterministic spy instead of the wall-clock ratio in the new test (§2).

[crowlKats]

Addressed the two non-blocking review notes in 6cef1e1:

• §1 — swallowing catches: discoverConfig now rethrows anything that isn't Deno.errors.NotFound (both the Deno.stat root check and the per-name readTextFile), so a permission/IO error on an existing deno.json surfaces instead of being reinterpreted as "no config here."
• §2 — timing-based test: replaced the wall-clock ratio with a deterministic invocation counter. readDeployFiles is now reached through an exported _internals seam; the test asserts the walk runs 0 times for a read-only action and exactly once when .files is read (reading .files twice confirms memoization). This also drops the 9000-file tree down to a handful of files, so the suite is cheaper.

deno fmt/deno lint clean and all 5 config tests pass. (The pre-existing resolve_config arity mismatch under deno check is unrelated — stale local wasm lib types, regenerated by deno task build.)

[uriva]

@piscisaureus any updates?