refactor(copilot): remove duplicate example implementation - #3879
refactor(copilot): remove duplicate example implementation#3879Carlos Hernandez (carloshvp) wants to merge 1 commit into
Conversation
Signed-off-by: Carlos Hernandez <carloshvp@gmail.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
Imran Siddique (imran-siddique)
left a comment
There was a problem hiding this comment.
Deleting 3,578 lines deserves proof that nothing goes with them, so I checked rather than took the
dedup claim on trust. It holds, with one conflict to sequence around.
The two policy.mjs copies really are the same implementation, and the package one is ahead.
examples/copilot-cli-agt/.github/extensions/agt-global-policy/lib/policy.mjs 1272 lines
agent-governance-copilot-cli/assets/extensions/agt-global-policy/lib/policy.mjs 1342 lines
diff between them: 104 changed lines
I pulled every line that exists only in the example copy, stripped brace-only lines, and got 16
distinct lines. Every symbol in them appears in the package copy at the same occurrence count:
summarizeText 3/3, safeJsonStringify 2/2, buildContextEntry 4/4, DEFAULT_TOOL_EFFECT 2/2,
outputText 4/4. So the divergence is ordering and formatting, not behaviour, and the package copy
is a strict superset. Your framing that "the package copy continued receiving fixes" is right.
Test coverage survives. The deleted examples/copilot-cli-agt/test/policy-engine.test.mjs has 9
tests against the package's 12. Two names appear only in the example:
evaluateDirectResourceAccess denies secret reads and reviews persistence writes— not lost.
The package hasevaluateDirectResourceAccess denies secret reads, allows env templates, reviews persistence writes, and blocks metadata URLs, which is a superset with 10 references to the same
function.example policy stays aligned with the hardened packaged baseline— genuinely goes away, and that
is correct. It is a drift guard between the two copies, and after this PR there is only one copy
for it to guard. Worth a sentence in the PR body saying so explicitly, since "deleted a test that
checks the thing this PR is about" reads badly in a changelog without the explanation.
One conflict to sequence. #3871 (the v5.0.1 security patch tree) modifies two files this PR
deletes:
examples/copilot-cli-agt/package.json
examples/copilot-cli-agt/.github/extensions/agt-global-policy/package.json
That is a delete-versus-modify conflict, and it will not resolve itself. #3871 is deliberately cut at
tag v5.0.0 and is not meant to be rebased onto main, so the two need a decision rather than a
merge order: either this waits until #3871 has shipped, or #3871's version-sync step drops those two
manifests on the grounds that they are about to stop existing.
Non-blocking. Keeping the scenario prompts, proof corpus, expected outcomes and tutorial while
removing the executable duplicate is the right line to draw. An example that carries a second runtime
is a second thing to keep correct, and the README/docs/tutorial edits pointing setup at the packaged
extension are what make the deletion safe rather than merely tidy.
Nothing blocking from me.
Related Issue
None.
Problem & Solution
Problem:
examples/copilot-cli-agtcarried a second copy of the Copilot extension runtime,four policy files, two installer wrappers, and nine policy tests. Its installer wrappers already
delegated to
agent-governance-copilot-cli, while the package copy continued receiving fixes.Maintaining both trees left 3,252 lines of runtime and support assets with two owners.
Solution: Keep the scenario prompts, proof corpus, expected outcomes, and tutorial in the
example. Remove its duplicate executable assets and point all setup, profile, source, and test
instructions to
agent-governance-copilot-cli. The package README now retains the original designattribution.
Impact on Your Work
The current source snapshot is 3,493 lines smaller: 85 additions and 3,578 deletions. Published
package contents and runtime behavior do not change. Contributors now test and edit the same
extension and policy files that users install.
Timeline
None.
Alternatives Considered
duplicate runtime and maintenance cost.
and npm packaging less predictable.
Type of Change
Package(s) Affected
Core & runtime:
Governance & security:
Platform & tooling:
CLI plugins:
Shared / other:
Testing
Unit Testing
npm --prefix agent-governance-copilot-cli test(18 passed)python3 -m pytest scripts/tests/test_docs_check_links.py scripts/tests/test_docs_check_frontmatter.py -q(51 passed)The removed nine-test example suite covered the same policy helpers. The retained package suite
also covers installer lifecycle, policy profiles, prompt-defense grading, and metadata URL guards.
One removed test,
example policy stays aligned with the hardened packaged baseline, was solely a drift guard between the two copies; it becomes obsolete once the duplicate copy is removed.Manual Testing
npm --prefix agent-governance-copilot-cli run checknpm --prefix agent-governance-copilot-cli pack --dry-run --jsonpython3 scripts/docs/check_links.py --root .python3 scripts/docs/check_frontmatter.py --root . --strictgit diff --checkChecklist
Attribution & Prior Art
Prior art / related projects:
AI Assistance
Codex audited the duplicate trees, prepared this cleanup at Carlos Hernandez's request, and ran the
validation listed above. Maintainer review is still required.
IP, Patents, and Licensing