Skip to content

test(wsl): run contained-delete script under sh in Linux CI - #18349

Open
fettpl wants to merge 1 commit into
stablyai:mainfrom
fettpl:test/wsl-contained-delete-script-ci
Open

test(wsl): run contained-delete script under sh in Linux CI#18349
fettpl wants to merge 1 commit into
stablyai:mainfrom
fettpl:test/wsl-contained-delete-script-ci

Conversation

@fettpl

@fettpl fettpl commented Sep 3, 2026

Copy link
Copy Markdown

ELI5

The WSL contained-delete helper is a real sh program that walks approved roots then deletes one leaf. Default tests only check the argv we would pass to wsl.exe; they never run the script. This PR runs that script under sh on Linux CI so a broken kind/symlink/missing-leaf path fails the unit shard instead of shipping green.

What Changed

  • Added src/main/wsl-contained-delete.script.test.ts.
  • The test builds production argv via containedDeleteCommand, then runProcesss sh with that argv (no shell: true, no mocked execFile).
  • Cases: happy-path directory delete, kind mismatch (exit 65 / ORCA_WSL_DELETE_REJECT:kind), intermediate symlink escape (ORCA_WSL_DELETE_REJECT:symlink, outside leaf survives), missing leaf (exit 0 no-op).
  • describe.skipIf(process.platform !== 'linux') — the script uses /proc/self/cwd, /proc/self/fd, and GNU stat -Lc, which Darwin and Windows do not provide. Ubuntu unit shards still run it. Existing wsl-unc-delete.test.ts argv tests are unchanged.

Why

PR unit tests run on ubuntu-latest. Mocked execFile cannot catch a regression in WSL_CONTAINED_DELETE_SCRIPT. The script is generic POSIX aside from Linux /proc + GNU stat, so Linux CI can execute it without wsl.exe or ORCA_REAL_WSL_DELETE_TEST.

Linked Issue

Fixes #18284

Visual Proof

N/A (no visual change). Tests only; no UI, layout, or renderer change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Verification:

  • pnpm test src/main/wsl-contained-delete.script.test.ts — 4 skipped on Darwin (linux-only gate)
  • pnpm test src/main/wsl-unc-delete.test.ts — 9 passed
  • pnpm tc:node — exit 0
  • pnpm run check:code-quality:changed — 0 findings
  • Linux Docker (node:22-bookworm): the same four cases via containedDeleteCommand + sh all passed (happy path, kind, symlink, missing leaf)

Platforms: macOS (skip path) and Linux (Docker, /proc + GNU stat). Windows skipped by the linux-only gate. SSH not exercised; this is a local CI characterization of a POSIX script.

AI Disclosure

Review

  • Cross-platform: Tests run only on Linux. Windows and macOS skip. Host fixture paths use path.join; containedDeleteCommand still emits POSIX components. Spawn goes through runProcess (shell: false, program sh). No metaKey.
  • SSH / remote: No wire, RPC, or execution-host change. The production script still runs inside WSL on Windows; this only executes it under sh in Linux CI. Loss of contact is not involved.
  • Security: Script body is unchanged. Fixtures live under a unique os.tmpdir() directory; cleanup refuses paths outside that tmp root. The symlink case asserts an outside leaf is not deleted. Tests never rm -rf / and never target the approved root itself (containedDeleteCommand returns null for that).
  • Performance: Four short sh invocations with a 10s timeout; no extra CI workflow.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

Did not change WSL_CONTAINED_DELETE_SCRIPT. Did not enable ORCA_REAL_WSL_DELETE_TEST on Windows CI. Did not commit plans/.

Checklist

  • This PR is small and focused
  • I explained what changed and why (including ELI5)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Execute WSL_CONTAINED_DELETE_SCRIPT with production argv on Linux so
kind, symlink, missing-leaf, and happy-path behavior cannot regress
behind mocked execFile. Skip non-Linux hosts: the script needs
/proc/self/cwd and GNU stat.

Fixes stablyai#18284
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: b43fd0ab-05b4-472a-a373-aac8147e5626

📥 Commits

Reviewing files that changed from the base of the PR and between 720c329 and 82254ef.

📒 Files selected for processing (1)
  • src/main/wsl-contained-delete.script.test.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one optional coverage note below.

Reviewed changes

  • src/main/wsl-contained-delete.script.test.ts — a new Linux-gated integration test that builds production argv via containedDeleteCommand and runs the real WSL_CONTAINED_DELETE_SCRIPT under sh (through runProcess, shell: false), asserting filesystem side effects for four cases: directory-leaf delete, kind mismatch rejection, intermediate-symlink escape rejection, and missing-leaf no-op.

I ran pnpm exec vitest run --config config/vitest.config.ts src/main/wsl-contained-delete.script.test.ts on this Linux runner — 4/4 passed, and the assertions are tight against real filesystem side effects (deleted leaf is gone, outside secret survives, exit 65 + rejection marker on stderr), so this genuinely fails on a script regression rather than passing vacuously.

ℹ️ Non-recursive (regular file / rm -f) branch is not exercised

All four cases pass recursive: true, so the script's expected_kind='regular file'rm -f -- "$leaf_path" success path (the else at src/main/wsl-contained-delete.ts:72) never runs end-to-end. That branch is production-relevant — deleting individual files under an approved root (e.g. session.json) is a real call path — and the existing argv-only tests in wsl-unc-delete.test.ts pin its argv but never execute the script, so a regression in the rm -f branch would still ship green. Consider adding one non-recursive file-delete case.

Technical details
# Add a non-recursive (regular file) delete case

## Affected sites
- src/main/wsl-contained-delete.script.test.ts:60 — every `it` case calls `runContainedDelete` with `true`

## Required outcome
- Exercise the `expected_kind = 'regular file'``rm -f --` path by calling `runContainedDelete(leaf, approvedRoot, false)` on a regular file and asserting `code === 0`, the file is gone, and an outside survivor is untouched.

## Suggested approach
- Add one `it('deletes an approved regular-file leaf')` case mirroring the happy path with a file plus `recursive: false`.

Pullfrog  | Fix it ➔View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Execute the WSL contained-delete shell script in Linux CI

1 participant