Repository navigation
fix: warn when H5I_AGENT is invalid - #489
Merged
Koukyosyumei merged 3 commits intoAug 13, 2026
Merged
Conversation
The env-var table claimed H5I_AGENT drives "the branch namespace and
automatic runtime profile selection", then that an invalid value falls
back to `human`. Only the first half falls back. AgentRuntime::detect()
reads the raw var and matches starts_with("codex") on it, so
H5I_AGENT=codex.code warns "using 'human'", namespaces the env under
human/, and still hands the box Codex credentials and OpenAI egress via
the auto-picked `agent` profile. The row told the reader the opposite.
Scope the fallback to the namespace and put back the concrete claude /
codex values the rewrite had dropped — "which runtime a box is scoped
to" is what someone reads this row to find out. Regenerated the /manual/
HTML from it so the freshness gate stays green.
Also document warn_invalid_agent_identity, including why the rejected
value is not echoed, and why agent_identity warns at all rather than
falling through in silence.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
CI's `clippy --workspace --all-targets -- -D warnings` failed on Linux
and macOS with collapsible_if on the nested `if let Some(value)` /
`if !value.is_empty()` in the new identity test. It did not reproduce
for the author: collapsible_if only learned to fire across a `let`
binding in clippy 1.97, and older clippy says nothing.
Clippy's own suggestion is a let-chain, which is exactly wrong to commit
here. CI compiles the pull_request MERGE commit, so it sees main's
edition 2024 and let chains parse; this branch is based on an older main
that is still edition 2021, where the same line is a hard parse error.
Anyone building the branch alone would hit it.
`Option::filter` collapses the nesting without a let chain, so it builds
on both editions and reads as the one condition it always was: skip the
empty-string case, which has nothing to look for and would match every
haystack via `contains("")`.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Koukyosyumei
approved these changes
Aug 13, 2026
Koukyosyumei
left a comment
Koukyosyumei
left a comment
Collaborator
There was a problem hiding this comment.
Approving. The behavior is right and the test earns its keep.
The core change is narrow and correct: agent_identity() now distinguishes "unset" from "present but unusable" instead of collapsing both through a filter(...).unwrap_or(...), so the silent bare-shell case stays silent while a typo'd identity says so. NotUnicode routes to the same warning rather than being swallowed, which is the case the old .ok() dropped on the floor. Not echoing the rejected value is the right call for ambient input of unknown provenance, and the test pins that rather than trusting it.
The test table is the part I'd have asked for if it weren't already here: punctuation, embedded whitespace, empty, overlong, non-ASCII, valid, trimmed-valid, and unset, each asserting the manifest agent, the env id, the branch, and that stdout stays parseable JSON. That last assertion is what keeps the warning from becoming a breaking change for anything piping --json.
Three commits added on top while reviewing:
6f1f854d0corrects theH5I_AGENTrow in the manual. It claimed the fallback covers "the branch namespace and automatic runtime profile selection", but only the namespace falls back.AgentRuntime::detect()reads the raw var andfrom_identitymatchesstarts_with("codex")on it, soH5I_AGENT=codex.codewarns "using 'human'", namespaces underhuman/, and still hands the box Codex credentials and OpenAI egress. The row told the reader the opposite of what happens.f33d3134efixes the CI break:clippy 1.97firescollapsible_ifacross the nestedif let Some(value)/if !value.is_empty(), which is invisible on older clippy. Worth noting that clippy's own suggestion here is a trap: it proposes a let-chain, and CI compiles the merge commit (edition 2024, parses fine) while this branch is still edition 2021, where the same line is a hard parse error.Option::filtercollapses it on both.
Two things for follow-up rather than blockers:
agent_identity()validates and falls back, butAgentRuntime::detect()bypasses it entirely and re-reads the raw var, so an invalid identity still selects a runtime from the unvalidated string. Unifying those would change behavior (codex.codewould flip from Codex to Claude credentials), so it does not belong in a warning-only PR, but it is the actual split underneath the doc bug fixed above.- This branch is based on an edition-2021 main. It merges cleanly and CI tests the merge, so nothing is broken, but a rebase would make what you build locally match what CI checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An invalid but present
H5I_AGENTcurrently falls back tohumanwithout telling the user. Emit one warning on stderr when the value fails the existing identity grammar, while preserving thehumanfallback and successful create behavior.Unset identities remain silent, valid values keep their current behavior, JSON stdout stays parseable, and the warning does not echo the invalid value. The integration test covers punctuation, whitespace, empty, overlong, non-ASCII, valid, trimmed, and unset identities. The manual now documents the validation and fallback behavior.
Closes #458
Testing
cargo test --locked --no-default-features --test env_integration create_warns_for_invalid_agent_identity_without_contaminating_json -- --exactcargo test --locked --no-default-features --bin h5icargo clippy --locked --no-default-features --bin h5i --test env_integration -- -D warningscargo build --locked --no-default-features --bin h5i --test env_integrationpython scripts/gen_manual.py