feat(deploy): send deno.json to the console on every deploy

[piscisaureus]

deno deploy uploaded the files but never passed the deno.json
contents to apps.initiateCliRevision, so the console fell back to the
app's stored config and the deploy section only took effect once,
when deno deploy create captured it.

• deno deploy looks up the app's build directory (apps.get) and
  sends that directory's deno.json/deno.jsonc as denoJsonFiles; the
  console applies its deploy section as it does for GitHub
  deployments. It is read from disk when deploy.include/exclude
  leaves it out of the upload, and a config selected with --config
  takes its place.
• create warns when deno.json will override the build config given to
  it (--do-not-use-detected-build-config, --build-timeout, a custom
  app directory, or a custom config in the interactive flow), except
  under --json, and no longer prompts for a timeout deno.json decides.

Behavior change: CLI apps whose deno.json has a deploy section now
build with it on every deploy instead of the stored config. The app's
build memory limit, cronsDisabled and executor are kept
(denoland/deployng#3773, in prod).

create also detects the build config of a local custom app directory
(with the root's package manager), so it offers the detected config and
warns as for workspace members. The excluded-config
fallback resolves symlinks and never reads outside the deploy root.

Not covered: create's warning does not inspect a --config file or a
custom GitHub app directory (that lookup's failure would exit the CLI),
and
a deno.json found only above the deploy root (a workspace config) is not
sent, as GitHub deployments also read only the app directory's.

[avocet-bot]

Review — #146 feat(deploy): send deno.json to the console on every deploy

Reviewed head SHA: 3f99929c3555adb5e4140e4296f0b57f461d545a
Status: Draft PR, CI green (deno success, jsr success). No blocking issues found — non-blocking notes below.

Review skill evidence

This review was produced by invoking the Claude Code pr-review-toolkit:code-reviewer subagent (via the Task tool) against the four changed files at the reviewed head SHA, plus my own independent trace of the diff, the @deno/framework-detect@0.3.3 type contract, and the --json output contract in util.ts. The subagent completed a full pass (19 tool calls) and returned no findings at confidence ≥ 80.

---

What this PR does (background for a reader new to the codebase)

deploy-cli is the deno deploy command-line tool. When you run deno deploy, it tars up your project's files, hashes them, and hands the manifest to the Deno Deploy console (the backend) over a tRPC call named apps.initiateCliRevision; the console then builds and runs your app. Your project's deno.json/deno.jsonc can carry a deploy section that describes the build configuration (install/build commands, timeout, etc.).

The bug this PR fixes: the CLI uploaded the files but never passed the contents of deno.json to apps.initiateCliRevision. As a result, the console fell back to the build config it had stored when the app was first created (deno deploy create), so a deploy section in deno.json only took effect once, at creation time — not on subsequent deploys. GitHub-sourced deployments already re-read the deploy section on every deploy; this makes CLI (local) deployments behave the same way.

The change has three parts:

1. deploy/publish.ts — while streaming/hashing the uploaded files, it now also captures the text of every deno.json/deno.jsonc into a new denoJsonFiles map (keyed by normalized relative path), and additionally reads the root deno.json/deno.jsonc from disk even when deploy.include/exclude left it out of the upload. This map is sent alongside manifest in apps.initiateCliRevision. A small exported helper isDenoJson(path) does the filename match.
2. deploy/create/flow.ts — the interactive create flow now prints DENO_JSON_PRECEDENCE_WARNING when the user supplies build settings that a deno.json deploy section will override, and it auto-selects a timeout (skipping the prompt) when deno.json already decides it.
3. deploy/create/mod.ts — the non-interactive create path prints the same warning when --do-not-use-detected-build-config or --build-timeout is given but the (possibly custom-directory) deno.json has a deploy section; it calls detectBuildConfig directly for a custom local app directory that is not a detected workspace member.

Key contract that makes the logic correct: in @deno/framework-detect@0.3.3, detectBuildConfig returns from: "deno.json" only when the deno.json actually has a deploy section (otherwise from: "detected"). So every from === "deno.json" guard in this PR is effectively "deno.json has a deploy section that will take precedence" — which is exactly the condition the warnings and the prompt-skip intend to key on.

---

Correctness assessment — no blockers

I traced the edge cases the diff introduces and they are handled:

• Path separators / isDenoJson (publish.ts:31-33, :103). isDenoJson slices after the last /. It is only ever called on paths already normalized to / (publish.ts:103), and the new unit test confirms it matches deno.json/deno.jsonc at any depth while rejecting package.json, deno.json.bak, and src/mydeno.json. The disk-read fallback uses join(rootPath, name) (:116), which correctly uses the OS separator for filesystem access while the map key stays the bare literal deno.json/deno.jsonc — consistent with the manifest keys the console already expects.
• Upload-vs-fallback consistency / no TOCTOU (publish.ts:113-120). The root fallback loop continues when a config is already present from the upload, so uploaded bytes are never overwritten by a second disk read. A deno.json and a deno.jsonc get distinct keys, so no collision when both are present.
• detectBuildConfig failure is non-fatal (mod.ts:258-263). It is awaited with .catch(() => undefined) and only for source === "local", so a bad custom directory degrades to "no warning" rather than throwing.
• No double-warning (flow.ts:263, 277). The finalBuildConfig === buildConfig guards keep the precedence warning from firing twice across the "use detected" and "explicit timeout" branches.
• --json envelope stays clean. The PR's new warnings are either gated by !options.json (mod.ts:251-252) or live in createFlow, which calls requireInteractive() at its top (flow.ts:120-123) and therefore cannot run under --json.

---

Non-blocking notes

1. Intended new data-egress path is untested (publish.ts:109-120). When a user deliberately excludes the root deno.json from the upload via deploy.include/exclude, its full contents are now read from disk and transmitted inline as denoJsonFiles["deno.json"] anyway; and every nested deno.json/deno.jsonc in the upload is sent, even though the comment notes the console only reads the app-directory one. This is the documented intent of commit 475550f and the destination is the same trusted Deploy backend the user is already uploading their whole source tree to, so the privacy risk is low and deno.json conventionally holds no secrets. Flagging only because it is a silent behavior change (an excluded file's contents now leave the machine) and no test covers the fallback or the "send all nested configs" behavior. A small test that drives publish's collection with an excluded root config and a nested config would lock in the intent.

2. Pre-existing ungated console.warn under --json (mod.ts:274-276, out of scope). The "No build configuration was detected in '…'." warning is not gated by !options.json, so under --json it writes non-JSON to stderr — the same contract sibling commit 0b2870a fixed for the new warning. This line is present verbatim on origin/main, so it is pre-existing and not introduced here; worth a follow-up but not a blocker for this PR.

3. Minor messaging overlap (not a bug). For a custom local app directory whose deno.json has a deploy section, passing --build-timeout can surface both the precedence warning and the pre-existing "No build configuration was detected" message. The outcome is functionally correct (the backend applies deno.json via denoJsonFiles); the paired messages just read as mildly contradictory.

4. Timeout placeholder is cosmetic (flow.ts:275-280). When deno.json decides the timeout, buildTimeout is set to AVAILABLE_BUILD_TIMEOUTS[0] purely to skip the prompt; the logged "X minutes" may not equal the value deno.json actually applies. The precedence warning already tells the user deno.json wins, so this is low-impact UX only.

---

Draft context

The PR is a draft, explicitly held until denoland/deployng#3773 reaches prod (so a deno.json override does not drop the app's stored memory limit / cronsDisabled / executor). That upstream guard is the real gate on the behavior change; this CLI change is correct on its own terms. Known gaps (a deno.json in a non-root app directory excluded from the upload is not sent; the create warning does not probe custom GitHub app directories) are acknowledged in the PR description.

Recommendation: No blocking issues. Posting as a comment given the draft status and the non-blocking test-coverage/privacy notes above.

[avocet-bot]

Review (re-review of reworked head) — #146 feat(deploy): send deno.json to the console on every deploy

Reviewed head SHA: e794f13675cf608774a22274501e1cfd61aa1e22
Status: No longer a draft; CI green (deno success, jsr success). No blocking issues — one non-blocking robustness/test-coverage concern and a minor symmetry nit below.

This is a re-review: the head was substantially reworked since the earlier pass (7 new commits, publish.ts grew to +70/−3, and the approach changed from "send every nested deno.json plus a root fallback" to "send only the app directory's deno.json").

Review skill evidence

Produced by invoking the Claude Code pr-review-toolkit:code-reviewer subagent (via the Task tool) against the true PR delta (git diff origin/main...HEAD) at the reviewed head, plus my own trace. I additionally ran the new test locally (deno test --no-check -A tests/deno_json_files.test.ts → 1 passed) and confirmed the type-check failure is solely a missing generated wasm artifact (lib/rs_lib.js, a build step), unrelated to this PR.

---

What this PR does (background for a reader new to the codebase)

deploy-cli is the deno deploy command-line tool. Running deno deploy tars up and hashes your project files, hands a manifest to the Deno Deploy console (the backend) over a tRPC call apps.initiateCliRevision, then uploads the missing files; the console builds and runs the app. A project's deno.json/deno.jsonc can carry a deploy section describing the build configuration.

The bug being fixed: the CLI uploaded the files but never sent the contents of deno.json, so the console fell back to the build config it stored at app-creation time — the deploy section only took effect once, at deno deploy create. GitHub-sourced deploys already re-read it every deploy; this brings local (CLI) deploys in line.

How the reworked version does it (all in deploy/publish.ts):

1. Learn the app's build directory. A new appBuildDirectory helper queries the tRPC apps.get endpoint for the app's stored build_config.buildDirectory and passes it through a new normalizeBuildDirectory helper, which mirrors the console's normalization: it drops empty and . path segments and maps any ..-escaping path to the root "".
2. Scope the config to that directory. During hashing, it captures only ${appDir}/deno.json or ${appDir}/deno.jsonc (the app directory's config — "the console reads no others").
3. --config stand-in. If the user passed --config somefile, its contents are read from disk and sent under the ${prefix}deno.json key (the .jsonc key is deleted), so the console applies the explicitly selected config as the app directory's deno.json.
4. Excluded-config fallback. Otherwise, if deploy.include/exclude left the app directory's config out of the upload, it is read from disk (guarded by an insideRoot relative-path check) so its deploy section still applies.

The create-flow changes (deploy/create/flow.ts, deploy/create/mod.ts) add a DENO_JSON_PRECEDENCE_WARNING and skip the timeout prompt when deno.json decides it; these are essentially unchanged from the earlier pass, except mod.ts now also warns when there is no detected workspace member (member === undefined, i.e. a custom app directory whose config is synthesized from flags).

Key contract that makes the warning logic correct: in @deno/framework-detect@0.3.3, detectBuildConfig returns from: "deno.json" only when the config actually has a deploy section, so every from === "deno.json" guard means "deno.json has a deploy section that will take precedence."

---

Correctness assessment — no blockers

I verified the pieces that could break and they hold:

• Write/read field agreement for the build directory. My first concern was that create sends buildDirectory top-level while publish reads it nested as build_config.buildDirectory. This is not a mismatch: createApp (deploy/create/mod.ts:429-434) nests buildDirectory inside the buildConfig object it sends to the server, and the inner camelCase key matches the established build_config.frameworkPreset precedent (deploy/apps.ts:25,130). Reading build_config.buildDirectory back is consistent with the write path.
• normalizeBuildDirectory behavior is directly unit-tested: ""/"."→"", "apps/web"/"./apps/web/"→"apps/web", backslash normalization "apps\\web"→"apps/web", ".."-escapes ("../private", "apps/../..", "..\\private")→"", and the deliberate non-escape "..app"→"..app". I re-ran the suite locally: all 9 cases pass.
• --config path reads context.config, which is discovered and read at the top of every action via discoverConfig (config.ts) long before publish, so an unreadable path fails earlier; the key is normalized to ${prefix}deno.json with the .jsonc key deleted, so exactly one config is sent.
• No --json envelope pollution. The new mod.ts warning is gated by !options.json; the flow.ts warnings live in createFlow, which calls requireInteractive() first and so cannot run under --json.
• Stream / concurrency. Awaiting apps.get before draining the counter tee does not deadlock (the stream is lazy) and does not change the pre-existing tee() buffering. No new race.
• insideRoot guard is defensive-redundant (since normalizeBuildDirectory already strips .., appDir can never escape rootPath), but harmless; Windows separators are handled consistently by relative()/SEPARATOR.

---

Non-blocking finding

Silent fall-back to the repo root if apps.get ever omits build_config.buildDirectory, with no end-to-end coverage (deploy/publish.ts:56-57).

Intended behavior: for an app whose stored build directory is, say, apps/web, the CLI must send apps/web/deno.json so the console applies that directory's deploy section.

The concern: the server's apps.get response is a projection, and the only typed consumer in this repo, AppDetail (deploy/apps.ts:25), lists just build_config.frameworkPreset — not buildDirectory. publish.ts reads the field via a loose cast as { build_config?: { buildDirectory?: string } } followed by ?? "". If the server's serializer does not actually project buildDirectory into the apps.get response (or renames it to snake_case), the value is undefined, appDir collapses to "", prefix becomes "", and the CLI captures/reads the repo-root config instead of the app directory's. For every non-root (monorepo) app the feature would then silently no-op — the console never receives the app directory's deno.json, and no error is surfaced because the ?? "" masks the absence.

Why I am not treating this as a blocker: I found no evidence the contract is actually broken. The author controls both sides; normalizeBuildDirectory is explicitly written "as the console does," and a dedicated commit ("ignore a stored build directory that leaves the deploy root") only makes sense if apps.get returns that stored directory — so the field is clearly expected to be present. CI is green. The risk is a latent one that would bite only if the server projection drifts.

Suggestions: (a) make the absence explicit rather than silent — e.g. log/debug when build_config is present but buildDirectory is missing, so a future serializer change surfaces instead of silently deploying the wrong config; and (b) add a test that exercises a non-root appDir end to end (that the correct ${prefix}deno.json key is produced, ideally pinned to the exact response field name the server returns). The current tests/deno_json_files.test.ts covers normalizeBuildDirectory in isolation but nothing wires apps.get → appDir → captured key.

Related, lower-confidence: normalizeBuildDirectory maps any .. segment to root, so a stored "a/../b" becomes "" rather than "b". The comment asserts the console does the same ("any ..-escape to root"), which would make this consistent; I cannot confirm console behavior from this repo. Exotic (stored directories are normally clean like apps/web), so low risk — a targeted test would settle it if the console source is reachable.

Minor nit (not a bug)

The --config branch (publish.ts:139-140) has no try/catch around Deno.readTextFile(context.config), unlike the else branch which gracefully ignores Deno.errors.NotFound. In practice context.config was already read successfully by discoverConfig earlier, so this throws only in a narrow TOCTOU window (file deleted/permission-changed mid-run), yielding a raw stack trace instead of a graceful error. A one-line catch for symmetry would be nice but is not required.

---

Recommendation: No blocking issues. Posting as a comment given the (unconfirmed-but-plausible) silent-fallback robustness concern and the absence of end-to-end coverage for a non-root app directory; neither rises to a true blocker against a green CI with the author controlling both sides of the contract.

[avocet-bot]

Review (re-review) — #146 feat(deploy): send deno.json to the console on every deploy

Reviewed head SHA: ca25625e430542f9307872176e982821fe143a2f
Status: Not a draft; CI green (deno success, jsr success). No blocking issues. One residual non-blocking note carried over from the prior pass.

This head adds a single commit on top of the version reviewed earlier (e794f13): refactor(deploy): type the apps.get build directory through AppDetail. The rest of the PR is unchanged from that earlier full review, so this write-up focuses on the delta and restates the standing assessment.

Review skill evidence

Produced by invoking the Claude Code pr-review-toolkit:code-reviewer subagent (via the Task tool), scoped to the two-file delta (git diff e794f13..HEAD), plus my own trace. I re-ran the unit test locally (deno test --no-check -A tests/deno_json_files.test.ts → 1 passed) and confirmed the only type-check error is the unrelated missing generated wasm artifact (lib/rs_lib.js, a build step).

---

What this PR does (background for a new reader)

deploy-cli is the deno deploy command-line tool. It tars and hashes your project files, hands a manifest to the Deno Deploy console (the backend) via a tRPC call apps.initiateCliRevision, uploads the missing files, and the console builds/runs the app. A project's deno.json/deno.jsonc can carry a deploy section describing the build configuration.

The fix: previously the CLI uploaded files but never sent the contents of deno.json, so the console fell back to the build config stored at app-creation time — the deploy section only took effect once. This PR sends the app directory's deno.json/deno.jsonc on every deploy (matching how GitHub-sourced deploys already behave). To know which directory is "the app directory," publish() queries the tRPC apps.get endpoint for the app's stored build_config.buildDirectory, normalizes it (normalizeBuildDirectory, which mirrors the console's handling — dropping ./empty segments and mapping any ..-escape to the root), and captures/sends only that directory's config (with --config and excluded-from-upload fallbacks). Companion changes in deploy/create/flow.ts and deploy/create/mod.ts warn when a deno.json deploy section would override build settings passed to create.

The delta in this head

1. deploy/apps.ts — the AppDetail interface (the CLI's type for an apps.get response row) is now exported, and its build_config member gains a documented optional field buildDirectory?: string. build_config remains typed {...} | null.
2. deploy/publish.ts — imports the AppDetail type and changes the apps.get response cast from an ad-hoc inline shape ({ build_config?: { buildDirectory?: string } }) to as AppDetail.

This is a direct, good-faith response to the type-contract observation from the prior review: the buildDirectory field the CLI relies on is now declared in the canonical shared AppDetail type (the same type apps.ts already uses for apps.get), rather than an inline cast private to publish.ts.

Delta assessment — clean, no regression

• The widened type is safe at the use site. publish.ts reads fullApp.build_config?.buildDirectory ?? "". The property type moved from the old inline optional property ({...} | undefined) to AppDetail's required-but-nullable ({...} | null). The optional-chain ?. short-circuits on both null and undefined, producing string | undefined, which ?? "" closes to a string. It compiles and cannot throw.
• No breakage for the other consumer. apps.ts reads detail.build_config?.frameworkPreset ?? null and the id/slug/created_at/updated_at fields; the new buildDirectory? member is additive and optional, so nothing there changes. AppDetail has exactly two consumers (apps.ts, publish.ts) and no stale inline reader was left behind.
• Runtime behavior is byte-identical. as AppDetail is a compile-time type assertion with no runtime validation — the same pattern used for every other tRPC response in this client (data as Revision, as Promise<AppDetail>, as string, etc.). Because a cast does not execute, the runtime is identical to the prior head.

Standing (non-blocking) note, carried over

Because as AppDetail still performs no runtime validation, the behavior I flagged last time persists: if the server's apps.get projection ever omitted build_config.buildDirectory (or renamed it), fullApp.build_config?.buildDirectory ?? "" would collapse appDir to the repo root, and a non-root (monorepo) app would silently send the root deno.json instead of its app directory's — the feature would no-op with no error surfaced. This commit reduces the risk by documenting the field as part of the shared response type (a clear statement of the expected contract) but does not eliminate it, and there is still no end-to-end test exercising a non-root appDir (the test covers normalizeBuildDirectory in isolation). I continue to treat this as non-blocking: I found no evidence the contract is actually broken, the author controls both client and console, normalizeBuildDirectory is explicitly written "as the console does," and CI is green. A debug log when build_config is present but buildDirectory is absent, plus a non-root-appDir end-to-end test, would close it out.

Everything else verified in the prior full review (the --config stand-in, the excluded-config disk fallback, the insideRoot guard, --json envelope cleanliness, stream tee behavior, and the create-flow warnings) is unchanged and still holds.

---

Recommendation: No blocking issues. The delta is a correct type-safety/documentation improvement that addresses prior feedback without changing runtime behavior. Posting as a comment given the one residual non-blocking item (unvalidated runtime cast → silent root-fallback, and the still-absent non-root end-to-end test).

[avocet-bot]

Review (re-review) — #146 feat(deploy): send deno.json to the console on every deploy

Reviewed head SHA: b47aae5918619ef24e1d97a63fc586a0af6d5375
Status: Not a draft; CI green (deno success, jsr success). No merge blockers. One should-fix robustness regression and a couple of minor notes, all non-blocking — details below.

This head adds two commits on top of the previously-reviewed ca25625:

• test(deploy): cover which deno.json files a deploy sends
• fix(deploy): resolve symlinks before reading an excluded config; detect custom app directories in create

This write-up covers that delta and restates the standing assessment; the rest of the PR is unchanged from the earlier full reviews.

Review skill evidence

Produced by invoking the Claude Code pr-review-toolkit:code-reviewer subagent (via the Task tool), scoped to the ca25625..b47aae5 delta, plus my own trace. I ran the full test file locally (deno test --no-check -A tests/deno_json_files.test.ts → 6 passed) and confirmed the only type-check error is the unrelated missing generated wasm artifact (lib/rs_lib.js, a build step).

---

What this PR does (background for a new reader)

deploy-cli is the deno deploy command-line tool. It tars and hashes your project files, hands a manifest to the Deno Deploy console (backend) via a tRPC call apps.initiateCliRevision, uploads the missing files, and the console builds/runs the app. A project's deno.json/deno.jsonc can carry a deploy section describing the build configuration.

The fix: previously the CLI never sent the contents of deno.json, so the console fell back to the build config stored at app-creation time and the deploy section took effect only once. This PR sends the app directory's deno.json/deno.jsonc on every deploy (matching GitHub-sourced deploys). The CLI learns the app directory by querying apps.get for the stored build_config.buildDirectory, normalizes it, and sends only that directory's config (with a --config stand-in and an excluded-from-upload disk fallback). Companion create changes warn when a deno.json deploy section would override build settings passed to create.

The delta in this head

1. deploy/publish.ts — refactor + security hardening. The deno.json collection is extracted into a new exported pure function collectDenoJsonFiles({rootPath, buildDirectory, uploaded, configPath}); appBuildDirectory now returns the raw stored build directory and the normalization happens inside collectDenoJsonFiles. The important addition: the disk-read fallback for a config the upload excluded now resolves symlinks first — it calls Deno.realPath on both the deploy root and the candidate config path, then rejects the read when relative(root, path) equals "..", starts with "..<separator>", or is absolute. This closes a path-traversal/exfiltration hole: a deno.json symlinked (directly, or via a symlinked app directory) to somewhere outside the deploy root can no longer be read from disk and transmitted to the console.
2. deploy/create/flow.ts + mod.ts — custom-directory detection. A new exported customDirectoryBuildConfig(trpcClient, rootPath, repo, path) detects the build config for an app directory that is not a detected workspace member. For a local source it calls detectBuildConfig(...).catch(() => null); for a GitHub source it queries a new tRPC endpoint github.detectBuildConfigForRepo. Both the interactive createFlow (custom directory selection) and the non-interactive create warning path now use it, so the deno.json-precedence warning also fires for custom GitHub app directories (previously a known gap).
3. tests/deno_json_files.test.ts — expanded 14 → 145 lines. New end-to-end coverage for collectDenoJsonFiles: non-root app directory selection, the excluded-config disk read, the --config stand-in, lexical root-escape rejection (../private, ..\private), and symlink-escape rejection (both a symlinked directory and a symlinked deno.json pointing at a sibling private dir → returns {}, the secret is never read). This resolves the "no end-to-end test for a non-root app directory" gap I raised previously.

Delta assessment

Symlink containment guard — correct and well-tested. Deno.realPath(join(rootPath, appDir, name)) canonicalizes every path component, so it protects both a symlinked app directory and a symlinked config file; the subsequent relative(realPath(rootPath), path) escape check then rejects anything resolving outside the root. Because normalizeBuildDirectory already strips any .. from appDir, no lexical trick can reach the guard, and the ..${SEPARATOR} / exact-".." / isAbsolute triplet matches relative()'s platform-native output on both POSIX and Windows. The two symlink tests exercise both escape shapes and pass.

Refactor is behavior-preserving. The capture loop now collects every deno.json/.jsonc by basename into uploadedDenoJsonFiles, and collectDenoJsonFiles filters to the ${prefix}${name} keys; keys stay /-separated and consistent with the manifest, the --config override (delete .jsonc, set deno.json) and the "skip if already uploaded" guard match the prior logic, and moving normalizeBuildDirectory into collectDenoJsonFiles preserves the final normalization. No key/prefix mismatch.

---

Non-blocking finding (should-fix): asymmetric, uncaught GitHub detection query makes a warning-only path fatal

Background / intended behavior. customDirectoryBuildConfig exists only to decide whether to print the advisory DENO_JSON_PRECEDENCE_WARNING during create. Its declared contract is Promise<DetectedBuildConfig | null>, and every caller treats null as "nothing detected, don't warn." Detection here is best-effort — it must never gate whether create succeeds.

The problem (deploy/create/flow.ts:60-69). The local branch degrades failures to null via .catch(() => null), but the GitHub branch lets the github.detectBuildConfigForRepo tRPC query throw:

if (repo !== undefined) {
  return await trpcClient.query("github.detectBuildConfigForRepo", { owner, repo, path }) as DetectedBuildConfig | null;  // no .catch
}
return await detectBuildConfig(new FrameworkFileSystemReader(resolve(rootPath, path))).catch(() => null);

Why this is a regression. At ca25625, a GitHub-source custom app directory (member === undefined) evaluated to undefined with no network call on both the non-interactive path (deploy/create/mod.ts:258-272) and the interactive path (deploy/create/flow.ts:256-265) — it could not fail. This head introduces a network call on those paths. github.detectBuildConfigForRepo is a brand-new endpoint; if it is unavailable during server rollout, 404s, or transiently 5xx's, the throw unwinds to handleCliError (main.ts) and aborts the command non-zero.

Practical impact. deno deploy create --source github --owner O --repo R --app-directory some/custom/dir … (where that directory is not a detected workspace member) previously succeeded (no warning, create proceeds); now a transient failure of the detection query aborts it. The blast radius is narrow (only create, only GitHub source, only a custom directory, only on endpoint error) and the failure is loud and recoverable rather than silent — which is why I am not treating it as a merge blocker — but a best-effort warning computation should not be able to fail the command.

Suggested fix. Mirror the local branch: append .catch(() => null) to the GitHub query (or wrap both branches). Note this is intentionally different from github.detectWorkspaceForRepo (mod.ts:227, flow.ts:636), which is essential to the flow and correctly allowed to throw — here null is the right failure mode.

Minor notes (not blocking)

• TOCTOU between realPath and readTextFile (publish.ts). The fallback catches NotFound on Deno.realPath (→ skip), then calls Deno.readTextFile(path) with no try/catch. Only a file deleted in the microsecond window between the two calls would throw uncaught; non-NotFound errors (permission, IsADirectory) were equally uncaught at ca25625, so there is no regression. Negligible.
• --config branch has no containment check and no try/catch (collectDenoJsonFiles). This is byte-for-byte the prior behavior, and the path is user-explicit (the user chose --config somefile), so reading it and sending it as the app directory's stand-in is intended. Acceptable.
• Standing runtime-cast note. apps.get is still consumed via the compile-time as AppDetail cast with a ?? "" fallback (now inside appBuildDirectory), so if the server projection ever omitted build_config.buildDirectory the app directory would silently resolve to root. Unchanged from ca25625, no evidence it is broken, now additionally backed by the new end-to-end tests for non-root directories.

---

Recommendation: No merge blockers. The security hardening (symlink containment) and the new end-to-end tests are solid improvements, and the publish.ts refactor is behavior-preserving. Posting as a comment: the one real item is the asymmetric uncaught github.detectBuildConfigForRepo query in customDirectoryBuildConfig (flow.ts:60-65), which turns a best-effort create warning into a potential command failure — a narrow, loud, one-line-fixable robustness regression worth addressing but not gating merge.

[avocet-bot]

Review (re-review — APPROVE) — #146 feat(deploy): send deno.json to the console on every deploy

Reviewed head SHA: c9eac00e9d8c69ecb7eec86e7b1eb9f38bad8fa8
Status: Not a draft; CI green (deno success, jsr success). Approving — every actionable finding from the prior review rounds is resolved and I see no blocking issues.

This head adds two commits on top of the previously-reviewed b47aae5:

• fix(create): don't let the custom-directory deno.json check fail create
• fix(create): detect a custom app directory with the root's package manager

Both land in deploy/create/flow.ts/mod.ts and directly address the one should-fix item from my last review.

Review skill evidence

Produced by invoking the Claude Code pr-review-toolkit:code-reviewer subagent (via the Task tool), scoped to the b47aae5..c9eac00 delta, plus my own trace. I ran the full test file locally (deno test --no-check -A tests/deno_json_files.test.ts → 6 passed), confirmed detectPackageManager is exported by jsr:@deno/framework-detect@0.3.3 with the signature the new code relies on, and confirmed the only type-check error is the unrelated missing generated wasm artifact (lib/rs_lib.js, a build step).

---

What this PR does (background for a new reader)

deploy-cli is the deno deploy command-line tool. It tars and hashes your project files, hands a manifest to the Deno Deploy console (backend) via a tRPC call apps.initiateCliRevision, uploads the missing files, and the console builds/runs the app. A project's deno.json/deno.jsonc can carry a deploy section describing the build configuration.

The fix: the CLI never used to send the contents of deno.json, so the console fell back to the build config stored at app-creation time and the deploy section took effect only once. This PR sends the app directory's deno.json/deno.jsonc on every deploy (matching GitHub-sourced deploys). The CLI learns the app directory by querying apps.get for the stored build_config.buildDirectory, normalizes it, and sends only that directory's config — with a --config stand-in, an excluded-from-upload disk fallback, and a symlink-containment guard that prevents reading a config resolving outside the deploy root. Companion create changes print an advisory warning when a deno.json deploy section would override build settings passed to create.

The delta in this head

Both commits rewrite the customDirectoryBuildConfig helper, which detects the build config for an app directory that is not a detected workspace member. Its result feeds only advisory behavior: the DENO_JSON_PRECEDENCE_WARNING and, in the interactive flow, the "use detected build config?" prompt.

1. GitHub branch no longer makes a fallible request. Previously it queried a new tRPC endpoint (github.detectBuildConfigForRepo) with no error handling, so a transient or rollout-timing failure of that endpoint could abort create — the regression I flagged last round, since this is a warning-only computation that should never gate command success. It now simply returns null (the doc comment states the reasoning explicitly: not worth a request whose failure exits the CLI, for what is only a warning). The only effect is that a custom GitHub app directory no longer gets the advisory warning; the console still applies the deno.json deploy section on deploy regardless, so there is no functional loss.
2. Local branch is now fully defensive and more accurate. The entire detection is wrapped in a single try { … } catch { return null } (previously only the detectBuildConfig call had a .catch). It also now determines the package manager from the root lockfile via detectPackageManager(new FrameworkFileSystemReader(rootPath)) and passes it into detectBuildConfig(new FrameworkFileSystemReader(resolve(rootPath, path)), packageManager) — matching how detectWorkspace resolves a nested app's package manager from the root lockfile. This is a strict improvement over the old branch, which passed no package manager.
3. Signature cleanup. The now-unneeded trpcClient parameter is removed and both callers (flow.ts createFlow, mod.ts non-interactive) updated.

Delta assessment — clean

• Fully defensive: the only statement outside the try is if (repo !== undefined) return null, which cannot throw; every awaited call is inside the try, so nothing can throw out of customDirectoryBuildConfig. The prior-round regression is resolved.
• Type-correct: detectPackageManager returns "deno" | "npm" | "yarn" | "pnpm" | null, exactly detectBuildConfig's optional maybePackageManager? union; passing null (no root lockfile) reproduces the old no-package-manager behavior, and a non-null value only sharpens detection.
• No dangling references: both callers use the new 3-arg signature, there are no remaining references to github.detectBuildConfigForRepo anywhere, and the removed trpcClient argument leaves no orphaned imports (createTrpcClient/TRPCClient remain used elsewhere in both files; detectPackageManager/resolve/FrameworkFileSystemReader are all still used).
• Tests: the suite (including the end-to-end collectDenoJsonFiles cases and the two symlink-escape tests added earlier) is unaffected and still passes (6/6).

Standing note (non-blocking, unchanged)

apps.get is consumed via a compile-time as AppDetail cast with a ?? "" fallback in appBuildDirectory, so if the server's apps.get projection ever omitted build_config.buildDirectory the app directory would silently resolve to the repo root. This is inherent to how this client casts every tRPC response (no runtime validation anywhere), there is no evidence the contract is broken, the field is now declared in the shared AppDetail type, and the behavior is additionally covered by the new non-root end-to-end tests. I note it only for completeness; it is not actionable and not a blocker.

---

Recommendation: approve. Across this PR's reviewed heads the feature has been verified end to end, hardened against symlink-based path traversal, given comprehensive tests, and every actionable finding I raised — most recently the uncaught custom-directory GitHub detection that could abort create — has been cleanly addressed. CI is green and there are no outstanding blocking or actionable non-blocking issues.