fix(resolvers): empty find must not fall through to cwd - #2483
Open
tranthanhnhatkhoa wants to merge 1 commit into
Open
fix(resolvers): empty find must not fall through to cwd#2483tranthanhnhatkhoa wants to merge 1 commit into
tranthanhnhatkhoa wants to merge 1 commit into
Conversation
Same defect as bin/gstack-codex-session-import: `find ... | xargs ls -t`
runs `ls -t` with NO OPERANDS when find matches nothing, and `ls -t`
with no operands lists the CURRENT DIRECTORY. Three sites, two of them
user-visible:
review.ts:859
PLAN becomes a random .md from wherever /review was invoked, and is
then read as the plan for the review.
generate-context-recovery.ts:15
"RECENT ARTIFACTS" lists unrelated files from cwd.
generate-context-recovery.ts:24
LATEST_CHECKPOINT points at a random file, which the skill is then
instructed to read.
All three only misfire when the projects dir has no ceo-plans /
checkpoints / plans yet — i.e. on a fresh install, where the wrong
output is least likely to be recognized as wrong.
`-exec ls -t {} +` never runs the command when nothing matched, and
avoids GNU-only `xargs -r` (this repo targets macOS and has no existing
`xargs -r` to follow).
Regenerated with `bun run gen:skill-docs` (46 files) and refreshed the
three ship goldens, which are byte-for-byte copies of generated output
and have no update script. Every regenerated line is one of the three
fixes or its comment — no unrelated drift.
Verified:
suite before 17 fails
suite after 17 fails, no golden or /spec regressions
goldens 3 fail -> 0
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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
1 participant
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.
Follow-up to #2482, which fixed this defect in
bin/gstack-codex-session-import. Same bug, three more sites, two of them user-visible.The bug
find ... | xargs ls -trunsls -twith no operands whenfindmatches nothing — andls -twith no operands lists the current directory.scripts/resolvers/review.ts:859PLANbecomes a random.mdfrom wherever/reviewwas invoked, then gets read as the planpreamble/generate-context-recovery.ts:15preamble/generate-context-recovery.ts:24LATEST_CHECKPOINTpoints at a random file, which the skill is then told to readAll three only misfire when the projects dir has no
ceo-plans/checkpoints/plansyet — a fresh install, which is exactly where a wrong answer is least likely to be recognized as wrong. A new user's first/reviewcan silently review against someone else's markdown.Lines 857–858 in
review.tspipe togrep -l, which reads stdin when given no files and correctly matches nothing. Left alone.The fix
-exec ls -t {} +never runs the command when nothing matched. Same substitution as #2482, and the same reason for avoidingxargs -r: GNU-only, repo targets macOS, no existing use to follow.Regeneration
These are resolvers, so the change propagates into generated docs:
bun run gen:skill-docs— 46 filestest/fixtures/golden/refreshed by hand; they're byte-for-byte copies of generated output and have no update script, sogen:skill-docsleaves them stale and the golden tests fail until they're copied overWorth knowing:
.agents/skills/gstack-ship/SKILL.mdand.factory/skills/gstack-ship/SKILL.mdare gitignored, sogit statusshows nothing while their goldens go stale. Only the test catches it.I checked every regenerated line — all are one of the three fixes or its comment. No unrelated drift.
Verified
The 17 are pre-existing and unrelated; #2480, #2481 and #2482 cover 15 of them, and the remaining 2 are environmental on my box (no
claudeon PATH for the test user, andcdp-e2eneeds Chrome).Environment: bun 1.3.14, Ubuntu 24.04.4 LTS,
main@960c3a8d(v1.60.2.0).🤖 Generated with Claude Code
https://claude.ai/code/session_01H3KRWmyJnSvzzxeooDxuU3