Repository navigation
fix: agent never-block reliability, MCP/version coherence, and CI/docs hardening - #146
Merged
Merged
Conversation
…rdening Deep audit + remediation pass (ADR pm-96wm). Four specialized read-only sub-agents audited code-quality/perf, SDK/extensions, agent-UX, and docs/CI; findings were recorded centrally in pm (deduped against the open backlog). This ships the focused, low-risk batch. Never-block (agents must never dead-end on the common path): - pm close / close-many without a reason returned a generic error that named the internal governance knob but not the fix; now returns actionable guidance (examples, next steps, how to disable governance.require_close_reason). [pm-g799] - --path / PM_PATH pointing at a project root hard-failed "tracker not initialized" while cwd-mode discovers .agents/pm; resolvePmRoot now redirects an explicit project-root into its .agents/pm, keeping pm init --path and direct tracker roots verbatim. [pm-ryik] MCP / contract coherence: - serverInfo.version now reflects package.json (was hardcoded "1.0.0"). [pm-2nvw] - error envelope includes structuredContent.result=null so consumers read .result uniformly across success and error. [pm-l40h] - pm_health defaults to the compact summary projection (~22x smaller; full=true opts into the deep payload). [pm-yjub] - corrected MCP tool-count docs (18/21 -> 22, +pm_copy; 3 -> 4 subagents) and locked the claude-plugin smoke to the live tools/list count. [pm-7tvx] CI / docs: - pinned codeql.yml init/analyze to immutable SHAs + a contract-test assertion that codeql.yml has no unpinned actions. [pm-ji5c] - corrected the inaccurate PR CI-matrix description in CONTRIBUTING.md. [pm-t73k] Cleanup: - removed dead code (runtime-schema IIFE, NESTED_SETTING_KEYS, _backgroundRefreshInternals) and deduped four package.json version readers into resolvePmCliVersion (also drops unused fs/path imports from the main.ts hot path). [pm-wrqk] Verified: typecheck, build, 2559 tests @ 100% coverage, static/contracts/docs/ security gates, and the claude-plugin smoke (22 tools). Deferred findings are tracked under pm-mpbb/pm-5k4v/pm-ugqx/pm-o2kc (incl. the settings-read cache perf lever pm-2bn5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Owner
Author
|
@codex review /gemini review @gemini-code-assist please review @coderabbitai review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughPR introduces centralized PM CLI version resolution, fixes MCP server to use real package version instead of hardcoded ChangesProduction Code & Integration Changes
Documentation & Plugin Description Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
To use Codex here, create a Codex account and connect to github. |
|
🧠 Learnings used✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/mcp-handshake.spec.ts`:
- Around line 69-73: The test currently asserts result.serverInfo?.version is
not "1.0.0", which will break if the real package version becomes 1.0.0; update
the assertion to compare against the repository's actual package version by
reading the package.json version (e.g. require('../..../package.json').version
or import the package JSON) and assert result.serverInfo?.version equals or
matches that version (or matches a regex derived from it) instead of asserting
inequality; adjust the assertion around result.serverInfo?.version in
mcp-handshake.spec.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2f550509-1f3c-418a-9001-16a5df25494f
📒 Files selected for processing (78)
.agents/pm/chores/pm-7tvx.toon.agents/pm/chores/pm-btwe.toon.agents/pm/chores/pm-e7dn.toon.agents/pm/chores/pm-l0jd.toon.agents/pm/chores/pm-q0kr.toon.agents/pm/chores/pm-t73k.toon.agents/pm/chores/pm-ul02.toon.agents/pm/chores/pm-wrqk.toon.agents/pm/decisions/pm-96wm.toon.agents/pm/features/pm-1js9.toon.agents/pm/features/pm-2bn5.toon.agents/pm/features/pm-k1e4.toon.agents/pm/features/pm-oll8.toon.agents/pm/features/pm-vhx6.toon.agents/pm/features/pm-yjub.toon.agents/pm/history/pm-1js9.jsonl.agents/pm/history/pm-2bn5.jsonl.agents/pm/history/pm-2nvw.jsonl.agents/pm/history/pm-5dbn.jsonl.agents/pm/history/pm-6l17.jsonl.agents/pm/history/pm-7tvx.jsonl.agents/pm/history/pm-96wm.jsonl.agents/pm/history/pm-b4cp.jsonl.agents/pm/history/pm-btwe.jsonl.agents/pm/history/pm-e7dn.jsonl.agents/pm/history/pm-g799.jsonl.agents/pm/history/pm-ji5c.jsonl.agents/pm/history/pm-jozc.jsonl.agents/pm/history/pm-k1e4.jsonl.agents/pm/history/pm-l0jd.jsonl.agents/pm/history/pm-l40h.jsonl.agents/pm/history/pm-ltbr.jsonl.agents/pm/history/pm-oll8.jsonl.agents/pm/history/pm-q0kr.jsonl.agents/pm/history/pm-ryik.jsonl.agents/pm/history/pm-srex.jsonl.agents/pm/history/pm-t73k.jsonl.agents/pm/history/pm-ul02.jsonl.agents/pm/history/pm-vhx6.jsonl.agents/pm/history/pm-wrqk.jsonl.agents/pm/history/pm-yjub.jsonl.agents/pm/issues/pm-2nvw.toon.agents/pm/issues/pm-6l17.toon.agents/pm/issues/pm-g799.toon.agents/pm/issues/pm-ji5c.toon.agents/pm/issues/pm-jozc.toon.agents/pm/issues/pm-l40h.toon.agents/pm/issues/pm-ltbr.toon.agents/pm/issues/pm-ryik.toon.agents/pm/stories/pm-5dbn.toon.agents/pm/stories/pm-b4cp.toon.agents/pm/stories/pm-srex.toon.claude-plugin/marketplace.json.github/workflows/codeql.ymlCHANGELOG.mdCONTRIBUTING.mdREADME.mddocs/CLAUDE_CODE_PLUGIN.mdmarketplace.jsonplugins/pm-claude/.claude-plugin/plugin.jsonplugins/pm-claude/README.mdplugins/pm-claude/commands/pm-init.mdscripts/smoke-claude-plugin.mjssrc/cli/commands/close-many.tssrc/cli/commands/close.tssrc/cli/commands/upgrade.tssrc/cli/main.tssrc/core/config/nested-settings.tssrc/core/packages/root.tssrc/core/schema/runtime-schema.tssrc/core/search/background-refresh.tssrc/core/sentry/instrument.tssrc/core/store/paths.tssrc/mcp/server.tstests/integration/ci-workflow-contract.spec.tstests/integration/mcp-handshake.spec.tstests/unit/close-command.spec.tstests/unit/store-paths.spec.ts
💤 Files with no reviewable changes (2)
- src/core/config/nested-settings.ts
- src/core/search/background-refresh.ts
Comment on lines
+69
to
73
| // pm-2nvw: serverInfo.version must reflect the real package.json version, | ||
| // not the former hard-coded "1.0.0". | ||
| expect(result.serverInfo?.version).not.toBe("1.0.0"); | ||
| expect(result.serverInfo?.version).toMatch(/^\d+\.\d+\./); | ||
| expect(typeof result.instructions).toBe("string"); |
There was a problem hiding this comment.
Avoid a future false-negative on the version assertion.
Line 71 will fail once the real package version legitimately becomes "1.0.0", even when wiring is correct. Assert against the repository package version instead of asserting inequality to a historical constant.
Suggested fix
+import { readFile } from "node:fs/promises";
import { describe, expect, it, vi } from "vitest";
import { handleRequest, processRpcLine } from "../../src/mcp/server.js";
import { createSerialQueue } from "../../src/core/shared/serial-queue.js";
import { PM_TOOL_ACTIONS } from "../../src/sdk/cli-contracts/enum-contracts.js";
import { withTempPmPath } from "../helpers/withTempPmPath.js";
@@
- expect(result.serverInfo?.version).not.toBe("1.0.0");
- expect(result.serverInfo?.version).toMatch(/^\d+\.\d+\./);
+ const packageJson = JSON.parse(
+ await readFile(new URL("../../package.json", import.meta.url), "utf8"),
+ ) as { version?: string };
+ expect(result.serverInfo?.version).toBe(packageJson.version);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // pm-2nvw: serverInfo.version must reflect the real package.json version, | |
| // not the former hard-coded "1.0.0". | |
| expect(result.serverInfo?.version).not.toBe("1.0.0"); | |
| expect(result.serverInfo?.version).toMatch(/^\d+\.\d+\./); | |
| expect(typeof result.instructions).toBe("string"); | |
| import { readFile } from "node:fs/promises"; | |
| import { describe, expect, it, vi } from "vitest"; | |
| import { handleRequest, processRpcLine } from "../../src/mcp/server.js"; | |
| import { createSerialQueue } from "../../src/core/shared/serial-queue.js"; | |
| import { PM_TOOL_ACTIONS } from "../../src/sdk/cli-contracts/enum-contracts.js"; | |
| import { withTempPmPath } from "../helpers/withTempPmPath.js"; | |
| // ... (rest of imports and test setup) | |
| // pm-2nvw: serverInfo.version must reflect the real package.json version, | |
| // not the former hard-coded "1.0.0". | |
| const packageJson = JSON.parse( | |
| await readFile(new URL("../../package.json", import.meta.url), "utf8"), | |
| ) as { version?: string }; | |
| expect(result.serverInfo?.version).toBe(packageJson.version); | |
| expect(typeof result.instructions).toBe("string"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/mcp-handshake.spec.ts` around lines 69 - 73, The test
currently asserts result.serverInfo?.version is not "1.0.0", which will break if
the real package version becomes 1.0.0; update the assertion to compare against
the repository's actual package version by reading the package.json version
(e.g. require('../..../package.json').version or import the package JSON) and
assert result.serverInfo?.version equals or matches that version (or matches a
regex derived from it) instead of asserting inequality; adjust the assertion
around result.serverInfo?.version in mcp-handshake.spec.ts accordingly.
Owner
Author
|
Review loop status:
All 14 CI checks pass (incl. Gates coverage @ 100%). No outstanding actionable findings → proceeding to merge. |
This was referenced Jul 4, 2026
Closed
Closed
Closed
pm health auto-detects missing Ollama embedding model and reports vectorization refresh failure
#455
Closed
This was referenced Jul 11, 2026
Closed
This was referenced Jul 24, 2026
This was referenced Aug 1, 2026
Owner
Author
|
PM context: pm-1js9. Historical linkage verified during the 2026-08-04 ecosystem review. This comment records canonical context only; it does not reopen or change this artifact. |
This was referenced Aug 7, 2026
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.
Summary
A deep audit + remediation pass over the pm CLI + SDK + ecosystem (ADR
pm-96wm). Four specialized read-only sub-agents audited code-quality/perf, SDK/extensions, agent-UX/token-efficiency, and docs/CI; the ecosystem was verified end-to-end in a temp sandbox + project smoke harnesses. Findings were recorded centrally inpm(deduped against the open backlog) to avoid concurrent-write history drift. This PR ships the focused, low-risk batch; larger findings are tracked (see below).Never-block (the most-emphasized goal)
pm close/close-manywithout a reason returned a generic error that named the internalgovernance.require_close_reasonknob but never told the agent how to recover — the single most common agent dead-end (default-on in every preset). Now returns actionable guidance (examples, next steps, how to disable).pm-g799--path/PM_PATHat a project root hard-failedtracker not initializedwhile cwd-mode discovers.agents/pm.resolvePmRootnow redirects an explicit project-root into its.agents/pm, keepingpm init --pathand direct tracker roots verbatim.pm-ryikMCP / contract coherence
serverInfo.versionnow reflectspackage.json(was hardcoded"1.0.0").pm-2nvwstructuredContent.result = nullso consumers read.resultuniformly across success/error.pm-l40hpm_healthdefaults to the compact summary projection (~22× smaller;full=trueopts into the deep payload).pm-yjubpm_copy; 3 → 4 subagents) and locked the claude-plugin smoke to the livetools/listcount.pm-7tvxCI / docs
codeql.ymlinit/analyzeto immutable SHAs + a contract-test assertion thatcodeql.ymlhas no unpinned actions.pm-ji5cCONTRIBUTING.md.pm-t73kCleanup
runtime-schema.ts, unusedNESTED_SETTING_KEYSand_backgroundRefreshInternals) and deduped fourpackage.jsonversion readers intoresolvePmCliVersion(drops unusedfs/pathfrom themain.tshot path).pm-wrqkVerification
pnpm typecheck✓ ·pnpm build✓Deferred findings (tracked, not in this PR)
Recorded for follow-up under the existing capability epics (kept out to keep this PR low-risk):
pm-2bn5settings read cache — the biggest remaining per-command lever; deferred because safe memoization must key onsettings.jsonandschema/*.jsonmtimes (schema files change independently) and stay correct for the long-lived MCP server.pm-vhx6compact-mode trailer trimming ·pm-6l17--tagsnever-block alias ·pm-jozcMCP top-level filter-key hoisting ·pm-q0krslim guided-error boilerplate.pm-oll8field-type validation w/ did-you-mean ·pm-l0jdmanifest JSON Schema +getWorkspaceContractsmemo +FlagDefinitionunify; enrichedpm-ltbr/pm-1js9/pm-k1e4.pm-e7dnrunUpdatetable-driven refactor ·pm-ul02removeas unknown ascasts.Closes #pm-g799, #pm-ryik, #pm-yjub, #pm-7tvx, #pm-t73k, #pm-wrqk, #pm-2nvw, #pm-l40h, #pm-ji5c (pm tracker IDs).
🤖 Generated with Claude Code