Repository navigation
feat(shared): Guild Automation Module Executor seam + AutoMessages pilot - #901
Merged
LucasSantana-Dev merged 2 commits intoMay 20, 2026
Merged
Conversation
…ocock skills configs
- CONTEXT.md: project-wide domain glossary (tenancy, music runtime, autoplay,
guild configuration, moderation, engagement, role granting, integrations).
Flags two known ambiguities (source field overload, dead engine field).
Canonical access path for Player is resolveGuildQueue (per the queueResolver ADR).
- docs/decisions/2026-05-19-automod-does-not-create-moderation-cases.md:
deliberate split between AutoMod (auto enforcement) and ModerationCase
(manual moderator audit trail with appeal flow). Considered options and
revisit triggers recorded.
- docs/decisions/2026-05-19-queue-resolver-defensive-fallback-chain.md:
keep the six-path resolveGuildQueue fallback chain pending production
telemetry (queue_resolution_source Sentry tag). Pilot pairs the
observability with cache-scan reorder, guard expansion to autoplay +
musicRecommendation, nodes.resolve doc comment, and a discord-player
canary CI job. Revisit 2026-06-02.
- docs/decisions/2026-05-19-guild-automation-module-executors.md:
replace the 1238-LOC GuildAutomationExecutionService + duplicated
bot/utils/guildAutomation/applyPlan.ts with seven per-module Module
Executors behind a Capture/Diff/Apply seam in
shared/src/services/guildAutomation/, plus a DiscordWriteAdapter lower
seam (bot DiscordJsAdapter, backend DiscordRestAdapter). Partial-success
rollback via existing GuildAutomationDrift. Pilot Executor = AutoMessages.
- docs/agents/{issue-tracker,triage-labels,domain}.md: matt-pocock skill
configs (GitHub issue tracker, canonical triage labels mapped 1:1, single-
context domain layout with ADRs in docs/decisions/ rather than docs/adr/).
- .gitignore: graphify-out/ build artifacts (graph.json, GRAPH_REPORT.md,
cache/, manifest.json, cost.json).
First concrete implementation of the Capture/Diff/Apply seam recorded in
docs/decisions/2026-05-19-guild-automation-module-executors.md. AutoMessages
is the simplest module (DB-only, no Discord REST) and proves the executor
shape before migrating the other six.
- Exposes a port-style `AutoMessagesPort` decoupled from `AutoMessageService`
so executors depend on a structural interface, not the full Prisma row
type (`Pick<AutoMessageService>` leaks the entire `auto_messages` table
shape).
- `capture(ctx) → AutoMessagesLiveState` reads welcome + leave rows in
parallel via the port.
- `diff(live, section) → { ops }` is pure; emits `create`, `update`, or
`noop` per type. Lock-in for monolith parity: empty `section[type]` and
missing `message` both produce `noop` (matches `upsertAutoMessage`'s
`if (!payload?.message) return`).
- `apply(diff, ctx) → { applied }` walks ops, calls port methods,
returns the applied action list for the orchestrator to persist on
`GuildAutomationRun.operations`.
Tests:
- happy path: create welcome + leave when neither exists
- update path: existing welcome → updateMessage, manifest-absent leave → noop
- noop path: manifest payload with no message
- noop path: empty manifest section
Wiring deferred (next pilot PR per ADR plan): adapter from real
`AutoMessageService` to `AutoMessagesPort`, export from
`shared/src/services/guildAutomation/index.ts`, backend orchestrator swap,
bot consumer + deletion of duplicated automessages logic in
`packages/bot/src/utils/guildAutomation/applyPlan.ts`.
Refs: docs/decisions/2026-05-19-guild-automation-module-executors.md
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Failed to generate code suggestions for PR |
📝 WalkthroughWalkthroughThis PR establishes domain vocabulary via a new glossary (CONTEXT.md), documents the Module Executor architecture pattern through an ADR, implements AutoMessages as the pilot executor with full test coverage, records supporting design decisions for moderation and queue resolution, and provides tooling documentation for team collaboration via the gh CLI and triage conventions. ChangesGuild Automation Architecture and Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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: 7
🤖 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/agents/domain.md`:
- Around line 9-20: The markdown fenced code block in docs/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.
In `@docs/agents/issue-tracker.md`:
- Line 8: The documentation line using the gh command is missing the --json flag
so jq filtering won't work; update the example `gh issue view` invocation in
docs/agents/issue-tracker.md to include `--json` with the appropriate fields
(for example `--json comments,labels` or `--json body,comments,labels`) so the
command emits structured JSON that jq can filter, and replace the existing `gh
issue view <number> --comments` snippet with the corrected command string.
In `@docs/decisions/2026-05-19-guild-automation-module-executors.md`:
- Line 61: The ADR uses inconsistent module path styles
(`shared/src/services/guildAutomation/` vs `packages/shared/...`); update all
occurrences to repo-root-relative paths (e.g., replace
`shared/src/services/guildAutomation/` and any `shared/...` variants with
`packages/shared/src/services/guildAutomation/...`) so links and copy/paste
navigation are reliable; search for the strings shown in the diff and also
update the other noted occurrences around lines referenced (including the
mentions at 80-80, 94-94, 97-97) to the normalized `packages/shared/...` form
and ensure `DiscordWriteAdapter` and any other referenced symbols still point to
the correct path.
In `@packages/shared/src/services/guildAutomation/autoMessagesExecutor.ts`:
- Around line 115-141: The apply method in autoMessagesExecutor.ts performs
create/update operations via svc.createMessage and svc.updateMessage but doesn't
emit ServerLogService audit entries; update apply to call ServerLogService.log
(or the project's ServerLogService API) after successful create and update
operations, including relevant details (guildId from ctx, operation type,
message id or created resource id, channelId, and the diff payload), ensuring
you log both the create branch (after svc.createMessage resolves) and the update
branch (after svc.updateMessage resolves) and only record the audit entry on
success; reference the apply function, svc.createMessage, svc.updateMessage, and
use the project's ServerLogService methods to record the actions.
- Around line 46-63: The public executor/port types use plain strings for
IDs—update ExecutorContext and AutoMessagesPort to use branded ID types (e.g.,
GuildId and ChannelId) instead of string: change ExecutorContext.guildId to
GuildId; change method parameters guildId in
getWelcomeMessage/getLeaveMessage/createMessage to GuildId; change
createMessage/options.channelId and updateMessage.data.channelId to ChannelId;
ensure the file imports the branded types (GuildId, ChannelId) from the shared
ID definitions so the signatures for getWelcomeMessage, getLeaveMessage,
createMessage, and updateMessage compile with the stronger types.
- Around line 74-143: The executor methods capture, diff, and apply must return
the shared Result<T> wrapper instead of raw values; update signatures to
Promise<Result<AutoMessagesLiveState>>, Result<AutoMessagesDiff>, and
Promise<Result<AutoMessagesResult>> respectively, import the Result (and helpers
like ok/err) from the shared types, wrap successful returns with ok(...) (for
diff which is synchronous return ok({ ops })), and surround async work in
capture/apply with try/catch to return err(...) on exceptions while preserving
existing logic (refer to function names capture, diff, apply, types
AutoMessagesLiveState, AutoMessagesDiff, AutoMessagesResult, and helpers
svc.createMessage/svc.updateMessage).
- Around line 90-110: The current logic in autoMessagesExecutor.ts treats any
manifest entry without a message as a noop, which prevents updates that only
change enabled or channelId; update the decision so that: if !have and no
message then still noop (can't create without message), but if have exists then
compute diffs for each field (message, channelId, enabled) and push an 'update'
op when any provided field in want is different from have; when constructing the
'update' op include only the fields present in want (avoid setting message to
undefined) and keep the existing behavior for 'create' and 'noop' branches;
refer to variables/functions/constructs ops, want, have and the
'create'/'update'/'noop' op kinds to locate where to change the logic.
🪄 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
Run ID: ef366c58-725f-46fc-8578-4ecc5311a828
📒 Files selected for processing (10)
.gitignoreCONTEXT.mddocs/agents/domain.mddocs/agents/issue-tracker.mddocs/agents/triage-labels.mddocs/decisions/2026-05-19-automod-does-not-create-moderation-cases.mddocs/decisions/2026-05-19-guild-automation-module-executors.mddocs/decisions/2026-05-19-queue-resolver-defensive-fallback-chain.mdpackages/shared/src/services/guildAutomation/autoMessagesExecutor.spec.tspackages/shared/src/services/guildAutomation/autoMessagesExecutor.ts
Comment on lines
+9
to
+20
| ``` | ||
| / | ||
| ├── 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 | ||
| └── ... | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced code block to satisfy markdownlint (MD040).
This fence is untyped, which will keep lint warnings active.
Suggested fix
-```
+```text
/
├── 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
└── ...</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
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.md around lines 9 - 20, The markdown fenced code block in
docs/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.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ## 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.
🧩 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 --json flag to enable jq filtering in the issue read command.
The gh issue view <number> --comments command doesn't output JSON by default, so jq filtering and label extraction won't work as described. The --json flag must be included to produce structured output.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **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]}'`. |
🤖 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/agents/issue-tracker.md` at line 8, The documentation line using the gh
command is missing the --json flag so jq filtering won't work; update the
example `gh issue view` invocation in docs/agents/issue-tracker.md to include
`--json` with the appropriate fields (for example `--json comments,labels` or
`--json body,comments,labels`) so the command emits structured JSON that jq can
filter, and replace the existing `gh issue view <number> --comments` snippet
with the corrected command string.
|
|
||
| ### 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.
Standardize shared-module paths to repo-root-relative paths.
This ADR mixes packages/shared/... and shared/... path styles. Please normalize to repo-root-relative paths (e.g., packages/shared/src/services/guildAutomation/...) so links and copy/paste navigation stay reliable.
Also applies to: 80-80, 94-94, 97-97
🤖 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/decisions/2026-05-19-guild-automation-module-executors.md` at line 61,
The ADR uses inconsistent module path styles
(`shared/src/services/guildAutomation/` vs `packages/shared/...`); update all
occurrences to repo-root-relative paths (e.g., replace
`shared/src/services/guildAutomation/` and any `shared/...` variants with
`packages/shared/src/services/guildAutomation/...`) so links and copy/paste
navigation are reliable; search for the strings shown in the diff and also
update the other noted occurrences around lines referenced (including the
mentions at 80-80, 94-94, 97-97) to the normalized `packages/shared/...` form
and ensure `DiscordWriteAdapter` and any other referenced symbols still point to
the correct path.
Comment on lines
+46
to
+63
| export type ExecutorContext = { guildId: string } | ||
|
|
||
| export type AutoMessagesPort = { | ||
| getWelcomeMessage(guildId: string): Promise<AutoMessageSnapshot> | ||
| getLeaveMessage(guildId: string): Promise<AutoMessageSnapshot> | ||
| createMessage( | ||
| guildId: string, | ||
| type: AutoMessageType, | ||
| data: { message: string }, | ||
| options?: { channelId?: string }, | ||
| ): Promise<{ id: string }> | ||
| updateMessage( | ||
| id: string, | ||
| data: { | ||
| message?: string | ||
| channelId?: string | ||
| enabled?: boolean | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use branded ID types in executor contracts.
guildId and channelId are plain string in the public executor/port types, which weakens type-level ID safety across the seam.
As per coding guidelines, packages/{bot,backend,shared}/src/**/*.ts: "Use branded types (e.g., GuildId, UserId, ChannelId) for Discord IDs throughout the codebase to prevent type-level ID confusion".
🤖 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 `@packages/shared/src/services/guildAutomation/autoMessagesExecutor.ts` around
lines 46 - 63, The public executor/port types use plain strings for IDs—update
ExecutorContext and AutoMessagesPort to use branded ID types (e.g., GuildId and
ChannelId) instead of string: change ExecutorContext.guildId to GuildId; change
method parameters guildId in getWelcomeMessage/getLeaveMessage/createMessage to
GuildId; change createMessage/options.channelId and updateMessage.data.channelId
to ChannelId; ensure the file imports the branded types (GuildId, ChannelId)
from the shared ID definitions so the signatures for getWelcomeMessage,
getLeaveMessage, createMessage, and updateMessage compile with the stronger
types.
Comment on lines
+74
to
+143
| async capture(ctx: ExecutorContext): Promise<AutoMessagesLiveState> { | ||
| const [welcome, leave] = await Promise.all([ | ||
| svc.getWelcomeMessage(ctx.guildId), | ||
| svc.getLeaveMessage(ctx.guildId), | ||
| ]) | ||
| return { welcome, leave } | ||
| }, | ||
|
|
||
| diff( | ||
| live: AutoMessagesLiveState, | ||
| section: AutoMessagesManifestSection, | ||
| ): AutoMessagesDiff { | ||
| const ops: AutoMessagesDiffOp[] = [] | ||
| for (const type of MODULE_TYPES) { | ||
| const want = section[type] | ||
| const have = live[type] | ||
| if (!want?.message) { | ||
| ops.push({ kind: 'noop', type }) | ||
| continue | ||
| } | ||
| if (!have) { | ||
| ops.push({ | ||
| kind: 'create', | ||
| type, | ||
| message: want.message, | ||
| channelId: want.channelId, | ||
| }) | ||
| continue | ||
| } | ||
| ops.push({ | ||
| kind: 'update', | ||
| type, | ||
| id: have.id, | ||
| message: want.message, | ||
| channelId: want.channelId, | ||
| enabled: want.enabled, | ||
| }) | ||
| } | ||
| return { ops } | ||
| }, | ||
|
|
||
| async apply( | ||
| diff: AutoMessagesDiff, | ||
| ctx: ExecutorContext, | ||
| ): Promise<AutoMessagesResult> { | ||
| const applied: AutoMessagesResult['applied'] = [] | ||
| for (const op of diff.ops) { | ||
| if (op.kind === 'create') { | ||
| await svc.createMessage( | ||
| ctx.guildId, | ||
| op.type, | ||
| { message: op.message }, | ||
| { channelId: op.channelId }, | ||
| ) | ||
| applied.push({ type: op.type, action: 'create' }) | ||
| continue | ||
| } | ||
| if (op.kind === 'update') { | ||
| await svc.updateMessage(op.id, { | ||
| message: op.message, | ||
| channelId: op.channelId, | ||
| enabled: op.enabled, | ||
| }) | ||
| applied.push({ type: op.type, action: 'update' }) | ||
| continue | ||
| } | ||
| applied.push({ type: op.type, action: 'noop' }) | ||
| } | ||
| return { applied } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Return Result<T> from executor methods.
capture, diff, and apply currently return raw values/Promises instead of the shared explicit-error wrapper contract.
As per coding guidelines, packages/shared/src/**/*.ts: "Use Result<T> type wrapper for function returns to enable explicit error handling without throwing exceptions".
🤖 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 `@packages/shared/src/services/guildAutomation/autoMessagesExecutor.ts` around
lines 74 - 143, The executor methods capture, diff, and apply must return the
shared Result<T> wrapper instead of raw values; update signatures to
Promise<Result<AutoMessagesLiveState>>, Result<AutoMessagesDiff>, and
Promise<Result<AutoMessagesResult>> respectively, import the Result (and helpers
like ok/err) from the shared types, wrap successful returns with ok(...) (for
diff which is synchronous return ok({ ops })), and surround async work in
capture/apply with try/catch to return err(...) on exceptions while preserving
existing logic (refer to function names capture, diff, apply, types
AutoMessagesLiveState, AutoMessagesDiff, AutoMessagesResult, and helpers
svc.createMessage/svc.updateMessage).
Comment on lines
+90
to
+110
| if (!want?.message) { | ||
| ops.push({ kind: 'noop', type }) | ||
| continue | ||
| } | ||
| if (!have) { | ||
| ops.push({ | ||
| kind: 'create', | ||
| type, | ||
| message: want.message, | ||
| channelId: want.channelId, | ||
| }) | ||
| continue | ||
| } | ||
| ops.push({ | ||
| kind: 'update', | ||
| type, | ||
| id: have.id, | ||
| message: want.message, | ||
| channelId: want.channelId, | ||
| enabled: want.enabled, | ||
| }) |
There was a problem hiding this comment.
enabled/channelId-only manifest updates are silently dropped.
Line 90 forces noop when message is absent, so payloads like { welcome: { enabled: false } } can never update an existing row even though the manifest type allows it.
💡 Proposed fix
- if (!want?.message) {
+ if (!want) {
ops.push({ kind: 'noop', type })
continue
}
- if (!have) {
+ if (!have && !want.message) {
+ ops.push({ kind: 'noop', type })
+ continue
+ }
+ if (!have) {
ops.push({
kind: 'create',
type,
message: want.message,
channelId: want.channelId,
})
continue
}
+ if (
+ want.message === undefined &&
+ want.channelId === undefined &&
+ want.enabled === undefined
+ ) {
+ ops.push({ kind: 'noop', type })
+ continue
+ }
ops.push({
kind: 'update',
type,
id: have.id,
message: want.message,
channelId: want.channelId,
enabled: want.enabled,
})🤖 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 `@packages/shared/src/services/guildAutomation/autoMessagesExecutor.ts` around
lines 90 - 110, The current logic in autoMessagesExecutor.ts treats any manifest
entry without a message as a noop, which prevents updates that only change
enabled or channelId; update the decision so that: if !have and no message then
still noop (can't create without message), but if have exists then compute diffs
for each field (message, channelId, enabled) and push an 'update' op when any
provided field in want is different from have; when constructing the 'update' op
include only the fields present in want (avoid setting message to undefined) and
keep the existing behavior for 'create' and 'noop' branches; refer to
variables/functions/constructs ops, want, have and the 'create'/'update'/'noop'
op kinds to locate where to change the logic.
Comment on lines
+115
to
+141
| async apply( | ||
| diff: AutoMessagesDiff, | ||
| ctx: ExecutorContext, | ||
| ): Promise<AutoMessagesResult> { | ||
| const applied: AutoMessagesResult['applied'] = [] | ||
| for (const op of diff.ops) { | ||
| if (op.kind === 'create') { | ||
| await svc.createMessage( | ||
| ctx.guildId, | ||
| op.type, | ||
| { message: op.message }, | ||
| { channelId: op.channelId }, | ||
| ) | ||
| applied.push({ type: op.type, action: 'create' }) | ||
| continue | ||
| } | ||
| if (op.kind === 'update') { | ||
| await svc.updateMessage(op.id, { | ||
| message: op.message, | ||
| channelId: op.channelId, | ||
| enabled: op.enabled, | ||
| }) | ||
| applied.push({ type: op.type, action: 'update' }) | ||
| continue | ||
| } | ||
| applied.push({ type: op.type, action: 'noop' }) | ||
| } |
There was a problem hiding this comment.
Add audit logging for create/update operations in apply().
This method performs administrative settings writes but does not emit ServerLogService entries, which leaves an audit trail gap.
As per coding guidelines, packages/shared/src/services/**/*.ts: "Use ServerLogService to record all administrative actions (moderation, settings changes, role updates) for audit trail compliance".
🤖 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 `@packages/shared/src/services/guildAutomation/autoMessagesExecutor.ts` around
lines 115 - 141, The apply method in autoMessagesExecutor.ts performs
create/update operations via svc.createMessage and svc.updateMessage but doesn't
emit ServerLogService audit entries; update apply to call ServerLogService.log
(or the project's ServerLogService API) after successful create and update
operations, including relevant details (guildId from ctx, operation type,
message id or created resource id, channelId, and the diff payload), ensuring
you log both the create branch (after svc.createMessage resolves) and the update
branch (after svc.updateMessage resolves) and only record the audit entry on
success; reference the apply function, svc.createMessage, svc.updateMessage, and
use the project's ServerLogService methods to record the actions.
|
1 of 2 tasks
LucasSantana-Dev
added a commit
that referenced
this pull request
May 20, 2026
…pec (#902) ## Summary The `autoMessagesExecutor.spec.ts` landed in #901 relied on ambient jest types. It passed in isolated `npx jest` runs (looser typing path) but fails under `npm run test:ci --workspace=packages/shared` (the workspace invocation, which is `jest --ci --silent` and enforces the shared package's strict tsconfig — which does not include `@types/jest` in `types`). ## What changed - Adds `import { describe, expect, it, jest } from '@jest/globals'` — matches the shared convention used by `PremiumService.spec.ts`, `LastFmLinkService/index.spec.ts`, and `SpotifyLinkService/index.spec.ts`. - Updates mock typing from the legacy `jest.fn<R, [Args]>()` two-arg form to `@jest/globals`'s required single-type-arg `jest.fn<Fn>()` form. ## Verification - `npx jest --config packages/shared/jest.config.cjs --ci --silent packages/shared/src/services/guildAutomation/autoMessagesExecutor.spec.ts` — 4/4 passing - `npm run test:ci --workspace=packages/shared` — failing test-suite count drops from 3 to 2 (this spec is no longer one of them). The remaining 2 failures (`FeatureToggleService.spec.ts`, `__tests__/utils/spotify/artistApi.test.ts`) are pre-existing tech debt last touched in #762/#614/#648 respectively, out of scope for this fix. ## Why this slipped through #901 PR #901 was merged on `UNSTABLE` while `Quality Gates` (the workspace test:ci runner) was still in flight. The check that would have caught this never reported before merge. Going forward, prefer waiting for `Quality Gates` on PRs touching `packages/shared`. ## Test plan - [x] `npm run test:ci --workspace=packages/shared` locally — autoMessagesExecutor spec green - [ ] Quality Gates check on this PR (the one we should have waited for) Refs: #901 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Tests** * Improved test infrastructure with enhanced type safety for mock functions. --- **Note:** This release contains internal test improvements with no changes to user-facing functionality. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/902?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/902?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This was referenced May 20, 2026
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
Baseline coverage (per `npm run test:ci --workspace=packages/shared -- --coverage`): - Statements: 21.43% - Branches: 18.42% - Functions: 17.24% - Lines: 20.51% Set thresholds to baseline minus 2% (no-regression floor): - Statements: 19% - Branches: 16% - Functions: 15% - Lines: 18% This establishes parity with packages/backend (70%) and packages/bot (65%) coverage gates. Plugs the gap documented in #909 where missing shared coverage threshold allowed PR #901 to merge with TS-only errors caught post-merge by Quality Gates. Note: CI's quality.yml workflow (delegated from org reusable) should include shared-workspace tests. Verify in follow-up to ensure the new threshold is enforced on PR submissions.
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
## Summary Establishes a coverage-regression floor for `packages/shared` — currently the only workspace without `coverageThreshold`. Baseline-minus-2% strategy prevents silent regressions while accommodating pre-existing low coverage. ## Baseline & Thresholds Coverage captured from `npm run test:ci --workspace=packages/shared -- --coverage`: | Metric | Baseline | Threshold | Floor | |--------|----------|-----------|-------| | Statements | 21.43% | 19% | no-regression | | Branches | 18.42% | 16% | no-regression | | Functions | 17.24% | 15% | no-regression | | Lines | 20.51% | 18% | no-regression | **Rationale:** Each threshold = `floor(baseline) - 2%`. Not aspirational 85%; matches project policy (backend 70%, bot 65%). Documented in memory note `feedback_wait_for_quality_gates_2026-05-19`: PR #901 merged despite shared-only TS errors; Quality Gates caught them post-merge → #902 follow-up fix. ## Acceptance Criteria - [x] `packages/shared/jest.config.cjs` has `coverageThreshold.global` with all four metrics - [x] Each threshold = current actual coverage minus 2% (no-regression floor) - [x] Coverage gate passes at new thresholds (test suite has pre-existing failures on `release/v2.12.0` unrelated to coverage) - [ ] **Follow-up:** Verify CI's Quality Gates workflow includes shared-workspace test invocation ## Notes - Single-file change (jest.config.cjs only) - Pre-existing TS errors in `FeatureToggleService.spec.ts` and `artistApi.test.ts` on base branch prevent full test suite pass, but coverage calculations are unaffected - CI integration: shared workspace is built & verified in `ci.yml` but not yet tested. The delegated org-wide `quality.yml` workflow should include test:ci for shared; recommend audit in separate issue to ensure threshold enforcement gates PRs Closes #909
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
## Summary Cut v2.13.0 of Lucky. Bumps root + 4 workspaces from `2.11.0` → `2.13.0` (skipping the archived `2.12.0`) and promotes the CHANGELOG `[Unreleased]` block to `[2.13.0] - 2026-05-21`. ## Headline changes since v2.11.0 **Added** - Guild Automation Module Executor seam + AutoMessages pilot (#901) - Sentry React SDK + Router v7 tracing/replay on frontend (#876) - Prometheus `/metrics` on backend (#875) + bot (#873) - Guild join/leave history tracking (#872) - Trivy image-scan on docker-publish, Phase A audit-only (#883) - Self-hosted developer-tooling register on landing page (#868) **Changed** - Backend migrated to Zod 4 API (#919) — unblocked the CVE patch + ended the lockfile fragility loop - 3 bot circular-deps clusters broken (#885, #886, #888) **Fixed** - brace-expansion DoS + ws uninit-memory CVEs patched (#921) - nginx-alpine CVEs (#881) - CI postinstall rate limit + madge actionlint (#878, #905) Full list in CHANGELOG.md. ## Next steps (after this PR merges) 1. Open `release/v2.13.0 → main` PR with merge-commit method 2. Tag `v2.13.0` on the merge commit 3. Cut next `release` (homelab-style bare branch) — Lucky's bare-release migration is still pending the user removing protection on `release/v2.11.0`
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
Four ADRs from the 2026-05-20 Guild Automation refactor that were created locally but never staged (referenced in PR #901 and #919 commit messages, but the files themselves slipped past the index): - executor-composition (#901's composition-roots pattern) - executor-partial-failure (best-effort + per-op try/catch) - drift-persistence (orchestrator updates GuildAutomationDrift) - manifest-absence-semantics (allowProtected handling) Three ADRs from today's 2026-05-21 work: - backend-zod-3-to-4-migration (rationale for #919, includes the locked 8-decision design from the /research-and-decide session) - discord-write-adapter-port (design ADR for the next Module Executor seam; strengthens executor-composition) - replace-plan-limited-review-tools (rationale for the org CI tools standard adopted in LucasSantana-Dev/.github) No code changes — pure documentation. Indexes into RAG (claude-mem) so future sessions can recall the decisions.
LucasSantana-Dev
added a commit
that referenced
this pull request
May 21, 2026
## Release v2.13.0 Promotes \`release/v2.13.0\` to \`main\` for the v2.13.0 cut. **$AHEAD commits across all merged PRs since v2.11.0 ship.** (Skipping v2.12.0 — the branch existed but its work was rolled forward into v2.13.0 alongside this session's Zod migration + CVE patches + standards adoption.) ## Headline changes **Added** — Guild Automation Module Executor pilot (#901), Sentry frontend (#876), Prometheus metrics on bot+backend (#873, #875), guild membership history (#872), Trivy image-scan Phase A (#883), landing redesign (#868). **Changed** — Backend migrated to Zod 4 API (#919), 3 bot circular-deps clusters broken (#885/#886/#888). **Fixed** — brace-expansion + ws moderate CVEs (#921), nginx-alpine CVEs (#881), CI postinstall rate limit (#878), madge actionlint (#905). **Internal** — shared coverageThreshold gate (#909/#914), Feature-removal sweep checklist + dangerfile guard (#908/#913), monitoring network, AI-doc policy, 4 new ADRs. Full list in [CHANGELOG.md](./CHANGELOG.md). ## Merge method This PR should land via **merge commit** (NOT squash) to preserve the individual PR SHAs in main's history. After merge: 1. Tag \`v2.13.0\` on the merge commit 2. Create GitHub release with notes from CHANGELOG.md 3. Fast-forward \`release/v2.13.0\` to match the new main HEAD ## Test plan - [ ] All 30 checks green except infra (snyk plan cap) - [ ] Verify \`gh pr view 922 --json mergeCommit\` shows the chore-bump commit on release tip - [ ] After merge: confirm \`origin/main\` contains the full $AHEAD commits
This was referenced May 23, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-commit branch: foundational architecture docs + first concrete implementation of the new Module Executor seam for Guild Automation.
docs(architecture)commitsourcefield overload acrossTrackMetadatavsRecommendationBasis, and the deadenginefield onTrackMetadata.docs/decisions/:2026-05-19-automod-does-not-create-moderation-cases.md— locks the deliberate split between AutoMod auto-enforcement and the appeal-flow-bearingModerationCaseaudit trail.2026-05-19-queue-resolver-defensive-fallback-chain.md— keeps the six-pathresolveGuildQueuefallback chain pending 14 days ofqueue_resolution_sourceSentry telemetry; ships observability + guardrail expansion + cache-scan reorder + discord-player canary CI as part of the same pilot. Revisit 2026-06-02.2026-05-19-guild-automation-module-executors.md— replaces the 1238-LOCGuildAutomationExecutionService+ the duplicatedbot/utils/guildAutomation/applyPlan.tswith seven per-module Module Executors behind a Capture / Diff / Apply seam inpackages/shared/src/services/guildAutomation/, plus a lowerDiscordWriteAdapterseam (botDiscordJsAdapter, backendDiscordRestAdapter). Partial-success rollback via existingGuildAutomationDrift.docs/agents/(3 files) —issue-tracker.md,triage-labels.md,domain.mdcapture the matt-pocock skill configs so downstream skills (to-issues,triage,improve-codebase-architecture, etc.) read the right vocabulary..gitignore— addsgraphify-out/(knowledge-graph build artifacts; rebuild with/graphify --update).feat(shared)commitFirst concrete Module Executor implementing the seam shape from the ADR: AutoMessages Module Executor (DB-only, no Discord REST, simplest module to validate the shape).
packages/shared/src/services/guildAutomation/autoMessagesExecutor.tsAutoMessagesPortdecoupled fromAutoMessageService(avoids the full Prisma row leaking throughPick<Service>).capture(ctx)reads welcome + leave rows in parallel.diff(live, section)is pure; emitscreate | update | noopper type. Monolith-parity behavior: emptysection[type]or missingmessage→noop(matches the existingif (!payload?.message) returninupsertAutoMessage).apply(diff, ctx)walks ops via the port, returns applied actions forGuildAutomationRun.operations.packages/shared/src/services/guildAutomation/autoMessagesExecutor.spec.ts— 4 tests covering create both, update existing + manifest-absent leave, noop-on-empty-message, noop-on-empty-section.Why
Came out of
/improve-codebase-architecturewalkingpackages/:GuildAutomationExecutionService(1238 LOC, 7 module types entangled) was the worst single-file friction.packages/bot/src/utils/guildAutomation/applyPlan.tsduplicates plan-building logic.Then
/grill-with-docsresolved the domain vocabulary,/research-and-decide(withcriticagent challenge) locked the four design choices, and/adt-tddproved the seam shape on AutoMessages.Not in this PR (per ADR pilot plan)
AutoMessageService→AutoMessagesPortshared/src/services/guildAutomation/index.tsGuildAutomationExecutionService.upsertAutoMessage→ call this executor)applyPlan.tsautomessages logicTest plan
packages/shared/src/services/guildAutomation/autoMessagesExecutor.spec.ts— 4/4 passing locallyRefs
docs/decisions/2026-05-19-guild-automation-module-executors.mddocs/decisions/2026-05-19-queue-resolver-defensive-fallback-chain.mddocs/decisions/2026-05-19-automod-does-not-create-moderation-cases.mdCONTEXT.mdSummary by CodeRabbit
Documentation
Tests
Chores