feat: deploy a source directory to insta-compute via the archive lane

[CarmenDou]

What

insta deploy <dir> stops assuming flyctl. It asks the platform which lane serves the target, and gains an archive lane that packs the directory, uploads it and lets the build gateway build it. That is what makes a directory deploy work against insta-compute, with or without a Dockerfile.

legacy (404) / flyctl / local-docker -> today's deploy-token path, unchanged
archive                              -> pack, upload, deploy
none                                 -> the platform's own refusal, verbatim

Depends on InsForge/insta-platform#397. Against today's platform, discovery 404s and every existing deploy behaves exactly as it does now.

How

The packer is written from scratch and the determinism is load-bearing, not cosmetic. The digest is a content address: the platform derives the storage id from it and a governance approval binds to a body carrying it, so the same tree must hash identically on every run and machine. Entry order, mtimes, uid/gid and the exec bit are pinned in the tar, and the gzip header's own mtime and OS byte are normalised too, because zlib fills both from the environment and a fixed tar can still gzip differently on another box. The exec bit is preserved rather than normalised to 0644, which would break any entrypoint script.

Ignore semantics are decided at the file level, not by merging patterns. A .dockerignore at the archive root wins outright and .gitignore is not consulted at all; without one, .gitignore applies with git semantics including nested files. Merging the two would exclude build artefacts the image needs, the classic gitignored dist/ that the Dockerfile COPYs, and diverge from what a local docker build in the same directory produces. Railway's CLI makes the same one-file-wins choice with .railwayignore over .gitignore. The git flavour is the ignore package, the gitignore(5) implementation the eslint and prettier tooling relies on, pinned to an exact version because its verdicts decide which files enter the archive and so are part of its identity. Three hand-written rounds of that grammar each shipped a file a valid rule had withheld (** placement, []], then the POSIX named classes), which is what settled it. The docker flavour stays hand-written to moby/patternmatcher, which no package implements. Nothing in this repo was reusable for either: gitignore.ts only appends entries to a project's .gitignore, contrary to what #241 and the design doc both say.

.git and .insta are always excluded: .git alone routinely exceeds the gateway's 10,000 entry cap, and .insta is CLI state. The root Dockerfile and .dockerignore always survive a catch-all exclude, matching docker, which avoids a tree that builds locally and fails remotely with "Dockerfile not found".

No resumable state is written to disk, and that falls out of an ordering choice. The status read comes before the gated mint. Because the packer is deterministic and the storage id is derived from content, a re-run after an approval recomputes the same digest, finds the object already uploaded, skips the mint it would otherwise need a second approval for, and submits a byte-identical deploy body. The object is the state. The design called for a resumable record on disk to escape an approval loop; ordering the two reads correctly removes the loop instead, and there is no stale file to refuse to resume from.

The deploy is one gated call, and the archive lane never touches /deploy. POST /projects/:id/archive-deploys enqueues build+deploy as a single operation on the platform and answers 202 at once; the CLI then polls GET /archive-deploys/:operationId until it is live or failed. A platform request has to finish inside the ALB's 60s and an image build runs minutes, so the wait is the CLI's, one short GET at a time. With the mint that is two gates on the lane, the same as the flyctl lane has always had. An earlier round split this into three calls (submit build, poll, then /deploy with the image), which cost one user decision three approvals and rebuilt the image on every re-run so an approved deploy body could never match; the operation is idempotent on the platform on (target, archive, effective spec: build kind, paths, port, websocket, replaceSource), so a re-run after approving lands on the operation it already started, and a re-run that changed any of those is a new operation rather than a reuse of the old one's values and deploys the image its approval was granted against. Port, websocket and replaceSource ride on this call because no /deploy follows it, and a repo-connected 409 gets the same flag-naming hint. The other lanes still resolve to an image for the ordinary /deploy call; prepareSource hands back either an image or a finished deploy, and the command prints one shape whichever it got.

An upload that does not land is caught by a second status read rather than by the deploy call, whose grant is consumed in the governance preHandler.

The archive PUT goes through plain fetch, never the api client: the presigned URL carries its own signature and the platform bearer must not be sent to a bucket.

Discovery is a GET so an old platform 404s identically for a human and an agent; a POST would answer an agent 403 and break the fallback for exactly the caller this is built for.

Deltas from the design a reviewer would otherwise flag

• Symlinks are refused by path, hardlinks are not. The builder fails the whole build on a symlink entry (cmd/instaflybuilder/main.go:872) and silently dropping one is worse: the file vanishes from the context and the build fails further away. That check keys on the tar entry type, and we only ever write '0', so a hardlink arrives as an ordinary file and refusing it would reject legitimate trees.
• Directories count toward the file cap. The worker increments its counter for every header before it looks at the type (main.go:840). Counting only regular files would pass a tree the worker then rejects.
• Both the tar and the .tar.gz are format-pinned. Early rounds pinned only the tar because the compressed stream was the runtime's zlib and varied between Node and Bun. The compressor is now fflate, pure JS and pinned to an exact version, so the compressed bytes are canonical across both install channels and the digest over them is the one identity the platform stores under; a second pin test defends that, and a caret on the dependency would silently undo it.

Verify

Run end to end on staging (2026-09-11), from this branch's source against insta-platform#397 deployed to staging and insta-compute#244 merged: a directory with a Dockerfile, a .dockerignore, a node_modules/ and a .env packed to 3 files / 363 bytes (the two ignored entries absent), minted, uploaded, and deployed through one operation in 71 seconds; the URL answered 200 with the page and 404 for the ignored paths. Running the same directory again answered in 5 seconds, logging resuming the deploy this archive already started, with the same image and URL. Three real trees followed on the same platform: Excalidraw (1130 files / 34.8 MB, its own Dockerfile, yarn build then nginx) and Homepage (1466 files / 6.9 MB, Next.js) both deployed and serve; and a Dockerfile-less Node app (package.json + server.js, 3 files / 541 bytes) went through the nixpacks build kind in 3m13s and serves on the routed port, so both build kinds are proven end to end. One thing this surfaced for a follow-up, not this PR: insta build . on a Dockerfile-less directory answers verdict: failed when nixpacks is not installed locally, although the deploy itself needs no local nixpacks on insta-compute. The first attempt had failed on a staging configuration fault unrelated to this PR (a stale gateway token on the platform box), which the lane surfaced as the curated 502 and which is now fixed.

Cross-runtime identity, measured on the two channels this CLI actually ships on. The same
fixture tree, packed by the bun build --compile --minify binary and by Node 25 running the npx
path:

compiled bun binary : 315 bytes  8b9ea4c4d79068c5877ee48b4dba0686e5189cdb721f4d2bd2160599547a19c9
node:25 (npx path)  : 315 bytes  8b9ea4c4d79068c5877ee48b4dba0686e5189cdb721f4d2bd2160599547a19c9

Under node:zlib the same tree was 360 bytes on Node and 353 on Bun, with different digests. That
is why the compressor is fflate (pure JS) rather than the runtime's native zlib: this digest is the
storage id, the dedup key and part of the approval-bound deploy body, so it cannot be allowed to
change with the install channel. The compiled leg is built through the real release command, not
just interpreted, so bundling and minification are covered too. test/pack.test.ts pins the
compressed bytes of a fixed tree beside the tar-level pin, so swapping the compressor back cannot
pass quietly.

• npm run typecheck clean; npm test 947 passing, 0 failing
• 94 packer and ignore tests: determinism across mtimes/order/location, the pinned header fields, the gzip header, exec-bit preservation, archive layout, symlink refusal by path including a dangling and a nested one, hardlink acceptance and the ino/dev identity check against a same-size rename, both ignore flavours with nesting and negation, the ** placement and bracket grammar each flavour actually has (git: ** crosses only at a segment boundary, []] names ], [[:digit:]] is a POSIX class, an unclosed [ is inert; docker: any ** is an optional run of whole directories that eats a following slash, so a**/b reaches the root ab and foo**bar never reaches fooXbar, with patternmatcher's suffix and prefix fast paths matched exactly; ! is a class member) asserted on the packed file list, the exact pin of ignore, the always-excluded and always-kept sets, and all three caps naming the limit hit
• Lane dispatch tests: 404 to legacy, flyctl, archive with both build types, none refusing with the platform's reason, and the discovered caps being enforced
• Upload ordering tests: the skip when the object is already there, the mint-upload-recheck path, stopping on a pending approval without uploading, and failing loudly when the object is still missing afterwards
• Symlink tests skip on Windows, where creating one needs elevation; CI runs a windows job
• Offline cross-repo interop: an archive from this packer extracts through insta-compute's extractArchive with the exec bit, an empty directory and uid/gid/mtime all zero, and the entry count and digest match what this reports

Not covered

No end-to-end run yet: it needs the lane deployed to staging.

The skills/insta/cli-reference.md mirror AGENTS.md:15-17 requires is done, in
InsForge/instacloud-skills#84, which rewrites the build and deploy rows for the per-target
Dockerfile rule. It is a separate repository (the superproject's skills/ submodule), so it
cannot ride in this diff. It must NOT merge before this CLI ships: it documents behaviour that
answers "source builds are not supported on this provider yet" until then.

Merge order: InsForge/insta-compute#244, then InsForge/insta-platform#397, then this.

🤖 Generated with Claude Code

---

Summary by cubic

insta deploy <dir> now discovers the platform's source-build lane instead of always using the deploy-token path. On insta-compute it uploads a deterministic source archive and waits for a gateway-built image from a Dockerfile or nixpacks; Fly-backed and older platforms keep their existing behavior. A discovery 404 naming a missing compute group now reports the missing service and hints at --group when the project has other groups, or insta services add compute when it has none; only a genuine route-not-found still means legacy.

Archive handling

• Pins fflate and ignore exactly so archive bytes and Git ignore verdicts are identical across Node and Bun, with case sensitivity forced on.
• Applies a root .dockerignore with docker semantics or Git-style .gitignore with ignore package semantics; docker ** matches whole directory runs and negated classes can match the path separator itself, git ** crosses only at segment boundaries.
• Always excludes .git and .insta; preserves the root Dockerfile and .dockerignore; rejects symlinks and keeps executable bits (Windows-packed trees lose them, with a warning).
• Enforces archive, extracted-size, entry-count, and positive-safe-integer limits.
• Uploads through a presigned URL without sending the platform bearer token.
• Checks upload status before minting approval, avoids resumable disk state, and polls the archive-deploy operation up to its 30-minute deadline.
• Each poll carries an AbortSignal for the time remaining so a stalled endpoint cannot hang the CLI.
• Validates lane, upload, operation responses, and --port before packing; a failed operation's message is used only when it's a non-empty string, and progress output stays off stdout in --json mode.

Migration
Requires InsForge/insta-compute#244, then InsForge/insta-platform#397, before this PR. Coverage includes lane fallback, archive packing and ignore behavior, approval recovery, polling, limits, response validation, and symlink-swap protection.

^{Written for commit f55fe8c. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/pack.ts">

<violation number="1" location="src/pack.ts:33">
P2: On native Windows, `Stats.mode` does not preserve Git's POSIX executable bit, so checked-out executable files are packed as 0644. A Docker `ENTRYPOINT` script copied without a later `chmod` then fails in the remote build; obtain executable metadata from Git for tracked files and define a fallback for untracked files.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: On native Windows, Stats.mode does not preserve Git's POSIX executable bit, so checked-out executable files are packed as 0644. A Docker ENTRYPOINT script copied without a later chmod then fails in the remote build; obtain executable metadata from Git for tracked files and define a fallback for untracked files.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack.ts, line 33:

<comment>On native Windows, `Stats.mode` does not preserve Git's POSIX executable bit, so checked-out executable files are packed as 0644. A Docker `ENTRYPOINT` script copied without a later `chmod` then fails in the remote build; obtain executable metadata from Git for tracked files and define a fallback for untracked files.</comment>

<file context>
@@ -0,0 +1,198 @@
+const PAD = Buffer.alloc(BLOCK, 0)
+
+// Only the exec bit matters: normalising to 0644 breaks entrypoints, raw mode leaks the umask.
+const fileMode = (mode: number): number => (mode & 0o111 ? 0o755 : 0o644)
+
+const octal = (n: number, width: number): string => n.toString(8).padStart(width - 1, '0') + '\0'
</file context>

[jwfing]

Summary
The implementation is directionally sound, but I found two Critical issues that should be fixed before merge.

Requirements Context
I used the PR description as the primary intent: insta deploy <dir> should discover the platform lane, keep legacy/fly/local-docker behavior, and add an archive lane that packs, uploads, and deploys Dockerfile or nixpacks builds. Local requirements came from AGENTS.md, .claude/skills/developing-insta-cli/SKILL.md, and README deploy docs; I did not find a local design doc or skills/insta/cli-reference.md in this checkout. I also checked Docker’s build-context docs for .dockerignore behavior: https://docs.docker.com/build/concepts/context/#dockerignore-files.

Findings

Critical

• Required command-surface docs/help are still on the old contract. The PR adds archive-lane directory deploys, including Dockerfile-less nixpacks archives, in src/commands/deploy.ts:53-76. However, the repo explicitly requires command/flag surface changes to be mirrored in the agent-facing reference (AGENTS.md:15-17, .claude/skills/developing-insta-cli/SKILL.md:36-38), and the local user-facing surfaces still say insta deploy <dir> needs a Dockerfile or is built on Fly (README.md:73-77, src/index.ts:208-215, src/commands/build.ts:173-190, install.sh:286-289). This will mislead both humans and agents after the new archive lane lands; update the required skills/insta/cli-reference.md in the superproject and align the local help/README/build/install guidance and tests with lane-dependent behavior.
• The content address is not canonical across gzip encoders. packDirectory canonicalizes the tar, then gzips it and computes sha256 over the gzip bytes (src/pack.ts:188-199), while the format-pin test intentionally hashes only the uncompressed tar (test/pack.test.ts:81-99). That can pass while different supported runtimes or zlib/Bun versions produce different compressed streams for the same tree, breaking the PR’s stated load-bearing requirement that the same tree hash identically across runs/machines. Hash the canonical tar bytes, or make the compressed stream itself part of the pinned cross-runtime contract and test it accordingly.

Suggestion

• Add a top-level deploy() archive-lane test. Current tests cover prepareSource dispatch and uploadArchive ordering (test/deploy-lane.test.ts:39-89, test/deploy-archive.test.ts:40-90), but nothing exercises the full deploy() path that posts the final archive body and formats --json output (src/commands/deploy.ts:130-139).
• Consider avoiding whole-archive buffering for max-size inputs. The packer can accept up to 1 GiB extracted / 256 MiB compressed by default (src/pack.ts:12-15), but it builds all tar chunks, concatenates them, gzips synchronously, and then uploads a full Buffer (src/pack.ts:153-199, src/deploy-archive.ts:20-24). This is probably acceptable for small projects, but near the documented cap it can spike memory and block the CLI for a long time.

Information

• Security review: no new dependency or obvious auth-token leak found. The presigned archive upload correctly uses plain fetch rather than the API client, so the platform bearer is not sent to the bucket (src/deploy-archive.ts:18-24), and package.json:44-53 shows no new dependencies.
• Verification: I did not run npm run typecheck or npm test because the review instructions were read-only; AGENTS.md:15-15 lists that pre-commit gate.

Verdict
Request changes due to the Critical findings above.

[cubic-dev-ai]

All reported issues were addressed across 11 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary

The archive deployment flow is thoughtfully decomposed and tested, but its determinism, ignore fidelity, and documentation currently fall short of stated requirements.

Requirements context

I assessed the change against the PR description, the user-facing behavior documented in README.md:74-80, the command surface in src/index.ts:213-225, and the repository requirements in AGENTS.md:8-17 and .claude/skills/developing-insta-cli/SKILL.md. No local design document was found, and the dependent private platform/compute PRs were not accessible, so their API contract was assessed from this PR's description and tests.

Findings

Critical

1. The archive identity is not deterministic across supported runtimes, contrary to the approval/resume requirement. The digest is taken over runtime-specific gzip output, and the code explicitly documents that Node and Bun produce different bytes for the same tree (src/pack.ts:18-24, src/pack.ts:192-204; test/pack.test.ts:83-96). Because this digest is included in both the gated upload request and deploy body, switching between the npm/Node CLI and compiled/Bun CLI—or upgrading runtimes between approval and retry—produces a different governed action and defeats the claimed content-addressed resume behavior. Either produce canonical compressed bytes across supported runtimes or persist/reuse the approved archive and digest.

2. The custom ignore parser does not provide the claimed Git semantics and can upload files users explicitly excluded. Parsing strips all trailing whitespace and treats backslashes literally, rather than supporting Git's escaped leading #/!, escaped glob characters, and escaped trailing spaces (src/pack-ignore.ts:65-88). For example, valid rules such as \#credentials, \!secrets, or private\ will not match their intended files, so potentially sensitive content is packed and uploaded. This needs standards-compatible parsing, with regression tests for escaping, before .gitignore can safely define the upload boundary.

3. The required agent-facing CLI reference update is missing. This PR changes the documented behavior of build and deploy (src/index.ts:213-225), while the repository explicitly requires command/flag surface changes to be mirrored into skills/insta/cli-reference.md (AGENTS.md:15-17). The PR description also acknowledges that this work was not done; the matching superproject documentation update must accompany the rollout.

Suggestion

1. Avoid constructing several full in-memory copies of large build contexts. Every file is synchronously read into chunks, then copied into a contiguous tar buffer, then compressed into another buffer before the archive-size check (src/pack.ts:157-199). Near the advertised 1 GiB extracted limit this can consume well over 2 GiB and terminate the CLI even when the resulting compressed archive is valid. A streaming tar/gzip pipeline—potentially backed by a temporary file—would keep memory bounded and allow size enforcement during production.

Information

1. Security review: The presigned upload correctly uses plain fetch without forwarding the platform bearer token, and no new dependencies, SQL, or shell interpolation were introduced (src/deploy-archive.ts:19-28). The ignore-boundary issue above is the security-relevant concern.

2. Software-engineering coverage: The change includes focused tests for lane dispatch, upload ordering, tar layout, limits, links, modes, and ignore behavior (test/deploy-lane.test.ts:39-90, test/deploy-archive.test.ts:39-91, test/pack.test.ts:57-385). I could not independently execute the required typecheck/test gate because this read-only checkout has no installed dependencies (tsc: not found).

3. Performance review: Apart from the peak-memory concern above, no N+1 network pattern or hot-path repeated API fetching was found; archive upload uses a bounded discovery/status/mint/status sequence (src/deploy-archive.ts:47-73).

Verdict

Request changes. The three Critical findings must be resolved before merge.

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary

The archive lane is thoughtfully structured and extensively unit-tested, but ignore-rule correctness and an explicit documentation requirement currently block merging.

Requirements context

I assessed the change against the PR description, the updated user-facing behavior in README.md:73-79, the deploy contract expressed by the new lane/upload tests, and the repository requirements in AGENTS.md:8-17 and .claude/skills/developing-insta-cli/SKILL.md:25-38. The linked platform and compute PR contracts were not available in this checkout, so cross-repository API details were assessed from this PR's description and tests. For the claimed compatibility, I also compared the matcher against the official Docker build-context documentation and gitignore specification.

Findings

Critical

• Security / functionality — .dockerignore preprocessing is not Docker-compatible and can upload explicitly excluded files. Docker preprocesses patterns with filepath.Clean, including trimming leading whitespace and normalizing path components; this parser only strips trailing whitespace and a leading ./. For example, Docker treats a line such as secrets.env as secrets.env, while this implementation retains the leading space and archives the actual secret file. It also treats a trailing slash as directory-only even though Docker disregards leading and trailing slashes. Because .dockerignore defines the archive boundary, these differences can disclose source or credentials that users reasonably expect not to be uploaded. Use the canonical matcher or implement Docker's full preprocessing semantics and add parity tests. src/pack-ignore.ts:95-130, src/pack.ts:142-150

• Functionality — trailing /** incorrectly excludes and prunes the parent directory, defeating valid Git negations. Translating abc/** to ^abc(?:/.*)?$ makes the rule match abc itself, whereas Git specifies that trailing /** matches everything inside that directory. With abc/** followed by !abc/keep.txt, Git can retain keep.txt because abc remains traversable; this implementation marks abc excluded and unconditionally prunes it for Git, so the re-included build input disappears from the archive. src/pack-ignore.ts:23-28, src/pack-ignore.ts:150-153, src/pack.ts:130-134

• Software engineering — the mandatory agent-facing command reference was not updated. This PR materially changes the behavior and help contract of insta deploy <dir>, while the repository explicitly requires command/flag changes to be mirrored into skills/insta/cli-reference.md; the PR description also acknowledges that this remains undone. Land the corresponding reference update as required before merging. AGENTS.md:15-17, src/index.ts:213-225

Suggestion

• Performance — packing near the server limits can require several times the source size in memory. Every file buffer is retained, then copied into a complete tar, synchronously compressed, and copied again into a Buffer. A valid tree near the 1 GiB extracted limit can therefore consume multiple GiB and block the event loop or terminate the CLI before upload. Consider streaming tar/gzip output to a temporary file while hashing and enforcing the compressed-size limit. src/pack.ts:162-205

Information

• Test coverage is otherwise strong. The suite covers lane dispatch, approval/upload ordering, archive determinism, header normalization, links, ignore basics, and server-provided caps; regression cases for the two matcher discrepancies above are missing. test/deploy-lane.test.ts:39-89, test/pack-ignore.test.ts:7-176, test/pack.test.ts:351-405

• Security positives: the presigned upload deliberately uses plain fetch, so the platform bearer is not forwarded to object storage, and the new dependency is integrity-pinned in the lockfile. src/deploy-archive.ts:21-27, package-lock.json:1130-1137

• I attempted the required typecheck and test command, but this read-only checkout has no installed development dependencies, so it stopped immediately with tsc: not found; I could not independently reproduce the reported passing suite. package.json:35-42

Verdict

Request changes. The ignore mismatches can change or disclose the uploaded build context, and the required CLI-reference update is explicitly outstanding.

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/pack.ts">

<violation number="1" location="src/pack.ts:9">
P2: Because this compressor produces the archive identity, a future fflate release can make npm-installed and bundled CLI builds assign different IDs to the same directory. Pin fflate to an exact version and update it deliberately when the archive format changes.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Because this compressor produces the archive identity, a future fflate release can make npm-installed and bundled CLI builds assign different IDs to the same directory. Pin fflate to an exact version and update it deliberately when the archive format changes.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack.ts, line 9:

<comment>Because this compressor produces the archive identity, a future fflate release can make npm-installed and bundled CLI builds assign different IDs to the same directory. Pin fflate to an exact version and update it deliberately when the archive format changes.</comment>

<file context>
@@ -1,7 +1,12 @@
+// output differs between the runtimes this CLI ships on -- the same tree packs to 360 bytes under
+// Node 25 and 353 under Bun -- so with it the identity of a tree changed with the install channel.
+// fflate is pure JS: same algorithm, same bytes, everywhere.
+import { gzipSync } from 'fflate'
 import { compileIgnore, type Ignore, type IgnoreFile, type Flavour } from './pack-ignore.js'
 
</file context>

[jwfing]

Summary

The archive lane is thoughtfully structured and well tested in isolation, but three blocking issues prevent it from safely satisfying the stated deployment contract.

Requirements context

I assessed the change against the supplied PR description, the updated README behavior, and the repository’s development requirements. No archive design document exists locally, and the linked private platform/compute PRs were inaccessible; the repository explicitly requires every command-surface change to update the external agent-facing CLI reference in the same change set (AGENTS.md:8-17, CONTRIBUTING.md:36-44).

Findings

Critical

• Functionality — approval recovery stops one stage too early. Every invocation unconditionally submits a new governed archive build, then later submits the separately governed final deploy. If the final deploy returns approval_required, rerunning must pass through /archive-builds again even though that build’s prior grant was already consumed. This causes another approval before the CLI can use the final-deploy approval; if the rebuilt image reference changes, the final request body changes as well and its approval cannot match. The tests cover recovery at upload mint and build submission independently, but never the complete build → final-deploy approval → rerun sequence. Resume or deduplicate the completed build before its gated POST, persist/recover its result, or combine the server-side operation so an approved rerun can reach a byte-identical final deploy. (src/deploy-archive.ts:87-117, src/commands/deploy.ts:76-86, src/commands/deploy.ts:142-147, test/deploy-archive.test.ts:95-134)

• Security / functionality — .dockerignore preprocessing is not Docker-compatible and can ship excluded secrets. The parser only removes trailing whitespace and a leading slash; it does not strip a UTF-8 BOM, trim leading whitespace, or clean path components, and it treats a trailing slash as directory-only. Docker’s parser performs BOM stripping, whitespace trimming, and path cleaning, including removing trailing slashes. Consequently, a common BOM-prefixed first rule such as secrets.env, or a cleaned rule such as foo/../secrets.env, does not match here and the supposedly excluded file is uploaded. Because ignore rules define the upload/security boundary, this is blocking; mirror Docker preprocessing and add regression fixtures for these cases. (src/pack-ignore.ts:92-130, src/pack.ts:142-150, test/pack-ignore.test.ts:77-113)

• Software engineering — the mandatory agent-facing command reference remains stale. This PR changes build and deploy semantics and help text from “Dockerfile required” to target-dependent archive/nixpacks behavior, but does not update skills/insta/cli-reference.md; the PR description explicitly acknowledges this omission. Repository policy says a command change is only half done without that same-change-set update, and agents would continue receiving incorrect deployment instructions. (CONTRIBUTING.md:41-44, AGENTS.md:15-17, src/index.ts:213-225, README.md:73-79)

Suggestion

• Performance — avoid materializing multiple copies of contexts up to 1 GiB. The packer retains every file buffer, concatenates a complete tar, and then creates the compressed buffer synchronously. Near the advertised extracted-size limit this can require well over 2 GiB of peak memory and block the process for the entire compression, making valid large contexts prone to OOM. Consider deterministic streaming into a temporary archive while hashing, or otherwise bounding peak memory substantially below the gateway limit. (src/pack.ts:162-205)

Information

• Testing: Coverage is extensive for deterministic headers/compression, modes, links, basic ignore behavior, caps, lane dispatch, upload ordering, polling, and JSON stdout. git diff --check passes. I could not execute the required typecheck/test gate because this checkout has no installed dependencies (tsc: not found). (package.json:35-54, test/pack.test.ts:48-406, test/deploy-lane.test.ts:47-130)

• Security: The presigned upload correctly uses plain fetch rather than the authenticated API client, so the platform bearer token is not sent to object storage. The new fflate dependency is integrity-pinned in the lockfile and was not itself flagged by the package audit. (src/deploy-archive.ts:22-29, package-lock.json:1130-1137)

• Architecture/readability: The archive packing, upload/build orchestration, and command dispatch are separated cleanly, and side effects expose injected upload/build runners consistent with repository conventions. (src/commands/deploy.ts:54-87, src/deploy-archive.ts:19-46)

Verdict

Request changes because the approval flow is not reliably resumable, ignore mismatches can disclose excluded files, and an explicit command-documentation requirement is unmet.

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/deploy.ts">

<violation number="1" location="src/commands/deploy.ts:82">
P2: When the final `/deploy` request requires approval, rerunning the command submits `/archive-builds` here again and rebuilds the same archive. Reuse a completed archive build and its image ref by archive digest before starting another build, otherwise approval retries take minutes again and can produce a different deploy body.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When the final /deploy request requires approval, rerunning the command submits /archive-builds here again and rebuilds the same archive. Reuse a completed archive build and its image ref by archive digest before starting another build, otherwise approval retries take minutes again and can produce a different deploy body.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/deploy.ts, line 82:

<comment>When the final `/deploy` request requires approval, rerunning the command submits `/archive-builds` here again and rebuilds the same archive. Reuse a completed archive build and its image ref by archive digest before starting another build, otherwise approval retries take minutes again and can produce a different deploy body.</comment>

<file context>
@@ -73,7 +74,16 @@ export async function prepareSource(
+  // The wait lives HERE, not in the platform's deploy handler: a deploy is answered
+  // synchronously and the ALB cuts an idle request at 60s, while an image build runs minutes.
+  // Each poll is its own short request, so the CLI can wait as long as the build takes.
+  const built = await buildArchive(api, projectId, ref, branch, opts)
+  if (!built) return null
+  if ('failed' in built) die(built.failed)
</file context>

[jwfing]

Summary

The archive deployment flow is thoughtfully implemented, but the .gitignore matcher can construct a build context different from Git’s, violating a stated requirement.

Requirements context

I reviewed against the PR description, the per-target Dockerfile behavior documented in README.md:74-80, and the repository conventions in .claude/skills/developing-insta-cli/SKILL.md:16-34. No archive-lane design document exists in this checkout; the linked platform, compute, and skills PRs were not publicly readable from this environment, so their contracts could only be assessed through this PR’s description and implementation. I also cross-checked the claimed Git semantics against the official gitignore documentation.

Findings

Critical

• Functionality — trailing /** incorrectly excludes the directory itself and defeats valid negation. translate() turns a terminal /** into an optional suffix, so abc/** matches abc as well as its contents (src/pack-ignore.ts:28-30). Git defines this pattern as matching everything inside abc, not abc itself. Consequently, with:

  abc/**
  !abc/keep.txt

  the walker considers abc excluded and immediately prunes it because Git-mode canPrune() always returns true (src/pack-ignore.ts:180-183, src/pack.ts:130-134). Git would descend into abc and retain keep.txt. This can silently omit required build inputs and directly contradicts the PR’s explicit promise that .gitignore is applied with Git semantics. The trailing form should require the slash and descendant portion, with a regression test covering re-inclusion beneath it.

Suggestion

• Performance — packing retains several complete copies of large contexts in memory. Every file buffer remains in chunks, Buffer.concat creates another full tar, and synchronous gzip then creates the compressed representation (src/pack.ts:184-198). A valid context near the 1 GiB extracted-size limit can therefore require roughly 2 GiB or more of transient memory before upload, potentially causing an OOM despite being within advertised limits. Consider an incremental tar/gzip pipeline or a temporary-file-backed archive (src/pack.ts:162-180).

Information

• Software engineering: Coverage is extensive across deterministic packing, ignore behavior, lane selection, approval recovery, polling, and JSON output (test/pack.test.ts:55-406, test/deploy-archive.test.ts:35-165, test/deploy-lane.test.ts:47-131). The dependency binaries are absent from this read-only checkout, so npm run typecheck stopped with tsc: not found and I could not independently execute the suite; git diff --check was clean.
• Security: Apart from the build-context boundary defect above, I found no additional security issue. The presigned upload intentionally uses plain fetch, so the platform bearer token is not sent to object storage, and upload failures do not expose the signed URL (src/deploy-archive.ts:22-29).
• Performance: Polling is sequential and bounded to 30 minutes, so there is no unbounded request loop or N+1 behavior (src/deploy-archive.ts:77-116). The archive memory peak is the only performance concern identified.

Verdict

Request changes because the custom matcher violates the required Git ignore semantics and can produce an incorrect deployment archive.

[cubic-dev-ai]

1 existing issue remains and no new issues found across 10 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary

The lane dispatch, deterministic archive format, approval recovery, and presigned upload flow are well structured, but the packer does not reliably enforce its promised source-directory boundary.

Requirements context

I assessed the change primarily against the PR description: discover the target lane, preserve legacy behavior on discovery 404, deterministically package an archive using Docker/Git ignore semantics, refuse symlinks, upload without the platform bearer token, support approval-safe retries, and wait for the gateway build. The repository documentation confirms the per-target Dockerfile behavior (README.md:73-79) and the existing thin-client/DI conventions (.claude/skills/developing-insta-cli/SKILL.md:14-31). No archive-lane design document exists in this checkout, and the linked private platform/compute PRs and separate skills-repository update were not locally available.

Findings

Critical

• Security / functionality — the symlink refusal can be bypassed between scanning and reading. During the initial walk, each path is classified with lstatSync and only its pathname and metadata are retained (src/pack.ts:119-138). All regular files are then reopened later with readFileSync, which follows symlinks (src/pack.ts:184-193). A file or ancestor directory replaced after the walk can therefore make the archive contain an arbitrary file outside the requested directory, despite the explicit promise that symlinks are refused. This is both a source-boundary disclosure risk and a correctness failure under concurrent filesystem changes. Read from handles opened without following links and verify their identity/type, including protection against swapped ancestor directories; add a regression test using an injected filesystem adapter or controlled swap.

Suggestion

• Performance — archive construction has multi-gigabyte peak-memory potential. Every file buffer is retained, then copied into a full tar buffer, and the full tar is synchronously compressed into another buffer before the 256 MiB compressed-size limit is checked (src/pack.ts:184-205). With the advertised 1 GiB extracted limit, peak memory can substantially exceed 2 GiB and block the CLI event loop for the entire compression. Consider streaming tar and gzip output into a temporary spool while hashing/counting it, then upload that spool after minting the presigned URL.

• Functionality — validate lane and build responses before dispatching. Discovery casts an arbitrary response to Lane, while every value other than archive or none silently enters the legacy/Fly path (src/commands/deploy.ts:43-68). Likewise, upload/build fields such as uploadUrl, buildId, and imageRef are consumed without runtime checks (src/deploy-archive.ts:55-72, src/deploy-archive.ts:97-114). Rejecting malformed or unknown contract responses would prevent server drift from triggering the wrong build lane or submitting an empty image.

Information

• Software engineering: Coverage is extensive and behavior-oriented: determinism, tar metadata, ignore precedence and negation, link handling, caps, lane dispatch, approvals, polling, and JSON-output isolation are all exercised (test/pack.test.ts:52-422, test/pack-ignore.test.ts:7-247, test/deploy-archive.test.ts:31-165, test/deploy-lane.test.ts:45-131). Dependency injection follows the repository convention, and git diff --check passed.

• Verification: The mandated npm run typecheck and npm test gate (AGENTS.md:15) could not be rerun because this read-only checkout has no installed typescript or vitest; both commands failed with “not found.”

• Security: Apart from the filesystem-boundary issue above, no additional token or dependency concern was found. The bucket upload correctly uses plain fetch without the API client’s bearer header (src/deploy-archive.ts:22-29), and fflate is integrity-locked in package-lock.json:1130-1136.

Verdict

Request changes because the archive can escape the validated directory through a symlink/ancestor replacement race.

[jwfing]

Summary

The archive lane is thoughtfully implemented, but its cross-install determinism and response-validation guarantees are not yet upheld.

Requirements context

I assessed the change against the PR description: legacy fallback on discovery 404, target-selected lane dispatch, deterministic bounded archives, Docker/git-compatible exclusions, approval-safe upload ordering, gateway build polling, and an unchanged final image deploy. No additional archive-lane design or linked-issue requirements are present in this repository; the development guide primarily adds the existing typecheck/test and CLI-reference requirements.

Findings

Critical

1. The compressor is not pinned for the npm/npx distribution channel. fflate is declared as ^0.8.3, so consumers installing the published package do not use this repository's lockfile and may resolve a later 0.8.x release. A patch release is allowed to change the valid gzip encoding without changing the decompressed tar, causing the same CLI release and source tree to produce different archive digests than the compiled binary. That directly breaks the PR's load-bearing cross-channel identity and can invalidate approval-bound reruns. Use an exact dependency version for the compressor (and keep the lockfile aligned). (package.json:44-48)

2. API responses are cast rather than fully validated, and unknown build states are treated as active for 30 minutes. Discovery checks only the lane tag, then trusts archive.limits and none.reason; malformed limits can silently fall back to local defaults or be coerced during comparisons. Likewise, polling treats a missing or unknown state exactly like building, contradicting the stated behavior that missing/unknown response fields fail clearly. Validate each discriminated response's required fields and explicitly accept only the platform's pending states before polling again. (src/commands/deploy.ts:43-56, src/commands/deploy.ts:73-84, src/deploy-archive.ts:110-127)

Suggestion

1. Reduce peak memory for large permitted contexts. Every file is retained in chunks, then copied into a complete tar, then synchronously compressed into another complete buffer. Near the advertised 1 GiB extracted limit this can require well over 2 GiB plus compressor working memory and block the event loop, making otherwise valid deployments prone to OOM. Consider streaming into a bounded temporary archive while hashing, or lower the client limit to a memory-safe value. (src/pack.ts:231-252)

Information

1. Software engineering: Coverage is extensive for dispatch, approval recovery, deterministic packing, limits, ignore behavior, filesystem swaps, JSON output, and polling. git diff --check passes. I could not execute the required test/typecheck gates because this read-only checkout has no installed dependencies (vitest and tsc are absent), and installing them would mutate the workspace. (test/deploy-lane.test.ts:47-130, test/deploy-archive.test.ts:35-164, test/pack.test.ts:51-474)

2. Security: No additional security finding beyond the correctness issues above. The presigned upload correctly uses plain fetch rather than the authenticated API client, the platform bearer is not forwarded to storage, and the new dependency is integrity-locked and has no transitive runtime dependencies. (src/deploy-archive.ts:22-29, package-lock.json:1129-1137)

3. Performance: Polling is bounded and sequential, so there is no unbounded request loop or N+1 behavior; the archive memory amplification is the only material performance concern found. (src/deploy-archive.ts:84-89, src/deploy-archive.ts:110-128)

Verdict

Request changes because the unpinned compressor breaks an explicit determinism requirement and external response validation is incomplete.

[cubic-dev-ai]

1 issue found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/deploy-archive.ts">

<violation number="1" location="src/deploy-archive.ts:121">
P3: The new fail-fast branch that throws on an absent or unknown build state has no test coverage, while every other branch of buildArchive (succeeded, building, failed, deadline) is tested. Add a case where the poll returns a 2xx body without a `state` (and one with an unexpected state string) asserting the error is thrown on the first poll, so a regression to re-trusting unknown states isn't silently reintroduced.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new fail-fast branch that throws on an absent or unknown build state has no test coverage, while every other branch of buildArchive (succeeded, building, failed, deadline) is tested. Add a case where the poll returns a 2xx body without a state (and one with an unexpected state string) asserting the error is thrown on the first poll, so a regression to re-trusting unknown states isn't silently reintroduced.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/deploy-archive.ts, line 121:

<comment>The new fail-fast branch that throws on an absent or unknown build state has no test coverage, while every other branch of buildArchive (succeeded, building, failed, deadline) is tested. Add a case where the poll returns a 2xx body without a `state` (and one with an unexpected state string) asserting the error is thrown on the first poll, so a regression to re-trusting unknown states isn't silently reintroduced.</comment>

<file context>
@@ -114,6 +114,13 @@ export async function buildArchive(
+    // state as "still building" meant a contract change, or a truncated response, spent the full
+    // deadline before saying anything -- half an hour of a spinner for a fault visible on the
+    // first poll.
+    if (state !== 'succeeded' && state !== 'building') {
+      throw new Error(`the platform reported an unknown build state (${JSON.stringify(state)}) — upgrade with \`insta upgrade\``)
+    }
</file context>

[cubic-dev-ai]

All reported issues were addressed across 10 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/deploy-archive.ts">

<violation number="1" location="src/deploy-archive.ts:116">
P2: When an archive deploy receives `--port abc`, `0`, or a value above `65535`, this request sends `null` or an out-of-range number to the platform. Validate the deploy port with the existing `1..65535` rule before constructing the archive request so invalid input fails locally rather than producing a rejected or misconfigured deploy.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When an archive deploy receives --port abc, 0, or a value above 65535, this request sends null or an out-of-range number to the platform. Validate the deploy port with the existing 1..65535 rule before constructing the archive request so invalid input fails locally rather than producing a rejected or misconfigured deploy.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/deploy-archive.ts, line 116:

<comment>When an archive deploy receives `--port abc`, `0`, or a value above `65535`, this request sends `null` or an out-of-range number to the platform. Validate the deploy port with the existing `1..65535` rule before constructing the archive request so invalid input fails locally rather than producing a rejected or misconfigured deploy.</comment>

<file context>
@@ -82,47 +82,89 @@ export async function uploadArchive(
     branch,
     group: opts.group,
     archive: ref,
+    port: opts.port ? Number(opts.port) : undefined,
+    websocket: typeof opts.websocket === 'boolean' ? opts.websocket : undefined,
+    replaceSource: opts.replaceSource === true ? true : undefined,
</file context>

[jwfing]

Summary

The archive deployment flow is broadly sound, but the ignore matcher can include files that valid .gitignore rules exclude.

Requirements context

I assessed the change against the PR description’s lane contract, deterministic content-addressing requirement, ignore-file precedence and Git/Docker semantics, approval recovery, upload isolation, and operation polling, plus the updated README and repository development guidance. The linked platform, compute, and skills PRs were not available from this checkout and returned 404 externally, so their API contracts could not be independently verified.

Findings

Critical

• src/pack-ignore.ts:35-37, src/pack-ignore.ts:44-52 — The custom glob translator does not implement the Git semantics promised by the PR. It treats every ** pair as directory-crossing, whereas Git only gives ** that meaning in specific positions; it also terminates a bracket expression at the first ], so valid expressions with an initial literal ], such as []], never match. This is an upload-boundary issue: for example, after *.env, a negation such as !a**b/keep.env can incorrectly re-include a/x/b/keep.env, and a file named ] is uploaded despite a valid []] exclusion. Please use a semantics-compatible matcher or cover the remaining Git grammar, with regression tests asserting the packed file list—not just matcher internals.

Suggestion

• src/pack.ts:237-263 — Packing retains all file buffers, creates a full concatenated tar, and then compresses that tar synchronously. Near the discovered 1 GiB extracted limit this can require multiple gigabytes of memory and make an otherwise server-valid archive fail or be killed locally. Consider a deterministic streaming or temporary-spool implementation, or enforce a lower client-side limit with a clear diagnostic.

Information

• test/deploy-lane.test.ts:47-131, test/deploy-archive.test.ts:35-219, test/pack.test.ts:57-474 — Software-engineering and functionality coverage is otherwise strong: legacy fallback, both build kinds, approval ordering, response validation, polling states, deterministic bytes, modes, links, ignores, and limits are exercised. The missing glob cases above are the material gap.
• src/deploy-archive.ts:15-27, package.json:44-48 — No additional security finding: the presigned upload uses plain fetch rather than the authenticated API client, and the deterministic compressor dependency is exactly pinned.
• src/deploy-archive.ts:81-85, src/deploy-archive.ts:126-166 — Aside from packing memory, no performance concern was found in the network flow; polling is bounded to 30 minutes and uses one request every three seconds.
• package.json:35-39 — git diff --check passed. I could not execute typechecking or tests because dependencies are absent in the review checkout (tsc: not found).

Verdict

Request changes: the ignore mismatch can send files that the user explicitly excluded, so it must be corrected before merge.

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="test/pack.test.ts">

<violation number="1" location="test/pack.test.ts:500">
P2: This test now runs on Windows (the old version used `itModes`, which skips win32), and it depends on two files having distinct `ino` values via `expect(lstatSync(other).ino).not.toBe(st.ino)`. Node's `fs.stat` reports `ino: 0` for every file on Windows, so the precondition fails there and the whole test breaks on Windows CI. Restore the `itModes` guard (the inode identity check is a POSIX concept the swap test cannot express on Windows), or gate the precondition on the platform.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This test now runs on Windows (the old version used itModes, which skips win32), and it depends on two files having distinct ino values via expect(lstatSync(other).ino).not.toBe(st.ino). Node's fs.stat reports ino: 0 for every file on Windows, so the precondition fails there and the whole test breaks on Windows CI. Restore the itModes guard (the inode identity check is a POSIX concept the swap test cannot express on Windows), or gate the precondition on the platform.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At test/pack.test.ts, line 500:

<comment>This test now runs on Windows (the old version used `itModes`, which skips win32), and it depends on two files having distinct `ino` values via `expect(lstatSync(other).ino).not.toBe(st.ino)`. Node's `fs.stat` reports `ino: 0` for every file on Windows, so the precondition fails there and the whole test breaks on Windows CI. Restore the `itModes` guard (the inode identity check is a POSIX concept the swap test cannot express on Windows), or gate the precondition on the platform.</comment>

<file context>
@@ -468,15 +497,25 @@ describe('readEntry — the file read cannot be swapped out from under the walk'
   })
 
-  itModes('refuses a file swapped for a DIFFERENT regular file, which O_NOFOLLOW allows', () => {
+  it('refuses a same-size file swapped in by rename after the walk', () => {
     const dir = mk()
     const abs = join(dir, 'a.txt')
</file context>

Suggested change

	it('refuses a same-size file swapped in by rename after the walk', () => {
	itModes('refuses a same-size file swapped in by rename after the walk', () => {

[jwfing]

Summary

The archive deploy flow is well designed overall, but the custom Git ignore implementation can upload files that valid .gitignore rules exclude.

Requirements context

I assessed the change against the PR description and the per-target deployment behavior documented in README.md:74-79. The intended behavior includes lane discovery with legacy fallback, deterministic content-addressed archives, Git/Docker-compatible ignore semantics, approval-safe upload ordering, a single asynchronous archive-deploy operation, and keeping platform credentials away from presigned uploads. No archive design document exists in this repository, and the referenced platform, compute, and skills PRs were not publicly accessible from this environment, so their contracts could not be independently inspected.

Findings

Critical

• src/pack-ignore.ts:56-60, src/pack-ignore.ts:87-108 — The Git matcher does not support POSIX named character classes. Git's wildmatch implementation recognizes classes including digit, alpha, space, and others inside bracket expressions (Git source). For example, [[:digit:]].env should exclude 1.env; this parser terminates the class at the inner ], produces a different regular expression, and leaves 1.env in the archive. Because the PR explicitly promises Git semantics and the result is uploading content the author deliberately excluded, this is blocking. Please implement Git's named classes and add both matcher-level and packed-file-list regression tests.

Suggestion

• src/pack.ts:237-256 — Packing retains all per-file buffers, concatenates another complete tar, and then synchronously compresses that tar. Near the advertised 1 GiB extracted-size ceiling this can require multiple GiB of memory and may OOM on otherwise supported developer machines. Consider streaming deterministic tar/gzip output into a temporary artifact, or otherwise bounding peak memory independently of total context size.

Information

• test/deploy-lane.test.ts:55-142, test/deploy-archive.test.ts:40-218, test/pack-ignore.test.ts:181-243 — Apart from the missing POSIX-class case, coverage is strong: lane fallback and dispatch, approval recovery, upload ordering, polling states and timeout, response validation, JSON output, limits, and common Git/Docker glob behavior are exercised. The implementation also follows the repository's dependency-injection convention.
• src/deploy-archive.ts:22-29, src/commands/deploy.ts:45-76 — Security review found no additional blocker: the presigned PUT deliberately bypasses the authenticated API client, platform response variants are validated, ports are validated before upload, and fflate is integrity-locked and exactly version-pinned. The ignore mismatch above is itself security-relevant because it can cross the intended upload boundary.
• package.json:35-54 — Performance review found no N+1 or unbounded network behavior; polling has a fixed cadence and deadline. I could not independently execute the required typecheck or test suite because this read-only checkout has no installed tsc or vitest binaries; both commands stopped with “not found,” rather than reporting code/test failures.

Verdict

Request changes: the .gitignore compatibility gap is a Critical correctness and data-boundary issue.

[cubic-dev-ai]

1 existing issue remains and no new issues found across 10 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 10 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary

The archive lane is thoughtfully implemented, but its .dockerignore matcher diverges from Moby in a way that can change—and broaden—the uploaded build context.

Requirements context

I assessed the change against the PR description’s lane-discovery, deterministic archive, Moby-compatible .dockerignore, approval-resumption, and bearer-free upload requirements. The repository README confirms the per-target Dockerfile behavior (README.md:73-79), while the development guide establishes the DI conventions and typecheck/test gates (.claude/skills/developing-insta-cli/SKILL.md:8-20); no archive-lane design document exists in this checkout, so the detailed protocol contract comes from the PR description.

Findings

Critical

• src/pack-ignore.ts:73-94 — The Docker globstar translation is not compatible with Moby for a ** followed by / when the globstar is not at a segment boundary. For example, Moby consumes the slash after any **, so a**/b compiles as an optional directory sequence and matches the root path ab; this implementation takes the fallback branch, emits .*, leaves /b mandatory, and therefore includes ab. The relevant behavior is visible in Moby’s matcher implementation. Because .dockerignore defines both the intended build context and the upload confidentiality boundary, such a path is packed and uploaded even though a local Docker build excludes it. The current Docker globstar tests cover a**b/..., **.log, and trailing /**, but not this valid form (test/pack-ignore.test.ts:221-225). Please align the translation with Moby and add regression coverage for both root and nested matches.

Suggestion

• src/pack.ts:237-263 — Packing retains every file buffer, concatenates a second full tar buffer, and then synchronously compresses it, while the accepted limits can permit a 1 GiB extracted tree and a 256 MiB compressed archive. This can consume several gigabytes and block the event loop or terminate the CLI on otherwise valid inputs. Consider streaming tar input into deterministic gzip while retaining only the final archive required for digest/mint/upload, or introduce a realistic client-side memory ceiling.

Information

• src/deploy-archive.ts:22-29, package.json:44-55 — No additional security findings: the presigned PUT correctly bypasses the authenticated API client, and both new runtime dependencies are exact-version pinned.
• src/deploy-archive.ts:40-78, src/deploy-archive.ts:102-170 — Upload ordering, approval recovery, response validation, and bounded polling are clearly separated and covered through injected side-effect seams, consistent with repository conventions.
• package.json:35-40 — git diff --check main...HEAD passed. I could not execute typecheck or tests because dependencies are absent in this read-only checkout (npm run typecheck failed with tsc: not found), and installing them would modify the workspace.

Verdict

Request changes: the .dockerignore mismatch is blocking because it can upload source files that the author explicitly excluded and produce a different context from Docker.

[jwfing]

Summary

The archive lane is generally well designed, but its .dockerignore implementation has a blocking incompatibility with Docker’s matcher.

Requirements context

I assessed the change against the PR description’s requirements: backward-compatible lane discovery, deterministic archives, Docker/Git ignore parity, approval-safe upload ordering, unauthenticated presigned uploads, and a single polled archive-deploy operation. The repository README documents the target-dependent Dockerfile behavior (README.md:74-80), while the development guide establishes the command architecture and verification gates (.claude/skills/developing-insta-cli/SKILL.md:8-27). No archive-lane design document is present in this checkout, so protocol details were assessed against the PR description; the dependent platform/compute implementations are outside this repository.

Findings

Critical

• src/pack-ignore.ts:73-96, test/pack-ignore.test.ts:221-224 — Internal ** patterns are translated too broadly. When ** appears inside a pattern, is not followed by /, and is not terminal, this implementation emits .*; Moby emits an optional sequence ending in a separator, (.*\/)? (reference implementation). Consequently, a pattern such as foo**bar incorrectly excludes fooXbar, whereas Docker includes that root file; Docker instead matches foobar and paths such as foo/x/bar. A Dockerfile that copies fooXbar can therefore build locally but fail through this archive lane because the file was omitted. Correct the translation and add regression assertions for both the non-matching fooXbar and matching foo/x/bar cases.

Suggestion

• src/pack.ts:237-256 — Packing retains every file buffer, concatenates a second full tar buffer, and then allocates compressed output plus compressor working memory. With discovered extracted limits potentially reaching 1 GiB, peak memory can be several GiB and may terminate the CLI on otherwise valid contexts. Consider spooling a canonical archive to a temporary file or introducing a streaming/two-pass design; synchronous execution itself is acceptable for this one-shot CLI, but the allocation profile is risky near the advertised limits.

• src/deploy-archive.ts:126-129, src/deploy-archive.ts:160-169 — The 30-minute deadline does not bound wall-clock waiting: it is checked only after each GET returns, and an individual request has no abort signal. A stalled endpoint can therefore hang indefinitely, while a live response received after the deadline is accepted before the deadline check. Bound each request by the remaining deadline if “up to 30 minutes” is intended as a real ceiling.

• src/commands/deploy.ts:92-94, test/deploy-lane.test.ts:50-67 — Lane tests cover legacy 404 and explicit flyctl, but not the accepted local-docker tag despite the PR description claiming all dispatch variants are covered. Add a case pinning its intended deploy-token/501 fallback behavior.

Information

• Software engineering: the new tests otherwise cover deterministic tar/gzip identity, modes, symlink handling, limits, approval recovery, upload ordering, operation states, and JSON-output discipline (test/pack.test.ts:57-140, test/deploy-archive.test.ts:40-96). git diff --check passed. I could not execute the required typecheck or test suite because this checkout has no installed development dependencies; the commands stopped with tsc: not found and vitest: not found (package.json:35-42).

• Security: no new SQL path or shell interpolation was introduced, the presigned upload deliberately uses plain fetch without the platform bearer (src/deploy-archive.ts:22-29), and both identity-sensitive dependencies are exactly pinned (package.json:44-49). I found no auth weakening or secret/PII logging issue.

• Performance beyond the buffering concern above: polling is sequential and fixed at one request every three seconds, so there is no N+1 or uncontrolled concurrency issue (src/deploy-archive.ts:126-169).

Verdict

Request changes because the Docker ignore mismatch can produce a build context that differs from a local Docker build, violating a core requirement of the archive lane.

[cubic-dev-ai]

1 issue found across 10 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/pack-ignore.ts">

<violation number="1" location="src/pack-ignore.ts:125">
P2: Docker `.dockerignore` ranges such as `[a-c].env` currently escape `-`, so they match only `a`, `-`, or `c` and upload excluded files such as `b.env`. Remove `-` from the characters escaped by `escapeInClass`.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Docker .dockerignore ranges such as [a-c].env currently escape -, so they match only a, -, or c and upload excluded files such as b.env. Remove - from the characters escaped by escapeInClass.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack-ignore.ts, line 125:

<comment>Docker `.dockerignore` ranges such as `[a-c].env` currently escape `-`, so they match only `a`, `-`, or `c` and upload excluded files such as `b.env`. Remove `-` from the characters escaped by `escapeInClass`.</comment>

<file context>
@@ -64,6 +121,38 @@ function translate(p: string): string {
 }
 
+// Inside a class only these carry regex meaning; `-` is left alone so a range stays a range.
+const escapeInClass = (c: string): string => (/[\\\]\[^-]/.test(c) ? '\\' + c : c)
+
+// A bracket expression starting at p[start], or null when no `]` closes it and the `[` is a
</file context>

Suggested change

	const escapeInClass = (c: string): string => (/[\\\]\[^-]/.test(c) ? '\\' + c : c)
	const escapeInClass = (c: string): string => (c === '\\' || c === ']' || c === '[' || c === '^' ? '\\' + c : c)

[cubic-dev-ai]

1 issue found across 11 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/deploy-archive.ts">

<violation number="1" location="src/deploy-archive.ts:138">
P2: When the access token expires during this status GET, the client's 401 refresh path ignores the poll signal, so a stalled `/auth/refresh` can hang the deploy despite the 20-second timeout. Pass the signal through the refresh request or otherwise bound token refresh before relying on this poll deadline.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When the access token expires during this status GET, the client's 401 refresh path ignores the poll signal, so a stalled /auth/refresh can hang the deploy despite the 20-second timeout. Pass the signal through the refresh request or otherwise bound token refresh before relying on this poll deadline.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/deploy-archive.ts, line 138:

<comment>When the access token expires during this status GET, the client's 401 refresh path ignores the poll signal, so a stalled `/auth/refresh` can hang the deploy despite the 20-second timeout. Pass the signal through the refresh request or otherwise bound token refresh before relying on this poll deadline.</comment>

<file context>
@@ -82,48 +82,101 @@ export async function uploadArchive(
+    // past it, and an answer that would arrive after it is not waited for.
+    const remaining = deadline - now()
+    if (remaining <= 0) throw overdue()
+    const res = await api.rawRequest('GET', `/projects/${projectId}/archive-deploys/${encodeURIComponent(operationId)}`, undefined, {
+      signal: AbortSignal.timeout(Math.min(remaining, POLL_REQUEST_TIMEOUT_MS)),
+    }).catch((e) => {
</file context>

[jwfing]

Summary

The archive lane is thoughtfully designed and extensively tested, but two boundary bugs violate its security and timeout guarantees.

Requirements context

I assessed the implementation against the PR description’s archive-lane, deterministic packing, Docker/Git ignore parity, approval recovery, and bounded-polling requirements, plus the documented per-target behavior in README.md:73-79. The linked insta-platform and insta-compute PRs were not accessible from this environment, so their contracts could not be independently checked. The separate CLI-reference update described by the author is also outside this checkout.

Findings

Critical

• Security / functionality — Docker ? matching can upload a file Docker excludes. The translator emits [^/] for ?, but constructs the JavaScript regex without the Unicode flag. JavaScript therefore matches UTF-16 code units, whereas moby/patternmatcher’s Go regex matches Unicode code points. For example, Docker treats ?.env as matching 😀.env, while this matcher does not; that file consequently enters and is uploaded in the archive despite the author explicitly excluding it. This is security-sensitive because ignore verdicts define the upload boundary. Construct the regex with Unicode semantics and add an archive-level regression test using a non-BMP filename. (src/pack-ignore.ts:104-109, src/pack-ignore.ts:214-220)

• Functionality — an authentication refresh can escape the poll deadline and hang indefinitely. Each status poll supplies a deadline-bound signal, but when that request returns 401, raw() invokes refresh() without forwarding the signal. A stalled /auth/refresh request can therefore hold a 30-minute deploy forever, contrary to the PR’s explicit bounded-polling guarantee; token expiry during a long build is also a realistic path into this branch. Thread the deadline signal through refresh while ensuring abort errors remain visible to deployArchive, and cover the 401 → stalled refresh case. (src/api.ts:76-80, src/api.ts:100-109, src/deploy-archive.ts:129-142)

Suggestion

• Performance — packing can require multiple gigabytes of resident memory for an allowed archive. Every file is retained in chunks, then copied into a complete tar buffer and synchronously compressed while that tar remains resident. With the advertised 1 GiB extracted-size limit, peak memory includes the per-file buffers, the full tar, compressed output, and compressor working storage, making an otherwise valid deployment liable to OOM on typical developer machines. Consider producing the deterministic archive through a streaming or temporary-file pipeline, or enforcing a client limit consistent with the in-memory implementation. (src/pack.ts:225-256)

Information

• Software engineering: Coverage is unusually thorough across deterministic bytes, link handling, ignore behavior, lane dispatch, approvals, validation, and polling, and the new side effects use the repository’s dependency-injection convention. The missing Unicode and refresh-timeout cases above are the notable gaps. (test/pack.test.ts:50-562, test/pack-ignore.test.ts:11-398, test/deploy-archive.test.ts:31-269, test/deploy-lane.test.ts:50-155)

• Security: Apart from the ignore-boundary issue, the presigned upload correctly uses plain fetch rather than the authenticated API client, so the platform bearer token is not attached to the bucket request. Both identity-affecting dependencies are exactly pinned, and the lockfile audit reported no known production vulnerabilities. (src/deploy-archive.ts:22-29, package.json:44-50)

• Performance: No database/N+1 or unbounded remote-fetch pattern was introduced; polling has a fixed cadence and nominal deadline. Independent execution of the required typecheck and test gate was unavailable because this checkout has no installed dependencies (tsc and vitest were missing); git diff --check passed. (AGENTS.md:15-15, package.json:35-42)

Verdict

Request changes. The two Critical findings must be resolved before merge.

[cubic-dev-ai]

All reported issues were addressed across 12 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary

The archive deployment flow is thoughtfully implemented, but a Docker-ignore compatibility bug can include explicitly excluded files in the uploaded archive.

Requirements context

I assessed the change against the PR description’s archive-lane contract—deterministic packing, Moby-compatible .dockerignore behavior, Git-compatible .gitignore fallback, approval-safe retries, token-free presigned uploads, and operation polling—plus the per-target behavior documented in README.md:73-79. No additional archive design/spec exists in this checkout. The linked platform, compute, and skills PRs were not accessible, so their contracts could not be independently inspected.

Findings

Critical

• src/pack-ignore.ts:162-178 — Docker negated character classes incorrectly exclude / by emitting [^/... ]. Moby’s patternmatcher passes bracket expressions directly to Go’s regexp compiler, so / remains a valid match for a negated class. For example, Moby compiles private[^x]token such that it matches and excludes private/token, while this implementation produces ^private[^/x]token$ and uploads that file. Because .dockerignore defines the upload boundary and the PR explicitly promises Moby parity, this is both a correctness and potential data-exposure issue. Add a Docker-specific regression alongside test/pack-ignore.test.ts:319-335 and match Moby’s actual behavior.

Suggestion

• src/pack.ts:237-256 — Packing retains every file buffer, concatenates a second full tar buffer, and then synchronously compresses it. With the advertised 1 GiB extracted-size limit, peak memory can reach several GiB and valid contexts may terminate through OOM rather than receive a controlled CLI error. Consider streaming the deterministic tar/gzip pipeline or otherwise bounding peak memory.

• src/deploy-archive.ts:132-142,179-180 — Individual status requests respect the remaining deadline, but the fixed three-second sleep does not. A response received just before the deadline can therefore keep the CLI alive beyond the claimed 30-minute ceiling. Recompute the remaining time after each response and cap the wait accordingly.

Information

• Software engineering/functionality coverage is otherwise strong: lane dispatch, approval recovery, response validation, deterministic archive bytes, ignore behavior, links, caps, and JSON output all have focused tests (test/deploy-archive.test.ts:39-268, test/deploy-lane.test.ts:49-154, test/pack.test.ts:55-578).
• No additional security finding was identified: the presigned PUT deliberately avoids the authenticated API client (src/deploy-archive.ts:22-29), and archive reads defend against final-component symlink swaps (src/pack.ts:173-193). The two identity-affecting dependencies are exactly pinned (package.json:44-49).
• The configured typecheck/test gates (package.json:35-40) could not be independently executed in this checkout because dependencies were absent (tsc: not found). No files were installed or modified because this review was read-only.

Verdict

Request changes: the Docker-ignore mismatch is Critical under the stated verdict rules because it violates an explicit compatibility/security boundary. The memory and deadline items are non-blocking suggestions.

[jwfing]

LGTM - approved.

[jwfing]

Summary

Reviewed at head d9e7bb8: the three Critical findings my earlier rounds raised (the non-Unicode ? regex, the refresh escaping the poll deadline, and the Docker negated class excluding /) are all fixed with tests, and I found no new blocking issue — the remaining items are one reproducible archive-shaping bug and four smaller notes.

Requirements context

No matching spec/plan found — this repo has no /docs/superpowers/ and no docs/specs/ (find over the checkout returns neither), so I assessed against the PR title/body, AGENTS.md, and the surrounding code. The companion PRs the body names (InsForge/insta-platform#397, InsForge/insta-compute#244, InsForge/instacloud-skills#84) are not reachable from this checkout, so the wire contracts for /source-build, /build-uploads and /archive-deploys, and the stated merge order, could not be independently verified.

Gates run in a clean npm ci clone of d9e7bb8: npm run typecheck clean, npx vitest run 1015 passed / 1015, 62 files.

Standing reviews: this PR carries 12 CHANGES_REQUESTED reviews from my earlier rounds. A COMMENT does not clear those states — if they still gate the merge, they need dismissing separately. I re-checked the two most recent ones (5f4d58c, 3fea61f) against this head and confirm every Critical in them is resolved here.

---

Findings

Critical

(none)

---

Suggestion

Functionality / performance — a wildcard-leading .dockerignore negation disables pruning globally, and every excluded directory below it still gets a tar header. src/pack-ignore.ts:278-280, src/pack.ts:137-141

literalHead() returns '' for any pattern starting with *, ? or [, and canPrune treats r.literal === '' as "a negation could reach anywhere" — so a single line like !*.md makes canPrune false for every directory in the tree. walk then descends into fully-excluded trees and pushes a directory header unconditionally at src/pack.ts:140 whenever it descends.

Reproduced on this head, .dockerignore = node_modules + !*.md, tree = app.js plus node_modules/{a,b,c}/{index.js,sub/deep.js}:

entries= 9 files= 2
['.dockerignore','app.js','node_modules/','node_modules/a/','node_modules/a/sub/',
 'node_modules/b/','node_modules/b/sub/','node_modules/c/','node_modules/c/sub/']

Control — the identical tree with the !*.md line removed:

entries= 2 files= 2
['.dockerignore','app.js']

The file-level verdicts are still right (no excluded file enters), so this is not a leak. But three things follow: directories docker build omits from its context are in the archive; they count against maxFiles — the packer's own error even says "(directories count)" — so a large excluded tree can trip archive has too many files … exclude what the build does not need on a tree the user did exclude; and the walk lstats the whole excluded subtree. Emitting the directory header lazily (only once a descendant survives), and/or scoping the empty-literal case to the negation's own base, fixes both arms.

Security — one-file-wins uploads .env when .dockerignore exists and does not mention it. src/pack.ts:195-204

The root-.dockerignore-wins rule is deliberate and well argued in the PR body, and it does match docker build. The blast radius is not the same, though: a local build context never leaves the machine, whereas this archive is PUT to a presigned bucket URL and stored under its content hash for the gateway to fetch. Verified on this head — a tree with .gitignore = node_modules, .env and .dockerignore = node_modules packs to ['.dockerignore', '.env', '.gitignore', 'app.js']. That combination (a .gitignore that hides secrets plus a narrower .dockerignore) is the common repo shape. A one-line stderr warning when a credential-shaped file (.env*, *.pem, id_rsa) enters the archive that .gitignore would have withheld would cost nothing and is consistent with the windowsModeCaveat precedent at src/pack.ts:210-213.

Functionality — the archive PUT is the one network call in this lane with no bound. src/deploy-archive.ts:26-29

This PR gave a bound to everything else: each poll gets AbortSignal.timeout(min(remaining, 20s)), the refresh now threads the caller's signal (src/api.ts:100-109), and the operation has a 30-minute ceiling. defaultUpload PUTs up to maxArchiveBytes (256 MiB by default) with no signal, so a bucket that completes the TCP handshake and then stalls holds insta deploy open indefinitely with no further output. Non-blocking because a flat timeout would be wrong for a legitimately long upload and the command is interactive, but an inactivity-based bound (or a generous deadline scaled off packed.archive.length) would close the last gap in the lane's own stated guarantee.

Performance — packing is fully in-memory and the compression is synchronous. src/pack.ts:237-260

Carried over unchanged from my 3fea61f round. Every file buffer is held in chunks, copied into one full tar Buffer, and then gzipSync(level 9) runs over it — fflate is pure JS, so that is single-threaded, blocks the process, and produces no progress output while it runs. Against the advertised 1 GiB extracted cap, peak resident is roughly two full copies of the tree plus the compressor's working set. Note also that maxArchiveBytes is only checked at src/pack.ts:262 — after the whole tree has been read and compressed — so the archive-size cap cannot save a run from the cost of producing the thing it rejects. Determinism does not require holding it all: a deterministic streaming tar into fflate's streaming Gzip emits the same bytes.

Software engineering — the five discoverLane guards are reachable but unpinned. src/commands/deploy.ts:56, :65, :71

Per the reachability check rather than by reading: I drove prepareSource with five malformed discovery bodies on this head and all five guards fire with their intended sentence (unknown tag "teleport"; archive with absent limits; archive with maxArchiveBytes: 0; none with no reason; none with a whitespace-only reason). So these are live, not dead code — they are simply the only branches in the new lane with no test, and the comments beside them say each one exists because an earlier round shipped the wrong behaviour. test/deploy-lane.test.ts already parameterises the discovery body, so pinning them is a handful of lines. Same for the new --port validation at src/commands/deploy.ts:162-167: parsePort itself is covered (test/services.test.ts:35-51), but nothing asserts deploy() rejects --port 0 before packing, which is the behaviour the comment claims.

---

Information

• src/deploy-archive.ts:180 — the fixed await wait(POLL_MS) still is not capped by the remaining deadline, so the CLI can overrun its own 30-minute ceiling before throwing overdue(). I filed this as a Suggestion at 3fea61f; having measured the bound it is at most 3 s, which is cosmetic. Downgrading my own earlier call.
• src/pack.ts:220 — packDirectory against a missing or non-directory path surfaces readdirSync's raw text (verified: ENOENT: no such file or directory, scandir '/tmp/definitely-not-here-xyz'), where the flyctl lane dies with a written message. A one-line statSync check at the top of packDirectory would match the rest of the CLI's tone.
• src/commands/deploy.ts:191 — const what = source.image is a leftover alias; source.image is used directly on the line above it.
• src/pack-ignore.ts:181-186 — a .dockerignore bracket expression containing a literal [: that is not a well-formed POSIX class throws invalid POSIX class in .dockerignore and aborts the deploy. Reading Go's parseNamedClass, it returns early when no :] follows and the [ becomes an ordinary member, so this looks like a divergence in the direction of failing a pattern Moby accepts. I did not measure it against Go, and it needs a filename shape that is essentially absent on POSIX, so it is a note rather than a finding.
• AGENTS.md:15-17 requires command/flag changes to be mirrored in skills/insta/cli-reference.md; src/index.ts:214 and :221 change two command descriptions. The body names InsForge/instacloud-skills#84 as the mirror plus a merge-order constraint (it must not land before this ships). Not verifiable from this checkout — flagging only so it is not lost at merge time.

---

Dimension coverage

• Software engineering — the new modules follow the repo's conventions: injectable side effects rather than global mocks (upload, now, wait, log, run), pure helpers exported for unit test (archiveBuildSpec, deployRequestBody, windowsModeCaveat), .js ESM import specifiers throughout. Coverage is unusually dense — 94 packer/ignore tests, lane dispatch, approval recovery at both gates, response validation, deadline behaviour — and the head commit added both a matcher-level and an archive-level regression for the fix it carries (test/pack-ignore.test.ts:324-339, test/pack.test.ts:58-68). The gaps are the five guards and --port above.
• Functionality — the lane split preserves the legacy path exactly: a 404 on discovery returns {lane:'legacy'} and falls through to the unchanged buildFromSource, which test/deploy-lane.test.ts:52-59 pins. The status-read-before-mint ordering does remove the second-approval loop as claimed, and test/deploy-archive.test.ts:141-160 proves the re-run body is byte-identical by running it twice rather than by reading the code. The one real defect is the directory-emission item above.
• Security — the presigned PUT correctly goes through plain fetch and never the authed client, so the platform bearer is not sent to the bucket (src/deploy-archive.ts:22-29); discovery is a GET so agent mode gets the same 404 a human does; operationId is encodeURIComponent'd into the poll path and the discovery query goes through URLSearchParams; .git and .insta are excluded unconditionally; no token, digest or URL is logged. Both new dependencies are zero-dependency, MIT, exactly pinned in package.json:47-48 and integrity-hashed in the lockfile — npm audit reports no production advisories. I verified the ignore usage against the current docs: ignorecase is the documented option name and test() does return {ignored, unignored}, so src/pack-ignore.ts:48,56 is current API, and I confirmed empirically that ignore({ignorecase:false}) really is case-sensitive ('Foo' does not match 'foo') — the claim the cross-machine digest rests on. The only security note is the .env item above.
• Performance — no DB work and no N+1 here. Polling is a fixed 3 s cadence with a 30-minute ceiling and every request individually bounded; the walk computes the entry and extracted-size caps before a byte is read. The two costs are the in-memory/synchronous packing above and the full walk of excluded trees that the pruning bug causes.

Verdict

approved (informational — a human still gives the GitHub approval). Zero Critical findings at d9e7bb8: typecheck clean, 1015/1015 tests green, and the blocking items from my earlier rounds are fixed and pinned. The Suggestions are worth a follow-up, and the pruning/directory-emission one is the one I would actually fix.

[cubic-dev-ai]

5 issues found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="test/deploy-lane.test.ts">

<violation number="1" location="test/deploy-lane.test.ts:9">
P3: Each test leaves an `insta-lane-*` directory in the OS temp dir; nothing removes it, so the suite leaks temp dirs on every run. Wrap the prepared dir in a try/finally or register cleanup (e.g. rmSync(..., {recursive:true, force:true})) in afterEach.</violation>
</file>

<file name="src/deploy-archive.ts">

<violation number="1" location="src/deploy-archive.ts:180">
P2: When an in-flight status response arrives with less than `POLL_MS` remaining, this fixed sleep runs past the advertised 30-minute deadline and can report a timeout after the operation has already completed. Cap the sleep by the remaining deadline and check for expiry before sleeping.</violation>
</file>

<file name="src/pack.ts">

<violation number="1" location="src/pack.ts:225">
P2: When the source tree greatly exceeds `maxFiles`, `walk()` still retains every entry before this check, so a large unignored tree can exhaust CLI memory or time instead of failing at the cap. Thread the limit through `walk` and abort as soon as the entry count exceeds it.</violation>
</file>

<file name="src/pack-ignore.ts">

<violation number="1" location="src/pack-ignore.ts:48">
P2: When a `.gitignore` starts with a UTF-8 BOM, `compileGit` passes that BOM into the first pattern, so the first rule does not match and withheld files can enter the archive. Strip the BOM before calling `ignore.add`, as `compileDocker` already does.</violation>

<violation number="2" location="src/pack-ignore.ts:224">
P2: On native Windows, `.dockerignore` rules using the Windows path separator do not exclude the corresponding archive paths. Normalize Docker patterns and walked paths with the same separator semantics before translating them.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When an in-flight status response arrives with less than POLL_MS remaining, this fixed sleep runs past the advertised 30-minute deadline and can report a timeout after the operation has already completed. Cap the sleep by the remaining deadline and check for expiry before sleeping.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/deploy-archive.ts, line 180:

<comment>When an in-flight status response arrives with less than `POLL_MS` remaining, this fixed sleep runs past the advertised 30-minute deadline and can report a timeout after the operation has already completed. Cap the sleep by the remaining deadline and check for expiry before sleeping.</comment>

<file context>
@@ -0,0 +1,182 @@
+      throw new Error(`the platform reported an unknown deploy state (${JSON.stringify(state)}) — upgrade with \`insta upgrade\``)
+    }
+    if (state !== last) { log(state === 'deploying' ? 'image built, deploying it' : `${state}…`); last = state }
+    await wait(POLL_MS)
+  }
+}
</file context>

Suggested change

	await wait(POLL_MS)
	const pause = Math.min(POLL_MS, deadline - now())
	if (pause <= 0) throw overdue()
	await wait(pause)

[cubic-dev-ai]

P2: When the source tree greatly exceeds maxFiles, walk() still retains every entry before this check, so a large unignored tree can exhaust CLI memory or time instead of failing at the cap. Thread the limit through walk and abort as soon as the entry count exceeds it.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack.ts, line 225:

<comment>When the source tree greatly exceeds `maxFiles`, `walk()` still retains every entry before this check, so a large unignored tree can exhaust CLI memory or time instead of failing at the cap. Thread the limit through `walk` and abort as soon as the entry count exceeds it.</comment>

<file context>
@@ -0,0 +1,274 @@
+  found.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0))
+
+  // Known from the walk alone, so both fail before a byte is read or compressed.
+  const extractedBytes = found.reduce((n, e) => n + e.size, 0)
+  if (found.length > cap.maxFiles) {
+    throw new Error(
</file context>

[cubic-dev-ai]

P2: On native Windows, .dockerignore rules using the Windows path separator do not exclude the corresponding archive paths. Normalize Docker patterns and walked paths with the same separator semantics before translating them.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack-ignore.ts, line 224:

<comment>On native Windows, `.dockerignore` rules using the Windows path separator do not exclude the corresponding archive paths. Normalize Docker patterns and walked paths with the same separator semantics before translating them.</comment>

<file context>
@@ -0,0 +1,282 @@
+// Treating that slash as directory-only, the way git does, under-excludes exactly the shape a
+// user writes when they mean "keep this out".
+function cleanDockerPattern(pat: string): string {
+  const normalized = posix.normalize(pat)
+  const cut = normalized.length > 1 ? normalized.replace(/\/+$/, '') : normalized
+  return cut === '' ? '.' : cut
</file context>

[cubic-dev-ai]

P2: When a .gitignore starts with a UTF-8 BOM, compileGit passes that BOM into the first pattern, so the first rule does not match and withheld files can enter the archive. Strip the BOM before calling ignore.add, as compileDocker already does.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/pack-ignore.ts, line 48:

<comment>When a `.gitignore` starts with a UTF-8 BOM, `compileGit` passes that BOM into the first pattern, so the first rule does not match and withheld files can enter the archive. Strip the BOM before calling `ignore.add`, as `compileDocker` already does.</comment>

<file context>
@@ -0,0 +1,282 @@
+function compileGit(files: IgnoreFile[]): Ignore {
+  const scoped = [...files]
+    .sort((a, b) => depth(a.base) - depth(b.base))
+    .map((f) => ({ base: f.base, ig: ignore({ ignorecase: false }).add(f.text) }))
+  return {
+    excludes(relPath, isDir) {
</file context>

Suggested change

	.map((f) => ({ base: f.base, ig: ignore({ ignorecase: false }).add(f.text) }))
	.map((f) => ({ base: f.base, ig: ignore({ ignorecase: false }).add(f.text.charCodeAt(0) === 0xfeff ? f.text.slice(1) : f.text) }))

[cubic-dev-ai]

P3: Each test leaves an insta-lane-* directory in the OS temp dir; nothing removes it, so the suite leaks temp dirs on every run. Wrap the prepared dir in a try/finally or register cleanup (e.g. rmSync(..., {recursive:true, force:true})) in afterEach.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At test/deploy-lane.test.ts, line 9:

<comment>Each test leaves an `insta-lane-*` directory in the OS temp dir; nothing removes it, so the suite leaks temp dirs on every run. Wrap the prepared dir in a try/finally or register cleanup (e.g. rmSync(..., {recursive:true, force:true})) in afterEach.</comment>

<file context>
@@ -0,0 +1,168 @@
+import { ApiError } from '../src/api.js'
+import type { BuildRunner } from '../src/flyctl-build.js'
+
+function srcDir(withDockerfile = true): string {
+  const dir = mkdtempSync(join(tmpdir(), 'insta-lane-'))
+  if (withDockerfile) writeFileSync(join(dir, 'Dockerfile'), 'FROM alpine\nEXPOSE 3000\n')
</file context>