Harden Windows runtime and skill precedence - #80
Conversation
![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: 99332bc0c5
ℹ️ 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".
| maxSkills, | ||
| precedence | ||
| ); | ||
| if (skillFiles.length >= maxSkills) break; |
There was a problem hiding this comment.
When an earlier root contains duplicate skill names, this break applies maxSkills to raw files before activeSkillRecords removes duplicates. For example, with max_skills: 2, two .codex/skills files named dup fill the limit, collapse to one result, and prevent a distinct .agents/skills skill from being scanned, so inventory returns fewer than the requested maximum and may hide valid user/plugin skills. Continue scanning lower-precedence roots until the active winner set reaches the limit, while retaining a separate bounded raw-scan safeguard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79715cb7d8
ℹ️ 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".
| const stat = fs.statSync(resolved); | ||
| if (!stat.isDirectory()) throw new Error(`Not a directory: ${resolved}`); | ||
| return fs.realpathSync(resolved); | ||
| return fs.realpathSync.native(resolved); |
There was a problem hiding this comment.
On Windows installations where fs.realpathSync.native() returns a different spelling from the prior fs.realpathSync() result (for example, canonical casing or a junction-resolved form), upgrading changes the root string passed to the unchanged hash-based profilePathForRoot. Existing profile and runtime files remain under the old hash, so settings show and start silently stop loading saved ports, tunnel credentials, and other configuration. Add a fallback lookup or migration using the legacy root identity before switching exclusively to the native representation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d0ce90686
ℹ️ 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".
| - name: Stress Test | ||
| run: npm run stress |
There was a problem hiding this comment.
On runners where bash -lc emits any startup diagnostic to stderr, this newly mandatory npm run stress step fails: runBash retains maxOutputBytes + 1 across stdout and stderr together, but src/bashOps.ts determines truncated for each stream separately. I reproduced the failure twice in this repository, where the streams totaled 4001 retained bytes while neither individually exceeded 4000, producing truncated: false and failing runBashOutputTerminationStress. Track truncation from the combined observed byte count before making the stress suite a CI gate.
Useful? React with 👍 / 👎.
3 participants
Summary
Contributor credit
058e7f5andebe401bonmainretain Zao's author metadata.9c50c60onmainretains misaka310's author metadata.Why
The original branches were based on an older
mainand conflicted after PR #79. This PR is the integration branch that preserves their authorship while resolving those conflicts against the current release.Validation
npm cinpm run buildskill-precedence-smokenpm run stressnpm audit --audit-level=high- 0 vulnerabilitiesnpm run release:packgit diff --checkIntegrates and supersedes #69 and #77.