8000 fix(sandbox2): default session LANG to a locale the rootfs ships by gominimal-aw-bot[bot] · Pull Request #1888 · gominimal/minimal · GitHub
[go: up one dir, main page]
Skip to content

fix(sandbox2): default session LANG to a locale the rootfs ships - #1888

Merged
norrietaylor merged 3 commits into
mainfrom
inbox-patch/fix-session-lang-locale-1787fed5ec42c2a9
Oct 4, 2026
Merged

fix(sandbox2): default session LANG to a locale the rootfs ships#1888
norrietaylor merged 3 commits into
mainfrom
inbox-patch/fix-session-lang-locale-1787fed5ec42c2a9

Conversation

@gominimal-aw-bot
@gominimal-aw-bot gominimal-aw-bot Bot commented Oct 3, 2026 •
edited by coderabbitai Bot
Loading
Copy link
Copy Markdown
Contributor

Routing-Key: inbox-route/I_kwDOSUhdos8AAAABUxkSPQ

Sessions defaulted LANG=C.UTF-8, but the session rootfs does not ship
that locale (locale -a lists en_US.utf8, not C.UTF-8), so every
setlocale caller warned "Cannot set LC_CTYPE to default locale".

This switches the session default to en_US.UTF-8 — the canonical alias
of the shipped en_US.utf8 — keeping LANG-only (no LC_ALL) so
composed env_vars and forwarded LANG/LC_* still override it. The
example project mirrors the same default, and a session e2e step now
asserts LANG=en_US.UTF-8 and that locale runs without the warning.

Verification

  • cargo fmt --all --check — pass
  • cargo clippy -p sandbox2 -p sessions -- -D warnings — pass (workspace
    clippy fails on pre-existing needless_borrows_for_generic_args lints in
    crates/args under the local 1.99.0 stable toolchain; repo pins 1.97.0)
  • cargo build --workspace --locked — pass
  • cargo test -p sandbox2 -p sessions --locked — pass (workspace test
    fails on pre-existing crates/checkouts git-clone/offline tests,
    unrelated to this change)
  • live-session: native-daemon-e2e — runs on this PR; not runnable in the
    fixer sandbox

Summary by CodeRabbit

  • Bug Fixes
    • Session sandboxes now use the en_US.UTF-8 locale, improving consistency for commands that depend on locale settings.
    • Configured environment variables can still override the default locale, and other sandbox types retain their existing locale settings.
    • Session checks now verify that the locale is applied successfully and flag locale-related warnings, helping identify problems during session setup.

@gominimal-aw-bot
fix(sandbox2): default session LANG to a locale the rootfs ships
ad08859 
Sessions defaulted LANG=C.UTF-8, but the session rootfs does not
ship that locale (locale -a lists en_US.utf8, not C.UTF-8), so every
setlocale caller warned "Cannot set LC_CTYPE to default locale".
Switch the session default to en_US.UTF-8, the canonical alias of the
shipped locale, keeping LANG-only (no LC_ALL) so composed env_vars and
forwarded LANG/LC_* still override it. Mirror the example project and
add a session e2e check that locale runs without the warning.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gominimal-aw-bot
gominimal-aw-bot Bot requested a review from a team as a code owner October 3, 2026 13:46
@gominimal-aw-bot gominimal-aw-bot Bot added the agent:fix Agentic fix PR eligible for review iteration; min-aw's worker-iterate gates on it. label Oct 3, 2026
@coderabbitai
coderabbitai Bot commented Oct 3, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 53 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 9b01f283-e770-4e72-be5d-d9cdb87796a7
📥 Commits

Reviewing files that changed from the base of the PR and between ad08859 and cea89fc.

📒 Files select 8000 ed for processing (2)
  • crates/sandbox2/src/config.rs
  • scripts/session-e2e.sh

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: c71b4916-af91-4289-8f21-f57527adbd44
📥 Commits

Reviewing files that changed from the base of the PR and between 3bf6b09 and ad08859.

📒 Files selected for processing (3)
  • crates/sandbox2/src/config.rs
  • crates/sessions/example_project/minimal.toml
  • scripts/session-e2e.sh

Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

Walkthrough

Session sandboxes now default to LANG=en_US.UTF-8. The example configuration, session environment test, and end-to-end proof reflect this locale. Non-session locale defaults remain unchanged.

Changes

Session locale

Layer / File(s) Summary
Session locale default and checks
crates/sandbox2/src/config.rs, crates/sessions/example_project/minimal.toml, scripts/session-e2e.sh		 The session locale default and example fallback change to en_US.UTF-8. The session environment test checks the new value. The end-to-end proof checks LANG and verifies that locale succeeds without a “Cannot set” warning.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: norrietaylor

Merge Risk: ⚪ Minimal · up to ad088

No concrete issue currently prevents merging. The live-session check should confirm that the shipped rootfs supports the new locale.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 …
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.
Title check ✅ Passed The title clearly and concisely describes the session LANG default change and follows the repository’s Conventional Commit format.
Description check ✅ Passed The description explains the change and its rationale, and lists verification results. It does not include the template’s Checklist section or explicitly address its documentation and breaking-change …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the locale at dawn,
en_US.UTF-8 greets the lawn.
No warning rustles through the air,
The session sprouts a setting fair.
Then off the rabbit hops with care.

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

@gominimal-aw-bot gominimal-aw-bot Bot added the agent:resolve Consent gate: pr-conflict-resolver may rewrite this branch label Oct 3, 2026
@norrietaylor norrietaylor changed the title fix: Default session LANG to a locale the rootfs ships fix(sandbox2): default session LANG to a locale the rootfs ships Oct 4, 2026
@gominimal-aw-bot
test(scripts): gate the session LANG equality check on a seeded project
cea89fc 
A caller's project or a forwarded LANG can override the uncomposed
default, so pin it only when the e2e seeded the project. The locale
stderr check stays unconditional as the regression guard.
@norrietaylor
norrietaylor commented Oct 4, 2026
Copy link
Copy Markdown
Member

/review

@minimal-foundry
minimal-foundry Bot commented Oct 4, 2026 •
edited
Loading
Copy link
Copy Markdown
Contributor

Approved: no findings at cea89fcc8323.

Nothing to do.

Clobber: none: git log -S C.UTF-8 shows the old default was introduced with the arbitrary-exec feature (e0f67c8, Aug 2026) and this PR deliberately corrects it based on live-session observation; nothing recently fixed is being reverted.

minimal-foundry[bot]
minimal-foundry Bot approved these changes Oct 4, 2026
View reviewed changes
@minimal-foundry minimal-foundry 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.

minimal-review — approved

Verdict APPROVE — No findings.

@minimal-foundry minimal-foundry Bot added the review:approved minimal-review approved the current head label Oct 4, 2026
8000
norrietaylor
norrietaylor approved these changes Oct 4, 2026
View reviewed changes
@norrietaylor norrietaylor left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved at cea89fc: minimal-review approved at head, 0 open threads, all lanes green.

@norrietaylor
norrietaylor added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit 932a9da Oct 4, 2026
35 checks passed
@norrietaylor
norrietaylor deleted the inbox-patch/fix-session-lang-locale-1787fed5ec42c2a9 branch October 4, 2026 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@norrietaylor norrietaylor norrietaylor approved these changes
@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes
@minimal-foundry minimal-foundry[bot] minimal-foundry[bot] approved these changes

Assignees

No one assigned

Labels

agent:fix Agentic fix PR eligible for review iteration; min-aw's worker-iterate gates on it. agent:resolve Consent gate: pr-conflict-resolver may rewrite this branch review:approved minimal-review approved the current head

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@norrietaylor
0