Delegatable auto-approval for hardware permanent rejects
Raygen
Rupe • @raygen
20h agoSign in to view attachments
Files and images attached to this public thread are only available to signed-in users.
Sign inI found two distinct capabilities in the existing flow:
- Ordinary hardware reviewers nominate a permanent rejection for an admin to decide.
- Admins’ own nominations are automatically approved, and admins can also approve or deny other reviewers’ nominations.
Should the new grantable permission auto-approve only the recipient’s own nominations, within their existing hardware-review scope, or also let them approve/deny other reviewers’ nominations?
That distinction changes server-side authorization materially. No files have been changed. I’ve relayed this question to the originating Puck thread; nothing was posted to Slack.
Implemented locally. Admins can grant Hardware Permanent Rejection Manager from a user’s Promote to… menu and revoke it through Demote from….
Recipients can:
- Auto-approve their own hardware permanent-rejection nominations within their existing review access.
- View nominations and approve or deny other reviewers’ nominations.
Server-side policy and model checks preserve claim requirements, project-membership restrictions, deleted-project safeguards, and audit trails. No unrelated admin powers or migrations.
roles.rb app/models/user
+def can_manage_hardware_permanent_rejections? = admin? || hardware_permanent_rejection_manager?Permanent-rejection authority is separate from general review and admin access.Verified: targeted Rails regression run passed (78 runs); expanded permission checks passed (15 runs, including 13 permission cases). RuboCop and ERB lint passed. Temporary generated tests were removed per your Stardance policy. UI inspection confirmed the manager dashboard, decision controls, direct-rejection dialog, and grant-menu option. UI lint still reports surrounding typography, contrast, and accessibility findings; it is not clean.



1 / 3
Nine files changed, uncommitted and unpushed. Reported the outcome to the originating Puck thread; nothing was posted to Slack.
rebase onto main, sorry. that was very outdated code.
Rebased onto latest origin/main, matching upstream main at 73e89e7a.
Resolved the conflict while preserving main’s new undo/history flow and reviewer attribution. The grantable role still covers own auto-approval and approving/denying others; undo remains admin-only.
permanent_rejection_nomination_policy.rb app/policies/admin/certification
−def undo? = approve? && record.approved?+def undo? = user&.admin? && approve? && record.approved?Expanding approval authority must not implicitly grant main’s newly added undo capability.Revalidation passed: 73 runs, 315 assertions, zero failures/errors, plus RuboCop, ERB lint, and diff checks. Temporary tests were removed per your policy.
Changes remain local, uncommitted and unpushed. Updated the originating Puck thread too.
Have a Task subagent review the current changes and report its findings.
The Task subagent found two actionable issues, which I confirmed in the code:
P2 — Navigation leads to forbidden pages. Non-admin managers can open nominations, but “Open project review” and “Hardware reviews” point to global review routes they may not have access to. Mission-only reviewers also receive the wrong route. The detailed audit-log link similarly requires separate permissions.
Suggested fix: policy-gate these links, use mission routes where appropriate, and provide an authorized fallback—not broader permissions.P3 — Staff notifications misrepresent the reviewer’s authority. The notification template says automatic approval happened because the reviewer “is an administrator.” That is false for the new role. Pending notifications also describe decisions as administrator-only.
Suggested fix: refer to authorized reviewers or permanent-rejection permission.
No additional server-side privilege escalation or undo-authority regression was identified. The review was read-only; it inspected the prior passing test results but did not rerun tests. No files were changed.
P3 - for these people, rather than admin say “because they’re Really Special And Awesome :3"
Updated P3. Non-admin auto-approvals now say:
Automatically approved because they’re Really Special And Awesome :3.
Admins keep the administrator wording. Pending nominations now await “an authorized reviewer’s decision.”
permanent_rejection_nomination.rb app/models/certification
+reviewer_admin: reviewer.admin?,Capture admin status when queuing the notification so the template selects the correct wording.Verified five rendering cases, including older queued notifications. RuboCop and diff checks passed. No Slack messages sent.
P2’s navigation fixes remain untouched. Changes are still uncommitted and unpushed.
Pushed to lordbagel42/stardance on feat/hardware-permanent-rejection-permission and opened draft PR #1606 against upstream main. The description follows the Stardance template and includes this thread’s URL.
The grantable role enables both requested permanent-rejection capabilities without admin powers. Server-side authorization is enforced, undo remains admin-only, and the requested “Really Special And Awesome :3” wording is included.
Targeted tests, notification rendering checks, and Ruby/ERB lint passed. Task review completed. Known limitation: some navigation/audit links still lead to separately restricted pages; this is disclosed in the PR.
CI and PR-feedback monitoring are active. Nothing was merged or deployed, and no Slack messages were posted.
No CI checks ran on the fork commit; this is not a test failure. The separate upstream CI subscription for draft PR #1606 remains active. No action needed.
PR #1606 is now ready for review. All nine currently listed checks passed, and there are no comments or reviews to address. No action needed.
Merging. let me know once its deployed.
I’m watching the merge and deployment. The PR is still open, and production currently serves the previous build. I’ll confirm once the live site reports a commit containing this change—not just when the deploy trigger succeeds.
PR #1606 is merged, and its production build and deploy workflow is running. Production still reports the previous build. I’m monitoring completion and will verify the live version before calling it deployed.
Not deployed yet. The image built and pushed successfully, but the deploy step timed out connecting to Coolify. The previous main build failed identically, so this predates our change.
Production still serves the old build. I’ll check the live version every 15 minutes and notify you once it includes this change. I haven’t retriggered deployment.
nvm.
Cancelled deployment monitoring. I won’t retry the deployment.