test(e2e): sandbox HOME and cover user-scope skill deletion

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.

[jackwener]

Why

#1517 let the Skills panel delete from ~/.maka/skills and ~/.agents/skills. buildE2eEnv never overrode HOME, so an E2E run enumerated the developer's real user-scope skills — and any spec that exercised the delete button would have removed one for real.

That is exactly why #1517 shipped without an e2e for its own destructive path. This closes that gap.

How

Set HOME (and USERPROFILE) to a directory inside the throwaway userData dir that teardown already removes — no second path to leak.

Overriding the home dir sandboxes every consumer at once, rather than threading a homeDir option through each skills API and hoping none is missed. That distinction matters here: if skills:list and skills:delete ever disagreed about which directories they mean, the bug would be both invisible and destructive.

userData is pinned separately via app.setPath, so this does not move the app's data dir.

e2eHomeDir(), not a fixture

The sandbox path is exposed as a plain accessor read from the test body. A fixture would have no declared dependency on the window fixture, so Playwright could resolve it before the window is set up and hand back a stale path.

New coverage

The invocable-skills seeder gains one user-scope skill under the sandbox, which is what makes the journey assertable. The spec covers what the unit tests in skills.test.ts cannot — the renderer sending a scope-aware ref through IPC, and the list agreeing afterwards:

• a user-scope skill is removed from disk and drops out of the list — with the directory still intact after the first of the two confirm clicks, so a single stray click provably cannot delete;
• a project-scope skill offers no delete button at all.

Verification

The disk assertion is not vacuous. Inverted it to expect 'present' after the delete; it fails with Received: "gone". (The original form used expect(promise).rejects, which I could not convince myself wasn't passing for the wrong reason — replaced with an explicit expect.poll.)

The sandbox introduces no new failures. Full e2e on this branch: 69 passed, 3 failed. All three baselined with the change stashed and the app rebuilt:

spec	with sandbox	stashed baseline
settings.spec.ts:141	✘	✘ pre-existing
sidebar-navigation.spec.ts:10	✘	✘ pre-existing
sidebar-navigation.spec.ts:30	✘	✓ flaky (flips both ways)

The first two fail identically without this change — local-environment issues on my machine, green in CI.

Gates: desktop test and typecheck exit 0 · format:check clean · knip --workspace apps/desktop clean.