Dotta
c6a0cf4bce
<!-- Write all pull request text in Simplified Technical English (ASD-STE100): short sentences, one instruction per sentence, simple approved vocabulary, and the active voice. --> ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server prepares task workspaces in `workspace-runtime.ts`. One helper, `resolveShell()`, picks the shell that workspace commands use. > - `resolveShell()` computes its fallback with one two-arm ternary: `"sh"` for win32, else `"/bin/sh"`. There is no separate darwin arm. > - The tests for this helper had three unset-SHELL fallback rows: linux, win32, and darwin. The darwin row repeated the linux row. It observed the same ternary arm and asserted the same value through the same code. > - This pull request removes the duplicated darwin row. The linux row keeps the non-win32 arm pinned. > - The benefit is one less repeated registration per suite run and a smaller platform matrix to maintain. No wall-time saving is claimed. ## Linked Issues or Issue Description - No public GitHub issue exists for this cleanup. The underlying problem: the `resolveShell` fallback tests registered two cases that could not fail separately. Any change to the non-win32 fallback arm breaks both rows at the same assertion. One row is enough to protect that arm. The audit receipt in `doc/plans/2026-10-17-workspace-runtime-resolveshell-test-audit.md` records the full decision table, the deferred candidate, and the retained rows. ## What Changed - Deleted `falls back to /bin/sh on darwin when SHELL is unset` from the `resolveShell (shell fallback)` describe in `server/src/__tests__/workspace-runtime.test.ts` (6 test lines). - Kept the win32 row (the only observation of the `"sh"` arm), the linux row (the only needed observation of the `"/bin/sh"` arm), the empty-string and whitespace-only rows (missing versus invalid input; whitespace-only also pins the trim-before-check order), and the missing-absolute-path row (the only observation of the `existsSync` guard). - Added the audit receipt `doc/plans/2026-10-17-workspace-runtime-resolveshell-test-audit.md` with evidence, commands, and the deferred candidate. - Corrected the receipt's mutation-control record (headd99284b): the `/bin/bash` control fails three retained rows, not one. See Verification. - Changed zero production lines, zero lockfile lines, and zero workflow lines. ## Verification - Focused owner run: `pnpm --filter @paperclipai/server exec vitest run -t "shell fallback" src/__tests__/workspace-runtime.test.ts` (vitest 5.0.3, Node 25.9.0). - Baseline before the edit: 9/9 passed. - Candidate after the edit: 8/8 passed. - Control (re-verified on headd99284b): the production fallback arm was temporarily changed to `"/bin/bash"`. Exactly three retained rows then failed with the same assertion (`expected '/bin/bash' to be '/bin/sh'`): the linux unset row (the deleted darwin row's keeper), the empty-`SHELL` row, and the missing-absolute-path row. The win32 rows and the set-`SHELL` rows still passed (3 failed, 5 passed). This proves the retained row carries the deleted row's failure sensitivity, and the shared fallback value keeps two more independent guards. Production was restored byte-exact (sha256 `335fd5332b4242ab0ff2a565323e27db48a69ddfb7bbd8d913e5148f79ff961b` before and after), and the focused run passed 8/8 again. - The full `workspace-runtime.test.ts` suite and the repo-wide typecheck/test/build were not run locally; the isolated container cannot finish the git-fixture cases inside their 15-second budgets (three unrelated reconciliation cases time out at 15,000 ms on this host both before and after the change). PR CI runs those lanes. - PR CI on headd99284b: all 57 checks green (workflow run 39022073948; the only non-success conclusions are conditional skips). ## Risks - Low risk. The change is a six-line deletion inside one describe block, plus a receipt correction. The only theoretical loss is a darwin-specific fallback change, which does not exist today: darwin and linux share the single non-win32 ternary arm. If a darwin-specific arm is added later, that arm needs its own row and its own contract decision. - Open PRs #12658, #11753, and #16213 also touch `server/src/__tests__/workspace-runtime.test.ts`, in different regions (pnpm provisioning case, restart scoping, and worktree lock work). This change edits only the `resolveShell` describe, so the hunks do not collide. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may be redirected — check the roadmap first. See [`CONTRIBUTING.md`](CONTRIBUTING.md). ## Model Used - GLM, a large language model from Z.ai. Exact model ID: `z-ai/glm-5.3` served through OpenRouter (`openrouter/z-ai/glm-5.3`). Context window: the session that produced this change. Capabilities used: repository file search and read, shell command execution, vitest runs, and a production mutation control. No model context-window size beyond the session is asserted because it was not disclosed to the agent. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [ ] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
5.1 KiB
workspace-runtime resolveShell duplicate-row retirement — 2026-10-17
One redundant registered execution is retired from
server/src/__tests__/workspace-runtime.test.ts. No production file
changes. No execution-time or CI-time benefit is claimed.
Discovery base: 5555817c2b18a1a5ebc3a3b01883c1b12b86df5a (master head).
Applied skill: installed test-audit, SKILL.md sha256
d2569031196778bebb7fd623d87b4379ed1a0d372d1ea004b0b3d330b352d1d2.
Change and keeper
| Candidate | Decision | Contract | Keeper | Evidence |
|---|---|---|---|---|
falls back to /bin/sh on darwin when SHELL is unset (resolveShell describe) |
Delete | Non-win32 fallback returns /bin/sh. |
falls back to /bin/sh on non-Windows when SHELL is unset (linux row) |
resolveShell() computes the fallback from a single two-arm ternary, process.platform === "win32" ? "sh" : "/bin/sh". darwin and linux both evaluate the same non-win32 arm and assert the same value through the same production path, so the darwin row cannot fail while the linux row passes. Inputs and assertions are otherwise identical (unset SHELL, mocked platform, fallback value). |
Retained siblings in the same describe: the win32 row (only observation of
the "sh" arm), the empty-string and whitespace-only rows (missing versus
invalid input; whitespace-only additionally pins trim-before-falsy-check),
and the missing-absolute-path row (only observation of the existsSync
guard). The non-Windows/linux row keeps the non-win32 arm pinned after this
change, verified by control below.
Validation
Focused owner runs, pnpm --filter @paperclipai/server exec vitest run -t "shell fallback" src/__tests__/workspace-runtime.test.ts (vitest 5.0.3,
Node 25.9.0, isolated checkout with pnpm install --filter @paperclipai/server...):
- Baseline (before edit): 9/9 passed.
- Candidate (darwin row deleted): 8/8 passed.
- Control: temporarily changed the production fallback arm to
"/bin/bash"and reran the same focused selection (re-verified 2026-10-17). Exactly three rows failed at the same assertion shape (expected '/bin/bash' to be '/bin/sh'): the retained linux unset row (the deleted darwin row's keeper), the empty-SHELLrow, and the missing-absolute-path row. The win32 rows and the set-SHELLrows still passed (3 failed, 5 passed). This proves the retained linux row carries the deleted row's failure sensitivity; it also shows the same non-win32 fallback value is observed by the empty-SHELLand missing-absolute-path rows, so that value keeps multiple guards after this change. An earlier draft of this receipt wrongly said the mutation "breaks nothing else" in the selected run; that claim is corrected here (Greptile review finding, PR #16639). - Production restored byte-exact (sha256
335fd5332b4242ab0ff2a565323e27db48a69ddfb7bbd8d913e5148f79ff961bbefore and after), rerun: 8/8 passed.
The full workspace-runtime.test.ts owner and the repo-wide
typecheck/test/build suite were not run locally in this session; this
container cannot complete the git-fixture cases inside their 15-second
budgets (three unrelated reconciliation cases time out at 15,000 ms here
before and after this change, and the production-internal 8-second git scan
budget also fires), so those runs are left to PR CI. The change is a
six-line deletion inside one describe; no production file, lockfile,
workflow, or shard manifest is touched.
Deferred candidate (evidence, not applied)
keeps forward reconciliation fail-closed for an unrelated task branch
(line ~3491) repeats keeps forward reconciliation fail-closed for same-content rewritten history (line ~3461): identical git topology (each
branch one commit ahead of main), identical assertion union through
expectPersistedBranchMismatchRejected (verdict diverged, reason
expected branch and current HEAD differ), and the differing fixture bytes
(file names, commit messages) are never read by
inspectGitWorktreeBranchIncoherence, which decides on cleanliness, branch
existence, ancestry, and worktree registration only. The third sibling
(live branch is behind) is retained: its topology makes the actual branch
an ancestor in the reverse direction, so it uniquely guards the ancestry
comparison's direction. Deletion of the unrelated-task row is deferred
because the local environment cannot produce a passing baseline for these
git-fixture cases (15-second per-test timeout); it needs validation on a
host where the fixtures complete, or via CI evidence.
Other examined-and-retained areas
native-runner-input-boundary.test.tsalias: retained per the 2026-10-13 execution-wrapper receipt; still the only entry point for the Runner contract suite in the root default run. Historical weight 204 ms.- Repo-wide duplicate-title and near-identical-body scans over the top-30 duration-weighted server suites: all other matches protect distinct contracts (different function, input class, platform arm, or boundary) and are retained.
Test source changes: −6 test lines, zero production lines, zero docs lines outside this receipt. One registration removed per complete-suite invocation. No measured wall-time saving is claimed.