Skip to content

Add Shell Society nomination flow#16

Open
VACInc wants to merge 6 commits into
openclaw:mainfrom
VACInc:feature/shell-society-nominations
Open

Add Shell Society nomination flow#16
VACInc wants to merge 6 commits into
openclaw:mainfrom
VACInc:feature/shell-society-nominations

Conversation

@VACInc

@VACInc VACInc commented Jun 23, 2026

Copy link
Copy Markdown

Summary

  • add /nominate with required user and reason options for Shell Society nominations
  • require exactly three configured approvals before adding the Shell Society role
  • post the public nomination through the bot-token channel message route after private validation, so approvers get a real channel message while submitter validation stays private
  • store nominations and distinct approvals in D1, with forward migrations that enforce one open submitted nomination per nominee/target role
  • expire submitted nominations after 48 hours, remove the approval button on expired nominations, and update the public nomination message to This nomination has expired. during the scheduled sweep
  • keep nomination validation and approval notices on Carbon v2 components, bound reasons to 500 characters, and keep submitter validation private

Root Cause

  • The nomination flow used raw content replies in new paths even though this repo requires Carbon v2 components.
  • The command inherited the repo base command's auto-defer behavior, so early validation responses could not reliably stay private once Carbon had already acknowledged the interaction.
  • The public nomination was briefly sent through the interaction webhook after an ephemeral defer; that could leave the nomination tied to the private interaction response path instead of creating an approver-visible channel message.
  • Duplicate open-nomination prevention only used an application-level pre-check; concurrent requests needed a D1 unique index so onConflictDoNothing() can reject the duplicate insert.
  • Nominations originally had no terminal state for non-approval, so an unapproved nomination could remain pending forever and block a replacement nomination.
  • Updating already-applied migrations would not affect any database that had already applied them, so the unique index and expiry changes needed forward migrations.

Real behavior proof

Closest feasible local proof was Bun tests plus a Wrangler bundle dry-run. The nomination migration test now applies the nomination migrations through 0007, verifies three configured approvals, rejects duplicate approver rows, rejects duplicate submitted nominations, allows later nominations after approved/expired statuses, backfills expiry for pre-expiry rows, and covers legacy/null expiry rows so they can be expired instead of getting stuck. The Worker also bundled successfully in dry-run mode without deploying.

Deployment notes

  • Hermit needs Manage Roles and its bot role must be above Shell Society.
  • D1 migrations need to run during deploy.
  • The scheduled Worker now has a 15-minute cron for nomination expiry and keeps the existing thread monitor gated to its prior two-hour cron.
  • No other app/codebase changes are required.

Verification

  • bun test tests/nominations.test.ts - 8 pass, 0 fail
  • bun test - 26 pass, 0 fail
  • bun run typecheck - passed
  • bun run db:generate - no schema changes, nothing to migrate
  • bun run deploy:dry-run - Worker bundle completed and exited with --dry-run
  • git diff --check - clean
  • codex review --uncommitted - no actionable correctness issues found after the expiry and channel-message fixes

What was not tested

  • Live Discord slash-command registration, button clicks, scheduled cron execution, public message visibility/edits, and actual role grant were not exercised locally.
  • Full end-to-end proof needs a deployed or tunneled Worker interaction endpoint, D1 migrations applied to that environment, and a test in the configured nomination channel with three distinct configured approvers.

@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 1:28 PM ET / 17:28 UTC.

Summary
The branch adds a guild-only /nominate Discord flow with Carbon components, D1 nomination/approval storage and migrations, scheduled expiry, and Shell Society role assignment after three approvals.

Reproducibility: not applicable. for a new feature PR. Current main has no Shell Society nomination flow, and the PR body explicitly says the live Discord paths were not exercised.

Review metrics: 3 noteworthy metrics.

  • Changed files: 18 files, +5811/-3. The diff spans command handling, components, D1 schema/migrations, cron config, and tests, so rollout review matters beyond compilation.
  • D1 migrations: 4 added. New persistent nomination and approval tables require upgrade-safe deployment proof before production rollout.
  • Cron cadence: 1 new 15-minute trigger. Scheduled expiry changes production Worker behavior alongside the existing two-hour thread monitor.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted live Discord proof for /nominate, three distinct approvals, expiry edit, and Shell Society role grant; redact private details before posting.
  • Get maintainer confirmation for the hardcoded role/channel/approval policy and D1/cron rollout plan.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Only Bun tests, dry-run bundling, and static review are provided; no redacted live Discord output proves the slash command, approval buttons, expiry edit, or role grant. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A visible proof run would materially reduce review risk for the slash command, approval buttons, expiry edit, and role grant. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify Discord /nominate posts a public nomination, three distinct approvals update it, expiry edits it, and Shell Society role is granted.

Risk before merge

  • [P1] Live Discord behavior remains unproven, including slash-command registration, public nomination visibility, approval button updates, expiry edits, and the final Shell Society role grant.
  • [P1] Four D1 migrations and a new 15-minute cron schedule require coordinated deployment and upgrade proof so nomination state and the existing two-hour thread monitor keep behaving correctly.
  • [P1] The role-granting path relies on hardcoded guild/channel/role IDs, approver-role membership, Manage Roles permission, and bot role order, which need maintainer ownership before merge.

Maintainer options:

  1. Add live rollout proof before merge (recommended)
    Have the contributor or a maintainer post redacted proof from a deployed or tunneled Worker showing /nominate, a visible public nomination, three distinct approvals, expiry edit, and final role grant after migrations are applied.
  2. Accept as a maintainer-owned rollout
    Maintainers may intentionally merge after independently verifying D1 migrations, cron cadence, Discord role order, and Manage Roles permissions in the production guild.
  3. Pause until role policy is sponsored
    If the Shell Society approval policy is not yet owned by the repo, keep the PR unmerged until a maintainer sponsors the workflow and exact IDs.

Next step before merge

  • [P1] Manual review and contributor action are needed because automation cannot provide the missing live Discord proof or approve the role-granting rollout policy.

Security
Needs attention: Needs attention because the diff adds a bot-token Discord role-granting path whose authorization policy and production permissions are not live-proven.

Review details

Best possible solution:

Keep the PR open until maintainers approve the Shell Society role policy and rollout plan, and redacted live Discord proof shows nomination, three approvals, expiry edit, and role grant end to end.

Do we have a high-confidence way to reproduce the issue?

Not applicable for a new feature PR. Current main has no Shell Society nomination flow, and the PR body explicitly says the live Discord paths were not exercised.

Is this the best way to solve the issue?

Unclear until live proof and maintainer approval are added. The implementation follows the repo's Carbon registration patterns and uses a channel-message route, but the role-granting policy and production rollout still need human ownership.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a8e175303311.

Label changes

Label justifications:

  • P2: This is a normal-priority feature PR with limited server scope, but it affects role grants, Discord message delivery, and D1 rollout.
  • merge-risk: 🚨 compatibility: The PR adds D1 migrations, a new cron schedule, and Discord permission prerequisites that production deployment must satisfy.
  • merge-risk: 🚨 message-delivery: The public nomination, approval button updates, and expiry edits are message-delivery behavior that still need live proof.
  • merge-risk: 🚨 security-boundary: The branch adds a new approval path that grants a Discord role using the bot token and needs maintainer-confirmed authorization policy plus live proof.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only Bun tests, dry-run bundling, and static review are provided; no redacted live Discord output proves the slash command, approval buttons, expiry edit, or role grant. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Confirm role-granting authorization boundary — src/components/nominationButtons.ts:46
    The approval flow can add the Shell Society role using the bot token after three configured approvals, while the PR explicitly says the actual role grant and production permissions were not exercised live.
    Confidence: 0.84

What I checked:

  • Repository policy read: AGENTS.md was read fully; its Carbon v2 component and explicit command/component registration guidance is directly relevant to this Discord command PR. (AGENTS.md:1, a8e175303311)
  • Current main lacks the feature: A focused search on current main returned no Shell Society nomination or /nominate implementation, so this PR is not obsolete on main. (a8e175303311)
  • PR scope and proof gap: The PR is open, mergeable, and changes 18 files with 5811 additions and 3 deletions; the body says live Discord slash-command registration, button clicks, scheduled expiry, public edits, and actual role grant were not exercised. (d64c5e5f450b)
  • Command and component registration: The PR registers NominateCommand and nominationComponents in the central Carbon client setup, matching the repository policy for new commands/listeners/components. (src/index.ts:59, d64c5e5f450b)
  • Public nomination route: The latest head posts the nomination through Routes.channelMessages(channelId) after private validation, which is the intended public channel-message route rather than an ephemeral interaction webhook response. (src/commands/nominate.ts:162, d64c5e5f450b)
  • Role-granting path: The approval component can add the target Discord role using the bot token after the approval threshold, making approver authorization, Manage Roles permission, and role order merge-critical. (src/components/nominationButtons.ts:46, d64c5e5f450b)

Likely related people:

  • Shadow: Git history and blame show Shadow introduced the central Carbon client wiring, FSC button component pattern, and thread monitor scheduler that this PR extends. (role: introduced Discord command/component and scheduler patterns; confidence: high; commits: 9b4c77a2a166, bbfa77bba20e, d4e8cdb499f6; files: src/index.ts, src/components/fscRequestButtons.ts, src/services/threadLengthMonitor.ts)
  • Ruby: Commit 0236007 added the maintainer-guest role acknowledgement flow, adjacent to this PR's Discord role-granting behavior. (role: adjacent role-flow contributor; confidence: medium; commits: 023600741b65; files: src/commands/role.ts)
  • fuller-stack-dev: Commit 2b97035 introduced the D1-backed claim deduplication pattern closest to this PR's nomination uniqueness model. (role: database uniqueness contributor; confidence: medium; commits: 2b9703501567; files: src/db/schema.ts, drizzle)
  • Patrick Erichsen: The current main commit touches adjacent D1 schema, Worker routing, and storage/API wiring near this PR's database and Worker surface. (role: recent adjacent schema and Worker contributor; confidence: medium; commits: a8e175303311; files: src/db/schema.ts, src/index.ts, wrangler.jsonc)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jun 23, 2026
@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

Updated PR head cf869c5:

  • corrected Shell Society nominations to two approvals
  • bounded nomination reasons, kept validation replies private on Carbon v2 components, and acknowledged slower /nominate paths before member/D1 work
  • added a forward D1 migration for the submitted-nomination unique index plus conflict handling

Validation rerun: bun test tests/nominations.test.ts, bun test, bun run typecheck, bun run db:generate, bun run deploy:dry-run, and codex review --uncommitted.

Remaining gap: live Discord slash-command/button/role-grant proof still needs a deployed or tunneled Worker endpoint with migrations applied and bot role permissions configured.

@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

Correction pushed at 3b3b2cc: Shell Society nominations now require three approvals, and the nomination migration test pins three configured approvals / three distinct approvers.

This supersedes my previous comment that said two approvals. Validation rerun: bun test tests/nominations.test.ts, bun test, bun run typecheck, bun run db:generate, bun run deploy:dry-run, and codex review --uncommitted.

@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

Pushed review-driven update at b924cef.

Changed:

  • added 48-hour nomination expiry with scheduled public message update to This nomination has expired.
  • kept approval requirement at exactly three approvals
  • fixed final-approval race so expired/failed role grants cannot leave nominations incorrectly complete
  • covered legacy/null expiry rows so they can expire instead of blocking replacements

Validation rerun:

  • bun test tests/nominations.test.ts - 8 pass
  • bun test - 26 pass
  • bun run typecheck - passed
  • bun run db:generate - no schema changes
  • bun run deploy:dry-run - passed
  • git diff --check - clean
  • codex review --uncommitted - no actionable correctness issues

@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@VACInc VACInc marked this pull request as draft June 23, 2026 20:08
@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

Pushed ClawSweeper P1 fix at d64c5e5.

Changed:

  • posts the nomination through the bot-token channel message route after private validation instead of the interaction webhook token, so approvers get a real public channel message/button
  • keeps /nominate validation and confirmation private
  • stores the real channel message id for expiry edits and cleanup

Validation rerun:

  • bun test tests/nominations.test.ts - 8 pass
  • bun test - 26 pass
  • bun run typecheck - passed
  • bun run db:generate - no schema changes
  • bun run deploy:dry-run - passed
  • git diff --check - clean
  • codex review --uncommitted - no actionable correctness issues found

Remaining gap: live Discord proof is still needed for slash command registration, button clicks, public message visibility/edits, scheduled expiry, and actual role grant.

@VACInc

VACInc commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 23, 2026
@VACInc VACInc marked this pull request as ready for review June 23, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant