Repository navigation
feat(shared): Guild Automation Module Executor seam + AutoMessages pilot #901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Domain Docs | ||
|
|
||
| How the engineering skills should consume this repo's domain documentation when exploring the codebase. | ||
|
|
||
| ## Layout | ||
|
|
||
| Single-context repo. ADRs live in **`docs/decisions/`** (not the conventional `docs/adr/`) to match the existing `/adr-write` workflow. | ||
|
|
||
| ``` | ||
| / | ||
| ├── CONTEXT.md ← not yet created; proceed silently if absent | ||
| ├── docs/ | ||
| │ ├── decisions/ ← ADRs live here | ||
| │ │ └── YYYY-MM-DD-<slug>.md | ||
| │ ├── specs/ ← /adt-specs-spec-new output | ||
| │ ├── plans/ ← /plan output | ||
| │ └── references/ | ||
| ├── packages/ ← bot, backend, frontend, shared | ||
| └── ... | ||
| ``` | ||
|
|
||
| ## Before exploring, read these | ||
|
|
||
| - **`CONTEXT.md`** at the repo root (if it exists). | ||
| - **`docs/decisions/`** — read ADRs that touch the area you're about to work in. Filter by date and slug. | ||
|
|
||
| If `CONTEXT.md` doesn't exist, **proceed silently**. Don't flag its absence; don't suggest creating it upfront. The producer skill (`/grill-with-docs`) creates it lazily when terms or decisions actually get resolved. | ||
|
|
||
| ## Use the glossary's vocabulary | ||
|
|
||
| When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary explicitly avoids. | ||
|
|
||
| If the concept you need isn't in the glossary yet, that's a signal — either you're inventing language the project doesn't use (reconsider) or there's a real gap (note it for `/grill-with-docs`). | ||
|
|
||
| ## Flag ADR conflicts | ||
|
|
||
| If your output contradicts an existing ADR in `docs/decisions/`, surface it explicitly rather than silently overriding: | ||
|
|
||
| > _Contradicts `docs/decisions/2026-05-16-dependabot-batch-handling-policy.md` — but worth reopening because…_ | ||
|
|
||
| Recent ADRs (as of 2026-05-19) cover: dependabot batch policy, no-AI-generated docs, security-scan policy, Docker decisions trio, token optimization, deploy target, refactor target selection, autoplay integration tests, branch strategy, bot test suite cleanup. Check `docs/decisions/` for the current set before assuming a decision is unmade. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,27 @@ | ||||||
| # Issue tracker: GitHub | ||||||
|
|
||||||
| Issues and PRDs for this repo live as GitHub issues in `LucasSantana-Dev/Lucky`. Use the `gh` CLI for all operations. | ||||||
|
|
||||||
| ## Conventions | ||||||
|
|
||||||
| - **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies. | ||||||
| - **Read an issue**: `gh issue view <number> --comments`, filtering comments by `jq` and also fetching labels. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify gh CLI output modes for issue view and confirm JSON fields for jq usage.
gh issue view --help | sed -n '1,220p' | rg -n --context 2 -- '--json|--jq|--comments'Repository: LucasSantana-Dev/Lucky Length of output: 416 Add The Suggested fix-- **Read an issue**: `gh issue view <number> --comments`, filtering comments by `jq` and also fetching labels.
+- **Read an issue**: `gh issue view <number> --json number,title,body,labels,comments --jq '{number, title, body, labels: [.labels[].name], comments: [.comments[].body]}'`.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| - **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters. | ||||||
| - **Comment on an issue**: `gh issue comment <number> --body "..."` | ||||||
| - **Apply / remove labels**: `gh issue edit <number> --add-label "..."` / `--remove-label "..."` | ||||||
| - **Close**: `gh issue close <number> --comment "..."` | ||||||
|
|
||||||
| Repo is inferred from `git remote -v` — `gh` picks it up automatically when run inside the clone. | ||||||
|
|
||||||
| ## When a skill says "publish to the issue tracker" | ||||||
|
|
||||||
| Create a GitHub issue. | ||||||
|
|
||||||
| ## When a skill says "fetch the relevant ticket" | ||||||
|
|
||||||
| Run `gh issue view <number> --comments`. | ||||||
|
|
||||||
| ## Repo-specific notes | ||||||
|
|
||||||
| - The `/backlog` skill already creates issues with `cat:*` category labels (`cat:bug`, `cat:feature`, `cat:refactor`, `cat:tech-debt`, `cat:perf`, `cat:test`, `cat:docs`, `cat:security`) and a `backlog-skill` marker. Triage labels (see `triage-labels.md`) are orthogonal — apply both when appropriate. | ||||||
| - Area labels (`bot`, `backend`, `frontend`, `shared`, `infra`, `ci`, `database`, `music`, `moderation`, `dx`) and size labels (`size/xs|s|m|l|xl`) are available; apply them when creating issues so existing dashboards and filters keep working. | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Triage Labels | ||
|
|
||
| The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's GitHub issue tracker. | ||
|
|
||
| | Canonical role | Label in this repo | Meaning | | ||
| | ----------------- | ------------------ | ---------------------------------------- | | ||
| | `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue | | ||
| | `needs-info` | `needs-info` | Waiting on reporter for more information | | ||
| | `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent | | ||
| | `ready-for-human` | `ready-for-human` | Requires human implementation | | ||
| | `wontfix` | `wontfix` | Will not be actioned | | ||
|
|
||
| When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table. | ||
|
|
||
| ## Relationship to other labels in this repo | ||
|
|
||
| These triage labels are **orthogonal** to: | ||
|
|
||
| - **Category labels** (`cat:bug`, `cat:feature`, `cat:refactor`, `cat:tech-debt`, `cat:perf`, `cat:test`, `cat:docs`, `cat:security`) — what _kind_ of work | ||
| - **Area labels** (`bot`, `backend`, `frontend`, `shared`, `infra`, `ci`, `database`, `music`, `moderation`, `dx`) — _where_ in the codebase | ||
| - **Size labels** (`size/xs|s|m|l|xl`) — _how big_ | ||
| - **`backlog-skill`** — created by `/backlog` | ||
|
|
||
| Triage labels answer _what's the next action and who owns it_. Apply alongside the others. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| --- | ||
| status: accepted | ||
| date: 2026-05-19 | ||
| --- | ||
|
|
||
| # AutoMod actions do not create Moderation Cases | ||
|
|
||
| AutoMod runs without a moderator in the loop and currently does not write to `ModerationCase`. Manual Moderation Actions (warn / mute / kick / ban / lockdown / slowmode / purge) write a `ModerationCase` row with case number, reason, duration, and an appeal trail; AutoMod hits do not. | ||
|
|
||
| This is deliberate, not a missing feature. AutoMod handles high-volume, low-judgement enforcement (spam, caps, banned-words, link blocks) where every triggered action would inflate the Case table and dilute the appeal flow, which is designed around moderator-reviewable decisions. | ||
|
|
||
| ## Considered options | ||
|
|
||
| - **Unified Cases for AutoMod and manual Moderation.** Rejected: floods the case audit trail with low-signal automatic hits, makes appeals less meaningful, and forces appeal-flow UX on enforcement decisions where appeal isn't appropriate. | ||
| - **Separate "AutoMod Hit" table mirroring Cases.** Rejected for now: no current consumer needs that data shape. AutoMod logs are sufficient for tuning rules; Sentry covers operational visibility. | ||
| - **Status quo: AutoMod writes nothing case-shaped.** Accepted. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - A moderator looking at a Member's `cases` history will not see AutoMod-triggered actions. If that becomes a real gap, surface it via a separate AutoMod-hit log rather than fusing the tables. | ||
| - "Member case count" metrics reflect _moderator-issued_ actions only; this is the correct baseline for moderation-load dashboards but must be disclosed in any user-facing transparency report. | ||
| - Reversing this decision later means a schema migration (either add fields to `ModerationCase` to disambiguate source, or introduce a separate `AutoModHit` table) plus rework of the appeal UI to handle the AutoMod path. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| --- | ||
| status: accepted | ||
| date: 2026-05-19 | ||
| revisit_after: after-pilot-executor | ||
| --- | ||
|
|
||
| # Guild Automation reconciles via Module Executors with a Capture / Diff / Apply seam in shared | ||
|
|
||
| `packages/backend/src/services/GuildAutomationExecutionService.ts` is a 1238-LOC class that reconciles 7 Manifest modules (Roles, Channels, Onboarding, Moderation, AutoMessages, ReactionRoles, CommandAccess) against live Discord + DB state. A parallel bot-side path lives at `packages/bot/src/utils/guildAutomation/` (`captureGuildState.ts`, `diff.ts`, `applyPlan.ts`) — duplicating much of the same logic with a different Discord write path (`discord.js` Client instead of REST). | ||
|
|
||
| We will extract per-module **Module Executors** behind a `Capture / Diff / Apply` seam in `packages/shared/src/services/guildAutomation/`, with a lower `DiscordWriteAdapter` seam so bot and backend both consume the shared executors. Runs remain partial-success-tolerant and reconcile on the next Run via existing `GuildAutomationDrift`. | ||
|
|
||
| ## Context | ||
|
|
||
| - `GuildAutomationExecutionService` mixes Discord REST infra, ID remapping across modules, per-module apply, stale pruning, and Run-record bookkeeping. Deletion test: removing any single module's logic still requires touching this one file. | ||
| - Bot has a separate apply path (`packages/bot/src/utils/guildAutomation/applyPlan.ts`, 329 LOC) for the same Manifest. Plan-building logic is duplicated; only the Discord write mechanism differs. | ||
| - Drift is already a first-class concept: `GuildAutomationDrift` Prisma model exists, `lastCapturedState` lives on `GuildAutomationManifest`, and `bot/utils/guildAutomation/diff.ts` computes diffs. The capture/diff/apply triplet is implicitly present already — just scattered. | ||
| - Tests of any single module (e.g. AutoMessages) currently require mocking infra for all 6 other modules because they all live in one class. | ||
|
|
||
| ## Decision | ||
|
|
||
| Four locked design choices: | ||
|
|
||
| 1. **Cut line.** Orchestrator owns Discord REST infra (`discordFetch`, `getBotToken`), cross-module ID remapping (`remap*Section`), `GuildAutomationRun` bookkeeping, and parity-checklist defaults. Each **Module Executor** owns Capture, Diff, Apply, and stale-pruning for its module only. | ||
| 2. **Seam shape.** **Capture / Diff / Apply** triplet. Each Module Executor exposes: | ||
| - `capture(ctx) → LiveState<T>` — reads live Discord/DB state into a typed shape. | ||
| - `diff(live, manifest) → Diff<T>` — computes operations + stale set. | ||
| - `apply(diff, ctx) → ModuleResult<T>` — executes operations, prunes stale, returns result. | ||
| 3. **Location.** Executors live in `packages/shared/src/services/guildAutomation/`. They depend on a `DiscordWriteAdapter` interface (also in shared). Bot provides `DiscordJsAdapter` (in-process `discord.js` Client). Backend provides `DiscordRestAdapter` (`fetch` + bot token). This collapses the duplicated bot/backend apply paths. | ||
| 4. **Rollback.** Partial-success + reconcile-on-next-Run. No reverse-op log. If executor N fails, executors 1..N−1 stay live; the next Run's Capture+Diff reconciles. Matches the existing `GuildAutomationRun` schema (`operations`, `protectedOperations`, `error`). | ||
|
|
||
| **Enumeration:** seven Module Executors, one per Manifest module section: | ||
|
|
||
| | Executor | Manifest section | Write target | | ||
| | ---------------------- | ------------------------------------------------------ | ---------------------------------------------------------------- | | ||
| | Roles Executor | `roles` | Discord REST/Client (create/edit/delete + ID remap source) | | ||
| | Channels Executor | `channels` | Discord REST/Client | | ||
| | Onboarding Executor | `onboarding` | Discord REST/Client (mostly read, rare writes) | | ||
| | Moderation Executor | `moderation.automod` + `moderation.moderationSettings` | DB (`updateModerationSettings`, `autoModService.updateSettings`) | | ||
| | AutoMessages Executor | `automessages` | DB + Discord channel ID resolution | | ||
| | ReactionRoles Executor | `reactionroles` | DB (`reactionRolesService`) | | ||
| | CommandAccess Executor | `commandaccess` | DB (`guildRoleAccessService`) | | ||
|
|
||
| ## Considered options | ||
|
|
||
| ### Cut line | ||
|
|
||
| - **A. Orchestrator: infra + remap + run; Executor: apply + prune + drift (accepted).** Clean separation of cross-cutting concerns from per-module concerns. | ||
| - **B. Thin orchestrator + fat executors.** Rejected: forces each executor to carry its own Discord REST adapter, duplicating infra across 7 places. | ||
| - **C. Fat orchestrator + thin executors.** Rejected: executors become pure-data Module descriptors with logic still centralized — weakest seam, smallest payoff, and `One adapter = hypothetical seam` problem (one impl of each executor type means no real seam). | ||
|
|
||
| ### Seam shape | ||
|
|
||
| - **A. Apply-only.** Rejected: loses native drift integration and pre-apply preview. Closest to current code but smallest refactor benefit. | ||
| - **B. Plan + Apply (two-phase).** Considered. Simpler than the triplet but doesn't natively produce a Drift; `GuildAutomationDrift` would remain a separate code path. | ||
| - **C. Capture / Diff / Apply triplet (accepted).** Aligns with existing `GuildAutomationDrift` model, `lastCapturedState` field, and the bot-side `captureGuildState.ts`/`diff.ts`/`applyPlan.ts` split. Drift detection becomes a side-product of normal flow. | ||
| - **D. Hybrid (apply-only for onboarding, plan+apply for others).** Rejected: two interface kinds for one seam adds learning cost without clear win. | ||
|
|
||
| ### Location | ||
|
|
||
| - **A. `shared/src/services/guildAutomation/` + `DiscordWriteAdapter` (accepted).** Both packages consume the same executors. Solves the bot↔backend duplication that would otherwise need a separate follow-up ADR. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Standardize shared-module paths to repo-root-relative paths. This ADR mixes Also applies to: 80-80, 94-94, 97-97 🤖 Prompt for AI Agents |
||
| - **B. `backend/src/services/guildAutomation/` only.** Rejected: leaves the bot-side `applyPlan.ts` duplication in place. Future-us would re-litigate this immediately. | ||
| - **C. Promote `bot/src/utils/guildAutomation/`.** Rejected outright. Violates the architecture invariant in `docs/ARCHITECTURE.md`: backend depends only on shared, not on bot. | ||
|
|
||
| ### Rollback | ||
|
|
||
| - **A. Partial-success + reconcile (accepted).** Matches the existing `GuildAutomationRun.operations` / `protectedOperations` / `error` schema. Drift becomes visible via `GuildAutomationDrift` and the next Run converges. | ||
| - **B. All-or-nothing rollback.** Rejected: doubles per-executor surface (each must emit reverse-ops), and Discord API doesn't cleanly reverse some operations (permission edits, onboarding mutations). | ||
| - **C. Idempotent retry only.** Rejected: no rollback path means broken intermediate state is user-visible in the Guild between Runs. | ||
| - **D. Hybrid best-effort revert per failing executor.** Rejected for the initial design — adds complexity without justification; revisit if partial-success data shows users frequently stuck on a half-applied state. | ||
|
|
||
| ## Consequences | ||
|
|
||
| **Positive:** | ||
|
|
||
| - Each Module Executor is unit-testable by mocking only the `DiscordWriteAdapter`. No real Discord client needed in tests. | ||
| - Bot↔backend duplication collapses; `bot/src/utils/guildAutomation/applyPlan.ts` becomes a thin caller of the shared executors via `DiscordJsAdapter`. | ||
| - New module types (e.g. a future "Voice Channels" or "Slowmode Policy" module) add one Executor file, not edits in two packages. | ||
| - Drift detection is automatic: Capture + Diff produces a Drift result that the orchestrator can persist to `GuildAutomationDrift` without separate code. | ||
| - Locality: each module's logic lives in one file under `shared/src/services/guildAutomation/<module>Executor.ts`. | ||
|
|
||
| **Negative:** | ||
|
|
||
| - Migration cost is real: 1238 LOC backend class + 329 LOC bot apply + scattered diff/capture/remap utilities all rewire. Expect ≥4 PRs to land safely. | ||
| - `DiscordWriteAdapter` interface must be designed carefully — wrong methods leak Discord internals to executors anyway. | ||
| - Cross-cutting infra (auth tokens, retry policies) must stay outside executors and inside the adapters/orchestrator; risk of accidentally re-introducing the monolith inside one Executor. | ||
|
|
||
| **Neutral:** | ||
|
|
||
| - `GuildAutomationRun` schema unchanged. `GuildAutomationDrift` schema unchanged. `GuildAutomationManifest.lastCapturedState` semantics unchanged. | ||
|
|
||
| ## Implementation plan (pilot-first) | ||
|
|
||
| 1. **Pilot Executor.** Pick the simplest write path — likely **AutoMessages Executor** (DB-only, no Discord REST) — and implement it in `shared/src/services/guildAutomation/autoMessagesExecutor.ts`. Define `ModuleExecutor<T>` and `DiscordWriteAdapter` interfaces. Backend orchestrator calls this Executor; old code path stays for the other 6 modules. | ||
| 2. **Bot consumer.** Wire bot to call the AutoMessages Executor through `DiscordJsAdapter`. Delete the duplicated automessages logic in `bot/src/utils/guildAutomation/applyPlan.ts`. This is the first cross-package consumption — proves the location works. | ||
| 3. **Migrate remaining 6 Executors** one PR per Executor, easiest first: Moderation (DB-only) → ReactionRoles → CommandAccess → Onboarding (mostly read) → Channels → Roles (most complex; ID remap-heavy). | ||
| 4. **Decommission the monolith.** Once all 7 executors are migrated, delete `GuildAutomationExecutionService` and the duplicated bot apply path. Verify the guardrail test in `bot/utils/guildAutomation/` is updated to enforce "no Manifest reconciliation outside `shared/services/guildAutomation/`". | ||
|
|
||
| ## Revisit when | ||
|
|
||
| - **After the pilot Executor PR lands** — re-open this ADR with concrete evidence on whether the seam shape held up. Adjust before migrating the other 6. | ||
| - The pilot reveals that `DiscordWriteAdapter` needs to leak so much Discord state that executors end up adapter-coupled anyway. | ||
| - A new Manifest module type would naturally fit a different shape (e.g. requires multi-phase commit, transactions across DB + Discord). | ||
| - Production data on `GuildAutomationDrift` shows partial-success Runs cause user-visible degradation that argues for option D (best-effort revert). | ||
| - Lucky adopts a different Discord library (currently `discord.js` v14) — would force a `DiscordWriteAdapter` redesign and is a natural moment to reconsider. | ||
|
|
||
| ## Cross-references | ||
|
|
||
| - `CONTEXT.md` — see entries: **Guild Automation**, **Manifest**, **Drift**, **Run**, **Module Executor**, **Discord Write Adapter**, **Capture / Diff / Apply**. | ||
| - `docs/ARCHITECTURE.md` — confirms the `backend → shared` dependency direction and `bot → shared` direction that make this design legal. | ||
| - `docs/decisions/2026-05-19-automod-does-not-create-moderation-cases.md` — confirms the Moderation Executor covers `automod` + `moderationSettings` sub-sections without bridging to `ModerationCase`. | ||
| - This ADR was produced via `/improve-codebase-architecture` grilling on Lucky's `packages/` graph (2026-05-19 session) — the candidate ranked #2 (worst friction) of seven deepening opportunities surfaced. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a language to the fenced code block to satisfy markdownlint (MD040).
This fence is untyped, which will keep lint warnings active.
Suggested fix
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/agents/domain.mdaround lines 9 - 20, The markdown fenced code block indocs/agents/domain.md is missing a language tag (MD040); update the
triple-backtick fence that contains the directory tree to include a language
identifier (e.g., use "text" so it becomes ```text) so markdownlint stops
warning; locate the untyped fence around the directory tree snippet and add the
language tag to the opening fence.