Skip to content

feat(browser): add opt-in export of saved browser files - #419

Merged
steipete merged 4 commits into
steipete:mainfrom
gwelinder:fix/frog-sandbox-artifact-write-output-20260826
Sep 7, 2026
Merged

feat(browser): add opt-in export of saved browser files#419
steipete merged 4 commits into
steipete:mainfrom
gwelinder:fix/frog-sandbox-artifact-write-output-20260826

Conversation

@gwelinder

@gwelinder gwelinder commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Integrate opt-in saved-file export with current main after #431 and #397 landed. Preserve both thinking-selection evidence and artifact-copy warnings in session metadata, including copy-failure paths.

The default remains answer-only --write-output. Explicit --write-artifacts requires browser mode and an output path, preserves canonical captures, verifies copied bytes/hashes, avoids collisions, and records non-fatal copy warnings.

Validation on current main: 2,171 tests passed, 45 skipped; lint/build/docs passed. Actual built-CLI/Chrome artifact export, all 27 attachment cases, and the effort-evidence/serve bridge matrix passed. Local and final branch P2 autoreview are clean. Exact-head CI is linked in the proof comment.

Signed-in generated-file downloads, current cookie/signed-URL behavior, and live remote-host artifact transfer remain unproven. Provider pages and files used here were synthetic. Contributor history was preserved with a merge of main and a fast-forward push.

Deferred changelog (carried by #464): Browser: optionally export captured downloadable files beside --write-output with --write-artifacts, preserving canonical artifacts, validating hashes, and avoiding filename collisions; thanks @gwelinder.

@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 26, 2026
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 12:23 PM ET / 16:23 UTC.

ClawSweeper review

What this changes

Adds a browser-only flag to export captured files beside the saved answer, with collision protection, integrity checks, persisted warnings, documentation, and CLI proof coverage.

Merge readiness

Ready for maintainer review

This remains useful work absent from current main. The maintainer-supported opt-in design resolves the earlier compatibility concern, and no blocking defect was found.

Priority: P2
Reviewed head: 48ba4f8f57a1014b9d3fc15ab9a9e839bbb6cc9e

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, supported implementation with relevant production-path proof and no blocking findings.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (linked_artifact): Owner-recorded execution at the reviewed head exercises the production CLI, real Chrome capture, and real filesystem export, demonstrating preserved defaults, identical bytes, collision safety, and non-fatal permission failures. Synthetic provider responses do not prove signed-in download authentication or live bridge transfer, neither of which this patch changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): Owner-recorded execution at the reviewed head exercises the production CLI, real Chrome capture, and real filesystem export, demonstrating preserved defaults, identical bytes, collision safety, and non-fatal permission failures. Synthetic provider responses do not prove signed-in download authentication or live bridge transfer, neither of which this patch changes.
Evidence reviewed 8 items Repository policy: Read the complete root AGENTS.md. The scoped-policy search found no additional AGENTS.md files, and .agents/maintainer-notes was absent. Browser smoke and changelog guidance informed the review; the owner explicitly assigned deferred release notes to #464.
Introduced behavior and compatibility: The complete local introduced diff preserves answer-only output unless writeArtifacts is true, validates the CLI combination, persists an optional boolean, and retains thinking-selection evidence when saving export warnings. Existing JSON readers accept the additive field without rewriting older records.
Filesystem and security boundary: Export reuses filename sanitization, copies with COPYFILE_EXCL, verifies recorded size and SHA-256, and removes failed copies. It consumes files already captured through the existing local or validated bridge pipeline; the patch does not change download authorization, bridge permissions, dependencies, or workflow credentials.
Findings None None.
Security None None.

How this fits together

Oracle’s browser runner captures ChatGPT answers and downloadable files into session storage. The CLI now optionally copies those captured files into the user’s answer-output directory while retaining the originals.

flowchart TD
  A[Browser response] --> B[Canonical captured files]
  C[CLI output options] --> D{Artifact export enabled?}
  B --> D
  D -->|No| E[Answer-only output]
  D -->|Yes| F[Copy without overwriting]
  F --> G[Verify size and hash]
  G --> H[Export metadata or non-fatal warning]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and validation growth Production +129 net lines; tests and proof harness +463 net lines Production growth implements the explicit export option and filesystem safeguards, with most added code devoted to validation.

Technical review

Best possible solution:

Retain the opt-in export contract and existing capture pipeline, with release notes following through #464.

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

Not applicable as a bug reproduction: this adds an explicit export capability, and the recorded built-CLI/Chrome scenarios exercise its observable filesystem behavior.

Is this the best way to solve the issue?

Yes. Reusing captured files behind an explicit flag is a narrow approach that preserves existing output behavior and avoids a second downloader.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 176cec67c4ef.

Labels

Label justifications:

  • P2: This is a bounded, opt-in browser workflow improvement that preserves existing defaults.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): Owner-recorded execution at the reviewed head exercises the production CLI, real Chrome capture, and real filesystem export, demonstrating preserved defaults, identical bytes, collision safety, and non-fatal permission failures. Synthetic provider responses do not prove signed-in download authentication or live bridge transfer, neither of which this patch changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. Owner-recorded execution at the reviewed head exercises the production CLI, real Chrome capture, and real filesystem export, demonstrating preserved defaults, identical bytes, collision safety, and non-fatal permission failures. Synthetic provider responses do not prove signed-in download authentication or live bridge transfer, neither of which this patch changes.

Evidence

What I checked:

  • Repository policy: Read the complete root AGENTS.md. The scoped-policy search found no additional AGENTS.md files, and .agents/maintainer-notes was absent. Browser smoke and changelog guidance informed the review; the owner explicitly assigned deferred release notes to docs(changelog): record browser and remote run improvements #464. (AGENTS.md:1, 48ba4f8f57a1)
  • Introduced behavior and compatibility: The complete local introduced diff preserves answer-only output unless writeArtifacts is true, validates the CLI combination, persists an optional boolean, and retains thinking-selection evidence when saving export warnings. Existing JSON readers accept the additive field without rewriting older records. (src/cli/sessionRunner.ts:146, 48ba4f8f57a1)
  • Filesystem and security boundary: Export reuses filename sanitization, copies with COPYFILE_EXCL, verifies recorded size and SHA-256, and removes failed copies. It consumes files already captured through the existing local or validated bridge pipeline; the patch does not change download authorization, bridge permissions, dependencies, or workflow credentials. (src/cli/sessionRunner.ts:1106, 48ba4f8f57a1)
  • Behavior proof tied to production execution: The owner’s captured comments report the actual built CLI and isolated Chrome exercising canonical capture and adjacent export: one send/download per case, matching binary hashes, preserved collision sentinel and originals, answer-only default, and a real POSIX permission failure persisted as a warning. The current-head confirmation is feat(browser): add opt-in export of saved browser files #419 (comment) and links https://github.com/steipete/oracle/actions/runs/34141654890. The complete inspected harness asserts these outcomes. Provider responses are synthetic; signed-in downloads and live bridge transfer are explicitly outside this proof. (scripts/artifact-export-proof.mjs:157, 48ba4f8f57a1)
  • Current-main necessity: Current main writes only the assistant answer at browser completion and has no adjacent captured-file export step. The related merged attachment and effort-evidence PRs address separate behavior and do not supersede this feature. (src/cli/sessionRunner.ts:143, 176cec67c4ef)
  • Release and history inspection limits: The local v0.18.0 tag resolves to the supplied release commit dated 2026-08-14. Older source-blob and blame reads encountered missing objects and failed because github.com DNS resolution was unavailable; no release implementation or source-line introduction is claimed from those failed reads. Current-head and current-main source inspection succeeded. (083bba7e61f4)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Piotr Durlej: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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-26T00:18:28.422Z sha 6d21d55 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-01T10:05:24.545Z sha 6d21d55 :: needs real behavior proof before merge. :: [P2] Document the expanded browser output contract
  • reviewed 2026-09-07T13:35:15.468Z sha 71af5ae :: needs maintainer review before merge. :: none
  • reviewed 2026-09-07T16:09:50.569Z sha 48ba4f8 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Sep 1, 2026
Preserve answer-only --write-output behavior. Add --write-artifacts for browser
runs, persist the option, retain canonical captures, and verify adjacent binary
copies without overwriting existing files. Keep copy failures visible as warnings.

Prove actual CLI capture and export using real Chrome with synthetic sandbox
responses, including defaults, collision preservation, and filesystem failures.

Co-authored-by: gwelinder <gwelinder@gmail.com>
@steipete steipete changed the title fix(browser): export saved files with write output feat(browser): add opt-in export of saved browser files Sep 7, 2026
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Sep 7, 2026
@steipete

steipete commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Maintainer repair verified at 71af5aeba31cca1a2181a1a4c20b2b911195efa8. All four CI jobs passed, including the built-CLI export proof on Linux: https://github.com/steipete/oracle/actions/runs/34127618572. Local and final branch autoreview through P2 are scoped-clean.

The public default is preserved: --write-output writes only the answer. Explicit --write-artifacts requires a browser run and an output path, persists with the session, and exports captured files without overwriting existing destination files.

Validation: 2,118 tests passed, 45 skipped; lint/typecheck/build/docs passed. The actual built CLI and real Chrome captured a synthetic sandbox binary through the existing browser fetch and canonical artifact pipeline. Default, opt-in, collision, and real POSIX permission-failure cases passed: one send/download each, preserved original captures, identical exported bytes and SHA-256, retained collision sentinel, and a persisted warning on copy failure. The fixture binary SHA-256 is dd927876d1d66c05f4b28dea75cbf2d687e4a669aec79765b6fb80cd836d8283. Invalid API and missing-output flag combinations were also rejected.

Unproven here: a fresh signed-in ChatGPT generated-file download, current authentication/cookie and signed-URL behavior, and live remote-host artifact transfer. Synthetic responses were intercepted locally in an isolated signed-out browser. Windows permission-failure behavior has mocked coverage; the real permission case runs on POSIX.

Thanks @gwelinder. Contributor history and human credit are preserved. The deferred changelog line is in the PR body, with no branch changelog delta. No merge or release was performed.

Co-authored-by: gwelinder <gwelinder@gmail.com>
@steipete

steipete commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Current-main integration verified at 48ba4f8, based on main 176cec6. Contributor history and human credit are preserved.

All four exact-head CI jobs passed: https://github.com/steipete/oracle/actions/runs/34141654890. Local and final branch autoreview through P2 are clean.

Validation: pnpm test (2,171 passed, 45 skipped), lint/build/docs, and the actual built-CLI artifact export, 27-case attachment, and effort-evidence matrices passed. The export regression now explicitly checks that thinkingSelection survives successful copying and copy warnings. The real Chrome file proof preserves default answer-only output, binary hashes, collision sentinels, original captures, and non-fatal POSIX copy-failure warnings.

Unproven: fresh signed-in generated-file downloads, cookie/signed-URL behavior, and live remote-host artifact transfer. All provider responses were synthetic. The four deferred notes are in #464, which must land after this PR and #400. No merge or release performed.

@steipete
steipete merged commit 2e206b6 into steipete:main Sep 7, 2026
5 checks passed
steipete added a commit that referenced this pull request Sep 7, 2026
Record the deferred entries for #431, #419, #397, and #400 in user-impact order after the existing MCP headlines. Preserve Highlights, existing notes, and released history.

This changes only CHANGELOG.md. No version bump, dependency change, tag, or publication is included.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants