Skip to content

fix: convert .md to .agent.md extension for GitHub Copilot subagents#2

Closed
flanny7 wants to merge 3 commits into
mainfrom
fix/copilot-subagent-agent-md-extension
Closed

fix: convert .md to .agent.md extension for GitHub Copilot subagents#2
flanny7 wants to merge 3 commits into
mainfrom
fix/copilot-subagent-agent-md-extension

Conversation

@flanny7
Copy link
Copy Markdown
Owner

@flanny7 flanny7 commented Apr 8, 2026

Summary

  • GitHub Copilot (VSCode) requires agent files to have the .agent.md extension to be recognized and loaded
  • rulesync generate was generating files as planner.md instead of planner.agent.md
  • Added toAgentMdFilePath() helper that converts .md.agent.md in fromRulesyncSubagent()
  • Files already ending in .agent.md are preserved unchanged

Related Issue

Fixes dyoshikawa#1432

Test plan

  • New test: converts .md extension to .agent.md in output file path
  • New test: preserves .agent.md extension when already present
  • All 4774 existing tests pass

🤖 Generated with Claude Code

flanny7 and others added 3 commits April 8, 2026 17:24
VSCode Copilot Chat requires agent files to have the .agent.md extension
to be recognized and loaded. Previously, rulesync generated files as
.md which caused them to be silently ignored by Copilot.

Fixes dyoshikawa#1432

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Change Copilot filePattern from "*.md" to "*.agent.md" so deletion
  (rulesync generate --delete) correctly finds and removes stale files
- Add fromAgentMdFilePath() to convert .agent.md back to .md in
  toRulesyncSubagent() for correct import paths
- Update subagents-processor test to use .agent.md file names

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Silently appending .agent.md to a non-.md path (e.g. planner.txt)
would produce malformed filenames like planner.txt.agent.md that
Copilot Chat would never load. Fail fast with a clear error instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
flanny7 pushed a commit that referenced this pull request Apr 22, 2026
- #1: use static import for resetDeprecationWarningForTests in tests
- #2: document why mutual-exclusivity is runtime-enforced, not a discriminated union
- dyoshikawa#3: stop emitting the deprecation warning from the Config constructor;
  the ConfigResolver is now the single emission point
- #4: cache validated ToolTarget[] for object-form targets in the constructor
  so getTargets() no longer rebuilds the ALL_TOOL_TARGETS set per call
- #5: fix misleading schema comment that claimed unknown-target warnings
  (the runtime path actually throws)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flanny7 pushed a commit that referenced this pull request Apr 22, 2026
…nd match-all bypasses

- Reject imported rules whose toolName maps to __proto__, constructor,
  or prototype to prevent prototype pollution when round-tripping
  untrusted TOML; use Object.hasOwn for lookups to avoid hitting
  inherited accessors. (Sec #1)
- Stop translating glob character classes to regex classes; emit '['
  and ']' as literals so that negated ([^a]) or wide-range ([!-~])
  classes cannot bypass the JSON field-boundary guard. (Sec #2)
- Skip empty patterns ('') with a warning (would match every bash
  invocation or nothing for other tools). Skip bash '*' and '**'
  with allow/deny decisions because they would silently grant or
  revoke every shell command; 'ask' remains supported. (Sec dyoshikawa#3)
- Update docs to reflect the new guardrails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
flanny7 pushed a commit that referenced this pull request Apr 22, 2026
Address Round 2 review findings for PR dyoshikawa#1526:

- HIGH-R2-#1: guard the stale-file cleanup loop in apm-install.ts
  against path traversal. Attacker-controlled deployed_files entries
  with ".." segments or absolute paths are now rejected by shape and
  via checkPathTraversal, with a warn log per offending entry, so a
  hostile lockfile cannot drive arbitrary removeFile calls.
- MID-R2-#2: make lockfile ordering deterministic for failed deps.
  The per-dep worker now returns the preserved prior entry via its
  result object, and the sequential post-loop pushes successes or
  preserved entries strictly in manifest order, not in
  promise-completion order.
- MID-R2-dyoshikawa#3: preserve top-level loose fields (mcp_servers and any
  looseObject extras) across lockfile rewrites by carrying forward
  existingLock through createEmptyApmLock.
- MID-R2-#4: relax the content_hash schema to accept arbitrary
  strings on parse so a lockfile produced by the upstream apm CLI
  does not break readApmLock. The --frozen integrity check now only
  compares hashes whose shape matches RULESYNC_CONTENT_HASH_REGEX
  and skips comparison otherwise (commit SHA pin still enforces
  integrity).

Tests added for each finding, including a two-dep ordering
regression and a frozen-mode interop check with a legacy
content_hash value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@flanny7 flanny7 closed this Apr 22, 2026
flanny7 pushed a commit that referenced this pull request May 11, 2026
…ode/cline/qwencode

Addresses 18 reviewer findings raised on PR dyoshikawa#1338. Highlights:

- #1 (critical): AugmentCode non-bash categories now fail-closed. A single
  deny rule in `read`/`edit`/`write`/`webfetch`/`websearch` collapses the
  whole tool to a catch-all `deny` entry; non-`*` allow/ask patterns are
  dropped with an aggregated warning instead of being silently downgraded
  to a catch-all `allow` that would shadow a deny.
- #2 (high): `toolPermissions` are sorted to make AugmentCode's
  first-match-wins evaluation safe — entries with `shellInputRegex` come
  before catch-alls, longer regex first, with deny < ask-user < allow as
  the tiebreaker.
- #5 (mid): existing `launch-process` deny entries are preserved across
  regeneration so a user-added shell deny rule cannot be silently
  downgraded; non-deny launch-process entries are still owned by rulesync.
- dyoshikawa#3 (mid): Qwencode generation uses `readFileContentOrNull` (no
  `readOrInitializeFileContent`) so dry-run does not create the `.qwen/`
  directory.
- #4 (mid): Kilo schema parsing is deferred and respects `params.validate`,
  so `forDeletion` and dry-run construction never throw on permissive input.
- #7, dyoshikawa#16 (mid/low): Cline drops non-bash / ask rules at `logger.error`
  level (rather than warn) and surfaces a defensive warn on allow/deny
  pattern collisions.
- #8, #13 (mid/low): Qwencode pattern parser uses the LAST `)` so nested
  parentheses (e.g. `Bash(echo (a))`) round-trip; malformed entries warn
  and fall back to `*`.
- #9 (low): Augment non-bash warnings are aggregated once per category.
- #11 (low): Kilo's wholesale-replace of the `permission` object is
  documented in `docs/reference/file-formats.md`.
- #10, #6 (low): glob→regex behaviour and round-trip caveats are
  documented.
- #14 (low): `mergedPermissions` is typed as
  `{ allow?: string[]; ask?: string[]; deny?: string[]; [k: string]: unknown }`.
- #15 (low): redundant Qwencode global-mode equivalence assert removed.
- dyoshikawa#17 (low): `permissions-processor.test.ts` gains `loadToolFiles` cases
  for AugmentCode, Cline, Kilo, and Qwencode.

Deferred:
- #12 (kilo home-mock migration): the existing kilo global-mode test
  already passes `outputRoot` directly and does not reach
  `getHomeDirectory()`, so introducing the home-mock pattern adds only
  ceremony without coverage.
- dyoshikawa#18 (tool-name-mapping helper extraction): postponed to keep this
  fix focused and to avoid touching files outside the permissions
  feature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
flanny7 pushed a commit that referenced this pull request May 11, 2026
- #1 (mid): augmentcode-permissions sort the COMBINED [generated, preserved] entries so a preserved launch-process deny cannot be shadowed by a generated catch-all allow/ask under first-match-wins. Adds regression test.

- #2 (low): sortAugmentEntries applies fail-closed type priority (deny < ask-user < allow) BEFORE the regex-length heuristic for has-regex entries. Heuristic limits documented in code.

- dyoshikawa#3 (low): cline-permissions downgrades translation-loss logs from logger.error to a single aggregated logger.warn per call (project convention; CI gates that treat error lines as failures no longer trip).

- #4 (low): documents in docs/reference/file-formats.md (and synced skills/rulesync/file-formats.md) that Cline allow/deny arrays are owned by rulesync entirely (no preservation), in contrast to Qwen Code and AugmentCode.

- #5 (low): qwencode-permissions forwards a logger to parseQwenPermissionEntry from both call sites (preservation filter and convertQwenToRulesyncPermissions) so the malformed-entry warnings are no longer dead code in production.

- #6 (low): qwencode-permissions.test extends the nested-paren round-trip test to cover sequential parens (Bash(grep (foo) | wc (-l))) and multi-nesting (Bash(echo ((deep)))).
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.

GitHub Copilot subagents generated by rulesync lack the required .agent.md extension

1 participant