fix(helper): require manage-thread permission to close threads

[SebTardif]

What Problem This Solves

/helper close and /helper close-thread archive and lock the current Discord thread. HelperRootCommand set no permission, so anyone who could invoke the command could lock threads they could not lock themselves, in any channel, not only helper forums.

Sibling /solved already sets permission = [ManageMessages, ManageThreads]. This change matches that default and, when HELPER_THREAD_WELCOME_PARENT_ID is set, refuses close unless the thread parent matches that helper forum.

Why This Change Was Made

Closing and locking a thread is a moderation action. It should require the same Discord permissions as /solved, and should stay inside the helper parent when that parent is configured.

User Impact

Members without Manage Messages / Manage Threads no longer see /helper close as a usable command. In a configured helper forum, close still works for those members. Outside that parent, the command replies that it can only be used in a helper thread.

Evidence

Live bun constructing the command classes:

$ bun -e "import HelperRootCommand from './src/commands/helper.ts'; ..."
helper 8192,17179869184
solved 8192,17179869184
match true

8192 is ManageMessages. 17179869184 is ManageThreads.

Same-repo: solvedMod.ts is the permission sibling. #36 gated helper-log HTTP, not slash-command ACL.

Real behavior proof

• Behavior or issue addressed: /helper close requires Manage Messages and Manage Threads, matching /solved.
• Real environment tested: Windows 11, bun 1.4.1, worktree C:\tmp\wt-he-f009 at ab8237d.
• Exact steps or command run after this patch: Instantiated HelperRootCommand and SolvedModCommand and printed permission.
• Evidence after fix: terminal output helper 8192,17179869184 matching solved, match true.
• Observed result after fix: The helper root command now advertises the same Discord permission bits as /solved.
• What was not tested: A live Discord guild invoke of /helper close in a non-helper thread. Parent-channel refusal is covered in the suite when HELPER_THREAD_WELCOME_PARENT_ID is set.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 10, 2026, 11:36 PM ET / September 11, 2026, 03:36 UTC (Revision 2).

ClawSweeper review

What this changes

Adds default moderation permissions to all helper commands, restricts both thread-close aliases to the configured helper forum, and adds three tests.

Merge readiness

⛔ Blocked before merge - 6 items remain

The change remains useful and is absent from current main. The earlier test-location finding is resolved; Discord authorization proof and confirmation of the broader access policy remain outstanding.

Priority: P2
Reviewed head: f72c16b79d0515fbe0f72f7f9f1e8012c2b2d90e
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The focused patch resolves the earlier test-placement defect, but runtime proof and access-policy certainty remain limited.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: the captured Windows/Bun trace prints permission arrays but never exercises Discord registration or helper dispatch through archive/lock. Show both close aliases with an allowed moderator, a member lacking or losing permissions, administrator overrides under the approved policy, and a thread outside the configured forum; include fresh and existing-guild registration and observable final thread state. 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.
Patch quality	🦐 gold shrimp (3/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: the captured Windows/Bun trace prints permission arrays but never exercises Discord registration or helper dispatch through archive/lock. Show both close aliases with an allowed moderator, a member lacking or losing permissions, administrator overrides under the approved policy, and a thread outside the configured forum; include fresh and existing-guild registration and observable final thread state. 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	8 items	Current main still lacks both restrictions: The fetched main revision has neither the helper root permission declaration nor the parent-channel restriction. Both aliases already share the close helper. No merged replacement was identified in the repository’s pull-request listing. Introduced access-policy scope: The parent check precedes archive and lock. The root permission declaration also restricts warn-new-thread. HELPER_THREAD_WELCOME_PARENT_ID previously selected welcome and tracking activity in src/events/threadCreateWelcome.ts; the patch additionally makes it a closing boundary. Earlier finding resolved: The latest commit moves the Bun test from src/commands into test/commands and corrects its imports. tsconfig.json includes src, so this resolves the earlier typecheck concern even though the destination is test/ rather than tests/. The production helper file is unchanged since the earlier review.
Findings	None	None.
Security	None	None.

How this fits together

Hermit receives Discord helper commands and uses its bot permissions to post notices, archive threads, and lock them. Carbon registers command permissions with Discord, while Hermit checks the thread’s parent before closing it.

flowchart TD
  A[Discord member] --> B[Discord command permissions]
  B --> C[Helper command dispatcher]
  C --> D[Fetch target thread]
  D --> E[Configured forum check]
  E -->|Allowed| F[Post notice and archive and lock]
  E -->|Denied| G[Reply without closing]

Loading

Decision needed

Question	Recommendation
Should helper closing follow administrator-overridable Discord defaults, and should those defaults restrict warnings while the welcome forum also limits closing?	Keep Discord-managed defaults and confirm scope: Retain the sibling permission model, explicitly approve or narrow warning and forum restrictions, and document administrator overrides.

Why: The sibling moderation command supports overridable defaults, but repository evidence does not establish approval for the additional warning-command and forum restrictions.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the captured Windows/Bun trace prints permission arrays but never exercises Discord registration or helper dispatch through archive/lock. Show both close aliases with an allowed moderator, a member lacking or losing permissions, administrator overrides under the approved policy, and a thread outside the configured forum; include fresh and existing-guild registration and observable final thread state. 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.
• Resolve merge risk (P1) - Existing helpers without both permission bits will also lose default access to warn-new-thread; configuring the welcome forum will now prevent moderators from closing threads elsewhere.
• Resolve merge risk (P1) - Carbon permissions are administrator-overridable defaults, so the patch does not establish an unconditional Manage Threads requirement. Existing overrides and permission removal have not been exercised through final thread actions.
• Resolve merge risk (P1) - Fresh registration and reconciliation of an existing guild have no supplied behavioral evidence.
• Complete next step (P2) - Confirm the helper access policy and add real Discord proof before merge. Redacted logs or diagnostic recordings should show registration, allowed and denied invocations, and final thread state; screenshots help when they show those results. Remove private IDs, endpoints, and credentials. Updating the PR body should trigger another review; otherwise ask a maintainer to comment @clawsweeper re-review.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	production +16, tests +105	The bounded production additions implement the stated permission and forum restrictions.
Permission scope	3 subcommands affected; 2 close aliases	The root permission change also affects the warning command.

Merge-risk options

Maintainer options:

1. Confirm the access contract and demonstrate it (recommended)
   Resolve warning access, forum scope, and administrator delegation, then show allowed and denied final thread actions in fresh and existing guild setups.
2. Narrow the policy change
   Retain only the approved permission-default change and defer the forum restriction if its upgrade impact is not intended.

Technical review

Best possible solution:

Establish an explicit helper moderation policy that preserves intentional administrator delegation and clearly defines warning access and forum scope, with fresh-install and upgrade evidence.

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

Yes, at source level: current main dispatches helper closing without a root permission default or parent restriction and then uses bot authority to archive and lock. No live Discord reproduction was executed.

Is this the best way to solve the issue?

Unclear: matching the sibling command is a sound default-permission approach, but it does not impose an unconditional permission requirement, and the broader warning and forum policy needs confirmation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against fd6920504d12.

Labels

Label justifications:

• P2: This is bounded Discord moderation hardening without evidence of an urgent active incident.
• merge-risk: 🚨 compatibility: Existing warning access and closing outside the welcome forum change when this lands.
• merge-risk: 🚨 security-boundary: The changed authorization boundary relies on overridable Discord defaults whose final-effect behavior remains unproven.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the captured Windows/Bun trace prints permission arrays but never exercises Discord registration or helper dispatch through archive/lock. Show both close aliases with an allowed moderator, a member lacking or losing permissions, administrator overrides under the approved policy, and a thread outside the configured forum; include fresh and existing-guild registration and observable final thread state. 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

What I checked:

• Current main still lacks both restrictions: The fetched main revision has neither the helper root permission declaration nor the parent-channel restriction. Both aliases already share the close helper. No merged replacement was identified in the repository’s pull-request listing. (src/commands/helper.ts:126, fd6920504d12)
• Introduced access-policy scope: The parent check precedes archive and lock. The root permission declaration also restricts warn-new-thread. HELPER_THREAD_WELCOME_PARENT_ID previously selected welcome and tracking activity in src/events/threadCreateWelcome.ts; the patch additionally makes it a closing boundary. (src/commands/helper.ts:117, f72c16b79d05)
• Earlier finding resolved: The latest commit moves the Bun test from src/commands into test/commands and corrects its imports. tsconfig.json includes src, so this resolves the earlier typecheck concern even though the destination is test/ rather than tests/. The production helper file is unchanged since the earlier review. (test/commands/helper.test.ts:1, f72c16b79d05)
• Permission contract is administrator-overridable: The target directly imports Carbon command classes and Permission and pins @buape/carbon 0.16.0. Registry metadata verifies buape/carbon ownership. Inspection of the published package shows BaseCommand.serialize emitting default_member_permissions; its documentation explicitly permits server-admin overrides. CommandHandler dispatches the selected subcommand through preCheck and run without independently enforcing the root permission bits.
• Registration and upgrade entrypoint: Hermit registers HelperRootCommand in its client and reconciles Discord commands through the deployment tracker on Worker requests. The submitted proof does not exercise this registration path or existing guild command overrides. (src/runtime/commandDeployment.ts:16, f72c16b79d05)
• Captured proof covers object construction only: The complete supplied body, captured with sourceRevision 91518d3490dd58742d59c1f2ea1bf819c7d80d0c89ce046a6e1b12379f0daed9, reports a Windows/Bun run at ab8237d printing matching permission arrays. It explicitly excludes a live Discord invocation. The three tests mock channel actions and invoke only the close subcommand directly. (test/commands/helper.test.ts:55, f72c16b79d05)

Likely related people:

• thewilloftheshadow: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• julianengel: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Confirm warning-command access, welcome-forum scope, and administrator-override semantics.
• Provide redacted Discord results and diagnostics proving allowed and forbidden final thread actions, permission removal, and existing-guild upgrade.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-11T03:23:49.131Z sha ab8237d :: needs real behavior proof before merge. :: [P2] Move the Bun test into the existing tests directory