Fix output_format override corrupting all command output, and adopt pm-cli 2026.7.27 - #40
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (8)
WalkthroughThe extension now declines unclaimed ChangesExtension corrections
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceRunner
participant Extension
participant Host
ServiceRunner->>Extension: invoke output_format override
Extension->>ServiceRunner: return handled=false
ServiceRunner->>Host: pass through original payload
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Reviewer's GuideUpdates the pm-starter extension to decline output_format service payloads correctly under pm-cli 2026.7.27, adds a regression test that exercises the real service runner, and bumps the package and pm-cli dependencies to the new version with corresponding changelog and metadata updates. Sequence diagram for updated output_format service override handlingsequenceDiagram
actor User
participant pm_cli
participant ServiceRunner
participant pm_starter_output_format
User->>pm_cli: pm list --json
pm_cli->>ServiceRunner: run output_format service
ServiceRunner->>pm_starter_output_format: api.registerService("output_format") handler
pm_starter_output_format-->>ServiceRunner: { handled: false }
ServiceRunner->>pm_cli: isServiceOverrideDecision({ handled: false })
pm_cli-->>User: render default command result (items, count)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Greptile SummaryThe PR corrects the
Confidence Score: 5/5The PR appears safe to merge. The previously reported runtime-version mismatch is resolved across package.json, manifest.json, index.ts, and dist/index.js, and no blocking failure remains.
|
| Filename | Overview |
|---|---|
| index.ts | Correctly declines the global output-format service and reports version 2026.7.27. |
| dist/index.js | Rebuilt installable output contains the corrected override and aligned runtime version. |
| test/smoke.test.ts | Adds real service-runner coverage and checks package, manifest, and extension version consistency. |
| package.json | Requires pm-cli 2026.7.27 for the updated service-override decision contract. |
| package-lock.json | Locks the development pm-cli dependency to 2026.7.27. |
Reviews (5): Last reviewed commit: "test: pin extension self-reported versio..." | Re-trigger Greptile
…version Greptile review on PR #40 caught that package.json declared 2026.7.27 while manifest.json and the extension's self-reported version constant in index.ts still declared 2026.7.26, so an installed pm-starter would report a stale version to the host and to `pm extension --manage`. Aligned both and rebuilt. The agent-driven bumps in pm-changelog, pm-gantt-chart and pm-graph had already handled manifest.json correctly — this was a gap in the hand-run bump, which rewrote only package.json. Gates re-run: build, typecheck, 10/10 tests, changelog:check up to date.
Bot feedback triage — all four read, voted, and acted on@greptile-apps 👍 — you found a real bug and it is now fixed. The version-metadata Root cause, for your model: the sibling bumps in this fleet (pm-changelog, pm-gantt-chart, Generalisable rule for this codebase: a pm extension declares its version in three places — Your 4/5 confidence and "safe to merge after considering a non-blocking mismatch" was well @sourcery-ai 👍 — the reviewer's guide and the sequence diagram are accurate, including the @coderabbitai 👎 — @gemini-code-assist 👎 — consumer Gemini Code Assist is sunset and all review activity has Note on the
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
ᕱ⑅ᕱ ✏️ Learnings added
✅ Action performedReview finished.
|
…t.json PR #40's version alignment (b383b26) fixed the immediate drift — package.json said 2026.7.27 while the extension export still said 2026.7.26 — but nothing prevented the next hand-run bump from drifting the same way. The version lives in three places that must agree (package.json, manifest.json, and the version constant in index.ts compiled into the committed dist/), and the extension cannot read package.json at runtime because standalone installs resolve only their own dist/, so the duplication is deliberate and needs a guard. Add a regression test asserting the default export's version equals both package.json and manifest.json. It imports the extension from dist/, so it also fails when source and the committed build are out of sync. No any casts (JSON parsed to unknown and narrowed); top-level imports only. Falsification verified: reverting index.ts to 2026.7.26 yields 10 pass / 1 fail; restoring yields 11/11. Gates: build, tsc --noEmit, 11/11 tests, changelog regenerated and changelog:check green. Closes pm-starter-1atg.
|
@greptileai @coderabbitai full review Two commits pushed since the last pass, both addressing the P2 finding on
Verified locally: 11/11 tests pass, |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 57 minutes. |
Under pm-cli 2026.7.27 an `output_format` service override's bare return value
IS what the host renders. This extension declined by returning the inbound
`ctx.payload`, which 2026.7.27 no longer reads as "not handled" — so EVERY
command in any workspace with pm-starter installed printed the whole command
context (`global`, `format`, `options`, …) instead of its own result.
Reproduced against real data in a throwaway workspace: `pm list` printed a
seeded item, installing pm-starter made it print the command context, and the
item renders correctly again after this fix.
The decline path now returns the `{ handled: false }` decision. pm's
`isServiceOverrideDecision` predicate accepts that only when `handled` is false
AND the object carries exactly that one key, so the literal is written
deliberately. The SDK's `declineServiceOverride()` returns the same object, but
it is a runtime value and a standalone-installed extension cannot resolve the
SDK at runtime (see the note at the top of index.ts); the matching
`ServiceOverrideDecision` type is not part of the public SDK surface either.
This matters disproportionately here: pm-starter is the reference template handed
to every new extension author, so the broken pattern was being taught.
Paired with the 2026.7.27 dependency adoption (`>=2026.7.27` / `^2026.7.27`,
version 2026.7.27) because 2026.7.26 has no `isServiceOverrideDecision` at all —
the regression test can only exercise the real contract once the dependency is
bumped. Confirmed: the pinned 2026.7.26 tree reported `handled: true` with
`result: { handled: false }`; on 2026.7.27 the runner correctly reports
`handled: false`.
The regression test uses the REAL SDK harness (`createExtensionTestHarness` +
`runRegisteredServiceOverrideForTest`) rather than this file's hand-rolled api
double. The double registered `registerService` as a no-op that discarded the
override, so its return value was never evaluated and this entire bug class was
invisible to the suite. Falsification verified: reverting index.ts to the old
`ctx.payload` form fails the new test (9 pass / 1 fail); restoring the fix passes
(10 pass / 0 fail).
Filed upstream as unbraind/pm-cli#776 — the semantics changed under a "Fixed"
changelog entry with no migration note, and the decision type is unexported.
Gates: build, typecheck, check, 10/10 tests, changelog:full + changelog:check
after `git fetch --tags --force`. Production proof: install ok with no warnings,
`pm list` unaffected, `pm list --json` valid, `pm starter --help` registers.
pm items:
- pm-starter-ejr6 (Issue) — output_format decline fix + 2026.7.27 adoption
…version Greptile review on PR #40 caught that package.json declared 2026.7.27 while manifest.json and the extension's self-reported version constant in index.ts still declared 2026.7.26, so an installed pm-starter would report a stale version to the host and to `pm extension --manage`. Aligned both and rebuilt. The agent-driven bumps in pm-changelog, pm-gantt-chart and pm-graph had already handled manifest.json correctly — this was a gap in the hand-run bump, which rewrote only package.json. Gates re-run: build, typecheck, 10/10 tests, changelog:check up to date.
…t.json PR #40's version alignment (b383b26) fixed the immediate drift — package.json said 2026.7.27 while the extension export still said 2026.7.26 — but nothing prevented the next hand-run bump from drifting the same way. The version lives in three places that must agree (package.json, manifest.json, and the version constant in index.ts compiled into the committed dist/), and the extension cannot read package.json at runtime because standalone installs resolve only their own dist/, so the duplication is deliberate and needs a guard. Add a regression test asserting the default export's version equals both package.json and manifest.json. It imports the extension from dist/, so it also fails when source and the committed build are out of sync. No any casts (JSON parsed to unknown and narrowed); top-level imports only. Falsification verified: reverting index.ts to 2026.7.26 yields 10 pass / 1 fail; restoring yields 11/11. Gates: build, tsc --noEmit, 11/11 tests, changelog regenerated and changelog:check green. Closes pm-starter-1atg.
cf2e51f to
7b3ced8
Compare
Summary
Fixes a bug that corrupted the output of every pm command in any workspace with pm-starter
installed, and adopts pm-cli 2026.7.27.
Under pm-cli 2026.7.27 an
output_formatservice override's bare return value IS what the hostrenders. This extension declined payloads by returning the inbound
ctx.payload, which2026.7.27 no longer reads as "not handled". The result: unrelated commands printed the whole
command context instead of their own output.
Reproduced against real data
After this fix, in a fresh workspace with two seeded items:
The fix
The decline path now returns the
{ handled: false }decision. pm'sisServiceOverrideDecisionpredicate accepts that only whenhandledisfalseand theobject carries exactly that one key, so the literal is written deliberately rather than spread
into a larger object.
The SDK does export
declineServiceOverride()(fromsdk/authoring), which returns the sameobject — but it is a runtime value, and a standalone-installed extension loads only its own
dist/and cannot resolve the SDK at runtime (see the note at the top ofindex.ts). Thematching
ServiceOverrideDecisiontype is not part of the public SDK surface either, sothere is nothing to type-import. Both points are raised upstream.
Why the dependency bump ships in the same PR
pm-cli 2026.7.26 has no
isServiceOverrideDecisionat all, so the decision object is notunderstood there. The regression test can only exercise the real contract once the dependency is
bumped. This was visible during development: against the pinned 2026.7.26 tree the harness
reported
handled: truewithresult: { handled: false }; on 2026.7.27 it correctly reportshandled: false.@unbrained/pm-clipeerDependencies>=2026.7.26→>=2026.7.27@unbrained/pm-clidevDependencies^2026.7.26→^2026.7.27version→2026.7.27The test that should have caught this
test/smoke.test.tsused a hand-rolled api double whoseregisterServicewas a no-op(
registerService: () => {}). It discarded the registered override, so the override's returnvalue was never evaluated — this entire bug class was invisible to the suite.
The new test drives pm's real service runner via the SDK harness
(
createExtensionTestHarness+runRegisteredServiceOverrideForTest) and assertshandled === false, that a declined payload is returned to the host untouched, and that noextension_service_override_*warnings are emitted.Falsification checked — this test is not vacuous:
ctx?.payloadform{ handled: false }fixUpstream
Filed as unbraind/pm-cli#776: the semantics
change shipped under a Fixed changelog entry ("First-party service collision: builtin-calendar
and builtin-guide-shell both override the global output_format service") with no breaking marker
and no migration note, it fails silently and globally with exit code 0, and
ServiceOverrideDecisionis unexported while its runtime constructors are exported. pm-graph andpm-slack-standup carried the same payload-echo pattern and are being fixed too.
Gates
build,typecheck,check, 10/10 tests,changelog:full+changelog:check(aftergit fetch origin --tags --force, since stale local tags otherwise red the CI changelog check).Production proof captured above with the built extension installed into a throwaway workspace —
not the dev runner.
pm health's only remaining warning isextension_migration_pending:project:pm-starter:pm-starter-0001-noop, which is the template'sown deliberate demo migration, not an activation failure.
Version-metadata fix (
b383b26) — caught in reviewA pm extension declares its version in three places:
package.json,manifest.json, and aversion:constant inside the module. The initial commit bumped onlypackage.json, so aninstalled pm-starter would have reported a stale
2026.7.26to the host and topm extension --manage. Greptile flagged the mismatch;b383b26alignsmanifest.jsonand theindex.tsconstant and rebuildsdist/.The sibling agent-driven bumps (pm-changelog, pm-gantt-chart, pm-graph) had handled
manifest.jsoncorrectly — this was a gap in a hand-run bump that rewrote only
package.json. The same drift wasfixed proactively in pm-slack-standup.
Gates re-run after the fix: build, typecheck, 10/10 tests,
changelog:checkup to date.pm items
pm-starter-ejr6— Issue: output_format decline fix + 2026.7.27 adoption