chore(e2e): review screenshots for #2335 - #2336
Closed
copse-release-bot[bot] wants to merge 2 commits into
Closed
copse-release-bot[bot] wants to merge 2 commits into
copse-release-bot[bot] wants to merge 2 commits into
Conversation
…tocol Moves the plugin-tool wire protocol, the author-facing SDK, the isolated worker entry, runtime source discovery and reviewed snapshots, the browser-service and model-turn adapters, and the MCP server configuration schema and types into packages/plugin-sdk (@copse/plugin-sdk). Registered through the workspace protocol; depends on @copse/agent, @copse/llm, @copse/std, and zod; no host-app imports. The one host fact the snapshot helper needs, the profile root the reviewed snapshots live under, becomes configurePluginSdk (default COPSE_DIR or ~/.copse); the app binds copseDataRoot in plugin-sdk-environment.ts. The standalone worker bundle keeps its entry path: src/main/services/plugins/ plugin-tool-worker.ts is now a one-line entry that imports the package worker, so scripts/main-bundles.mts, the stdout-protocol guard, and the host's dist lookup are unchanged. Every previous import path is a re-export of the package; the one stub with no remaining importers (plugin-tool-sdk) was deleted. custom-tools-config stays in the app because it depends on the app's ToolDefinition contract. docs/plugins.md's module layout names the package. Closes #2309. Part of #2303. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
🖥️ PR preview
|
Contributor
|
Closing automatically: parent PR #2335 is closed, so these candidates have no review context left. Reference screenshots only ever move through a live parent PR. |
jonathanKingston
added a commit
that referenced
this pull request
Sep 5, 2026
) ## Why The screenshot review PR for #2335 ([#2336](#2336)) re-rendered 19 reference PNGs with no UI change behind them. Pixel-diffing each pair traced every band to something the frame read from the live runner, repo, or clock instead of a fixture. ## What churned, and the fix | Seen in the diffs | Cause | Fix | | --- | --- | --- | | `runner@runnervmgx7h7:~/work/…$` → `runner@runnervmejwal` | Runner rc files set the bash prompt; hostname changes per run | `SHELL` = `tests/e2e/fixtures/e2e-shell.sh` under e2e (bash `--norc --noprofile`, prompt `$ `) | | Ports rail `:9647 ss`, `:33569 electron`, `:36355 chromedriver` | Live host port scan | `listPortRows` reports nothing listening under `COPSE_E2E` unless a spec seeds rows, so the rail hides | | Context wheel 10% → 11% (in almost every shot) | Pre-send estimate counts the checkout's `AGENTS.md` + skills; specs open `process.cwd()` | `seedStableWorkspace()`: fixed temp project, few files, one commit at a fixed date. Migrated the specs behind these shots | | Changes rail listing CI's re-rendered `*.png` as `M` | Same: the checkout is the project | Same | | `@shell` picker results from the real tree | Same | Same | | `…selection-ZZvJEL ⤳ work` in the titlebar | `mkdtemp` suffix | Same (fixed leaf name) | | `http://127.0.0.1:44701/reference` | `listen(0)` | `listenOnFixturePort(server, port)` — fixed per-spec port, ephemeral fallback | | `Background task 84e03bbc-… exited` | `randomUUID()` in the wake message | Sequential ids under `COPSE_E2E` (`nextBackgroundOperationId`) | | Sidebar `New Thread` vs `Mock response to: …` | Auto-title lands an IPC round trip after the turn | `waitForActiveThreadTitle()` before the capture | | Settings shots shifted ~8px / ~80px | Smooth scroll captured mid-flight | `saveAppScreenshot`/`saveElementScreenshot` wait for scroll offsets and the subject's box to hold still (`waitForSettledLayout`) | | `Latest · Sep 3, 2026, 12:08 AM` | Run the real scheduler just fired; no fixture can seed it | `pinTextForCapture` swaps that one text node for the capture and restores it; the DOM assertion on the real text stays | | Changes pop-out `Unstaged (16)` with `.bashrc`, `.gitconfig`, `.claude/…` | bubblewrap materialises every missing mandatory-deny path as an empty bind-mount target; ASRT defers cleanup while another sandboxed command runs, so a concurrent `git status` (main window + pop-out) lists them | `waitForFixtureRows()` in `git-changes-popout-embed.e2e.ts` waits for the Changes list to settle on the four files the fixture seeds before each capture. **Spec-side only** — see below | `docs/testing-strategy.md` → "Deterministic screenshots" gains the matching rules. ## The one product-behaviour change that was here, and is not any more An earlier revision filtered those bind-mount targets out of `getGitStatus` (`withoutGhostMountPoints`). Review found the flaw: it ran on every Linux `git status` and identified files by path and emptiness, so a user's own zero-byte `.gitconfig` or `.bashrc` would be hidden even where no sandbox had created one. Name and size are not provenance. Recording the paths bwrap actually materialised is the sound version, and it is not viable today: `@anthropic-ai/sandbox-runtime` keeps `bwrapMountPoints` module-private, so the record would have to be ours at wrap time — and `afterSandboxedCommand` is not called by `spawnBackgroundProcess` or `spawnPtyInProjectSandbox`, so an in-flight counter would drift upward and suppress forever. Balancing that lifecycle is its own change, not a screenshot PR. Reverted in 6747fed. The product reports exactly what git reports; the determinism moved into the spec that needed it. The underlying transient remains for Linux users during overlapping sandboxed commands and deserves a separate issue against the sandbox layer. ## Validation - `pnpm run typecheck`, `lint`, `format:check`, `check:dead-code`, `check:oracle`, `check:e2e-syntax` green; new unit tests for the ports seam and background ids. - Ran the affected specs locally under Xvfb: `acp-stale-model-picker` (element-typed capture), `terminal-display`, `shell-mention`, `settings-styling`, `settings-usage-plan-worth-it`, `settings-usage-value-map-card-link`, `background-task-lifecycle`, `monaco-selection-chat`, `thread-terminal-rename-archive`, `project-switch-panels`, `portrait-panel-controls`, `automation-trigger`, `automation-attention` and `git-changes-popout-embed` pass. - The oracle plans a **full** e2e run for this diff, so the whole reference set re-renders once; the review PR the bot opens touches every shot with a terminal rail or a context wheel. ## Not done here `seedEmptyProject(process.cwd(), …)` remains in ~160 other specs. Their wheels still track the real `AGENTS.md`; migrating them is mechanical but each needs a look at whether it relies on repo contents. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JNuSTyzLnTCSRoYzbCZGzx --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jonathan Kingston <KingstonMailBox@gmail.com>
This branch was previously deployed
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.
Screenshot candidates rendered for parent PR
#2335 at
71c068f63e93cea469e5392aa5cb4822aa44e4b8byCI run 33816195270.
Review GitHub's image diffs, then merge this PR (or enable auto-merge) to apply the
accepted references to
claude/extract-plugin-sdk. This branch containsonly PNG candidates from the immutable
reference-screenshot-candidates-33816195270artifact and never targetsmain.If the parent branch has advanced beyond the source SHA above, do not merge this PR;
the successful CI run for the new head will replace it.