Repository navigation
SDK-owned terminal lifecycle policy and durable project history - #846
Merged
Merged
Conversation
Move the close transaction behind public SDK operation contracts, centralize author-controlled terminal reason governance across create/update/close routes, and retain predecessor dependencies across close/reopen cycles. Add SDK documentation, executable examples, complete regression coverage, public-surface provenance, PM evidence, and generated changelog entries for pm-z5pmf8, pm-xm0id4, and pm-2ew0w3.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Owner
Author
|
@greptileai please review this exact head. @coderabbitai full review |
|
✅ 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 59 minutes. |
|
Warning Review limit reached
Next review available in: 38 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: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR moves close handling into the SDK, enforces author-controlled terminal reasons, preserves or removes ordering edges through policy, updates CLI and MCP integrations, expands public exports, and adds documentation and regression coverage. ChangesTerminal lifecycle governance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant MCP
participant lifecycleClose
participant lifecyclePolicy
participant ItemStore
CLI->>lifecycleClose: Route terminal close
MCP->>lifecycleClose: Route terminal close
lifecycleClose->>lifecyclePolicy: Resolve and require terminal reason
lifecycleClose->>ItemStore: Apply terminal metadata and ordering policy
ItemStore-->>lifecycleClose: Updated item and warnings
lifecycleClose-->>CLI: Close result
lifecycleClose-->>MCP: Close result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 GuideThis PR centralizes terminal lifecycle policy in the SDK, introduces public primitives for terminal reasons and predecessor-edge handling, and updates CLI/MCP paths and tests so all terminal transitions share the same reason governance and preserve historical ordering edges by default. Sequence diagram for SDK-owned closeItem operationsequenceDiagram
participant Caller as PresentationSurface
participant Close as closeItem
participant Runtime as runtime_primitives
participant Policy as lifecycle_policy
participant Store as Filesystem
Caller->>Close: closeItem(id, closeReasonText, options, global)
Close->>Runtime: resolvePmRoot, getSettingsPath, pathExists
Close->>Runtime: readSettings
Close->>Runtime: resolveRuntimeStatusRegistry
Close->>Runtime: resolveAuthor
Close->>Runtime: resolveItemTypeRegistry
Close->>Runtime: locateItem
alt item_not_found
Close->>Caller: throw buildItemNotFoundError
end
Close->>Runtime: readLocatedItem (via assertDuplicateTargetExists)
Close->>Policy: requireTerminalReason({explicit, duplicateOf, resolution, message}, requireCloseReason)
Close->>Runtime: resolveIsoOrRelative(completedAt)
Close->>Runtime: mutateItem(op="close", mutate)
activate Runtime
Runtime->>Policy: applyTerminalOrderingPolicy(metadata, lifecyclePolicy)
deactivate Runtime
Runtime-->>Close: {item, changedFields, warnings}
Close->>Runtime: listAllItemMetadataLight (autoUnblockResolvedDependents)
Close->>Runtime: mutateItem(op="update")
Close-->>Caller: CloseOperationResult{item, changed_fields, warnings}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 @.agents/pm/issues/pm-2ew0w3.toon:
- Around line 33-39: Update the owning item pm-2ew0w3 through pm files to
include all six listed source and test paths, ensuring every change is linked.
Then verify the live record state and confirm the table-driven terminal-route
acceptance criterion includes the negative control for a terminal route that
skips rule-set evaluation.
In `@src/sdk/index.ts`:
- Line 460: Update sdk/public-surface.json to record the public symbols exported
by lifecycle-policy.ts, matching the re-export added in src/sdk/index.ts.
Include all newly exposed lifecycle policy symbols so the recorded SDK surface
stays synchronized with the public exports.
In `@src/sdk/lifecycle-policy.ts`:
- Around line 164-176: Update the blocked_by cleanup in the lifecycle transition
to check field presence rather than the trimmed previousBlockedBy value. Use
metadata.blocked_by !== undefined for the deletion and changedFields update,
while retaining previousBlockedBy for any existing normalized-value logic.
In `@src/sdk/lifecycle/close.ts`:
- Around line 298-311: Update closeItem to call listAllItemMetadataLight once
and pass the resulting metadata collection into both findActiveChildIds and
findAutoUnblockCandidates. Refactor those helpers to accept and reuse the
preloaded items instead of performing their own tracker scans, preserving their
existing filtering and unblock behavior.
- Around line 518-531: Update applyCloseReason so an undefined closeReason
preserves any existing metadata.close_reason and does not delete it; only apply
and report close_reason when a new reason is supplied, while retaining the
empty-change result when neither value exists.
- Around line 671-687: Update the closeReason construction in the lifecycle
close flow to always pass options.message to requireTerminalReason, regardless
of lifecyclePolicy.requireCloseReason. Keep the policy flag controlling whether
a reason is required, but ensure author-provided message text remains available
as closure evidence when the requirement is disabled.
- Around line 694-699: Use the canonical identifier returned by locateItem
throughout close validation: pass located.id to findActiveChildIds and use it as
closingId for assertDuplicateTargetExists and circular-chain checks. Keep the
existing located.id usage at the later validation site consistent so child,
duplicate, and cycle comparisons match stored canonical item IDs.
In `@tests/integration/release-readiness-runtime.spec.ts`:
- Around line 2536-2543: Update the assertions in the auto-route close test
around autoRouteJson to verify that item.close_reason equals the author-supplied
message and that warnings does not contain "close_reason_defaulted", while
preserving the existing closed-status and auto-route warning assertions.
In `@tests/unit/commands/close-command.spec.ts`:
- Around line 1202-1253: Extend the acceptance coverage around the existing
predecessor-history test and the relevant close/status-update flows: verify that
update --status closed auto-routes through the same preservation behavior, and
add a corpus-level assertion that blocked_by relationships survive at the rate
required by pm-xm0id4. Ensure the checks also cover clearClosedBlockerSignals
removing blocked_by rows from closed items, or adjust the implementation to
satisfy those criteria while preserving the existing reclose assertions.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 62dcbdc7-eadb-43bf-97be-46730d33a510
📒 Files selected for processing (31)
.agents/pm/history/pm-2ew0w3.jsonl.agents/pm/history/pm-xm0id4.jsonl.agents/pm/history/pm-z5pmf8.jsonl.agents/pm/issues/pm-2ew0w3.toon.agents/pm/issues/pm-xm0id4.toon.agents/pm/issues/pm-z5pmf8.toonCHANGELOG.mddocs/SDK_LIFECYCLE.mddocs/examples/sdk-lifecycle-policy/index.tssdk/public-surface.jsonsrc/cli/commands/close.tssrc/cli/commands/create.tssrc/cli/commands/update.tssrc/core/shared/constants.tssrc/sdk/cli-contracts/typed-action-inputs.tssrc/sdk/generated-error-code-catalog.tssrc/sdk/index.tssrc/sdk/lifecycle-policy.tssrc/sdk/lifecycle/close.tssrc/sdk/runtime.tstests/integration/cli.integration.spec.tstests/integration/duplicates-command.integration.spec.tstests/integration/mcp-dynamic-package-actions.spec.tstests/integration/release-readiness-runtime.spec.tstests/unit/commands/close-command.spec.tstests/unit/commands/create-command.spec.tstests/unit/commands/dedupe-merge-command.spec.tstests/unit/commands/update-command.spec.tstests/unit/mcp/mcp-server-branch-residual.spec.tstests/unit/sdk/lifecycle-policy.spec.tstests/unit/sdk/sdk-index.spec.ts
Address exact-head review feedback by preserving existing reasons, normalizing canonical ids, clearing present blocked scalars, sharing the tracker scan, proving update-route and corpus ordering survival, asserting reason provenance, and regenerating the runtime contract fixture. Record review evidence on the canonical PM lineage.
Owner
Author
|
Bot review acknowledgements for exact head 43dc0e0: Gemini sunset notice was accurate but informational; Sourcery was down-voted because its rate-limit response contained no review; CodeRabbit invocation, walkthrough, and nine findings were usefulness-voted and each inline thread received a specific verification or fix response. All actionable findings are covered by 6,613 passing tests at 100/100/100/100. |
Owner
Author
|
@greptileai please re-review exact head 43dc0e0 after the review fixes. @coderabbitai full review |
|
✅ 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 38 minutes. |
Owner
Author
|
Final review-loop acknowledgement for exact head 43dc0e0: all nine CodeRabbit threads are resolved and their confirmation replies/reviews were usefulness-voted. Sourcery reviewer guidance was also acknowledged; its sequence description predates the follow-up optimization, where closeItem now performs one metadata scan, and pm-z5pmf8 is open and unclaimed. Required checks are terminal green with no failed checks. |
Owner
Author
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.
What changed
update --status closedand direct terminal create from synthesizing immutable close reasonsblocked_bydependency rows through close, reopen, and reclose while clearing only transient blocked-state scalarsWhy
Terminal lifecycle policy lived privately in CLI command modules. That caused two concrete defects: one terminal route bypassed close-reason governance by inventing evidence, and closing a dependent deleted the ordering edges needed to understand historical project context. The SDK now owns this tranche of lifecycle semantics so every presentation surface uses one operation and consumers can inspect or select policy explicitly.
PM lineage
pm-xm0id4andpm-2ew0w3are complete.pm-z5pmf8remains open and unclaimed because the create/update/delete/restore/context/planning SDK-to-CLI inversions are explicitly outside this delivered lifecycle tranche.Verification
close_reason_requirednpx/bunxbinariespnpm audit --audit-level moderate: no known vulnerabilitiesHosted analysis and review gates are expected to complete against this immutable PR head.
Summary by Sourcery
Move terminal lifecycle semantics into an SDK-owned close operation while aligning reason governance and predecessor-edge policy across CLI, SDK, and MCP surfaces.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by cubic
Moves terminal lifecycle to SDK-owned operations for consistent close behavior across CLI, SDK, and MCP. Fixes reason governance, preserves predecessor edges, and hardens close provenance (keeps existing reasons and normalizes ids) to keep project history intact. Addresses pm-2ew0w3 and pm-xm0id4; advances pm-z5pmf8.
Bug Fixes
close_reason_requiredacross direct close,update --status closed, and direct terminal create; no synthesized reasons, and existing reasons are never overwritten.pm-prefix) and clear only transient blocked state while preservingblocked_byedges through close → reopen → reclose.resolveTerminalReason/requireTerminalReason; update tests and runtime contracts to assert ordering survival and provenance.Migration
closeItem/runCloseplusrequireTerminalReason,applyTerminalOrderingPolicy, andTerminalTransitionPolicyfrom@unbrained/pm-cli/sdk.pm create --status closedorpm update --status closed, pass a reason via--close-reasonor--message; reasonless transitions now fail withclose_reason_requiredunder governance. Expectblocked_byedges to remain after close.Written for commit 43dc0e0. Summary will update on new commits.