fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init#427
Merged
github-actions[bot] merged 16 commits intomainfrom Apr 26, 2026
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe npm wrapper and install script were modified to improve detection and persistence of the user-local CLI path, suppress automatic init during Unix installs, and provide explicit error handling for installer and executable failures. Tests and a gitleaks allowlist were added/updated to validate these behaviors. Changes
Sequence DiagramsequenceDiagram
actor User
participant npm as "npm wrapper\n(npm/bin/autosearch-ai.js)"
participant bash as "bash installer"
participant fs as "Filesystem\n(~/.local/bin, shell profile)"
participant autosearch as "autosearch CLI"
User->>npm: run CLI (e.g., --yes doctor)
npm->>fs: check for autosearch in PATH and expected `$HOME/.local/bin`
alt autosearch found
npm->>autosearch: spawnSync (executes CLI)
autosearch-->>npm: exit/result
else autosearch missing
npm->>bash: spawn installer with `bash -s -- --no-init`
bash->>fs: install autosearch to `~/.local/bin`
bash-->>npm: installer exit (success/failure)
opt installer success
npm->>fs: write PATH export to chosen shell profile
npm->>npm: prepend `~/.local/bin` to process.env.PATH
npm->>autosearch: spawnSync (executes CLI)
autosearch-->>npm: exit/result
end
opt installer fails
bash-->>npm: stderr/exit -> npm prints error and exits non-zero
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
@copilot please address the blocking signals on this PR and push a fix commit. Trigger: ci-failure Follow the PR's |
![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: 49caa1399d
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Fixes the npx autosearch-ai wrapper and installer script so missing binaries and PATH propagation failures no longer “fake success”, and ensures installs don’t implicitly run autosearch init.
Changes:
- Update the npm wrapper to detect
spawnSyncENOENT, print actionable hints, pass--no-initto the installer, and refreshprocess.env.PATHpost-install. - Update
scripts/install.shto persist~/.local/bininto the user’s shell profile based on the original PATH, and add a--check-path-persistenceflag. - Add smoke/unit tests covering ENOENT handling, post-install PATH visibility,
--no-init, and PATH persistence behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
npm/bin/autosearch-ai.js |
Improves install/run reliability by handling ENOENT, updating PATH post-install, and passing --no-init. |
scripts/install.sh |
Adds ORIGINAL_PATH-based PATH persistence + a test-focused --check-path-persistence flag. |
tests/unit/test_install_sh_flags.py |
Adds unit coverage for PATH persistence behavior in install.sh. |
tests/smoke/test_npm_wrapper.py |
Adds smoke tests for wrapper ENOENT, PATH-after-install behavior, and --no-init propagation. |
tests/smoke/test_install_script.py |
Adds an install→run end-to-end smoke test through the npm wrapper. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/install.sh (1)
26-42:⚠️ Potential issue | 🟡 Minor
Usage:synopsis is missing--check-path-persistence.The flag is documented under
Flags:(lines 38–39) but the one-line synopsis at line 28 still only lists[--dry-run] [--no-init] [--version VERSION]. Users running--helpmay miss the new flag in the quick scan.📝 Proposed doc fix
Usage: - install.sh [--dry-run] [--no-init] [--version VERSION] + install.sh [--dry-run] [--no-init] [--version VERSION] [--check-path-persistence]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install.sh` around lines 26 - 42, Update the one-line synopsis in the install.sh usage help to include the new flag --check-path-persistence so the quick "Usage:" summary matches the detailed "Flags:" section; locate the Usage: block (the heredoc printed by the usage function / EOF block) and add [--check-path-persistence] to the bracketed options list (alongside [--dry-run] [--no-init] [--version VERSION]) so --help shows the flag in the synopsis.
🧹 Nitpick comments (4)
npm/bin/autosearch-ai.js (2)
58-69:prependHomeLocalBinToPathis effectively a no-op on Windows.
~/.local/binis not wherepipx/pip --userplace scripts on Windows (those go under%APPDATA%\Python\PythonXY\Scripts). On Windows the function will either early-return (noHOME) or prepend a non-existent dir. Not harmful, but the Windows install path leaves the wrapper without a corresponding PATH fix-up, so a freshly-installed-on-Windows binary may not be visible to the subsequentspawnSync("autosearch", ...). Worth either skipping on Windows or adding the Windows Scripts dir analog.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm/bin/autosearch-ai.js` around lines 58 - 69, The function prependHomeLocalBinToPath currently tries to add ~/.local/bin even on Windows, which is wrong; update prependHomeLocalBinToPath so that on Windows it either no-ops or instead computes and prepends the Windows per-user Python Scripts path (from %APPDATA%/Python/…/Scripts or the standard %APPDATA%\Python\PythonXY\Scripts) so the spawned autosearch child (spawnSync("autosearch", ...)) can find the binary; use isWindows() to branch, read process.env.APPDATA and inflect the Python version if available (or fall back to skipping on Windows), and ensure you check PATH membership using the same separator logic before prepending.
51-56: HardcodedPython312will go stale.The Windows hint will mis-point users on Python 3.13+, which has been GA for a while. Since this is purely a diagnostic hint string, low impact, but consider parameterizing the segment or trimming to the parent dir (
...\\Python\\Scripts) so the hint ages better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm/bin/autosearch-ai.js` around lines 51 - 56, The Windows diagnostic string in expectedAutosearchPath (used alongside isWindows()) hardcodes "Python312" which will become stale; change the hint to a version-agnostic path (e.g., use a parent dir like "\\Python\\Scripts\\autosearch.exe" or a wildcard-style "Python*\\Scripts\\autosearch.exe") or parameterize the Python version segment so the hint survives future Python releases.tests/unit/test_install_sh_flags.py (1)
75-118: Consider covering the in-profile idempotency branch.The two scenarios here exercise (a) the "write" path and (b) the
ORIGINAL_PATHshort-circuit atinstall.shline 150. The third meaningful branch —PATHlacks~/.local/binbut the profile already contains the export line (thegrep -Fqxshort-circuit at line 155) — isn't covered. Adding a tiny third assertion (pre-seed.zshrcwith the export line, run with PATH lacking~/.local/bin, assert file unchanged and no duplicate) would lock the actual idempotency invariant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_install_sh_flags.py` around lines 75 - 118, Add a third idempotency branch to test_install_persists_path_when_not_in_original: pre-seed the profile (.zshrc) with the exact export line 'export PATH="$HOME/.local/bin:$PATH"', set env["HOME"] to a temp dir and env["PATH"] to a value that does NOT contain $HOME/.local/bin (to exercise the grep -Fqx short-circuit in install.sh around the ORIGINAL_PATH/grep check), run subprocess.run(["bash", str(INSTALL_SH), "--check-path-persistence"], ...) and then assert the profile file content is unchanged (no duplicate export line) and the returncode is 0; reference the test function name test_install_persists_path_when_not_in_original and INSTALL_SH to locate where to add this case.tests/smoke/test_npm_wrapper.py (1)
156-198:--no-initassertion is a substring match — works, but not argv-precise.Because the wrapper invokes
bash -c "curl ... | bash -s -- --no-init", the fake bash sees["-c", "curl ...|bash -s -- --no-init"], andprintf "%s\n" "$@"writes the whole command string. The"--no-init" in install_argv.read_text(...)check therefore passes off a substring of the-cstring rather than a discrete argv slot. Fine for now, but ifrunInstallis ever refactored to pass installer args directly (e.g.,spawnSync("bash", [scriptPath, "--no-init"], ...)), this test will keep passing for the wrong reason. Consider a stricter regex liker"(^|\s)--no-init(\s|$)"to future-proof.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke/test_npm_wrapper.py` around lines 156 - 198, The current assertion in test_install_passes_no_init uses a substring check on install_argv which can match inside a single shell -c string; update the assertion to perform a stricter token-level match by using a regex search (e.g., re.search with pattern r"(^|\\s)--no-init(\\s|$)") against install_argv.read_text(...) so the test only passes when --no-init appears as a discrete argument; ensure you import re at top of the test file and update the assertion in test_install_passes_no_init (referencing install_argv and fake_bash) accordingly so future changes to runInstall invocation won’t produce false positives.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@npm/bin/autosearch-ai.js`:
- Around line 285-290: The spawnSync error handling only checks for ENOENT and
on other errors falls through to exit code 0; update the logic around the
autosearch spawnSync (the `result` from `spawnSync("autosearch", cmd, ...)`) to
treat any `result.error` as a failure: if `result.error` exists and
`result.error.code === "ENOENT"` call
`printAutosearchNotFoundHint(result.error)` and exit non-zero, otherwise log or
print a generic spawn error message for other `result.error` codes and exit
non-zero; apply the same change to the installer spawn block (the earlier
`spawnSync`/`result` handling) so both handlers treat any `result.error` as a
non-zero exit while giving a specific hint only for ENOENT.
---
Outside diff comments:
In `@scripts/install.sh`:
- Around line 26-42: Update the one-line synopsis in the install.sh usage help
to include the new flag --check-path-persistence so the quick "Usage:" summary
matches the detailed "Flags:" section; locate the Usage: block (the heredoc
printed by the usage function / EOF block) and add [--check-path-persistence] to
the bracketed options list (alongside [--dry-run] [--no-init] [--version
VERSION]) so --help shows the flag in the synopsis.
---
Nitpick comments:
In `@npm/bin/autosearch-ai.js`:
- Around line 58-69: The function prependHomeLocalBinToPath currently tries to
add ~/.local/bin even on Windows, which is wrong; update
prependHomeLocalBinToPath so that on Windows it either no-ops or instead
computes and prepends the Windows per-user Python Scripts path (from
%APPDATA%/Python/…/Scripts or the standard %APPDATA%\Python\PythonXY\Scripts) so
the spawned autosearch child (spawnSync("autosearch", ...)) can find the binary;
use isWindows() to branch, read process.env.APPDATA and inflect the Python
version if available (or fall back to skipping on Windows), and ensure you check
PATH membership using the same separator logic before prepending.
- Around line 51-56: The Windows diagnostic string in expectedAutosearchPath
(used alongside isWindows()) hardcodes "Python312" which will become stale;
change the hint to a version-agnostic path (e.g., use a parent dir like
"\\Python\\Scripts\\autosearch.exe" or a wildcard-style
"Python*\\Scripts\\autosearch.exe") or parameterize the Python version segment
so the hint survives future Python releases.
In `@tests/smoke/test_npm_wrapper.py`:
- Around line 156-198: The current assertion in test_install_passes_no_init uses
a substring check on install_argv which can match inside a single shell -c
string; update the assertion to perform a stricter token-level match by using a
regex search (e.g., re.search with pattern r"(^|\\s)--no-init(\\s|$)") against
install_argv.read_text(...) so the test only passes when --no-init appears as a
discrete argument; ensure you import re at top of the test file and update the
assertion in test_install_passes_no_init (referencing install_argv and
fake_bash) accordingly so future changes to runInstall invocation won’t produce
false positives.
In `@tests/unit/test_install_sh_flags.py`:
- Around line 75-118: Add a third idempotency branch to
test_install_persists_path_when_not_in_original: pre-seed the profile (.zshrc)
with the exact export line 'export PATH="$HOME/.local/bin:$PATH"', set
env["HOME"] to a temp dir and env["PATH"] to a value that does NOT contain
$HOME/.local/bin (to exercise the grep -Fqx short-circuit in install.sh around
the ORIGINAL_PATH/grep check), run subprocess.run(["bash", str(INSTALL_SH),
"--check-path-persistence"], ...) and then assert the profile file content is
unchanged (no duplicate export line) and the returncode is 0; reference the test
function name test_install_persists_path_when_not_in_original and INSTALL_SH to
locate where to add this case.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 707ae26c-ed77-406f-899f-3f3906020022
📒 Files selected for processing (5)
npm/bin/autosearch-ai.jsscripts/install.shtests/smoke/test_install_script.pytests/smoke/test_npm_wrapper.pytests/unit/test_install_sh_flags.py
… dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Contributor
|
PR size warning: This PR changes 532 lines (+516/-16). Consider splitting. |
Contributor
|
@copilot please address the blocking signals on this PR and push a fix commit. Trigger: ci-failure Follow the PR's |
* test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4)
Contributor
|
PR size warning: This PR changes 649 lines (+619/-30). Consider splitting. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/unit/test_install_sh_flags.py (1)
77-120: Single test covers two distinct behaviors; consider splitting for clearer failures.
test_install_persists_path_when_not_in_originalasserts both (a) the export line is written when~/.local/binis missing from$PATH, and (b) the profile is left unchanged when~/.local/binis already in$PATH. The second scenario is actually idempotency-of-ORIGINAL_PATH, not "not in original". Splitting into two tests (e.g.,test_persists_path_when_missingandtest_skips_when_local_bin_already_in_path) would yield clearer names and pinpoint failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_install_sh_flags.py` around lines 77 - 120, The test test_install_persists_path_when_not_in_original is exercising two behaviors; split it into two focused tests (e.g., test_persists_path_when_missing and test_skips_when_local_bin_already_in_path) so failures are isolated: keep the setup that writes a minimal profile and env with PATH lacking $HOME/.local/bin and assert the export line is appended in the new test (use INSTALL_SH and --check-path-persistence as currently), and move the second scenario into a separate test that creates already_configured_home with PATH containing str(already_configured_home / ".local" / "bin") and asserts already_configured_profile.read_text() equals the original_profile; preserve subprocess.run args, combined_output assertions, and timeouts/encoding in both new tests.tests/smoke/test_npm_wrapper.py (1)
75-112: ENOENT test couples to wrapper's internal--versionprobing; consider a brief comment.The test relies on the wrapper invoking
autosearch --version(triggering the self-rm) before the actualdoctorspawn, so that the second spawn hits ENOENT. This implicit ordering between the wrapper's internal probe and the user-arg spawn isn't obvious from the test body. A one-line comment near the fake script would help future maintainers understand why the binary self-removes on--version, and prevents the test from silently passing for a wrong reason if the probe is ever removed.Also consider promoting
nodediscovery to a module-level skip (as is already done intests/unit/test_npm_wrapper_permissions.py) instead ofshutil.which("node") or "node"on Line 98 — the fallback would yield a confusing FileNotFoundError on hosts without node rather than a clear skip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/smoke/test_npm_wrapper.py` around lines 75 - 112, Add a one-line comment next to the fake_autosearch script in test_spawn_enoent_returns_nonzero explaining that the script self-removes when invoked with "--version" to simulate the wrapper's internal probe so the subsequent spawn returns ENOENT; this documents the implicit ordering between the wrapper's probe and the user-arg spawn. Also replace the inline node discovery (the shutil.which("node") or "node" fallback used when invoking node) with a module-level skip guard (like in tests/unit/test_npm_wrapper_permissions.py) that calls shutil.which("node") and skips the entire test file if node is absent, so we get a clear skip rather than a FileNotFoundError at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitleaks.toml:
- Around line 38-42: The allowlist for the gitleaks tailscale-domain rule is
missing the new test file, so add a path pattern for
tests/unit/test_npm_wrapper_permissions.py to the existing [[rules.allowlists]]
paths array; update the paths entry (the same block containing
'''tests/unit/test_install_sh_flags\.py$''') to include a regex for
'''tests/unit/test_npm_wrapper_permissions\.py$''', ensuring the tests/unit test
that contains "$HOME/.local" is covered by the allowlist while leaving the
regexes (like '''\.local/(bin|share|lib|var|state|cache|tmp)''' and
'''\$HOME/\.local''') unchanged.
In `@scripts/install.sh`:
- Around line 140-154: The fallback in shell_profile currently returns
"$HOME/.bashrc" for unknown shells, which causes POSIX bash syntax to be written
into a file that non-bash shells won't source; update shell_profile so the
default (*) branch does not return .bashrc but instead emits an empty value or a
marker (and prints a warning) when the shell is not bash or zsh; also update
persist_path (or the caller that writes PATH) to check shell_profile's result
and skip writing the export line (and print an explicit message instructing the
user how to persist PATH manually) when shell_profile indicates an unsupported
shell. Ensure you reference and modify the shell_profile function and the
persist_path write path check so unsupported shells are handled gracefully.
---
Nitpick comments:
In `@tests/smoke/test_npm_wrapper.py`:
- Around line 75-112: Add a one-line comment next to the fake_autosearch script
in test_spawn_enoent_returns_nonzero explaining that the script self-removes
when invoked with "--version" to simulate the wrapper's internal probe so the
subsequent spawn returns ENOENT; this documents the implicit ordering between
the wrapper's probe and the user-arg spawn. Also replace the inline node
discovery (the shutil.which("node") or "node" fallback used when invoking node)
with a module-level skip guard (like in
tests/unit/test_npm_wrapper_permissions.py) that calls shutil.which("node") and
skips the entire test file if node is absent, so we get a clear skip rather than
a FileNotFoundError at runtime.
In `@tests/unit/test_install_sh_flags.py`:
- Around line 77-120: The test test_install_persists_path_when_not_in_original
is exercising two behaviors; split it into two focused tests (e.g.,
test_persists_path_when_missing and test_skips_when_local_bin_already_in_path)
so failures are isolated: keep the setup that writes a minimal profile and env
with PATH lacking $HOME/.local/bin and assert the export line is appended in the
new test (use INSTALL_SH and --check-path-persistence as currently), and move
the second scenario into a separate test that creates already_configured_home
with PATH containing str(already_configured_home / ".local" / "bin") and asserts
already_configured_profile.read_text() equals the original_profile; preserve
subprocess.run args, combined_output assertions, and timeouts/encoding in both
new tests.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4adbb62f-d83a-464b-bbea-207e5377379f
📒 Files selected for processing (7)
.gitleaks.tomlnpm/bin/autosearch-ai.jsscripts/install.shtests/smoke/test_install_script.pytests/smoke/test_npm_wrapper.pytests/unit/test_install_sh_flags.pytests/unit/test_npm_wrapper_permissions.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/smoke/test_install_script.py
- npm/bin/autosearch-ai.js
Contributor
|
@copilot please address the blocking signals on this PR and push a fix commit. Trigger: blocking-review 🧹 Nitpick comments (2)
🤖 Prompt for all review comments with AI agents🪄 Autofix (Beta)Fix all unresolved CodeRabbit comments on this PR:
ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (2)
Follow the PR's |
…sh syntax to .bashrc)
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me read the relevant files before planning the fixes. 🏁 Script executed: Length of output: 25352
Quick note: I'm replying to the latest automated trigger without access to the full earlier conversation thread. After reading all four files, here's what I found:
Changes across 3 files
🧠 Learnings usedFailed to handle agent chat message. Please try again. |
Contributor
|
PR size warning: This PR changes 653 lines (+623/-30). Consider splitting. |
0xmariowu
added a commit
that referenced
this pull request
Apr 26, 2026
…o-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
0xmariowu
added a commit
that referenced
this pull request
Apr 26, 2026
) * test(transcribe-openai): add failing scaffold for local path redact (F004 S3) * feat(redact): add redact_path_for_output helper + apply in video-to-text-openai (F004 S4) * docs(transcribe-openai): clarify SKILL.md output contract for local input (F004 S4) * fix(transcribe-groq): apply redact_path_for_output to local path output (F004 S5) * docs(transcribe-groq): clarify SKILL.md output contract for local input (F004 S5) * test(transcribe): avoid path_redact pytest module collision (F004) * test(redact): use generic fixture path to avoid personal-path gitleaks false positive * Revert "test(redact): use generic fixture path to avoid personal-path gitleaks false positive" This reverts commit 64bfd34. * chore(gitleaks): allowlist /Users/<placeholder>/ in personal-path rule * fix(redact): handle Windows drive-letter paths + schemeless URLs in redact_path_for_output * fix(transcribe-openai): redact extra values in _failure + use redact_path_for_output in logs * fix(transcribe-groq): redact extra values in _failure + use redact_path_for_output in logs * refactor(redact): make redact_path_for_output accept str | None natively (drop type: ignore) * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * style(transcribe): apply ruff format on openai/groq transcribe modules --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
0xmariowu
added a commit
that referenced
this pull request
Apr 26, 2026
…o-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
0xmariowu
added a commit
that referenced
this pull request
Apr 26, 2026
) * test(transcribe-openai): add failing scaffold for local path redact (F004 S3) * feat(redact): add redact_path_for_output helper + apply in video-to-text-openai (F004 S4) * docs(transcribe-openai): clarify SKILL.md output contract for local input (F004 S4) * fix(transcribe-groq): apply redact_path_for_output to local path output (F004 S5) * docs(transcribe-groq): clarify SKILL.md output contract for local input (F004 S5) * test(transcribe): avoid path_redact pytest module collision (F004) * test(redact): use generic fixture path to avoid personal-path gitleaks false positive * Revert "test(redact): use generic fixture path to avoid personal-path gitleaks false positive" This reverts commit 64bfd34. * chore(gitleaks): allowlist /Users/<placeholder>/ in personal-path rule * fix(redact): handle Windows drive-letter paths + schemeless URLs in redact_path_for_output * fix(transcribe-openai): redact extra values in _failure + use redact_path_for_output in logs * fix(transcribe-groq): redact extra values in _failure + use redact_path_for_output in logs * refactor(redact): make redact_path_for_output accept str | None natively (drop type: ignore) * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * style(transcribe): apply ruff format on openai/groq transcribe modules --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 26, 2026
…handling (#436) * test(kr36): add failing scaffold for 404 transient_error mapping (F015 S1) * docs(kr36): research candidate endpoints for 36kr search (F015 S2) * fix(kr36): apply S2 result — endpoint update or graceful transient_error (F015 S3) * refactor(kr36): replace monkeypatch identity with explicit feature flag for legacy HTML path * refactor(kr36): remove unreachable raise after _raise_search_endpoint_unavailable * refactor(kr36): annotate _raise_search_endpoint_unavailable as NoReturn * fix(kr36): read article fields from nested templateMaterial payload * fix(kr36): skip empty values when falling back across API keys * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * fix(p0-2): redact local path in cloud transcribe structured output (#428) * test(transcribe-openai): add failing scaffold for local path redact (F004 S3) * feat(redact): add redact_path_for_output helper + apply in video-to-text-openai (F004 S4) * docs(transcribe-openai): clarify SKILL.md output contract for local input (F004 S4) * fix(transcribe-groq): apply redact_path_for_output to local path output (F004 S5) * docs(transcribe-groq): clarify SKILL.md output contract for local input (F004 S5) * test(transcribe): avoid path_redact pytest module collision (F004) * test(redact): use generic fixture path to avoid personal-path gitleaks false positive * Revert "test(redact): use generic fixture path to avoid personal-path gitleaks false positive" This reverts commit 64bfd34. * chore(gitleaks): allowlist /Users/<placeholder>/ in personal-path rule * fix(redact): handle Windows drive-letter paths + schemeless URLs in redact_path_for_output * fix(transcribe-openai): redact extra values in _failure + use redact_path_for_output in logs * fix(transcribe-groq): redact extra values in _failure + use redact_path_for_output in logs * refactor(redact): make redact_path_for_output accept str | None natively (drop type: ignore) * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * style(transcribe): apply ruff format on openai/groq transcribe modules --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> * fix(p0-3): redact signed URL + reason in bcut transcribe (#429) * test(bcut): add failing scaffolds for log + result + output redact (F005 S1) * fix(bcut): redact source URL + reason text in log calls (F005 S2) * fix(bcut): redact source URL in transcribe() return dict (F005 S3) * fix(bcut): guard non-string source before redact + add coerce * fix(bcut): replace LOGGER.exception with redacted LOGGER.warning to avoid traceback leak * fix(kr36): align legacy test fixture with new endpoint schema --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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.
Summary
P0 fix for
reports/autosearch-p0-fix-plan.md§1 — npm wrapper fakes success when binary is missing or PATH was not properly persisted. Three root causes addressed:spawnSync("autosearch", ...)returned status 0 even when the binary was absent (result.errornot checked). Wrapper now exits non-zero with a fix hint pointing to the install location.scripts/install.shmutatedPATHfor the current shell only; if the user's profile didn't already have~/.local/bin, every future shell missed the binary. Now appendsexport PATH="$HOME/.local/bin:$PATH"to the matching shell profile (zsh/bash) only when the original PATH lacked it AND the line is not already present. Adds--check-path-persistencedry-run flag for testing.runInstall()returned, the wrapper immediately spawnedautosearchwithout updatingprocess.env.PATH, so a freshly installed binary in~/.local/binwas invisible. Wrapper now prepends~/.local/bintoprocess.env.PATHbefore the post-install spawn.runInstall()now passes--no-initto the underlying installer so install does not silently runautosearch init. Runtime no-args behavior is unchanged (out of scope for this PR; tracked separately).Plan
docs/exec-plans/active/autosearch-0426-p0-fix-plan-execution.md§ F003 (S1–S7, all done).Commits (TDD-style: failing test → fix)
7 commits exceeds the project's "PR stays under 5 commits" target. Tightly coupled to a single P0 root cause (npm wrapper fake success); commit titles partition cleanly into npm-wrapper (S1/S2/S5/S6), install-sh (S3/S4), and e2e (S7) blocks. Splitting into two PRs would require sequencing PR-A merge → PR-B rebase with no incremental review value. Happy to split if reviewer prefers.
Test plan
pytest tests/smoke/test_npm_wrapper.py tests/smoke/test_install_script.py tests/unit/test_install_sh_flags.py -x -q— 17 passed in 1.79spytest tests/unit/ -x -q -m "not real_llm and not slow and not network"— 674 passed, 3 skipped, 6.31spytest -x -q -m "not real_llm and not slow and not network"(pre-push hook) — 1109 passed, 3 skipped, 20.88sruff check && ruff format --check— clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests