Skip to content

feat(browser): default multi-file source uploads to ZIP - #406

Open
tristanmanchester wants to merge 3 commits into
steipete:mainfrom
tristanmanchester:feat/browser-zip-default-bundle
Open

feat(browser): default multi-file source uploads to ZIP#406
tristanmanchester wants to merge 3 commits into
steipete:mainfrom
tristanmanchester:feat/browser-zip-default-bundle

Conversation

@tristanmanchester

@tristanmanchester tristanmanchester commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keep flattened text as the auto bundle format for text-only uploads, matching the previous contract
  • Use a byte-preserving ZIP when raw/native files are present, or when --browser-bundle-format zip is set, and tell ChatGPT to extract that archive into its sandbox
  • Keep a single text/source file as a native upload, and leave images, PDFs, archives, and other native attachments separate while the 10-attachment cap permits
  • Make --browser-bundle-files actually select the upload plan under default auto
  • Create fallback bundles only if ChatGPT rejects an inline paste, including on remote hosts, and delete generated oracle-browser-bundle-* directories after the run

Why

auto already had a text-only contract: flatten text uploads into one Markdown-style bundle. Switching that to ZIP would have forced existing CLI and MCP workflows to extract an archive before reading source. This branch keeps that contract and treats ZIP as the format for mixed/raw inputs and for callers that opt in.

ZIP still matters when the files are not plain text. An unexplained archive is otherwise treated as an opaque attachment, so ZIP submissions include a short extract instruction.

Bundling is still narrower than --browser-bundle-files. Images and PDFs lose their native handling inside an archive, so they stay separate unless the flag is set or the attachment cap forces a single bundle. resolveBrowserBundleScope returns none, text-only, or all.

Behavior change

--browser-bundle-format auto still chooses flattened text for text-only uploads and ZIP when raw files are present. Use --browser-bundle-format zip when you want a filesystem tree for source files. --browser-bundle-files now bypasses auto's inline path so small files are uploaded as one bundle instead of being pasted.

Review follow-up

  • Restored the established auto text-only bundle format (ClawSweeper P1)
  • Remote clients no longer materialize a fallback ZIP before the primary submit (ClawSweeper P2)
  • --browser-bundle-files now selects the upload plan under default auto
  • Generated oracle-browser-bundle-* directories are deleted after browser runs and dry-runs

Verification

  • pnpm vitest run tests/browser/prompt.test.ts tests/browser/sessionRunner.test.ts tests/browser/index.test.ts tests/remote/server.test.ts — 144 passed
  • tsc --noEmit clean

Live proof

Session zip-bundle-extract-proof on GPT-5.5 browser (--browser-attachments always, two small source files, 57.9s). That run used the ZIP path (now --browser-bundle-format zip for text-only inputs). Oracle uploaded one attachments-bundle.zip, the composer included the extract instruction, and ChatGPT quoted both relative paths and sentinels. The generated bundle directory was gone after the run.

Uploading attachment: .../oracle-browser-bundle-iwhikb/attachments-bundle.zip
Attachment queued (UI anchored, file input confirmed)
All attachments uploaded

Submitted composer:

Extract the attached bundle, report both relative paths, and quote each sentinel.

The attached `attachments-bundle.zip` contains 2 selected files with relative paths preserved. Extract it into a temporary directory, then inspect the resulting file tree with filesystem and search tools before answering.

Answer:

Extracted and inspected the bundle. The two files are:

| Relative path                      | Sentinel                        |
| ---------------------------------- | ------------------------------- |
| `oracle-zip-smoke-406/src/one.txt` | `ORACLE_ZIP_SENTINEL_ALPHA_406` |
| `oracle-zip-smoke-406/src/two.txt` | `ORACLE_ZIP_SENTINEL_BETA_406`  |

@clawsweeper

clawsweeper Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93a0687a9f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/browser/prompt.ts
Comment thread src/browser/prompt.ts Outdated
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 22, 2026
@clawsweeper

clawsweeper Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed August 29, 2026, 11:09 AM ET / 15:09 UTC.

ClawSweeper review

What this changes

This PR refines browser-mode file upload planning, preserving flattened automatic text bundles while adding ZIP handling, extraction guidance, lazy fallback creation, and bundle cleanup.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

Keep open: the final branch resolves the two prior review findings and preserves the established automatic text-bundle contract, but remote inline-fallback runs still leave a generated source bundle outside the per-run cleanup directory.

Priority: P2
Reviewed head: 0f0ff3b4d598637bbd39b4a68475c0df6d6f8a3b

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Real browser proof supports the ZIP behavior, while one contained remote lifecycle defect remains before merge.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The captured GPT-5.5 browser session exercises the changed ZIP production path from upload through ChatGPT extraction and reports the observed files and sentinels; the remote cleanup defect is a patch correction, not absent contributor proof.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The captured GPT-5.5 browser session exercises the changed ZIP production path from upload through ChatGPT extraction and reports the observed files and sentinels; the remote cleanup defect is a patch correction, not absent contributor proof.
Evidence reviewed 6 items Verified PR ownership: The host-verified introduction delta is exactly base bbc1b3b to head 0f0ff3b; the verified test merge has those exact base and head parents.
Prior compatibility finding is fixed: The planner selects the upload plan when attachments are forced, retains text bundles for text-only automatic uploads, and emits ZIP guidance only for ZIP bundles.
Remote fallback leaves generated bundle behind: The remote fallback prepare callback creates a bundle through materializeStagedFallbackBundle, whose writer creates an oracle-browser-bundle-* directory in the system temp directory. The server finalizer removes only runDir, so a bundle created after fallback is not covered by cleanup.
Findings 1 actionable finding [P2] Clean up materialized remote fallback bundles
Security Needs attention Remote fallback retains generated source bundle: The generated bundle is outside the per-request directory removed by the server finalizer, leaving selected file contents on the browser host after fallback execution.

How this fits together

Oracle turns selected local files into pasted composer text or browser attachments before automating a signed-in ChatGPT conversation. The changed planner also serializes fallback attachments to remote browser hosts, where the remote server stages them and invokes browser automation.

flowchart LR
  A[Selected local files] --> B[Browser prompt planner]
  B --> C{Inline or upload}
  C --> D[Composer text]
  C --> E[Text or ZIP bundle]
  E --> F[Local browser run]
  E --> G[Remote browser host]
  F --> H[ChatGPT submission]
  G --> H
Loading

Before merge

  • Clean up materialized remote fallback bundles (P2) - When the remote fallback is actually used, prepare() creates an oracle-browser-bundle-* directory under the system temp root. The request finalizer only removes runDir, so the generated source bundle survives; retain its path and remove it after the browser run on both success and failure.
  • Resolve security concern: Remote fallback retains generated source bundle - The generated bundle is outside the per-request directory removed by the server finalizer, leaving selected file contents on the browser host after fallback execution.
  • Resolve merge risk (P1) - A remote run whose inline prompt is rejected can retain a generated ZIP or text bundle containing selected source files in the host's system temp directory after the request completes.
  • Complete next step (P2) - A concrete P2 remote lifecycle repair and regression test can be made without changing the approved upload-policy direction.

Findings

  • [P2] Clean up materialized remote fallback bundles — src/remote/server.ts:254-264
  • [low] Remote fallback retains generated source bundle — src/remote/server.ts:256
Agent review details

Security

Needs attention: The new remote fallback path can retain uploaded source content in a generated system-temporary bundle after the request lifecycle ends.

Review metrics

Metric Value Why it matters
Implementation and coverage delta production +412/-130, tests +554/-90 The attachment-planning rewrite is substantial, so the remaining remote lifecycle path warrants direct regression coverage before merge.

Merge-risk options

Maintainer options:

  1. Clean remote fallback bundles before merge (recommended)
    Track the generated remote fallback bundle and remove its directory in the request finalizer after browser automation has finished, then test the triggered fallback path.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Clean up generated remote fallback bundle directories after success and failure, and add a regression test that invokes fallback preparation.

Technical review

Best possible solution:

Keep the compatible upload behavior, and make remote fallback bundles part of the remote request lifecycle so they are deleted after either success or failure, with a regression test that triggers preparation.

Do we have a high-confidence way to reproduce the issue?

Not applicable as an issue report; source inspection establishes the remote fallback cleanup failure, while the supplied live run demonstrates the explicit ZIP submission behavior.

Is this the best way to solve the issue?

No: the restored automatic text-bundle contract is the maintainable direction, but remote materialization must join the existing cleanup lifecycle before this implementation is complete.

Full review comments:

  • [P2] Clean up materialized remote fallback bundles — src/remote/server.ts:254-264
    When the remote fallback is actually used, prepare() creates an oracle-browser-bundle-* directory under the system temp root. The request finalizer only removes runDir, so the generated source bundle survives; retain its path and remove it after the browser run on both success and failure.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bbc1b3b0261d.

Labels

Label changes:

  • remove merge-risk: 🚨 compatibility: Current PR review merge-risk labels are merge-risk: 🚨 other.

Label justifications:

  • P2: The remaining defect affects remote fallback runs and can retain user-selected source material, but it is bounded to a less common recovery path.
  • merge-risk: 🚨 other: The new remote fallback path can leave generated temporary source bundles after a completed or failed browser run.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The captured GPT-5.5 browser session exercises the changed ZIP production path from upload through ChatGPT extraction and reports the observed files and sentinels; the remote cleanup defect is a patch correction, not absent contributor proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured GPT-5.5 browser session exercises the changed ZIP production path from upload through ChatGPT extraction and reports the observed files and sentinels; the remote cleanup defect is a patch correction, not absent contributor proof.

Evidence

Security concerns:

  • [low] Remote fallback retains generated source bundle — src/remote/server.ts:256
    The generated bundle is outside the per-request directory removed by the server finalizer, leaving selected file contents on the browser host after fallback execution.
    Confidence: 0.95

Acceptance criteria:

  • [P1] pnpm vitest run tests/remote/server.test.ts tests/browser/prompt.test.ts tests/browser/sessionRunner.test.ts tests/browser/index.test.ts.
  • [P1] tsc --noEmit.

What I checked:

  • Verified PR ownership: The host-verified introduction delta is exactly base bbc1b3b to head 0f0ff3b; the verified test merge has those exact base and head parents. (0f0ff3b4d598)
  • Prior compatibility finding is fixed: The planner selects the upload plan when attachments are forced, retains text bundles for text-only automatic uploads, and emits ZIP guidance only for ZIP bundles. (src/browser/prompt.ts:525, 0f0ff3b4d598)
  • Remote fallback leaves generated bundle behind: The remote fallback prepare callback creates a bundle through materializeStagedFallbackBundle, whose writer creates an oracle-browser-bundle-* directory in the system temp directory. The server finalizer removes only runDir, so a bundle created after fallback is not covered by cleanup. (src/remote/server.ts:256, 0f0ff3b4d598)
  • Remote coverage does not exercise cleanup: The new remote test verifies that fallback preparation is deferred, but its injected browser runner never calls fallbackSubmission.prepare(), so it cannot observe cleanup after a materialized bundle. (tests/remote/server.test.ts:166, 0f0ff3b4d598)
  • Positive live behavior proof: The captured PR body records a real GPT-5.5 browser run that uploaded one ZIP, included the extraction instruction, and returned both requested relative paths and sentinels; this directly covers the explicit ZIP submission path. (0f0ff3b4d598)
  • Current-main and release status: This head is an unmerged PR rather than current main; the local tag containment check produced no release tag containing the PR head, so it is not already shipped. (0f0ff3b4d598)

Likely related people:

  • steipete: Recent work on remote attachment staging and the bridge service is directly adjacent to the remaining cleanup path. (role: recent remote-service contributor; confidence: high; commits: 6eb5d99987fa; files: src/remote/server.ts)
  • Renfei Zhou: Commit bc66fad introduced the byte-preserving browser ZIP bundle behavior that this PR extends. (role: introduced ZIP bundle behavior; confidence: high; commits: bc66fad88396; files: src/browser/prompt.ts)
  • Trịnh Đức Hoàng: The remote service lifecycle dates through the secure bridge work, making this a relevant adjacent owner for remote temporary-file handling. (role: remote bridge contributor; confidence: medium; commits: bda0326d43b0; files: src/remote/server.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Delete remote-generated fallback bundles after both successful and failed runs, and exercise that path in the remote server test.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)
  • reviewed 2026-08-22T11:02:37.994Z sha 93a0687 :: needs real behavior proof before merge. :: [P1] Preserve or explicitly approve the established auto contract | [P2] Make the force-bundle flag select the upload plan | [P2] Clean up the eager fallback ZIP
  • reviewed 2026-08-29T14:28:56.385Z sha ea6cc37 :: needs real behavior proof before merge. :: [P1] Preserve or explicitly approve the established auto contract | [P2] Defer fallback archive transfer in remote consults
  • reviewed 2026-08-29T14:37:36.127Z sha ea6cc37 :: found issues before merge. :: [P1] Preserve the established auto bundle contract | [P2] Defer remote fallback materialization
  • reviewed 2026-08-29T14:57:49.828Z sha 37ddbb2 :: found issues before merge. :: [P1] Preserve or explicitly approve the established auto contract | [P2] Defer remote fallback archive materialization

@tristanmanchester

Copy link
Copy Markdown
Contributor Author

Should existing --browser-bundle-format auto users be migrated to ZIP uploads by default, despite the prior flattened-text behavior?

This PR keeps auto's small-text inline path. ZIP is the default only when files are uploaded (over the inline budget, raw/native files, or --browser-bundle-files). --browser-bundle-format text remains the explicit legacy flattened-text hatch.

Review defects from the last pass are fixed in ea6cc37:

  • --browser-bundle-files now bypasses auto inline selection
  • Fallback ZIPs are created lazily and generated bundle dirs are removed after the run

Dry-run proof (default auto attachments):

# --browser-bundle-files with two small source files
[preview] Bundled upload:
  • .../oracle-browser-bundle-GlvtSt/attachments-bundle.zip (513 B)

# same files without the force flag
[preview] Inline file content:
  • 2 files pasted directly into the composer.

The generated oracle-browser-bundle-GlvtSt directory was gone after the dry-run.

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. label Aug 29, 2026
@tristanmanchester

Copy link
Copy Markdown
Contributor Author

Live ChatGPT proof for the ZIP default is in. Session zip-bundle-extract-proof, GPT-5.5 browser, 57.9s.

Oracle uploaded one attachments-bundle.zip, the composer included the extract instruction, and the answer quoted both relative paths and sentinels (ORACLE_ZIP_SENTINEL_ALPHA_406, ORACLE_ZIP_SENTINEL_BETA_406). The generated oracle-browser-bundle-iwhikb directory was gone after the run.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 29, 2026
tristanmanchester and others added 2 commits August 29, 2026 16:47
Browser uploads previously flattened multiple text/source files into a
single Markdown-style text bundle, discarding the directory tree that
ChatGPT needs to navigate a codebase.

Multiple text/source files are now packed into a byte-preserving ZIP with
relative paths intact, and the composer carries a short instruction to
extract the archive before inspecting it. A single text/source file stays
a native upload, and images, PDFs, and archives remain separate
attachments while the 10-attachment cap permits.

The legacy flattened bundle remains available via
--browser-bundle-format text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--browser-bundle-files now selects the upload plan under auto, so small
files become one ZIP instead of an inline paste. Fallback archives are
created only when ChatGPT rejects the inline paste, and generated
oracle-browser-bundle-* directories are removed after browser runs and
dry-runs.
@tristanmanchester
tristanmanchester force-pushed the feat/browser-zip-default-bundle branch from ea6cc37 to 37ddbb2 Compare August 29, 2026 14:53
Restore the established auto bundle format: flattened text for text-only
uploads, ZIP when raw files are present or format is zip. Remote clients
no longer materialize a fallback archive before the primary submit.
@tristanmanchester

Copy link
Copy Markdown
Contributor Author

Addressed the remaining ClawSweeper items in 0f0ff3b:

Should existing --browser-bundle-format auto uploads switch from flattened text to ZIP by default?

Kept auto backward-compatible. Text-only uploads still flatten to text. ZIP is used when raw/native files are present, or when --browser-bundle-format zip is set.

The remote executor calls prepare() before sending the primary request.

Remote no longer materializes a fallback archive up front. Unbundled fallback files are sent to the host, and the ZIP/text bundle is created there only if ChatGPT rejects the inline paste.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant