fix(delivery): simplify phase authorization - #117
Merged
Conversation
|
Warning Review limit reached
Next review available in: 7 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: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR revises delivery authorization, orchestration, recovery, reviewer evidence, and greenfield story contracts around explicit owner-requested scope. It also replaces candidate-package integrity checks with delivery-surface manifests and detects retired delivery gates. ChangesDelivery coordination and authorization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 576d6ef333
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/delivery/greenfield/stories/GF-050.md (1)
90-98: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove residual activation-gate terminology across both story contracts. The new model authorizes implementation through an explicit current-owner or named-delegate request; remaining activation wording can reintroduce the retired hosted/external gate.
docs/delivery/greenfield/stories/GF-047.md#L69-69: update the DR-5/fallback wording at Line 119 from “external owner activation” to the current owner/delegate request contract.docs/delivery/greenfield/stories/GF-050.md#L90-L98: update the realization-authorization wording at Line 214 from “exact activation” to the current owner/delegate request contract.🤖 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 `@docs/delivery/greenfield/stories/GF-050.md` around lines 90 - 98, Update the DR-5/fallback wording in GF-047 and the realization-authorization wording in GF-050 to remove “external owner activation” and “exact activation” terminology. Express both contracts using only an explicit current-owner or named-delegate implementation request, preserving the existing provenance and evidence requirements.
🤖 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/skills/orchestrate-phase-delivery/references/phase-protocol.md:
- Around line 75-78: Update the exhaustive recovery-fact rule in the phase
protocol to include tracker, clean-status, sanitized-environment, ignored-state,
reviewer-independence, required-check, and ownership facts alongside the
existing branch/object/path/base/evidence facts. Ensure these localized blockers
use the same affected-story and descendant blocking behavior, while retaining
OWNER_DECISION_REQUIRED only for material decisions.
- Around line 57-59: Clarify in the phase protocol that terminally blocked work
is an explicit exception to the confirmed-landing cleanup requirement. Before
deleting any non-landed worktrees under this exception, require reconciliation
or an applicable material owner decision, retained ledger/evidence, and explicit
authorization for the scoped cleanup; preserve the provider-managed hosted CI
exception as workspace-only.
In @.agents/skills/orchestrate-phase-delivery/SKILL.md:
- Around line 39-44: Expand the localized-block rule in the affected
orchestration guidance to explicitly include missing tracker, base/path/status,
sanitized-environment, ignored-state, required-check, reviewer-independence, and
ownership facts, matching phase-protocol.md and phase-handoff-template.md. Keep
these conditions as story-local blockers that do not revoke authorization or
block independent ready stories, while reserving OWNER_DECISION_REQUIRED for
genuine authority, scope, realization, reachability, or trade-off ambiguity.
In `@docs/delivery/greenfield/decisions.md`:
- Line 26: Update the DR-7 decision entry to remove the “await activation”
wording and any implication of an undefined external activation gate. State
instead that the TypeScript SDK, CLI, and stdio MCP remain unavailable until
explicitly selected and their existing qualification and reachability gates
pass.
- Around line 76-95: Revise the “Implementation authorization” section to remove
authorization by inference from “necessarily incorporates.” Require both the
implementation request and external phase ledger to explicitly name every
selected bounded default and DR-* choice, with deterministic authoritative
bindings where applicable; otherwise preserve fail-closed
OWNER_DECISION_REQUIRED behavior for missing or conflicting selections.
In `@docs/delivery/greenfield/phase-orchestration.md`:
- Line 62: Update the recovery blocking criteria in the phase orchestration
guidance to include every handoff-contract fact: tracker, path/status,
environment allowlist, ignored-state, reviewer, required-check, and ownership,
in addition to object, branch, base, evidence, and clean reconciliation. Ensure
any missing, ambiguous, dirty, or irreconcilable fact blocks only the affected
story and its descendants, while independent ready stories continue.
- Around line 29-34: Clarify the terminally blocked cleanup rule in the story
worktree retention guidance: allow deletion of a non-landed story worktree only
through an explicit terminal-blocked disposition, such as reconciliation, an
applicable material owner decision, or explicitly scoped cleanup. Require
preserving the story’s external-ledger evidence, including its terminal status
and cleanup authorization, before deleting the worktree or branch; keep
confirmed landing as the normal cleanup prerequisite.
In `@docs/delivery/greenfield/reviewer/README.md`:
- Around line 35-38: Update the protocol wording in README.md and
review-checklist.md at the specified ranges to say “No delivery-package
qualification or delivery-surface digest approval” instead of the ambiguous “No
package qualification.” Preserve the existing provider qualification
requirement, including CF-GATE-PROVIDER, so unqualified provider paths remain
unreachable until named qualification evidence passes.
In `@docs/delivery/greenfield/stories/GF-001.md`:
- Around line 151-153: Update GF-001 to remove remaining “exact owner
activation” and “owner-ratified” terminology, replacing each occurrence with
wording that requires a ledger-recorded current-owner or named-delegate
implementation request selecting the workspace/toolchain tuple and story; if
retaining “activation,” explicitly define it as that exact request.
In `@docs/delivery/greenfield/stories/GF-019.md`:
- Around line 140-141: Update the authorization wording in
docs/delivery/greenfield/stories/GF-019.md lines 140-141 and
docs/delivery/greenfield/stories/GF-020.md lines 153-154 to require a “current
owner or named-delegate implementation request,” while preserving the existing
restrictions on parser, policy, authority, and widening delegation.
In `@docs/delivery/greenfield/stories/GF-039.md`:
- Line 85: Rewrite the CP-MEDIATOR behavior so RT-CONTROLLER first authorizes
the exact Operation, then CP-MEDIATOR validates its manifest, owner/delegate,
and binding before dispatching through PORT-WORKSPACE. State that dispatch fails
closed when authorization or binding is unverifiable, and preserve the
provider’s limited activation role and existing evidence/authority constraints.
In `@docs/delivery/greenfield/stories/GF-047.md`:
- Line 69: Update the DR-5/fallback wording around the qualification flow to
remove the obsolete “external owner activation” requirement, while retaining the
current owner or named-delegate request and GF-022 admission requirements.
Ensure the provider cannot be interpreted as requiring any retired
external-activation artifact.
- Line 93: Update the local-verifier manifest and corresponding start/acceptance
evidence requirements to record the authorizing principal as either the current
owner or a named delegate, along with request freshness, exact phase/story
scope, and constraints. Preserve these fields identically in both evidence
points and retain the governing design’s exact IDs, authority boundaries, and
immutable bindings; apply the same change to the related section around the
acceptance requirements.
In `@docs/delivery/greenfield/stories/GF-050.md`:
- Around line 201-209: Update the exact-acceptance sentence in GF-050 to remove
the residual “approved delivery” package-approval terminology and explicitly
identify the delivery manifest as informational evidence only. Preserve the
requirements that the candidate tree and separately bound evidence are reviewed,
while ensuring acceptance does not imply a package/digest gate or authorize
landing.
In `@docs/delivery/greenfield/stories/GF-051.md`:
- Around line 146-147: Update the GF-051 authorization clause to allow
activation through either an explicit current-owner selection or a valid
named-delegate request for the named story, while preserving the existing
widening ownership and read-only constraint rules.
In `@docs/delivery/greenfield/track.json`:
- Line 47: Update the line 47 policy wording to say “No separate
delivery-package qualification” instead of “No separate package qualification,”
preserving the requirement that provider qualification evidence must pass before
any provider, adapter, or effect path is reachable. Add or retain a negative
test demonstrating configuration cannot expose an unqualified path.
In `@docs/delivery/README.md`:
- Around line 116-118: Update the delivery requirements near the existing
detached/fresh-clone wording to explicitly prohibit local clones as delivery,
review, or recovery workspaces. State that clones cannot replace
registered-worktree evidence, while preserving the existing list of inactive
gates and phase-contract terminology.
In `@tools/repo-guard/bin/check-delivery-track.mjs`:
- Around line 45-54: Expand RETIRED_DELIVERY_GATE_PATTERNS to detect all retired
gate classes: delivery-package qualification, digest approval, hosted activation
artifacts, and landed-commit equivalence. Add case-insensitive patterns covering
the missing terminology while preserving the existing patterns and ensure the
corresponding detector logic near the other occurrence uses the same complete
coverage.
---
Outside diff comments:
In `@docs/delivery/greenfield/stories/GF-050.md`:
- Around line 90-98: Update the DR-5/fallback wording in GF-047 and the
realization-authorization wording in GF-050 to remove “external owner
activation” and “exact activation” terminology. Express both contracts using
only an explicit current-owner or named-delegate implementation request,
preserving the existing provenance and evidence requirements.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6062254d-f516-410a-9e49-3b631519cbe3
📒 Files selected for processing (56)
.agents/skills/orchestrate-phase-delivery/README.md.agents/skills/orchestrate-phase-delivery/SKILL.md.agents/skills/orchestrate-phase-delivery/evals/evals.json.agents/skills/orchestrate-phase-delivery/evals/trigger_queries.json.agents/skills/orchestrate-phase-delivery/references/phase-protocol.mdREADME.mddocs/delivery/AGENTS.mddocs/delivery/README.mddocs/delivery/greenfield/baseline-and-findings.mddocs/delivery/greenfield/decisions.mddocs/delivery/greenfield/delivery-policy.mddocs/delivery/greenfield/phase-handoff-template.mddocs/delivery/greenfield/phase-orchestration.mddocs/delivery/greenfield/research-ledger.mddocs/delivery/greenfield/reviewer/README.mddocs/delivery/greenfield/reviewer/review-checklist.mddocs/delivery/greenfield/risks-and-owner-decisions.mddocs/delivery/greenfield/stories/GF-001.mddocs/delivery/greenfield/stories/GF-002.mddocs/delivery/greenfield/stories/GF-003.mddocs/delivery/greenfield/stories/GF-004.mddocs/delivery/greenfield/stories/GF-005.mddocs/delivery/greenfield/stories/GF-010.mddocs/delivery/greenfield/stories/GF-011.mddocs/delivery/greenfield/stories/GF-012.mddocs/delivery/greenfield/stories/GF-013.mddocs/delivery/greenfield/stories/GF-014.mddocs/delivery/greenfield/stories/GF-015.mddocs/delivery/greenfield/stories/GF-019.mddocs/delivery/greenfield/stories/GF-020.mddocs/delivery/greenfield/stories/GF-021.mddocs/delivery/greenfield/stories/GF-024.mddocs/delivery/greenfield/stories/GF-033.mddocs/delivery/greenfield/stories/GF-034.mddocs/delivery/greenfield/stories/GF-037.mddocs/delivery/greenfield/stories/GF-039.mddocs/delivery/greenfield/stories/GF-040.mddocs/delivery/greenfield/stories/GF-041.mddocs/delivery/greenfield/stories/GF-042.mddocs/delivery/greenfield/stories/GF-043.mddocs/delivery/greenfield/stories/GF-046.mddocs/delivery/greenfield/stories/GF-047.mddocs/delivery/greenfield/stories/GF-050.mddocs/delivery/greenfield/stories/GF-051.mddocs/delivery/greenfield/stories/GF-052.mddocs/delivery/greenfield/stories/GF-053.mddocs/delivery/greenfield/stories/GF-054.mddocs/delivery/greenfield/stories/GF-056.mddocs/delivery/greenfield/stories/GF-057.mddocs/delivery/greenfield/stories/GF-060.mddocs/delivery/greenfield/stories/GF-061.mddocs/delivery/greenfield/stories/GF-062.mddocs/delivery/greenfield/story-contract.mddocs/delivery/greenfield/track.jsontools/repo-guard/bin/check-delivery-track.mjstools/repo-guard/tests/check-delivery-track.test.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/repo-guard/tests/check-delivery-track.test.mjs (1)
1347-1353: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression cases for allowed negated gate wording.
The table exercises only affirmative retired-gate phrases. Add cases such as
does not require,doesn't require, andNo separate ... is required, asserting that active-policy validation accepts them.🤖 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 `@tools/repo-guard/tests/check-delivery-track.test.mjs` around lines 1347 - 1353, Add regression cases to the relevant validation table in check-delivery-track tests for allowed negated gate wording, including “does not require,” “doesn't require,” and “No separate ... is required” variants. Assert that active-policy validation accepts each case while preserving the existing affirmative retired-gate coverage.
🤖 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 `@tools/repo-guard/bin/check-delivery-track.mjs`:
- Around line 45-53: The retired-gate patterns in
tools/repo-guard/bin/check-delivery-track.mjs lines 45-53 need consistent
negation handling so suffix and contraction forms such as “does not require” and
“doesn't require” do not match. Update RETIRED_DELIVERY_GATE_PATTERNS
accordingly, and add regression cases in
tools/repo-guard/tests/check-delivery-track.test.mjs lines 1347-1353 confirming
acceptance of “does not require,” “doesn't require,” and “No separate ... is
required.”
---
Nitpick comments:
In `@tools/repo-guard/tests/check-delivery-track.test.mjs`:
- Around line 1347-1353: Add regression cases to the relevant validation table
in check-delivery-track tests for allowed negated gate wording, including “does
not require,” “doesn't require,” and “No separate ... is required” variants.
Assert that active-policy validation accepts each case while preserving the
existing affirmative retired-gate coverage.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cdbc92e4-0840-4c50-b68e-4cbd6f422c61
📒 Files selected for processing (20)
.agents/skills/orchestrate-phase-delivery/SKILL.md.agents/skills/orchestrate-phase-delivery/evals/evals.json.agents/skills/orchestrate-phase-delivery/references/phase-protocol.mdREADME.mddocs/delivery/README.mddocs/delivery/greenfield/decisions.mddocs/delivery/greenfield/delivery-policy.mddocs/delivery/greenfield/phase-orchestration.mddocs/delivery/greenfield/reviewer/README.mddocs/delivery/greenfield/reviewer/review-checklist.mddocs/delivery/greenfield/risks-and-owner-decisions.mddocs/delivery/greenfield/stories/GF-019.mddocs/delivery/greenfield/stories/GF-020.mddocs/delivery/greenfield/stories/GF-039.mddocs/delivery/greenfield/stories/GF-047.mddocs/delivery/greenfield/stories/GF-050.mddocs/delivery/greenfield/stories/GF-051.mddocs/delivery/greenfield/track.jsontools/repo-guard/bin/check-delivery-track.mjstools/repo-guard/tests/check-delivery-track.test.mjs
🚧 Files skipped from review as they are similar to previous changes (14)
- docs/delivery/greenfield/stories/GF-020.md
- docs/delivery/greenfield/stories/GF-051.md
- docs/delivery/greenfield/stories/GF-019.md
- .agents/skills/orchestrate-phase-delivery/SKILL.md
- docs/delivery/greenfield/risks-and-owner-decisions.md
- .agents/skills/orchestrate-phase-delivery/evals/evals.json
- docs/delivery/README.md
- docs/delivery/greenfield/reviewer/review-checklist.md
- docs/delivery/greenfield/reviewer/README.md
- .agents/skills/orchestrate-phase-delivery/references/phase-protocol.md
- README.md
- docs/delivery/greenfield/stories/GF-050.md
- docs/delivery/greenfield/delivery-policy.md
- docs/delivery/greenfield/phase-orchestration.md
This was referenced Jul 30, 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
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
orchestrate-phase-deliveryskill, evals, reviewer packet, handoff, story contracts, and validator terminologyBehavior change
A direct request such as “implement Phase 1” now starts orchestration within the current product/design/track scope. Missing recoverable execution evidence blocks only the affected story and descendants;
OWNER_DECISION_REQUIREDis reserved for material product, architecture, authority, scope, dependency, realization, provider-reachability, or accepted-trade-off ambiguity.No track dependency or dependency-edge record changed. No custom sealer or fresh-clone workflow is restored.
Validation
pnpm check— 20 tasks passed; repo-guard 93/93 tests passedpnpm delivery:check— 55/55 delivery mutation tests passedpnpm format:checkmainexactlygit diff --checkSummary by CodeRabbit
OWNER_DECISION_REQUIREDand blocking behavior so only the affected story (and descendants) are held until ledger facts are reconciled.