Skip to content

Remove unused basename import from VS Code customization bundler script#282

Merged
data-douser merged 2 commits into
copilot/ship-portable-custom-agentsfrom
copilot/fix-review-comment-281
May 20, 2026
Merged

Remove unused basename import from VS Code customization bundler script#282
data-douser merged 2 commits into
copilot/ship-portable-custom-agentsfrom
copilot/fix-review-comment-281

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

This PR addresses the linked review comment on PR #281 by applying the requested change only: remove an unused import from extensions/vscode/scripts/bundle-customizations.js. Scope is intentionally limited to that single cleanup.

  • Change

    • Removed basename from the path import list in extensions/vscode/scripts/bundle-customizations.js.
  • Impact

    • No behavior change; this is a dead-code cleanup in the script dependency list.
// before
import { basename, dirname, join, normalize, resolve } from 'path';

// after
import { dirname, join, normalize, resolve } from 'path';

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code as per review comment on PR 281 Remove unused basename import from VS Code customization bundler script May 20, 2026
Copilot AI requested a review from data-douser May 20, 2026 00:41
@data-douser data-douser marked this pull request as ready for review May 20, 2026 00:41
@data-douser data-douser requested review from a team and enyil as code owners May 20, 2026 00:41
@data-douser data-douser merged commit 2df7d26 into copilot/ship-portable-custom-agents May 20, 2026
@data-douser data-douser deleted the copilot/fix-review-comment-281 branch May 20, 2026 00:41
data-douser added a commit that referenced this pull request May 20, 2026
…y-developer, codeql-workshop-author) (#281)

* Initial plan

* feat(vscode): ship portable built-in custom agents with user-extensible packaging

- Add codeql-query-developer and codeql-workshop-author .agent.md files
- Add bundle-customizations.js script with overlay support
- Add AgentRegistrar class managing chat.agentFilesLocations registration
- Add bundle:customizations npm script; update vscode:prepublish and clean
- Add codeql-mcp.agents.enabled and additionalAgentDirs settings
- Add codeql-mcp.showAgentsStatus command
- Add contributes.chatAgents, chatPromptFiles, chatSkills static declarations
- Add Vitest unit tests for bundler and AgentRegistrar
- Add Mocha integration test suite for agents
- Add team-customizations example overlay
- Update README.md with Built-in Custom Agents section
- Update CHANGELOG.md [Unreleased] with new VS Code Extension features

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* Ignore generated customizations bundle outputs (agents/, prompts/, skills/, manifest)

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* Fix overlay manifest path for nested files; use project-local .tmp/ in tests

Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>

* Lint fixes for extensions/vscode/**/*.md

* Remove "model" from .github/agents/*.md

* fix(vscode): drop broken AgentRegistrar; contribute agents declaratively

Switch to `contributes.chatAgents` (declarative) only — the same channel
GitHub.copilot-chat resolves its own Plan/Ask/Explore agents through.

Changes:
- Remove `src/customizations/agent-registrar.ts` and its unit tests.
- Drop `codeql-mcp.agents.enabled` and `codeql-mcp.additionalAgentDirs`
  settings (unsupported; users set `chat.agentFilesLocations` to a
  workspace-relative path directly).
- Repurpose `codeql-mcp.showAgentsStatus` to report agents from
  `contributes.chatAgents` in the manifest.
- Add red→green integration tests: assert no absolute path leaks into
  `chat.agentFilesLocations`; assert `contributes.chatAgents` lists both
  bundled agents and that each referenced file exists on disk.
- Remove stale toggle/`additionalAgentDirs` integration tests.
- Update README + CHANGELOG to reflect declarative-only registration.
- Strip `model: Claude Opus 4.6 (copilot)` from the two `.github/prompts/`
  files that still set it (no static model in any agent or prompt).

Tests: 185/185 unit, 78/78 integration, lint clean.

* Fix extension agent packaging

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>

* chore: replace whitelist/blacklist terminology with allowlist/denylist

Across the VS Code extension bundler and the server-side CLI executor,
rename "whitelist" to "allowlist" (and follow the same convention for any
future "blacklist" → "denylist") in:

- Doc comments and header descriptions
- JSDoc and inline comments
- Identifiers:
  - bundle-customizations.js:
    promptWhitelist → promptAllowlist
    skillWhitelist  → skillAllowlist
- Error messages
- Test descriptions and comments

Files updated:
- extensions/vscode/customizations/bundle-customizations.config.js
- extensions/vscode/scripts/bundle-customizations.js
- extensions/vscode/test/customizations/bundle-customizations.test.ts
- server/src/lib/cli-executor.ts
- server/test/src/lib/cli-executor.test.ts

Out of scope (untouched):
- server/dist/*.js.map (generated; will pick up new strings on next build)
- server/ql/rust/**/*.testproj/**/rust/library/core/**.rs (vendored Rust
  stdlib test fixtures)
- .codeql/ql-mcp/query-logs/** (gitignored runtime logs)
- client/gh-ql-mcp-client (compiled binary)

Tests: 186/186 vscode unit, 78/78 server cli-executor unit, lint clean.
No behavior changes.

* docs(changelog): link bundled VS Code extension entries to PR #281

Replace placeholder NNN PR references with the actual PR number, now
that the bundled-customizations entries sit in the [Unreleased] section
alongside the supply-chain hardening work from #279.

* Address PR #281 review: doc accuracy + faster link-validity test

- README: tighten the 'Extending at Build Time' intro so the framing
  matches the limitation documented just below (manifest patch is
  required for net-new contributions; overlays only override or extend
  the already-contributed list).
- agents.integration.test.ts: rewrite the obsolete file header. It used
  to claim the tests verify chat.agentFilesLocations is 'updated
  correctly', but the suite now asserts the extension does NOT write
  the bundled-agents path there (VS Code rejects absolute paths).
- bundle-customizations.test.ts: the 'real source tree' link-validity
  test now copies only the allowlisted prompts and skill directories
  (read from the actual bundler config) instead of the entire
  server/src/prompts/ and .github/skills/ trees. Run time on this test
  drops from ~1.2s to ~0.7s and the I/O is bounded by the allowlist
  rather than by the size of the example workshop fixtures.

Tests: 186 unit (all pass); integration noWorkspace 83/83 exit 0.

* Address PR #281 review #4322045254

- extension.ts: extract `readBundledAgentsStatus(context)` helper that
  reads the manifest from `context.extensionUri.fsPath` directly,
  removing the previous `context.extension.id` dependency.
  `showAgentsStatus` now calls the helper, and the same helper is
  exposed on `ExtensionApi.getBundledAgentsStatus()` for test
  introspection.
- agents.integration.test.ts (TDD): new test asserts the extension API
  exposes `getBundledAgentsStatus()` and that it returns both bundled
  agent paths (`ql-mcp-ext-query-developer.agent.md`,
  `ql-mcp-ext-workshop-author.agent.md`) plus a `bundledDir` resolved
  via `extensionUri`. Red against the previous code, green after the
  helper extraction.
- CHANGELOG.md: reorder `[Unreleased]` sections to match the repo's
  changelog guidance (`Highlights` -> `Added` -> `Changed` -> `Fixed`
  -> `Security` -> `Dependencies`). The new `Added > VS Code Extension`
  bullets now sit immediately after `Highlights`.

Tests: 186 unit (vitest); integration noWorkspace 84, singleFolder 86,
multiRoot 89; all exit 0.

* feat(vscode): drop bundled prompts; expand MCP-prompt references in agents

Workflow prompts now ship exclusively via the `ql-mcp` MCP server's
`prompts/list` and surface as slash commands in Copilot Chat. The four
bundled `ql-mcp-ext-*` `.prompt.md` files duplicated content the server
already exposes under canonical IDs (`/ql_tdd_basic`, etc.), creating
two slash commands for the same workflow. Bundled agents and skills are
unchanged in form; only their prose changes to reference a richer
palette of MCP slash IDs.

Changes:
- bundle-customizations.config.js: `prompts` allowlist is now empty.
  Header doc explains why and notes the array is kept for future
  reversibility.
- bundle-customizations.js: prompts copy loop is preserved but gated
  on a non-empty allowlist; the `prompts/` target dir is only created
  when at least one prompt would be copied.
- package.json: drop `contributes.chatPromptFiles` entirely.
- customizations/agents/ql-mcp-ext-query-developer.agent.md: split the
  old 'Bundled Skills and Prompts' section into 'Bundled Skills' (2
  bullets) and 'MCP Prompts' (7 bullets) referencing canonical slash
  IDs. Adds `/ql_lsp_iterative_development`, `/explain_codeql_query`,
  `/document_codeql_query`, `/data_extension_development` to the
  workflow palette.
- customizations/agents/ql-mcp-ext-workshop-author.agent.md: same
  split. The MCP Prompts list now has 5 bullets, adding
  `/explain_codeql_query` and `/document_codeql_query`.
- README.md 'Built-in Custom Agents': describes agents + skills only;
  a single sentence points users at the MCP-served slash commands.
- CHANGELOG.md [Unreleased]: rename the 'Bundled prompts and skills'
  bullet to 'Bundled skills'; add a 'Changed > VS Code Extension'
  bullet that explains the prompts-via-MCP migration.

TDD (new integration tests in agents.integration.test.ts):
- 'package.json does NOT contribute chatPromptFiles' — asserts
  `contributes.chatPromptFiles` is absent or empty.
- 'Bundled prompts/ directory is not produced by the bundler' —
  asserts `<extensionPath>/prompts/` does not exist.
- 'Every MCP prompt slash ID referenced by shipped agents maps to a
  real MCP prompt' — scans each agent's body for `/<id>` tokens,
  asserts each is in the 15-ID set registered by
  server/src/prompts/workflow-prompts.ts, and asserts each agent
  references at least one.

All three failed against the previous tip and pass now.

Also: bundled-markdown-links.integration.test.ts splits its
'required' vs 'optional' bundled-dir lists so the absence of the
`prompts/` dir no longer trips the vacuous-pass guard.

Tests: 186 unit (vitest); integration noWorkspace 87, singleFolder 89,
multiRoot 92; all exit 0.

* Address PR #281 review #4323697978

- src/extension.ts: extend `ExtensionApi` to include `environmentBuilder`
  and `serverManager`. The activate() return value was always shaped
  this way (the bridge/workspace integration tests rely on these
  hooks), but the type only listed `mcpProvider` and
  `getBundledAgentsStatus`, which is an excess-property violation
  under TS strict checks. Document both as part of the public
  contract.
- test/suite/extension.integration.test.ts: new TDD assertion that the
  runtime `ExtensionApi` exposes `mcpProvider`,
  `environmentBuilder`, `serverManager`, and a callable
  `getBundledAgentsStatus`. Guards against any future drift between
  the declared interface and the returned shape.
- README.md "Adding Your Own Agents at Runtime": clarify exactly which
  path forms `chat.agentFilesLocations` accepts (workspace-relative
  and home-relative `~/foo` form) and which it rejects (absolute,
  drive-letter, backslash separators, glob characters). Resolves the
  reviewer's complaint that `~/.copilot/agents` was listed as a
  workspace-relative example.
- CHANGELOG.md `[Unreleased]` Highlights: condense the second
  supply-chain hardening bullet to a brief lead-in that points at the
  Security section, eliminating the duplicate prose previously
  flagged by repo changelog rules (no entry repeated across multiple
  sections).

Tests: 186 unit (vitest); integration noWorkspace 88, singleFolder 90,
multiRoot 93; all exit 0.

The three remaining unresolved reviewer comments
(`extension.ts:85`, `cli-executor.ts:35`, `.github/agents/...:4`) all
concern the PR description text on GitHub claiming a narrower scope
than the actual diff. The code is consistent; the PR description
should be updated via the GitHub UI.

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>

* Remove unused `basename` import from VS Code customization bundler script (#282)

---------

Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Co-authored-by: Nathan Randall <data-douser@github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

2 participants