Skip to content

agent-browser: make Rust CLI + pnpm tree reproducible#264

Merged
bryan-minimal merged 1 commit into
mainfrom
bryan/agent-browser-reproducible
Jun 18, 2026
Merged

agent-browser: make Rust CLI + pnpm tree reproducible#264
bryan-minimal merged 1 commit into
mainfrom
bryan/agent-browser-reproducible

Conversation

@bryan-minimal

@bryan-minimal bryan-minimal commented Jun 17, 2026

Copy link
Copy Markdown
Member

What

Makes the agent-browser package reproducible — its Rust CLI and its pnpm node_modules tree both diverged across rebuilds.

Why / fixes

Two independent causes, both addressed:

  1. Rust CLI codegen non-determinism. The release build split each crate into parallel codegen units (non-deterministic .llvm.<hash> symbols + function partitioning) and ahash/const-random baked a compile-time random seed. Fixed by extending RUSTFLAGS:

    export RUSTFLAGS="-C linker=gcc --remap-path-prefix=$(pwd)=/builddir --remap-path-prefix=$HOME/.cargo=/cargo -C codegen-units=1"
    export CONST_RANDOM_SEED=0
  2. pnpm state-file timestamps. pnpm writes wall-clock timestamps into two node_modules state files that aren't needed at runtime — .modules.yaml (prunedAt) and .pnpm-workspace-state-v1.json (lastValidatedTimestamp). Drop both before packaging.

Verification

Two from-scratch forced rebuilds (--rebuild --no-fetch, aarch64) produced output identical in 12,737 / 12,738 files — with the codegen fixes in place, the only remaining difference was the .pnpm-workspace-state-v1.json timestamp. After removing that file the two builds are byte-for-byte identical (repro-check diff, 0 differences). The Rust binary and all other artifacts were already byte-identical, confirming the codegen flags resolve the CLI non-determinism.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Improved build script reproducibility and determinism by adding compiler flags and removing non-deterministic artifacts during packaging.

Two independent causes of cross-rebuild divergence:

1. Rust CLI codegen: parallel codegen-units (.llvm.<hash> symbols) plus
   ahash/const-random's compile-time seed. Pin both via RUSTFLAGS
   (-C codegen-units=1, path remaps) and CONST_RANDOM_SEED=0.

2. pnpm state files embed wall-clock timestamps (.modules.yaml prunedAt,
   .pnpm-workspace-state-v1.json lastValidatedTimestamp) and aren't needed
   at runtime. Drop both before packaging.

Verified on aarch64: with the codegen fixes, two from-scratch builds were
identical in 12737/12738 files (sole diff = the pnpm timestamp file);
removing it yields byte-identical output (repro-check diff, 0 differences).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

packages/agent-browser/build.sh is updated to improve build reproducibility: RUSTFLAGS gains --remap-path-prefix mappings and -C codegen-units=1, CONST_RANDOM_SEED=0 is added to pin compile-time randomness, and a cleanup step removes pnpm-generated timestamp state files before package assembly.

Changes

Reproducible Build Hardening

Layer / File(s) Summary
Rust determinism flags and seed pinning
packages/agent-browser/build.sh
RUSTFLAGS is expanded with --remap-path-prefix entries (covering $HOME/.cargo) and -C codegen-units=1; CONST_RANDOM_SEED=0 is set to pin the ahash/const-random compile-time seed.
Pre-packaging pnpm state file cleanup
packages/agent-browser/build.sh
A cleanup block is added before copying/package assembly to delete .modules.yaml and .pnpm-workspace-state-v1.json, removing pnpm-generated files that embed wall-clock timestamps.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • gominimal/pkgs#259: Applies the same Rust determinism flags (--remap-path-prefix, -C codegen-units=1, CONST_RANDOM_SEED=0) and pnpm state file cleanup steps to packages/agent-browser/build.sh.

Poem

🐇 Hop, hop — no timestamps allowed!
The paths are remapped, the seeds are vowed,
One codegen unit, steady and true,
.modules.yaml? Deleted, adieu!
Deterministic builds, crisp as morning dew~ 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title precisely describes the main change: making the agent-browser Rust CLI and pnpm tree reproducible, which matches both the file changes and PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bryan/agent-browser-reproducible

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/agent-browser/build.sh (2)

1-2: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enable required strict mode for build scripts.

This script currently sets only -e; it should enforce unset-variable and pipeline failure checks too.

As per coding guidelines: "packages/*/build.sh: Use set -euo pipefail at the beginning of build scripts to ensure error handling and strict variable expansion."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent-browser/build.sh` around lines 1 - 2, Update the shell script
strict mode initialization by replacing the current `set -e` line with `set -euo
pipefail`. This will enable all three strict mode options: exit on error (e),
error on unset variables (u), and error on pipe failures (o pipefail), which
ensures comprehensive error handling and variable expansion checks throughout
the build script execution.

Source: Coding guidelines


42-49: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Drop libtool archives from the output tree after install.

The script should remove .la files from $OUTPUT_DIR as part of post-install cleanup.

Suggested patch
 cp -R dist bin node_modules package.json $OUTPUT_DIR/usr/libexec/agent-browser/
+find "$OUTPUT_DIR" -name '*.la' -delete

As per coding guidelines: "Post-install, drop libtool archives with find "$OUTPUT_DIR" -name '*.la' -delete."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent-browser/build.sh` around lines 42 - 49, The build script is
missing post-install cleanup to remove libtool archive files (.la files) from
the output directory. After the chmod command that sets executable permissions
on the agent-browser wrapper script, add a find command that searches through
$OUTPUT_DIR for all files matching the pattern '*.la' and deletes them using the
-delete action to comply with post-install cleanup guidelines.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/agent-browser/build.sh`:
- Around line 6-8: In the build.sh script, add two additional environment
variable exports for deterministic builds after the existing RUSTFLAGS and
CONST_RANDOM_SEED exports. Export SOURCE_DATE_EPOCH=0 to pin the build timestamp
and PYTHONHASHSEED=0 to ensure deterministic Python hashing during any
Python-based build steps, as required by the reproducible-build contract.

---

Outside diff comments:
In `@packages/agent-browser/build.sh`:
- Around line 1-2: Update the shell script strict mode initialization by
replacing the current `set -e` line with `set -euo pipefail`. This will enable
all three strict mode options: exit on error (e), error on unset variables (u),
and error on pipe failures (o pipefail), which ensures comprehensive error
handling and variable expansion checks throughout the build script execution.
- Around line 42-49: The build script is missing post-install cleanup to remove
libtool archive files (.la files) from the output directory. After the chmod
command that sets executable permissions on the agent-browser wrapper script,
add a find command that searches through $OUTPUT_DIR for all files matching the
pattern '*.la' and deletes them using the -delete action to comply with
post-install cleanup guidelines.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c63048a-add2-4e5d-a9de-4952f053af39

📥 Commits

Reviewing files that changed from the base of the PR and between 8144780 and 258b817.

📒 Files selected for processing (1)
  • packages/agent-browser/build.sh

Comment on lines +6 to 8
export RUSTFLAGS="-C linker=gcc --remap-path-prefix=$(pwd)=/builddir --remap-path-prefix=$HOME/.cargo=/cargo -C codegen-units=1"
export CONST_RANDOM_SEED=0 # pin ahash/const-random compile-time seed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add deterministic timestamp/hash environment exports.

Please export SOURCE_DATE_EPOCH=0 and PYTHONHASHSEED=0 in this script to satisfy the reproducible-build contract.

Suggested patch
 export LD=gcc
 export RUSTFLAGS="-C linker=gcc --remap-path-prefix=$(pwd)=/builddir --remap-path-prefix=$HOME/.cargo=/cargo -C codegen-units=1"
 export CONST_RANDOM_SEED=0   # pin ahash/const-random compile-time seed
+export SOURCE_DATE_EPOCH=0
+export PYTHONHASHSEED=0

As per coding guidelines: "For embedded timestamps and Python-based build steps, export SOURCE_DATE_EPOCH=0 and PYTHONHASHSEED=0."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export RUSTFLAGS="-C linker=gcc --remap-path-prefix=$(pwd)=/builddir --remap-path-prefix=$HOME/.cargo=/cargo -C codegen-units=1"
export CONST_RANDOM_SEED=0 # pin ahash/const-random compile-time seed
export RUSTFLAGS="-C linker=gcc --remap-path-prefix=$(pwd)=/builddir --remap-path-prefix=$HOME/.cargo=/cargo -C codegen-units=1"
export CONST_RANDOM_SEED=0 # pin ahash/const-random compile-time seed
export SOURCE_DATE_EPOCH=0
export PYTHONHASHSEED=0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent-browser/build.sh` around lines 6 - 8, In the build.sh script,
add two additional environment variable exports for deterministic builds after
the existing RUSTFLAGS and CONST_RANDOM_SEED exports. Export SOURCE_DATE_EPOCH=0
to pin the build timestamp and PYTHONHASHSEED=0 to ensure deterministic Python
hashing during any Python-based build steps, as required by the
reproducible-build contract.

Source: Coding guidelines

@bryan-minimal

Copy link
Copy Markdown
Member Author

Thanks @coderabbitai — dispositions on the three findings:

  1. set -euo pipefail (line 1): leaving as set -e. This script is #!/bin/sh and bash is only a runtime dep here (not a build_dep), so set -o pipefail (a bashism) isn't available at build time without promoting bash to a build dep. The package is already verified byte-reproducible and minimal-check is green, so I'd rather not destabilize it for a strict-mode change; happy to revisit if maintainers want bash added as a build dep.

  2. export SOURCE_DATE_EPOCH=0 + PYTHONHASHSEED=0 (line 8): not applying — this would fail CI. The build sandbox already exports both, and minimal-check's build script disallowed-patterns checker explicitly rejects re-setting them ("already set by the build sandbox"). This suggestion stems from the AGENTS.md guidance, which had this backwards — I'm correcting it in docs(AGENTS): fix reproducibility guidance (sandbox sets SOURCE_DATE_EPOCH) #266.

  3. .la cleanup (line 42): not applying — agent-browser is a Node + Rust package and produces no libtool archives, so find … -name '*.la' -delete is a no-op here. (The corrected guideline in docs(AGENTS): fix reproducibility guidance (sandbox sets SOURCE_DATE_EPOCH) #266 scopes .la cleanup to C/autotools builds.)

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@bryan-minimal Thanks for the detailed explanations — all three make complete sense and I'm withdrawing those suggestions.

  1. Noted: #!/bin/sh scripts can't use set -o pipefail without bash as a build_dep, so set -e is the correct choice here.
  2. Understood: the build sandbox already injects SOURCE_DATE_EPOCH and PYTHONHASHSEED, and minimal-check's build script disallowed-patterns checker will reject re-exporting them. Good catch on the inverted AGENTS.md guidance — thanks for fixing it in #266.
  3. Agreed: .la cleanup is only meaningful for C/autotools builds; it's a no-op for Node + Rust packages.

✏️ Learnings added
Learnt from: bryan-minimal
URL: https://github.com/gominimal/pkgs/pull/264

Timestamp: 2026-06-17T22:43:22.656Z
Learning: In the gominimal/pkgs repository, `build.sh` scripts use `#!/bin/sh`. `bash` is listed only as a *runtime* dependency, not a `build_dep`. Therefore, `set -o pipefail` (a bashism) is not available at build time and should not be suggested unless `bash` is promoted to a build dependency. `set -e` is the correct and sufficient choice.

Learnt from: bryan-minimal
URL: https://github.com/gominimal/pkgs/pull/264

Timestamp: 2026-06-17T22:43:22.656Z
Learning: In the gominimal/pkgs repository, the build sandbox automatically exports `SOURCE_DATE_EPOCH` and `PYTHONHASHSEED`. The `minimal-check` `build script disallowed-patterns` checker explicitly rejects any `build.sh` that re-sets (re-exports) these variables, causing CI failure. Never suggest adding `export SOURCE_DATE_EPOCH=0` or `export PYTHONHASHSEED=0` to `build.sh` scripts in this repository.

Learnt from: bryan-minimal
URL: https://github.com/gominimal/pkgs/pull/264

Timestamp: 2026-06-17T22:43:22.656Z
Learning: In the gominimal/pkgs repository, `find … -name '*.la' -delete` cleanup in `build.sh` should only be suggested for C/autotools packages. It is a no-op and inappropriate for Node.js, Rust, or combined Node+Rust packages (such as `agent-browser`) that produce no libtool archives.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@bryan-minimal bryan-minimal added this pull request to the merge queue Jun 18, 2026
Merged via the queue into main with commit 94a7944 Jun 18, 2026
4 checks passed
@bryan-minimal bryan-minimal deleted the bryan/agent-browser-reproducible branch June 18, 2026 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants