fix(windows): follow active Codex home for tray listener - #644
fix(windows): follow active Codex home for tray listener#644clover980805-creator wants to merge 6 commits into
Conversation
* fix(ci): let the PR labeler actually write labels * test(ci): pin the labeler to the permissions the API demands
The .codexclaw/ goalplans and ledgers are per-machine agent state. They were committed with 'git add -f' despite the ignore rule, and once tracked the rule stopped applying, so they rode along into main and preview. Untrack them (files stay on disk), drop two .DS_Store files that got in the same way, and add tests/repo-hygiene.test.ts so a forced add fails CI instead of landing silently. Also drop a registry.ts comment pointing at a .codexclaw evidence file that was never committed and cannot be resolved by any reader.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR removes tracked local goal-plan state, strengthens repository hygiene checks, grants the PR labeler write access, updates Windows Codex home resolution, increments the package version, and removes an obsolete provider comment. ChangesRepository hygiene
Pull request labeler permissions
Runtime and release metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/tray/windows.ts`:
- Around line 77-81: Add focused regression coverage for currentCodexHome,
verifying that a trimmed CODEX_HOME value such as E:\codex is returned and that
the fallback resolves to the expected user-profile .codex path. Mock or
configure the environment and resolveCodexHomeDir consistently with existing
test patterns, preserving both active-home and fallback behavior.
In `@tests/repo-hygiene.test.ts`:
- Around line 50-55: Update the gitignore assertion in the “gitignore still
declares the agent-state directories” test to parse active, non-comment,
non-negated rules instead of using raw substring matching. Assert the four
required root and nested entries exactly, ensuring commented or negated patterns
do not satisfy the check.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 707c4820-5fc8-4e0f-a05f-111fccf6b798
⛔ Files ignored due to path filters (2)
devlog/.DS_Storeis excluded by!**/.DS_Storedevlog/_plan/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (11)
.codexclaw/goalplans/opencodex-4-pabcd-work-phase-wp1-wp2-dev2-go-pr/goalplan.json.codexclaw/goalplans/opencodex-4-pabcd-work-phase-wp1-wp2-dev2-go-pr/ledger.jsonl.codexclaw/goalplans/opencodex-live-unfinished-issues-and-prs-triage/goalplan.json.codexclaw/goalplans/opencodex-live-unfinished-issues-and-prs-triage/ledger.jsonl.github/scripts/pr-labeler.test.cjs.github/workflows/pr-labeler.yml.gitignorepackage.jsonsrc/providers/registry.tssrc/tray/windows.tstests/repo-hygiene.test.ts
💤 Files with no reviewable changes (5)
- .codexclaw/goalplans/opencodex-live-unfinished-issues-and-prs-triage/goalplan.json
- .codexclaw/goalplans/opencodex-4-pabcd-work-phase-wp1-wp2-dev2-go-pr/ledger.jsonl
- .codexclaw/goalplans/opencodex-live-unfinished-issues-and-prs-triage/ledger.jsonl
- .codexclaw/goalplans/opencodex-4-pabcd-work-phase-wp1-wp2-dev2-go-pr/goalplan.json
- src/providers/registry.ts
| function currentCodexHome(): string { | ||
| const raw = process.env.CODEX_HOME?.trim(); | ||
| return raw ? resolve(expandUserPath(raw)) : join(homedir(), ".codex"); | ||
| // Keep the tray's listener/launcher target aligned with Codex's active home. | ||
| // This matters on the Windows desktop setup, where CODEX_HOME is E:\\codex | ||
| // instead of the historical %USERPROFILE%\\.codex fallback. | ||
| return resolveCodexHomeDir(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a focused regression test for the active Codex home.
This changes tray behavior from local resolution to resolveCodexHomeDir(). Add a test covering a trimmed CODEX_HOME such as E:\codex and the fallback path, so a future tray change cannot silently revert to %USERPROFILE%\.codex.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile, execFileSync, spawn } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@src/tray/windows.ts` around lines 77 - 81, Add focused regression coverage
for currentCodexHome, verifying that a trimmed CODEX_HOME value such as E:\codex
is returned and that the fallback resolves to the expected user-profile .codex
path. Mock or configure the environment and resolveCodexHomeDir consistently
with existing test patterns, preserving both active-home and fallback behavior.
Source: Path instructions
| test("gitignore still declares the agent-state directories", async () => { | ||
| const ignore = await Bun.file(new URL("../.gitignore", import.meta.url)).text(); | ||
|
|
||
| for (const dir of FORBIDDEN_TRACKED_DIRS) { | ||
| expect(ignore).toContain(`${dir}/`); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert exact active ignore rules.
Raw substring matching passes for commented or negated rules (for example, !.omo/) and does not independently require both root and nested patterns. Parse non-comment rules and assert the four required entries exactly.
Proposed fix
+const REQUIRED_AGENT_STATE_IGNORE_RULES = [
+ ".codexclaw/",
+ "**/.codexclaw/",
+ ".omo/",
+ "**/.omo/",
+];
+
- test("gitignore still declares the agent-state directories", async () => {
+ test("gitignore retains active agent-state rules", async () => {
const ignore = await Bun.file(new URL("../.gitignore", import.meta.url)).text();
+ const rules = new Set(
+ ignore
+ .split(/\r?\n/)
+ .map((line) => line.trim())
+ .filter((line) => line && !line.startsWith("#")),
+ );
- for (const dir of FORBIDDEN_TRACKED_DIRS) {
- expect(ignore).toContain(`${dir}/`);
+ for (const rule of REQUIRED_AGENT_STATE_IGNORE_RULES) {
+ expect(rules.has(rule)).toBe(true);
}
});📝 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.
| test("gitignore still declares the agent-state directories", async () => { | |
| const ignore = await Bun.file(new URL("../.gitignore", import.meta.url)).text(); | |
| for (const dir of FORBIDDEN_TRACKED_DIRS) { | |
| expect(ignore).toContain(`${dir}/`); | |
| } | |
| const REQUIRED_AGENT_STATE_IGNORE_RULES = [ | |
| ".codexclaw/", | |
| "**/.codexclaw/", | |
| ".omo/", | |
| "**/.omo/", | |
| ]; | |
| test("gitignore retains active agent-state rules", async () => { | |
| const ignore = await Bun.file(new URL("../.gitignore", import.meta.url)).text(); | |
| const rules = new Set( | |
| ignore | |
| .split(/\r?\n/) | |
| .map((line) => line.trim()) | |
| .filter((line) => line && !line.startsWith("#")), | |
| ); | |
| for (const rule of REQUIRED_AGENT_STATE_IGNORE_RULES) { | |
| expect(rules.has(rule)).toBe(true); | |
| } | |
| }); |
🤖 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 `@tests/repo-hygiene.test.ts` around lines 50 - 55, Update the gitignore
assertion in the “gitignore still declares the agent-state directories” test to
parse active, non-comment, non-negated rules instead of using raw substring
matching. Assert the four required root and nested entries exactly, ensuring
commented or negated patterns do not satisfy the check.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74e2747650
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Keep the tray's listener/launcher target aligned with Codex's active home. | ||
| // This matters on the Windows desktop setup, where CODEX_HOME is E:\\codex | ||
| // instead of the historical %USERPROFILE%\\.codex fallback. | ||
| return resolveCodexHomeDir(); |
There was a problem hiding this comment.
On Windows, resolveCodexHomeDir() is semantically identical to the removed implementation: a set CODEX_HOME is still trimmed, expanded, and resolved, while an unset value still becomes homedir()/.codex because WSL discovery is inapplicable. Consequently, the cited CODEX_HOME=E:\codex scenario produces exactly the same tray entry before and after this change, so the reported tray mis-targeting remains unresolved. Trace the persisted/listener path that is selecting the wrong home and add a focused Windows tray regression test demonstrating the failure and fix.
AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
|
Hey @clover980805-creator thanks for this PR. Please adjust your description. |
3 participants
What changed\n- make the Windows tray launcher resolve the Codex home through the shared CODEX_HOME-aware helper\n- keep the listener and launcher target aligned with the active Codex workspace, including E:\codex\n\n## Validation\n- git diff --check\n- Bun typecheck not run: Bun is not available on this host session
Summary by CodeRabbit
New Features
Bug Fixes
Chores