Add containers-canvas: a Copilot plugin for local Docker - #621
Open
Patrick Verbrugge (patverb) wants to merge 29 commits into
Open
Patrick Verbrugge (patverb) wants to merge 29 commits into
Patrick Verbrugge (patverb) wants to merge 29 commits into
Conversation
Adds extensions/containers-canvas, a Copilot plugin that renders local Docker state in a panel: container and image lists, live log streaming, CPU/memory charts, a filesystem browser, image layer breakdown, Dockerfile provenance, and an optional PTY terminal. The panel tracks the daemon through `docker events` with a 10s poll as a safety net, so changes made outside it appear without a manual refresh. Three things in here look like mistakes and are not: * .gitignore negates dist/, NOTICE.html and LICENSE.md for this package only. Copilot plugins install straight from a git ref with no build or install step, so the bundle and the legal files have to be committed; postinstall never runs for an installed plugin. * .gitattributes marks dist/ and NOTICE.html as -text. esbuild writes LF and the notice embeds third-party licence text verbatim, so letting autocrlf normalise them would make every rebuild look like a change and would alter quoted licence text. * package.json pins typescript. @trpc/server declares a required peer on it, and without an explicit version pnpm satisfies that with the TypeScript 7 the root installs as @typescript/native. That compiler has no API, so typescript-eslint silently loses type information and `pnpm -r lint` fails across the whole repository. NOTICE.html is generated at build time from esbuild's metafiles, covering the 57 packages that actually contribute bytes to the bundle. It excludes build tools and tree-shaken code, and fails the build if a bundled package ships no readable licence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Installing the extension from a URL produced an extension that could not load: `Cannot find module ...\containers-canvas\dist\host.mjs`. The file is committed and present on the branch, so it was dropped in transit. The packaging flow excludes a directory named `dist` as regenerable build output. For most extensions that is right; for this one `dist/` held the only thing that runs, because Copilot plugins install from a git ref with no build step. Confirmed by sharing a probe extension containing three subdirectories: assets/nested/c.js survived bundle/a.js survived dist/b.js dropped So nested directories are preserved and the exclusion is specifically the `dist` name. Renaming the build output to `bundle/` keeps it. Also updates the .gitignore negation and the .gitattributes -text rule that were pinned to the old path, and the webview root resolution in server.mjs. Note for anyone hitting the same wall: the gist route cannot carry this extension either, but for a different reason -- share enforces a 1 MB per-file limit and the webview bundle is 1.35 MB. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Installing from a URL failed with:
File `extensions/containers-canvas/bundle/webview/main.js` is too large
(1348568 bytes > 1000000 byte limit)
The installer rejects any file over 1 MB. The previous symptom was worse
than an error: an oversized file was dropped silently, so the extension
installed and then failed to load with a missing-module error.
The sub-views now load on demand via React.lazy, so the single 1.35 MB
webview bundle becomes a 350 KB entry plus chunks. Largest shipped file is
now 709 KB (bundle/host.mjs). xterm, at 337 KB the biggest single
dependency, no longer loads unless the terminal is opened.
Verified against the running panel: all nine dynamic import specifiers
resolve 200 with `text/javascript`, and chunk names and bytes are stable
across rebuilds so the committed bundle does not churn.
Two details worth knowing:
* RunImageDialog was always mounted with `open={false}`. Left as-is it
would have fetched its chunk during first paint and given up the split,
so it is now mounted only while a target is set.
* esbuild also emits a duplicate stylesheet beside the terminal chunk.
Nothing loads it -- importing a JS chunk does not pull in a sibling CSS
file -- so index.html continues to load main.css eagerly, which is what
keeps the terminal styled.
The build now fails if any shipped file exceeds the limit. This constraint
is easy to drift back over, and the failure it causes is silent and
happens on someone else's machine.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The canvas had no discoverability story. Whether it opened at all depended on the agent picking it from the canvas description, so "why did web stop?" was as likely to produce pasted `docker logs` output as a log viewer. Adds skills/containers-canvas/SKILL.md, declared from .plugin/plugin.json, following the pattern used by coreai-microsoft/canvases-cloud-foundation. It routes container and image questions to the panel, carries the deep-link mapping so a question lands on the view that answers it, and says what to do when registration fails. The frontmatter description states what the panel is not for as well as what it is for -- authoring Dockerfiles, registry and cloud services, and build-time failures that never produced a container all belong elsewhere. Verified by installing a renamed copy from a clean staging directory: Plugin "containers-skilltest" installed successfully. Installed 1 skill. and `copilot skill list` shows it under "Plugin skills". Note for anyone editing the description: it must stay quoted. An unquoted ": " is parsed as a YAML mapping and the skill fails to load with only a `copilot skill list` error to show for it. That is how the first version of this file failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
…arning
Adds .github/plugin/marketplace.json listing the containers plugin, using a
relative same-repository source. Catalog and payload live in one git
snapshot, so there is no SHA to bump when the plugin changes.
Every install route we have today prints "Direct plugin installs (repos,
URLs, local paths) are deprecated. Only plugin@marketplace installs will be
supported in a future release." A catalog is the supported path.
Verified against a fixture that reproduces this repository's layout and
plugin depth, renamed so it could not collide with a real install:
copilot plugin install containers-mktest@mpdepth
-> Plugin "containers-mktest" installed successfully. Installed 1 skill.
-> no deprecation warning
It also confirms the relative source resolves at ./extensions/<name> depth,
and that a local-path marketplace loads the plugin live from the clone
rather than copying it, which is a better development loop than the current
copy-out-of-the-worktree-and-install dance.
This is additive. No existing install route is removed, and the file is
inert until someone runs `marketplace add`: having it in the workspace does
not register anything. Repo-wide lint and tests are unaffected, and the
root .github/ directory is already excluded from the VS Code extension
package.
`marketplace add microsoft/vscode-containers` reads the catalog from the
default branch, so the remote form starts working when this lands on main.
Until then a local clone path works and follows the checked-out branch.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
…from package.json here has two audiences. pnpm reads it as a workspace manifest; users read it as part of an installed plugin, because a plugin installs from a git ref with no build step, so the source manifest is the shipped manifest. That split already shipped one defect. `"esbuild": "catalog:"` is valid pnpm and meaningless to npm, and the README asks users to run `npm install @lydell/node-pty` in the plugin root to enable the terminal. Anyone who followed it got EUNSUPPORTEDPROTOCOL. scripts/verifyManifest.mjs now runs first in the build and rejects any dependency specifier npm cannot resolve: catalog:, workspace:, link:, portal:, file:. Reintroducing the original bug fails the build with the offending field named. It checks devDependencies as well as runtime dependencies. That is not belt-and-braces -- measured against npm, `npm install <pkg> --omit=dev` still fails with EUNSUPPORTEDPROTOCOL when a devDependency uses a pnpm-only protocol, because npm parses every dependency field before deciding what to install. Checking only runtime dependencies would have missed the exact case that shipped. Also corrects the terminal instruction to `npm install --omit=dev`. Without it a user pulls the 13 build-time devDependencies: measured at 104 packages against 2. Nothing at runtime imports them. The underlying awkwardness stands: one file cannot be both a workspace manifest and a minimal runtime manifest. The full fix is to separate the plugin root from the workspace package so the build can emit a stripped manifest, which is a layout change and would mean re-verifying every install route. Documented in the README rather than left to be rediscovered. Adapted from the artifact verifier in coreai-microsoft/canvases-cloud-foundation, which asserts that installed artifacts cannot require npm installation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
…ource bundle/ and NOTICE.html are committed because a Copilot plugin installs from a git ref with no build step -- the committed bytes are what users execute. Nothing verified they still matched the source. That failure is silent. Editing a source file and not rebuilding leaves every gate green: source contains probe : True bundle contains probe : False tests : 104 pass lint : exit 0 git : only src/ modified The commit lands, users installing from it run the previous bundle, and the repository describes behaviour the shipped code does not have. CI did not help: it rebuilds into its own workspace and never compares the result with what was committed. scripts/verifyBundle.mjs runs as the `package` script, which the shared vscode-azuretools CI template runs directly after `build`, and fails when bundle/ or NOTICE.html differ from the commit. It uses `git status --porcelain` rather than `git diff` so untracked generated files are reported as well. That is the more dangerous direction: a chunk the build emits locally but which was never committed is missing for every other user, and a diff would ignore it. Verified in four states: clean tree passes; a rebuilt bundle after a source edit fails naming the file; an uncommitted new chunk fails; and outside a git checkout it skips rather than fails, because an installed plugin is not a repository. The repo-wide sequence lint -> build -> package -> test passes. Adapted from the checksums.json/verifyArtifact step in coreai-microsoft/canvases-cloud-foundation, reduced to what a committed-artifact layout needs: the build is already byte-deterministic across rebuilds, so git is a sufficient oracle. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
execSessions.mjs claimed "the UI says so rather than implying a sandbox that does not exist". It did not. hostEscapeRisks was implemented and covered by 18 test references, and referenced zero times in TerminalView -- the safeguard was built, tested, and never connected, while a SECURITY BOUNDARY comment asserted it was live. The server now computes the risks for the target and sends them with the `ready` frame; TerminalView renders them above the terminal. Verified against real containers through the panel's own WebSocket: privtest ready risks=["it runs privileged"] plaintest ready risks=[] and containerHostEscapeRisks flags buildx_buildkit_desktop-linux, the container the original comment cites as the example. This is deliberately weaker than execCommand, which refuses outright unless acknowledgeHostAccess is set. That path is driven by the agent, which may not know what it is asking for. This one is driven by a person who already chose the container, so it informs rather than blocks. Also makes the build clean bundle/ first. Chunk names carry a content hash, so editing a view emits a new file and orphans the old one. This change produced a second TerminalView chunk beside the 341 KB one it replaced, and because the bundle is committed, that dead chunk would have shipped to users and stayed in the repository. Found by the drift check added in 01fe832, which reported the new chunk as untracked. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
This package had `"lint": ""` -- 27 files and 7,432 lines with no static analysis, while every other package in the repository is linted. The shared config could not be reused as-is. Both azExtEslintRecommended and its type-checked variant drive typescript-eslint through the project service and include type-aware rules such as no-floating-promises, which need a TypeScript program. This package is plain JavaScript and JSX, so those rules error out rather than degrade. The config composes the parts that do apply: the repository's copyright header rule, ESLint's recommended set, and the TypeScript parser used purely as a syntax parser so it can read JSX. No new dependencies -- the tooling is already declared at the repository root. 41 findings, all fixed: * 34 x header/header: the repository convention is a blank line after the copyright header. Mechanical. * onExec was passed to Detail and never called, so the Commands view was reachable only through an agent deep-link. A person using the panel had no way to open a view the README documents. Now rendered as a button beside terminal. This is the same failure as the terminal warning fixed in cb2fd8c: built, wired at one end only, and invisible without a tool that looks for unused bindings. * useRef in main.jsx and ArrowSortRegular in LayersView.jsx were dead imports; ArrowSortRegular was still being bundled into a chunk. * startCanvasServer destructured an instanceId it never used, and extension.mjs passed one. Both removed. * host-entry.mjs initialised `latest` with a value the loop always overwrites before reading. * The ANSI-escape strip in followStats legitimately needs a control character, so it carries a scoped disable with the reason. `$schema` rest-sibling omission is allowed through ignoreRestSiblings rather than renamed, since that is the idiomatic way to drop a property. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The loopback server set no security headers. Adds them once at the top of the request handler rather than at each of the 16 writeHead sites, so error paths carry them too -- verified on 200, 404 and the 403 origin rejection. nosniff matters most on the routes that echo container output. Logs and command results are attacker-influenced text, and without it a browser may sniff a response into something executable regardless of the Content-Type. CORP is same-origin. I first shipped cross-origin, reasoning that a strict value could break a panel the host frames from another origin under COEP. That reasoning was wrong, and testing it rather than trusting it is what caught it: with same-origin the iframe still loads and still holds its SSE stream, so the stricter value is what ships. The comment now records what was verified, plus the symptom and fix if a future host does enable COEP. Rendering was confirmed by checking for an established connection to the panel port, which only exists once the iframe has loaded and run its client code. Chrome DevTools was not exposed in this session. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
… rules Several rules in this package look arbitrary and are not. They exist because something shipped broken, and the reasoning lived only in commit messages, which nobody reads before editing a build file. docs/constraints.md states the contract those rules serve: An installed plugin folder must run with no build step, no npm install, no repository-root resolution, and no assumption about the working directory. Every runtime import must resolve from files inside the plugin folder or from the host SDK. and then records each constraint with the failure that produced it and the commit that fixed it: the catalog: protocol that broke npm for users, the packaging flow dropping any directory named dist, the 1,000,000-byte install limit, committed generated files going stale invisibly, orphaned content-hashed chunks, line-ending normalisation of the bundle, and the typescript pin that keeps lint working repository-wide. Citing the incident is deliberate. A rule with a reason can be argued with and removed when the reason expires; a rule without one gets cargo-culted or deleted by someone who assumes it is noise. It closes with the two cases where a comment claimed a safeguard the code did not have, and the near-miss where testing a third claim showed it was wrong. The README now points at it as required reading before changing the build, the packaging layout, the shipped package.json, or anything under bundle/. Every file path, enforcement claim and commit SHA cited was verified. The borrowed idea is from coreai-microsoft/canvases-cloud-foundation, whose skills carry `source: earned (<incident>)` metadata and whose portability guide is declared required reading for specific kinds of change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
…mitation The changelog described the package as of 2026-09-15 and had not moved since, while the release gained live daemon tracking, a code-split webview, a router skill, a plugin catalog, response security headers and the terminal host-access warning. 1.0.0 has not shipped, so these fold into it rather than inventing a version. It also listed as a known limitation: Files copied out of a container accumulate in .copilot-containers/ in the session working directory and are not cleaned up automatically. That is not true. tidyExtractionDir keeps the ten most recent extractions and prunes anything older than a day, and it is called on every extraction. The other limitations were re-checked and still hold: Podman remains unsupported, Windows remains the only tested platform, and compose containers managed by Docker Desktop still report empty labels. Bulk pruning is still unexposed, and the mount restrictions are still enforced and covered by tests. Replaces it with the real one: changes made outside the panel appear within a few seconds, and anything the event stream misses waits for the next poll. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
@fluentui/react-icons declares MIT and ships no licence file, so generateNotice.mjs supplies the text. The entry used the licence @fluentui/react-components ships, on the assumption that a sibling package from the same publisher carries the same terms. It does not. Fetched from microsoft/fluentui-system-icons, the package's own repository, the licence is plain MIT -- "Copyright (c) 2020 Microsoft Corporation" -- with none of the assets-clause wording react-components adds about fonts and icons, and different phrasing throughout. The entry looked right, cited a real source, and was wrong. It is now the upstream text, compared rather than inferred, with the repository URL as its provenance. The assets clause still appears 34 times in the notice, correctly: those are Fluent packages that ship it in their own licence files. Also records in docs/constraints.md the two release gates this build cannot enforce: the notice has not been through the repository's release pipeline, which is external tooling and unavailable here, and any future licence override deserves the same suspicion this one earned. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The constraints doc claimed this package's NOTICE.html "has not been through the release pipeline" and had to be accepted before publishing. That framed a normal difference as a gate, and the premise was wrong: the repository's root notice is maintained by hand in the release commit, not produced by tooling this package is failing to run. The 2.5.2 release changed it by one line. Generating ours is a deliberate difference rather than a shortfall. This package bundles a different dependency set from the VS Code extension, and deriving the notice from the bytes esbuild actually emitted is what keeps it accurate as dependencies change. The drift check means it cannot fall behind what ships. What remains is the ordinary human read any notice gets at release, and specifically the one entry supplied by override -- which is worth looking at, having already been wrong once. Invented gates cost as much as missing ones: this doc exists to stop rules being cargo-culted, so it should not add one of its own. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Every existing check reads bytes. Unit tests import src/, the build measures file sizes, the drift check compares hashes. None can see whether the panel renders, whether a lazily-loaded view arrives, or whether a button is connected to anything -- and that gap has produced four bugs that shipped past green tests: a dist path error that would have blanked every panel, detail views rendering a stale snapshot, a privileged-container warning wired at one end only, and a Commands button passed a handler it never called. scripts/verifyPanel.mjs closes it. It starts the loopback server in-process, launches headless Edge, and drives the built UI against containers it creates and removes itself. Each assertion corresponds to one of those bugs. No new dependencies: the browser is driven over the DevTools protocol with the ws client the server already uses. Docker and Edge are required; when either is absent it exits 0 with an explanation rather than failing a machine that cannot run it. Writing it found two problems in changes I had previously called verified: * Clicking a lifecycle button silently did nothing while an operation was in flight. The state badge is refreshed by a daemon event independently of the disabled flag, so a control can read as current and still reject the click for another half second. Evaluating the click once looked exactly like a missing button; clicks now wait for the control to be enabled. * The Commands view assertion matched innerText against a placeholder, which is an attribute and never appears in text. It passed nothing and failed everything. One artifact is reported rather than hidden: a single /rpc request is aborted during startup, before any interaction. State still arrives and the panel renders, so it costs a wasted request rather than correctness. It is not StrictMode -- this is a production React build -- and the cause is not established, so the run prints it instead of filtering it away. Stable across three consecutive runs at 9/9. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The skill treated every unavailable canvas as a broken install. On a host with no canvases at all - the CLI - it answered the question correctly from docker and then advised a reinstall of a plugin that was installed and healthy. Split the two cases: no open_canvas tool means this host has no canvases, which is expected and should be answered with docker quietly; open_canvas present but reporting the canvas unavailable keeps the reload-and-retry path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The plugin installs on every Copilot client but only the app can render a canvas. Nothing in the manifest or marketplace schema gates a plugin by client, so the only place to set the expectation is the text users read while browsing. Adds the client note to both published descriptions and to the README requirements, and states plainly that a CLI install is not a broken one. The canvas description in extension.mjs is agent routing text and is left alone. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
plugin.json declared the Agent Plugins 1.0 schema while sitting at the legacy .plugin/ location and carrying a skills path field. The CLI is lenient about where it looks, so it loaded, and the field appeared to be doing the work. A probe showed it was not: with an Agent Plugins 1.0 manifest pointing skills at ./altskills/, the skill loaded only from the conventional skills/ directory and not from the one the manifest named. Ours worked by coincidence. Move the manifest to the plugin root, which the format requires and which any other client implementing it will look for, and drop the dead field. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
verifyBundle rebuilt the bundle and diffed the bytes. CI runs on ubuntu-latest and every committed byte was built on Windows, so that check would have failed on every pull request. The cause is pnpm, not esbuild: its virtual store directory is named differently on Windows, which shortens it, and esbuild writes those paths into the host bundle as comments and into the [hash] of each shared webview chunk. Two chunks with identical contents came out as chunk-BMJR3RQE.js and chunk-PMF66EPL.js, and every importer then differs - 14 of 18 files. The Linux output is still correct: it passed the browser harness 9/9, and NOTICE.html is byte-identical. Minifying the host leaves the chunk hashes, and dropping the hash collides the three shared vendor chunks, so both were rejected. Equal bytes would mean pinning pnpm's store layout for the whole repository. Digest the source instead - src, build.mjs, generateNotice.mjs, package.json - and record it in bundle/build-inputs.json. Line endings are normalised so a CRLF checkout agrees with CI. NOTICE.html keeps its byte comparison, being reproducible. Verified by rebuilding a Windows-built bundle on Linux: chunk names changed and the check passed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The comment still said dist/, which this package renamed to bundle/ because the extension packaging flow drops a directory called dist. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Copilot started reviewing on behalf of
Patrick Verbrugge (patverb)
September 23, 2026 21:31
View session
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Copilot review overview
🟡 Changes recommended
Critical safety and bundle-integrity findings remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 7
Open (13)
What changed in this PR
Adds a Copilot plugin providing a live Docker container/image canvas with logs, stats, files, terminals, agent actions, and committed installable bundles.
Changes:
- Adds Docker runtime operations and webview views.
- Adds plugin metadata, marketplace registration, routing skill, and CLI fallback.
- Adds packaging, licensing, bundle validation, and tests.
Unresolved findings include missing host-side confirmation for destructive actions; runtime validation and path-safety gaps; stale or incomplete bundle verification; server and PTY cleanup issues; and UI, audit, logging, installation, and localization issues.
| File | Reviewed changes |
|---|---|
scripts/postinstall.mjs |
Copies plugin licensing during postinstall. |
pnpm-workspace.yaml |
Registers the plugin workspace. |
extensions/containers-canvas/src/webview/TerminalView.jsx |
Implements terminal UI; reconnect and handshake cleanup need fixes. |
extensions/containers-canvas/src/webview/StatsView.jsx |
Implements container statistics. |
extensions/containers-canvas/src/webview/panelUrl.js |
Builds tokenized panel URLs. |
extensions/containers-canvas/src/webview/LogView.jsx |
Implements streamed logs; trailing partial lines need flushing. |
extensions/containers-canvas/src/webview/LayersView.jsx |
Displays image layers. |
extensions/containers-canvas/src/webview/index.html |
Defines the webview shell and CSP. |
extensions/containers-canvas/src/webview/FilesView.jsx |
Implements filesystem browsing. |
extensions/containers-canvas/src/webview/ExecView.jsx |
Provides command execution UI. |
extensions/containers-canvas/src/webview/ExecHistory.jsx |
Displays command history; duplicate entries and mutable-name filtering need fixes. |
extensions/containers-canvas/src/webview/DockerfileView.jsx |
Displays Dockerfile provenance. |
extensions/containers-canvas/src/webview/CodeView.jsx |
Renders structured output. |
extensions/containers-canvas/src/webview/canvasVsCodeApi.js |
Provides the webview transport adapter; rejected fetches need handling. |
extensions/containers-canvas/src/vscode-stub.mjs |
Provides VS Code compatibility stubs. |
extensions/containers-canvas/src/stubPanel.mjs |
Provides panel transport stubs. |
extensions/containers-canvas/src/security.test.mjs |
Tests security behavior. |
extensions/containers-canvas/src/index.mjs |
Defines the host bundle entry point. |
extensions/containers-canvas/src/host-entry.mjs |
Bridges host and RPC operations. |
extensions/containers-canvas/src/agentActions.mjs |
Generates agent actions. |
extensions/containers-canvas/skills/containers-canvas/SKILL.md |
Routes container questions to canvas views. |
extensions/containers-canvas/scripts/verifyManifest.test.mjs |
Tests manifest validation. |
extensions/containers-canvas/scripts/verifyManifest.mjs |
Validates install metadata. |
extensions/containers-canvas/scripts/verifyBundle.mjs |
Verifies bundle inputs; output inventory and integrity checks are incomplete. |
extensions/containers-canvas/scripts/generateNotice.test.mjs |
Tests notice generation. |
extensions/containers-canvas/scripts/generateNotice.mjs |
Generates third-party notices. |
extensions/containers-canvas/scripts/buildInputs.test.mjs |
Tests build-input digests. |
extensions/containers-canvas/scripts/buildInputs.mjs |
Computes build digests; host sources and dependency resolution are incomplete. |
extensions/containers-canvas/plugin.json |
Defines plugin metadata. |
extensions/containers-canvas/package.json |
Defines dependencies and package scripts. |
extensions/containers-canvas/LICENSE.md |
Supplies plugin licensing. |
extensions/containers-canvas/extension.mjs |
Registers the canvas and agent actions; destructive operations lack host-side confirmation. |
extensions/containers-canvas/execSessions.mjs |
Manages optional PTY sessions; the unavailable-install command is inconsistent with documentation. |
extensions/containers-canvas/eslint.config.mjs |
Configures linting. |
extensions/containers-canvas/com.github.copilot/extensions/containers-canvas/extension.mjs |
Provides the distribution entry point. |
extensions/containers-canvas/CHANGELOG.md |
Contains release notes; this hand-maintained file should remain unchanged. |
extensions/containers-canvas/bundle/webview/main.css |
Provides bundled webview styles. |
extensions/containers-canvas/bundle/webview/index.html |
Provides the bundled webview shell. |
extensions/containers-canvas/bundle/webview/chunks/TerminalView-MO2UQ5XN.css |
Provides terminal chunk styles. |
extensions/containers-canvas/bundle/webview/chunks/StatsView-BRUNNGK7.js |
Provides bundled statistics view code. |
extensions/containers-canvas/bundle/webview/chunks/RunImageDialog-JPAWVPY2.js |
Provides bundled image-run dialog code. |
extensions/containers-canvas/bundle/webview/chunks/LogView-ILJEIKWI.js |
Provides bundled log view code. |
extensions/containers-canvas/bundle/webview/chunks/LayersView-MCQU74A7.js |
Provides bundled layers view code. |
extensions/containers-canvas/bundle/webview/chunks/FilesView-HW7GUK6Z.js |
Provides bundled files view code. |
extensions/containers-canvas/bundle/webview/chunks/ExecView-YIP6PBGY.js |
Provides bundled execution view code. |
extensions/containers-canvas/bundle/webview/chunks/DockerfileView-MVGPKSXN.js |
Provides bundled Dockerfile view code. |
extensions/containers-canvas/bundle/webview/chunks/CodeView-JVLDNWYI.js |
Provides bundled code view code. |
extensions/containers-canvas/bundle/webview/chunks/chunk-K5VEBUWY.js |
Provides shared bundled webview code. |
extensions/containers-canvas/bundle/webview/chunks/chunk-DNZLB6FF.js |
Provides shared bundled webview code. |
extensions/containers-canvas/bundle/build-inputs.json |
Records committed bundle inputs. |
extensions/containers-canvas/build.mjs |
Builds host and webview bundles. |
.gitignore |
Preserves committed plugin artifacts. |
.github/plugin/marketplace.json |
Registers the plugin in the marketplace. |
.gitattributes |
Configures generated artifact handling. |
Files not reviewed (11)
- extensions/containers-canvas/bundle/webview/chunks/DockerfileView-MVGPKSXN.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/ExecView-YIP6PBGY.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/FilesView-HW7GUK6Z.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/LayersView-MCQU74A7.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/LogView-ILJEIKWI.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/RunImageDialog-JPAWVPY2.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/StatsView-BRUNNGK7.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/TerminalView-MO2UQ5XN.css: Generated file
- extensions/containers-canvas/bundle/webview/chunks/chunk-DNZLB6FF.js: Generated file
- extensions/containers-canvas/bundle/webview/chunks/chunk-K5VEBUWY.js: Generated file
- extensions/containers-canvas/bundle/webview/main.css: Generated file
Files excluded by content exclusion policy (1)
- pnpm-lock.yaml
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The backend accepted remove and forceRemove from the start, and the skill told the agent the panel's own controls ask before destroying anything. Neither was true of the UI: a stopped container offered only start, so removing one meant leaving the panel. Offer remove once the container is stopped. Running and paused are left out deliberately - Docker refuses to remove a running container without -f, so the button would be the guaranteed error the paused row already avoids, and force removal should not be one click from a running container. It stays available to the agent. Removal asks first, inline next to the container it is about, following the tagging pattern rather than adding a dialog. The confirmation is labelled differently from the verb that opens it, because two controls reading remove in one pane are ambiguous. It is cleared when the selection changes: Detail is not keyed by item, so a confirmation left open would have pointed at whatever was selected next. The changelog claimed both remove and kill were lifecycle controls. Remove is now true; kill is still agent-only, so say so. verify:panel covers both halves - that the verb is offered, and that confirming it destroys the container - checked against the daemon rather than the UI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Remove was offered only once a container was stopped, which left the common case - discarding something that is running - outside the panel. Offer it in every state and pick the operation from the state: running and paused use forceRemove, everything else uses remove. Force is a consequence of what is being removed rather than a second control to go find. The confirmation is now a modal rather than the inline row tagging uses. Tagging is recoverable and this is not, so it is worth interrupting for, and modalType=alert stops a stray click outside from dismissing it. It names the container, says plainly that a running one will be killed first, and shows the exact command - so -f is in front of the user before the click instead of being inferred from a verb. verify:panel now also force-removes a running container and asserts the confirmation showed 'docker rm -f', since agreeing to something milder than what runs is the failure that matters here. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Findings from the PR review, each reproduced before being changed.
runtime
- runImage appended spec.image without validation. docker run parses options
until its first positional, so an image of '--privileged' became a flag and
the first command element became the image - a privileged container while
every mount and capability check above still passed. Use assertImageRef, the
guard tag/pull/push already use and whose own test says it refuses anything
that could become another argument.
- path.basename('/..') is '..', and joining that onto the container folder
resolves to the shared extraction directory, which the directory branch then
removed recursively. Reject dot segments and separators, verify containment,
and split with POSIX rules since a backslash is a legal Linux filename
character.
- assertSafeMount tested the raw path, so '/tmp/../etc' walked past the rule
'/etc' trips over. Probing also found '//etc/passwd' did the same. Refuse
'..' segments and match the deny-list against a separator-collapsed form,
keeping the UNC prefix the named-pipe rule depends on.
- hostEscapeRisks missed CapAdd ALL, which grants SYS_ADMIN along with
everything else. Compare normalised capability names instead.
server and webview
- The exec socket registered its close handler after awaiting execs.start, so
a panel navigating away during shell detection left the PTY unreachable and
alive. Claim ownership first and kill the session if the socket already went.
- LogView held a partial trailing line and dropped it on 'end', losing the last
line of any container that exits without a newline.
- Terminal reconnect replaced socketRef without closing the previous socket.
- canvasVsCodeApi's fire-and-forget POST had no catch, so the disconnection it
exists to detect surfaced as an unhandled rejection.
verifyBundle
- Switching to an input digest lost the ability to notice a missing bundle
file: deleting bundle/webview/main.js kept the digest valid and the check
green. Record an output inventory and compare it, and treat a worktree
deletion or an uncommitted chunk as fatal while still tolerating the
platform-dependent byte differences the digest exists to work around.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The review's point stands: prose in the skill is not an enforcement boundary, and containerOp/imageOp accepted remove and forceRemove from the generated action surface with nothing in the way. Require acknowledgeDestructive on those two, mirroring acknowledgeHostAccess on execInContainer. The honest limit is the same - a caller that means it can set the flag - so this is not protection against a determined agent. What it stops is the accidental case: every other verb in that enum is reversible, removal is not, so removal has to be asked for by name. The panel passes the flag only after its confirmation dialog, and the skill now tells the agent to ask the user first and only then repeat the call with it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The committed-file check added in 7b3c0dd failed every CI run. CI rebuilds on Linux, where content-hashed chunk names differ from the ones committed from Windows, so the old names showed as deleted and the new ones as untracked - and the check treated both as a hole in the bundle. That is the same platform difference the input digest exists to tolerate, so routing it back in through git status was a mistake. Compare the commit against itself instead: every output the committed build-inputs.json names must be a file that is actually committed, and every committed bundle file must be one it names. That still catches a chunk emitted but never added, or removed without regenerating the inventory, and it does not care what the local rebuild produced. Verified both directions: renaming a chunk on disk and updating the on-disk inventory - which is what CI looks like, ' D' plus '??' in git status - now passes, and a commit that drops a chunk while still listing it fails. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The header said none of @microsoft/vscode-ext-webview, tRPC, or a vscode stub was needed. All three are used: initWebviewTrpc in appRouter, attachTrpc in host-entry, connectTrpc in the webview, bundled into both halves and attributed in NOTICE.html - and the stub exists precisely because the host barrel requires vscode. It also described the package as a second front end for comparing against a sibling containers extension that no longer exists, using that extension's runtime adapter rather than this package's own src/runtime.mjs. Replace it with what the file actually does, and with the three things that are otherwise puzzling when reading it: why a loopback server exists per panel, why the webview package is a dependency at all, and why the import is ./bundle/host.mjs rather than src/. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
The digest walked src/ plus three named extras, which missed server.mjs and execSessions.mjs - both compiled into bundle/host.mjs, both outside src/. Editing either left the digest unchanged and a stale bundle looking current. Found by editing a comment in server.mjs and watching the digest not move. Take the list from esbuild's metafile instead, so it cannot drift from what is compiled, and record it alongside the digest so verify recomputes over exactly the same set. index.html is added by hand because it is copied rather than bundled and so never appears as an input. Test files drop out, which is correct: they do not affect the bundle. Also two stale claims. server.mjs said the runtime adapter came from a sibling containers extension so the two canvases could not drift; it imports ./src/runtime.mjs and there is one canvas. The changelog said 'only tested on Windows' - CI builds and runs the tests on Linux, though nothing there has a daemon or a browser, so say that precisely instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Brandon Waterloo [MSFT] (bwateratmsft)
left a comment
Brandon Waterloo [MSFT] (bwateratmsft)
left a comment
Collaborator
There was a problem hiding this comment.
We are probably gonna wanna turn on LFS for the big bundle files
containerStats had no callers and was already tree-shaken out of the bundle - removing it leaves bundle/host.mjs byte-identical, which is the evidence it was never shipped. Live stats come from followStats over the /stats SSE route. Unlike prune and pushImage, which carry comments explaining why they stay unexposed, this one had no stated intent to keep. Also write the English-only strings into the known limitations, so the gap is recorded somewhere a reader will find it rather than living only in a review thread. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
Brandon Waterloo [MSFT] (bwateratmsft)
left a comment
Brandon Waterloo [MSFT] (bwateratmsft)
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed with PeerPilot.
Sixteen review findings, reproduced before each was changed.
Terminal sessions could outlive the panel. The eight-session check ran
before shell detection and the pty import, so concurrent sockets each
saw room: twelve starts spawned twelve PTYs while the map tracked two,
leaving ten that nothing could kill. Disposal set no flag, so a start
already past the check spawned into a closed panel. Capacity is now
reserved before the first await and disposal latches, re-checked after
each await and after spawn.
Writing that test surfaced a collision it did not set out to find:
session ids were containerId + Date.now(), so two terminals on one
container inside a millisecond produced the same key and the second
evicted the first.
A failed read was reported to Copilot as an empty inventory. With the
daemon stopped, the list action returned counts of zero and empty
arrays -- indistinguishable from a healthy host with nothing on it.
Separate execs back the two lists, so one could fail while the other
succeeded and look entirely normal. loadState now reports per-list
failures and the action returns them, with runtime availability.
A per-target view arriving without a target was dropped in silence,
leaving the previous pane on screen while the agent believed it had
opened resource usage. It now opens the list and says why. Resolving
that exposed an older leak: instances.set ran before validation and
onClose only fires for a canvas that opened, so any throw stranded a
loopback server. open now closes a server it just started.
Four races where a slower response overwrote a newer one: file reads
and listings, log snapshots landing after Follow began, and events
from a replaced terminal socket. Each request now carries a token, or
checks it is still current, before touching state. A failed snapshot
no longer keeps its error while logs stream, a handshake failure no
longer leaves the badge on connecting, and RPCs wait for the stream
that carries their reply.
Two parsers were wrong in ways clicking would not reveal:
--label="hello world" split into three arguments with the quotes
retained, and a repository name containing a dot was truncated, so
org/service.api linked to org/service. A recorded revision was
ignored, hiding known commits. JSON previews unwrapped single-element
arrays meant for docker inspect, misrepresenting real files, and
rendered [] and {} as a blank pane.
The pure logic behind these moved where a test can reach it:
extension.mjs joins the session on import and node --test cannot parse
JSX, so several of these had no way to be covered at all. openPanel.mjs
sits beside extension.mjs rather than in the bundle, and is listed in
EXTRA_INPUTS so the digest still covers it.
124 tests to 170. Panel verification 12/12.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 874105b2-c7c3-4921-bbf3-017bc6886dfa
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.
Adds Containers for Copilot — a canvas plugin that puts your local Docker containers and images in a live panel, so Copilot can answer container questions by showing you the thing instead of pasting command output.
Ask "what's running?" and the panel opens. Ask "why did
webstop?" and it opens on that container's logs.What you can do
--followstreaming with filtering, adjustable tail, copy and saveImages can also be pulled and tagged from the panel.
It stays live
The panel tracks the daemon, not a snapshot. Start a container in another terminal, run
docker compose up, let something crash — it shows up in one to three seconds without being asked.A shared
docker eventsstream does the work; a 10-second poll is only a safety net for what a stream cannot report (dropped connection, daemon restart, waking from sleep). On an idle machine it does no work at all, and actions the panel itself causes are ignored, so opening a folder doesn't make it reload itself.Copilot drives it too
The plugin ships a skill that routes container questions to the right view, so the agent lands on logs for "why did it stop?" and on layers for "why is this image 1.2 GB?". Copilot can also run the actions itself — and everything it runs shows up in the panel's Commands history, so you can see what happened rather than trusting a summary.
Destructive actions (remove, prune) ask first, whether they come from you or from Copilot.
Trying it
Then start a new Copilot session — plugins are discovered at session start.
Until this lands on
main, point the marketplace at a local clone instead; a local path is read live from your working tree:Requires Docker on
PATH. The panel itself is a GitHub Copilot app feature; on other clients (Copilot CLI, VS Code) the plugin installs and the skill answers the same questions withdocker.Notes for review
This is a Copilot plugin rather than a VS Code extension, so it doesn't change the
vscode-containersextension or any existing package — it just lives in the same repo, as the same product area.Being a plugin does mean it breaks one repo convention:
bundle/andNOTICE.htmlare committed, because plugins install straight from a git ref with no build step, so the committed bytes are what users run. That's what the.gitignore/.gitattributes/postinstallchanges are for.Verified with the full CI sequence locally — repo-wide lint, build, package and test, 716 tests — plus a browser harness that drives the real panel over CDP (9/9), and a Linux container build to check cross-platform behaviour. Untested on macOS and Podman.
extensions/containers-canvas/docs/constraints.mdexplains each non-obvious constraint with the incident behind it, if you hit something in the diff that looks arbitrary.