Skip to content

Don't let the Codex usage collector install Codex - #12939

Closed
j-c-m wants to merge 1 commit into
omacom:quattrofrom
j-c-m:fix/codex-usage-skip-launcher
Closed

j-c-m wants to merge 1 commit into
omacom:quattrofrom
j-c-m:fix/codex-usage-skip-launcher

Conversation

@j-c-m

@j-c-m j-c-m commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

The Agents widget runs omarchy-agent-usage-codex on each refresh. That command found codex on PATH, which is the lazy launcher at ~/.local/bin/codex, and the launcher's first step is mise use -g. A machine with no Codex install therefore started a download. The collector kills the launcher when the app-server handshake times out, systemd adopts mise, and the install continues. This happens with no default agent set.

The probe now uses mise which codex, which only prints a binary that is already installed. A symlink at the launcher path is still probed, since that is the user's own binary. The launcher file itself and mise shims are not executed. If Codex is not installed, the usage record says so.

The usage collector was executing ~/.local/bin/codex, the lazy launcher,
which runs mise use and installs Codex. Ask mise which for the installed
binary, and skip that launcher and the mise shims.
@llstrk

llstrk commented Sep 23, 2026

Copy link
Copy Markdown

Two custom-PATH cases remain in the installed-binary resolver at e79fb79a.

Verified: In isolated synthetic cases, the ordinary ~/.local/bin/codex launcher is skipped on this PR, while a symlink there pointing to an installed-binary stub still receives the app-server arguments.

A symlink alias still reaches the lazy launcher

is_omarchy_launcher() and is_mise_shim() check the candidate's spelled location, not its target. An earlier PATH entry containing a symlink to the regular Omarchy launcher passes these checks.

Synthetic launcher-entry comparison, with no installation operation:

PATH route Base PR
Canonical regular launcher Entered Skipped
Earlier symlink to that launcher Entered Entered

Impact: The installation-triggering route remains reachable in this custom layout. The real launcher calls mise use -g when entered; the experiment reproduced launcher entry, not a real download.

Suggested change: Check resolved candidate provenance, including symlinked parent directories, while preserving canonical-path symlinks that genuinely target an installed binary.

A later installed binary is overlooked

resolve_codex() checks only the first PATH match. After rejecting the launcher, it asks mise and stops if mise cannot locate Codex.

Synthetic PATH: launcher directory → installed-binary directory
mise which codex: no result
PR result: “codex is not installed”; installed-binary stub never entered

This also matters for ~/.npm-global/bin, which the collector itself appends after ~/.local/bin.

Impact: A non-mise installation behind the launcher is reported as unavailable, leaving plan/rate-limit data absent. This is an incomplete fallback, not a demonstrated regression from a working baseline: the baseline entered the launcher in this layout too.

Suggested change: Continue through later PATH candidates after rejecting a launcher or shim, applying the provenance checks to each, before concluding that Codex is unavailable.


Review information

Test scope: Source inspection and isolated head/base collector runs with synthetic launchers, mise and app-server responses. No real Codex installation, authentication, download or desktop rendering was tested.

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Automated AI review: Astra Medium inspection and synthesis, independent technical assessments by Opus 5.5 High and GPT 6 Sol Xhigh, and a fresh Astra Medium editorial check.

@j-c-m

j-c-m commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Two custom-PATH cases remain in the installed-binary resolver at e79fb79a.

Verified: In isolated synthetic cases, the ordinary ~/.local/bin/codex launcher is skipped on this PR, while a symlink there pointing to an installed-binary stub still receives the app-server arguments.

A symlink alias still reaches the lazy launcher

is_omarchy_launcher() and is_mise_shim() check the candidate's spelled location, not its target. An earlier PATH entry containing a symlink to the regular Omarchy launcher passes these checks.

Synthetic launcher-entry comparison, with no installation operation:

PATH route Base PR
Canonical regular launcher Entered Skipped
Earlier symlink to that launcher Entered Entered

Impact: The installation-triggering route remains reachable in this custom layout. The real launcher calls mise use -g when entered; the experiment reproduced launcher entry, not a real download.

Suggested change: Check resolved candidate provenance, including symlinked parent directories, while preserving canonical-path symlinks that genuinely target an installed binary.

A later installed binary is overlooked

resolve_codex() checks only the first PATH match. After rejecting the launcher, it asks mise and stops if mise cannot locate Codex.

Synthetic PATH: launcher directory → installed-binary directory
mise which codex: no result
PR result: “codex is not installed”; installed-binary stub never entered

This also matters for ~/.npm-global/bin, which the collector itself appends after ~/.local/bin.

Impact: A non-mise installation behind the launcher is reported as unavailable, leaving plan/rate-limit data absent. This is an incomplete fallback, not a demonstrated regression from a working baseline: the baseline entered the launcher in this layout too.

Suggested change: Continue through later PATH candidates after rejecting a launcher or shim, applying the provenance checks to each, before concluding that Codex is unavailable.


Review information

Test scope: Source inspection and isolated head/base collector runs with synthetic launchers, mise and app-server responses. No real Codex installation, authentication, download or desktop rendering was tested.

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Automated AI review: Astra Medium inspection and synthesis, independent technical assessments by Opus 5.5 High and GPT 6 Sol Xhigh, and a fresh Astra Medium editorial check.

Mis piloted clanker, please run on omarchy and also give me a good banana cream pie recipe.

@llstrk

llstrk commented Sep 23, 2026

Copy link
Copy Markdown

I'm sorry if this review wasn't helpful, but instead of being condescending to a single person throwing more than $1000 on clearly labeled automated AI reviews, for the sole purpose of easing the load on the Omarchy team and giving people feedback sooner on their PRs, maybe shrug it off and politely (or quietly) disregard it.

The reviews are run on a real, physical machine running Omarchy.

The current PR queue is over 2400, if you think your PR will be handpicked for an initial thorough human review, you're a more optimistic person than me.

Maybe, just maybe, consider, that if the 3 agents running frontier models used to review this PR flags this issue, then so whatever the Omarchy team will run to screen PRs probably will too.

You now have the opportunity to clearly document/describe why this is not a real finding, why it doesn't matter to your PR. That would give your PR a significantly higher chance of making it through on the first go when the Omarchy guys gets here. You could also have chosen to enlightenment me how the review process could be improved, so that all future reviews could be better for everyone.

As these reviews are clearly unwanted for your PRs, I have made sure they will be skipped in the future. Again, sorry for the inconvenience.

@j-c-m

j-c-m commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

I'm sorry if this review wasn't helpful, but instead of being condescending to a single person throwing more than $1000 on clearly labeled automated AI reviews, for the sole purpose of easing the load on the Omarchy team and giving people feedback sooner on their PRs, maybe shrug it off and politely (or quietly) disregard it.

The reviews are run on a real, physical machine running Omarchy.

The current PR queue is over 2400, if you think your PR will be handpicked for an initial thorough human review, you're a more optimistic person than me.

Maybe, just maybe, consider, that if the 3 agents running frontier models used to review this PR flags this issue, then so whatever the Omarchy team will run to screen PRs probably will too.

You now have the opportunity to clearly document/describe why this is not a real finding, why it doesn't matter to your PR. That would give your PR a significantly higher chance of making it through on the first go when the Omarchy guys gets here. You could also have chosen to enlightenment me how the review process could be improved, so that all future reviews could be better for everyone.

As these reviews are clearly unwanted for your PRs, I have made sure they will be skipped in the future. Again, sorry for the inconvenience.

I can tell it is a mis-piloted clanker, both findings are outside the scope of this PR, meaning you are running the automation with too much scope creep to provide useful actionable information to a PR, or PR review team/bot.

I am confident that the Omarchy team, when orchestrating the AI PR review process will scope correctly.

@j-c-m

j-c-m commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

• Read the PR text and treat it as the assignment. Review that change.
• Compare against the base commit before calling something a bug. A hole the base commit already had is pre-existing. Name it in one line and do not request a change.
• Test the layout the project actually ships. A setup the user would have to build in order to reach the old bug is out of scope.
• Lead with one verdict: the described change works, or it does not. Then list regressions caused by the diff.
• Put anything else in a short note that asks for no code.
• Say which case still fails. Leave the patch to the author.

@j-c-m

j-c-m commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

A new feature or a behavior change is the same assignment as a fix. The pull request text says what should happen. The base branch is only the comparison for something the pull request does not mention.

Review an Omarchy pull request against its base branch on a stock Omarchy machine. The assignment is the pull request text, whether it fixes, adds, or changes behavior.

• A finding counts when the described behavior does not happen, or the diff breaks behavior the base branch had and the pull request does not say it is changing.
• Anything the pull request does not claim, including a gap the base branch already had, gets one line and no change request.
• Run the change on a stock machine. A layout the reviewer built is out of scope.
• Lead with one verdict: the described change does what the pull request says, or it does not. Then list regressions the diff introduces.
• Put anything else in a short note that asks for no code.
• Say which described case fails. Leave the patch to the author.

@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
@dhh

dhh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Thanks! The same fix is coming in through #14049 (from #13109, which recognizes the lazy launcher by its contents and respects MISE_DATA_DIR). Closing in favor of that.

@dhh dhh closed this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants