demo: make Admin delete schedule-aware - #55
Conversation
|
@coderabbitai review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 51 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe interactive authorization demo now schedule-controls Admin’s ChangesInteractive schedule enforcement
Repository workflow maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ScheduleUI
participant evaluateSchedule
participant PolicyFacts
participant AuthorizationDecisions
participant PolicyViews
User->>ScheduleUI: set schedule or clock state
ScheduleUI->>evaluateSchedule: evaluate Admin cars.delete
evaluateSchedule->>PolicyFacts: update schedule-controlled fact
evaluateSchedule->>AuthorizationDecisions: evaluate at clockTime
AuthorizationDecisions-->>evaluateSchedule: return allow or deny
evaluateSchedule->>PolicyViews: refresh table and permission sidebar
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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 @.agents/wisdom/process.md:
- Around line 10-20: Update the “CodeRabbit SUCCESS with no review” guidance to
avoid treating empty review output as definitive evidence of rate limiting.
Require verifying whether the review covered the latest commit, distinguish
incremental skips from rate-limited or unavailable results, and obtain an
explicit human decision before proceeding in either uncertain case; align the
instructions with the behavior described in adversarial-review guidance and
engineering workflow documentation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 947550f4-550b-4986-b47c-869c732c008d
📒 Files selected for processing (6)
.agents/wisdom/process.md.gitignoreexamples/interactive-decision-demo/README.mdexamples/interactive-decision-demo/index.htmlexamples/interactive-decision-demo/main.tsexamples/interactive-decision-demo/style.css
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@docs/demo-video-script.md`:
- Line 3: Align the target duration with the storyboard by either trimming at
least three seconds from the final sequence so it ends by 2:55, or updating the
target range to include the 2:58 storyboard endpoint. Keep the overall “never
over 3:00” constraint intact.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 376037a5-be48-4b11-9e97-e64903284c63
📒 Files selected for processing (2)
.agents/wisdom/process.mddocs/demo-video-script.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/wisdom/process.md
| @@ -0,0 +1,57 @@ | |||
| # Mizan demo video — YouTube / OpenAI Build Week | |||
|
|
|||
| Target: 2:40–2:55, landscape 16:9, public YouTube, never over 3:00. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the target duration with the storyboard.
The stated target ends at 2:55, but the storyboard ends at 2:58. Either trim at least three seconds from the final sequence or update the target range so production does not treat an out-of-range cut as complete.
Also applies to: 25-26
🤖 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/demo-video-script.md` at line 3, Align the target duration with the
storyboard by either trimming at least three seconds from the final sequence so
it ends by 2:55, or updating the target range to include the 2:58 storyboard
endpoint. Keep the overall “never over 3:00” constraint intact.
553ae45 to
4dc15fd
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
5c215bb to
08b39b4
Compare
…Admin cars.delete only, restore manage-policy gate
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/interactive-decision-demo/main.ts (2)
209-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate admin
cars.deletefact-construction logic.The scheduled-vs-unrestricted
admin/cars.deletefact block is duplicated verbatim inrestorePolicyFactsFromSaved(Lines 221-235) andevaluateSchedule(Lines 759-770). Any future change to the fact shape (e.g. adding astartsAt, changing the effect) needs to be kept in sync by hand in two places.♻️ Suggested consolidation
+function applyAdminCarsDeleteFact(enabled: boolean, startH: number, startM: number, endH: number, endM: number): void { + policySource.removeAllFacts("admin", "cars.delete"); + policySource.addFact("admin", { + permission: "cars.delete", + effect: "grant", + ...(enabled ? { schedule: makeWeekSchedule(startH, startM, endH, endM) } : {}), + }); +}Then call this from both
restorePolicyFactsFromSavedandevaluateSchedule.Also applies to: 756-792
🤖 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 `@examples/interactive-decision-demo/main.ts` around lines 209 - 236, Extract the shared admin `cars.delete` fact construction from `restorePolicyFactsFromSaved` and `evaluateSchedule` into a helper that uses the schedule settings to create either the scheduled or unrestricted grant fact. Replace both duplicated blocks with calls to this helper, preserving the existing behavior and fact shape.
839-868: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant
saveState()calls afterattemptManagement().
attemptManagement()already callssaveState()internally when the decision isallow(Line 519), capturing state afteronAllowed()has mutated it. Each clock handler here then callssaveState()again afterevaluateSchedule()(Lines 846, 856, 866) even thoughevaluateSchedule()only rewrites derivedpolicySourcefacts from the same primitives that were already persisted — no new state to save. This is a newly-introduced instance of a pattern that also exists in the (unchanged) toggle/hour-input handlers above.♻️ Suggested fix
byId("clock-inc").addEventListener("click", async () => { const granted = await attemptManagement(() => { clockTime = new Date(clockTime.getTime() + 3_600_000); }, "Advance clock"); if (!granted) return; updateClockDisplay(); await evaluateSchedule(); - saveState(); });(apply similarly to the
clock-decandclock-resethandlers)🤖 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 `@examples/interactive-decision-demo/main.ts` around lines 839 - 868, Remove the redundant saveState() calls from the clock-inc, clock-dec, and clock-reset click handlers. Keep attemptManagement() responsible for persisting the state after the allowed mutation, while preserving each handler’s updateClockDisplay() and evaluateSchedule() calls.
🤖 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 `@examples/interactive-decision-demo/README.md`:
- Around line 58-60: Update the README section describing schedule enforcement
to state that the Admin cars.delete decision, evaluated at clockTime, enforces
the schedule, while manage-policy only authorizes users to modify schedule
settings.
---
Nitpick comments:
In `@examples/interactive-decision-demo/main.ts`:
- Around line 209-236: Extract the shared admin `cars.delete` fact construction
from `restorePolicyFactsFromSaved` and `evaluateSchedule` into a helper that
uses the schedule settings to create either the scheduled or unrestricted grant
fact. Replace both duplicated blocks with calls to this helper, preserving the
existing behavior and fact shape.
- Around line 839-868: Remove the redundant saveState() calls from the
clock-inc, clock-dec, and clock-reset click handlers. Keep attemptManagement()
responsible for persisting the state after the allowed mutation, while
preserving each handler’s updateClockDisplay() and evaluateSchedule() calls.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 22d2c1a6-dcdd-4e14-91a1-146e8b6a3404
📒 Files selected for processing (5)
.gitignoreexamples/interactive-decision-demo/README.mdexamples/interactive-decision-demo/index.htmlexamples/interactive-decision-demo/main.tsexamples/interactive-decision-demo/style.css
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@README.md`:
- Line 107: Update the Admin permission descriptions in both README locations to
state that scheduled delete access is restricted to the configured work window
only when scheduling is enabled; when the schedule is disabled, clarify that car
deletion is unrestricted while policy management remains locked.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f0c6879-e832-4de9-9b82-8e796a40ee36
📒 Files selected for processing (2)
README.mdexamples/interactive-decision-demo/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/interactive-decision-demo/README.md
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Purpose
Prepare the interactive decision demo to show that a permission can be granted only during a configured time window, while still being decided by the real Mizan engine.
Implementation scope
cars.deletedecision use the demo's weekday/work-hours schedule.cars.deletedeny override unconditional somatching-denialremains visible.examples/interactive-decision-demo/and use the existing memory/demo sources.Constraints
Verification
Please run the repository checks and verify the browser flow for Admin inside/outside schedule, Support denial, schedule toggle, and clock changes.
Refs #36
Summary by CodeRabbit
New Features
cars.delete, while only Super Admin can edit the schedule (controls remain visible to all).Documentation
Style / Chores
.gitignoreto exclude video and Playwright artifacts.