Migrate JSONC reader + line locators to agent-gov-core@v0.1.1#40
Merged
Conversation
Replaces local copies of stripJsonComments, stripTrailingCommas, lineOfJsonKey, and lineOfJsonStringValue with the shared primitives. This is the same migration ScopeTrail PR #35 did — PolicyMesh's discovery.ts now delegates parsing/location logic to agent-gov-core and keeps only PolicyMesh-specific behavior: - async wrapper with ENOENT → empty object (PolicyMesh runs against repos that haven't adopted these surfaces yet) - JsonParseError = { message, line } shape that the parsers/* layer depends on (line is derived from SyntaxError 'position N') The action now `npm ci --omit=dev` so runtime imports resolve at action invocation time — same change ScopeTrail made. Loosened the workflow.test.mjs assertion from "no npm ci" to "no npm run build"; dist/ is still committed and consumers still skip the TypeScript compile. All 38 existing tests pass against the migrated reader. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
Conalh
added a commit
that referenced
this pull request
May 22, 2026
Closes the false-positive class flagged in the PolicyMesh audit: two surfaces that differ only in cosmetically neutral ways (`npx -y <pkg>` vs `npx <pkg>`, `.cmd` vs unsuffixed, flag reordering) were being reported as high-severity command mismatches. What changed - McpServer gains a `canonicalIdentity: string` field, computed by agent-gov-core@v0.1.2's normalizeMcpCommand from (command, args, url). Both the JSON and Codex TOML parsers populate it. - `detectMcpCommandMismatch` now groups by `canonicalIdentity` instead of the raw joined `command` string. The human-readable command list in the finding message still uses `command` so the finding stays actionable. - Env is deliberately omitted from `canonicalIdentity`. Env drift has its own detector (mcp_env_mismatch); including env here would have surfaced two findings for what users perceive as one issue (and broke the mcp-env-value-mismatch fixture test). Regression test pinned `mcp-command-neutral-flag-equivalence` fixture: root MCP runs `npx -y @modelcontextprotocol/server-github@1.2.3`, Cursor runs the same without `-y`. Before this change the audit emitted a high-severity mcp_command_mismatch finding; after it emits none. Test 'CLI does not flag mcp_command_mismatch on neutral -y flag drift between surfaces' asserts the post-fix behavior — it fails on the pre-fix engine, passes here. 39 PolicyMesh tests pass. Stacked on #40 (JSONC migration); merge that one first. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Conalh
added a commit
that referenced
this pull request
May 22, 2026
Closes the false-positive class flagged in the PolicyMesh audit: two surfaces that differ only in cosmetically neutral ways (`npx -y <pkg>` vs `npx <pkg>`, `.cmd` vs unsuffixed, flag reordering) were being reported as high-severity command mismatches. What changed - McpServer gains a `canonicalIdentity: string` field, computed by agent-gov-core@v0.1.2's normalizeMcpCommand from (command, args, url). Both the JSON and Codex TOML parsers populate it. - `detectMcpCommandMismatch` now groups by `canonicalIdentity` instead of the raw joined `command` string. The human-readable command list in the finding message still uses `command` so the finding stays actionable. - Env is deliberately omitted from `canonicalIdentity`. Env drift has its own detector (mcp_env_mismatch); including env here would have surfaced two findings for what users perceive as one issue (and broke the mcp-env-value-mismatch fixture test). Regression test pinned `mcp-command-neutral-flag-equivalence` fixture: root MCP runs `npx -y @modelcontextprotocol/server-github@1.2.3`, Cursor runs the same without `-y`. Before this change the audit emitted a high-severity mcp_command_mismatch finding; after it emits none. Test 'CLI does not flag mcp_command_mismatch on neutral -y flag drift between surfaces' asserts the post-fix behavior — it fails on the pre-fix engine, passes here. 39 PolicyMesh tests pass. Stacked on #40 (JSONC migration); merge that one first. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2 tasks
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.
Summary
stripJsonComments,stripTrailingCommas,lineOfJsonKey, andlineOfJsonStringValuewith the shared primitives fromagent-gov-core@v0.1.1.src/discovery.tsshrinks from 207 to 91 lines and stops duplicating logic that now lives in one place.What stays local
readJsonObjectWithSourcewrapper with ENOENT → empty-object semantics (PolicyMesh runs against repos that haven't adopted these surfaces yet).JsonParseError = { message, line }shape —parsers/*anderrors.tsdepend on it.lineis still derived fromSyntaxError'sposition N.Action-time install
action.ymlnownpm ci --omit=dev --no-audit --no-funds before invoking the bundled CLI, so the newagent-gov-coreruntime import resolves. Same change ScopeTrail made. Theworkflow.test.mjsassertion is loosened from "nonpm ci" to "nonpm run build" —dist/is still committed.Test plan
npm test— all 38 tests pass locally against the migrated readerPolicyMesh workflow self-dogfoods the actiontest was untouched)🤖 Generated with Claude Code