Skip to content

Graceful Shutdown Handling#3

Merged
BillChirico merged 8 commits intomainfrom
auto-claude/002-graceful-shutdown-handling
Feb 4, 2026
Merged

Graceful Shutdown Handling#3
BillChirico merged 8 commits intomainfrom
auto-claude/002-graceful-shutdown-handling

Conversation

@BillChirico
Copy link
Collaborator

Implement proper shutdown handling that saves conversation state, completes pending requests, and cleanly disconnects from Discord before exiting.

BillChirico and others added 5 commits February 3, 2026 20:29
…persistence

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added pendingRequests Set to track active AI requests during shutdown.
The generateResponse function now registers requests on start and
removes them on completion (success or error) using try-finally.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add gracefulShutdown function that saves state, waits for pending requests, and destroys Discord client
- Handle SIGTERM and SIGINT signals for clean shutdown
- Add 5-second timeout for pending requests during shutdown
- Add loadState() call on startup to restore previous state
- Include detailed logging for shutdown process
- Fix state.json path to use data/ directory (critical security fix)
- Update shutdown timeout from 5s to 10s per spec

Addresses QA review feedback from session 1

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 24 minutes and 7 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 596cb8a and 52a7050.

📒 Files selected for processing (4)
  • .gitignore
  • data/.gitkeep
  • src/index.js
  • src/modules/ai.js
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch auto-claude/002-graceful-shutdown-handling

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cursor
Copy link

cursor bot commented Feb 4, 2026

Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.

  • ✅ Fixed: State save fails if data directory missing
    • Added mkdirSync with recursive option to ensure the data directory exists before writing state.json.
  • ✅ Fixed: Local development files accidentally committed to repository
    • Removed the three local dev files from git tracking and added them to .gitignore.
  • ✅ Fixed: State saved before pending requests complete loses data
    • Reordered graceful shutdown to wait for pending requests to complete before saving state.

Create PR

Or push these changes by commenting:

@cursor push aee2067c22

…utdown (#5)

- Add mkdirSync to ensure data directory exists before saving state
- Remove accidentally committed local dev files (.auto-claude-security.json,
  .auto-claude-status, .claude_settings.json) and add to .gitignore
- Reorder graceful shutdown to wait for pending requests before saving state
  to ensure in-flight conversation history is captured

Co-authored-by: Cursor Agent <agent@cursor.com>
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 4, 2026
Resolve conflicts by integrating graceful shutdown features with
the modular architecture from main:
- Keep modular imports (config, events, health modules)
- Add graceful shutdown handler with SIGTERM/SIGINT support
- Add state persistence for conversation history
- Keep command loading and permission system
- Export conversation history functions from ai.js for state persistence
- Include all auto-claude ignore patterns from both branches
- Keep readdirSync for dynamic command loading
- Use module-level dataDir constant (already defined at top)
- Get conversationHistory from ai.js module for state persistence
@BillChirico BillChirico merged commit e2489f9 into main Feb 4, 2026
2 checks passed
@BillChirico BillChirico deleted the auto-claude/002-graceful-shutdown-handling branch February 4, 2026 02:13
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before Autofix could start.

*/
export function removePendingRequest(requestId) {
pendingRequests.delete(requestId);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending request tracking functions never called

Medium Severity

The registerPendingRequest and removePendingRequest functions are exported but never imported or called anywhere in the codebase. The generateResponse function in events.js makes AI requests without registering them. As a result, pendingRequests will always be empty, and the graceful shutdown logic that waits for pending requests to complete will never actually wait for anything—requests in flight during shutdown will be dropped.

Additional Locations (1)

Fix in Cursor Fix in Web

BillChirico added a commit that referenced this pull request Feb 11, 2026
Remove dead code: `const finalKey = parts[parts.length - 1]` was assigned
but never referenced in the function body.

Resolves Bugbot review thread #3.
BillChirico added a commit that referenced this pull request Feb 15, 2026
…cord values

- getThreadConfig now validates reuseWindowMinutes (must be positive number)
  and autoArchiveMinutes (must be valid number), falling back to safe defaults
- Add snapAutoArchiveDuration() that snaps to nearest Discord-allowed value
  (60, 1440, 4320, or 10080 minutes)
- Invalid config values (NaN, negative, non-number) gracefully degrade

Addresses PR #57 review threads #3 and #6.
BillChirico added a commit that referenced this pull request Feb 15, 2026
…cord values

- getThreadConfig now validates reuseWindowMinutes (must be positive number)
  and autoArchiveMinutes (must be valid number), falling back to safe defaults
- Add snapAutoArchiveDuration() that snaps to nearest Discord-allowed value
  (60, 1440, 4320, or 10080 minutes)
- Invalid config values (NaN, negative, non-number) gracefully degrade

Addresses PR #57 review threads #3 and #6.
BillChirico added a commit that referenced this pull request Feb 16, 2026
…s in memory command

- Use splitMessage() utility instead of manual substring truncation for
  Discord's 2000-char limit, properly handling multi-byte characters (#1)
- Include memory IDs in searchMemories results and use them directly in
  handleForgetTopic instead of fragile text equality matching (#2)
- Parallelize deleteMemory calls with Promise.allSettled instead of
  sequential for loop (#3)
- Verify deferReply is called in all forget test variants (#7)
BillChirico added a commit that referenced this pull request Feb 16, 2026
…s in memory command

- Use splitMessage() utility instead of manual substring truncation for
  Discord's 2000-char limit, properly handling multi-byte characters (#1)
- Include memory IDs in searchMemories results and use them directly in
  handleForgetTopic instead of fragile text equality matching (#2)
- Parallelize deleteMemory calls with Promise.allSettled instead of
  sequential for loop (#3)
- Verify deferReply is called in all forget test variants (#7)
BillChirico added a commit that referenced this pull request Feb 16, 2026
…s in memory command

- Use splitMessage() utility instead of manual substring truncation for
  Discord's 2000-char limit, properly handling multi-byte characters (#1)
- Include memory IDs in searchMemories results and use them directly in
  handleForgetTopic instead of fragile text equality matching (#2)
- Parallelize deleteMemory calls with Promise.allSettled instead of
  sequential for loop (#3)
- Verify deferReply is called in all forget test variants (#7)
BillChirico added a commit that referenced this pull request Feb 16, 2026
…uard

- Add cursor-based pagination with `after` param to fetchUserGuilds,
  looping until all guilds are fetched (issue #2)
- Propagate RefreshTokenError to session and auto-redirect to sign-in
  via SessionGuard component in Providers (issue #3)
- Add dockerContext = ".." to railway.toml for monorepo Docker builds (issue #4)
- Create /api/health returning 200 JSON; update railway.toml healthcheckPath (issue #5)
- Conditionally render Add to Server buttons only when CLIENT_ID is set (issue #6)
- Add AbortController cleanup to guild fetch useEffect in ServerSelector (issue #7)
- Refuse unauthenticated bot API requests when BOT_API_SECRET is missing (issue #9)
- Add retry + empty/error states to ServerSelector (issue #13 partial)
BillChirico added a commit that referenced this pull request Feb 17, 2026
- Differentiate requireGuildAdmin (ADMINISTRATOR only) from
  requireGuildModerator (ADMINISTRATOR | MANAGE_GUILD), aligning REST
  admin check with slash-command isAdmin (#1, #2, #12)
- Add botOwners startup warning when using default upstream ID (#3)
- Add SESSION_SECRET, DISCORD_CLIENT_SECRET, DISCORD_REDIRECT_URI to
  README deployment table (#4)
- Pass actual permission level to getPermissionError so modlog denial
  says 'moderator' not 'administrator' (#5)
- Guard _seedOAuthState with NODE_ENV production check (#6)
- Add test: valid JWT with no server-side session (#7)
- Add DiscordApiError class with HTTP status (#8)
- Add moderatorRoleId support to isModerator (#9)
- Remove no-op delete override from SessionStore (#10)
- Cap oauthStates at 10k entries (#11)
- Fix hasOAuthGuildPermission docstring for bitwise OR semantics (#12)
- Handle dashboard URL fragment collision (#13)
- Cap guildCache at 10k entries (#14)
- Add SESSION_TTL_MS co-location comment with JWT expiry (#15)
- Cache SESSION_SECRET via lazy getter in verifyJwt (#16)
- Remove PII (username) from OAuth auth log (#17)
BillChirico added a commit that referenced this pull request Feb 17, 2026
- Differentiate requireGuildAdmin (ADMINISTRATOR only) from
  requireGuildModerator (ADMINISTRATOR | MANAGE_GUILD), aligning REST
  admin check with slash-command isAdmin (#1, #2, #12)
- Add botOwners startup warning when using default upstream ID (#3)
- Add SESSION_SECRET, DISCORD_CLIENT_SECRET, DISCORD_REDIRECT_URI to
  README deployment table (#4)
- Pass actual permission level to getPermissionError so modlog denial
  says 'moderator' not 'administrator' (#5)
- Guard _seedOAuthState with NODE_ENV production check (#6)
- Add test: valid JWT with no server-side session (#7)
- Add DiscordApiError class with HTTP status (#8)
- Add moderatorRoleId support to isModerator (#9)
- Remove no-op delete override from SessionStore (#10)
- Cap oauthStates at 10k entries (#11)
- Fix hasOAuthGuildPermission docstring for bitwise OR semantics (#12)
- Handle dashboard URL fragment collision (#13)
- Cap guildCache at 10k entries (#14)
- Add SESSION_TTL_MS co-location comment with JWT expiry (#15)
- Cache SESSION_SECRET via lazy getter in verifyJwt (#16)
- Remove PII (username) from OAuth auth log (#17)
BillChirico added a commit that referenced this pull request Feb 28, 2026
…tus, add 30-day default window, verify conversationId on flag POST

- Import escapeIlike() instead of inline regex (DRY #4)
- Default to last 30 days when no `from` filter to prevent unbounded LIMIT 5000 scan (#3)
- Fix Map construction: iterate ORDER BY DESC rows and only set first occurrence per key so most-recent flag status wins (#1)
- Verify flagged messageId belongs to the conversation's channel before inserting (#2)
BillChirico added a commit that referenced this pull request Feb 28, 2026
* feat(conversations): add flagged_messages migration

* feat(conversations): add API endpoints for list, detail, search, flag, stats

* feat(conversations): mount router in API index

* feat(conversations): add conversation list and replay dashboard pages

* feat(conversations): add Next.js API proxy routes

* test(conversations): add API and grouping tests

* fix(conversations): escape ILIKE wildcards to prevent wildcard injection

* fix(conversations): remove unused _totalChars variable

* fix(conversations): cap list query to 5000 rows to prevent memory exhaustion

* fix(conversations): replace in-memory stats grouping with SQL aggregates

* fix(conversations): bound conversation detail query by time window instead of full channel scan

* style: alphabetize imports and format authorizeGuildAdmin calls

* test(conversations): fix stats mock to match SQL conversation counting

* test(conversations): add POST flag endpoint to guild validation test

The auth test already covered all 5 endpoints including POST .../flag,
but the guild-validation test only checked 4 GET endpoints, leaving the
flag endpoint's guild validation untested.

Resolves review thread PRRT_kwDORICdSM5xTeiw

* fix(conversations): parameterize SQL interval and fix flag button a11y

Thread 3: Replace string interpolation of CONVERSATION_GAP_MINUTES in
the window-function SQL with a $2 parameter to avoid hardcoded literals.
Passes CONVERSATION_GAP_MINUTES as a query value instead.

Thread 4: Change flag button wrapper from `focus-within:opacity-100`
to `group-focus-within:opacity-100` so the button becomes visible
whenever any element in the message bubble group receives keyboard focus,
not just when the button's own children are focused — matching the
group-hover pattern and ensuring proper keyboard accessibility.

Also: biome --write reformatted label.tsx and textarea.tsx (pre-existing
style issues).

* test: add ILIKE wildcard escape coverage for % and _ characters

Adds test cases verifying that % and _ characters in conversation
search queries are properly escaped before being used in ILIKE
patterns, preventing them from acting as SQL wildcards.

* fix: deterministic flag status for duplicate flagged_messages rows

When a message has been flagged multiple times (flagged_messages has no
UNIQUE constraint on message_id), the previous Map construction would
silently overwrite entries in iteration order, making the displayed
status non-deterministic.

Order the SELECT by created_at DESC so the first row per message_id
that lands in the Map is always the most recently created flag, giving
a predictable 'latest wins' behaviour.

* refactor: extract escapeIlike utility from logQuery inline impl

Creates src/utils/escapeIlike.js as a shared, exported utility.
Conversations route now imports it instead of duplicating the regex.

* fix(conversations): use escapeIlike(), fix non-deterministic flag status, add 30-day default window, verify conversationId on flag POST

- Import escapeIlike() instead of inline regex (DRY #4)
- Default to last 30 days when no `from` filter to prevent unbounded LIMIT 5000 scan (#3)
- Fix Map construction: iterate ORDER BY DESC rows and only set first occurrence per key so most-recent flag status wins (#1)
- Verify flagged messageId belongs to the conversation's channel before inserting (#2)

* test(conversations): add ILIKE backslash escape test and fix flag mocks for new anchor check

- Add test for backslash (\) escaping in ILIKE search (#7)
- Update 'flag a message' mock to include anchorCheck query result

* refactor(web): add LOG_PREFIX constant to all 5 conversation proxy routes (#6)

Each route previously inlined its prefix string on every call.
Extracts to module-scope const matching the pattern used by
config/members/roles routes.

* fix(ui): use MessagesSquare icon for Conversations sidebar entry (#8)

AI Chat already uses MessageSquare. Conversations now uses MessagesSquare
to distinguish the two nav items visually.

* fix(ui): show last 4 digits of channel snowflake instead of raw ID (#9)

Raw Discord snowflakes are 18+ digit numbers. Showing `${channelId.slice(-4)}`
gives a minimal but less jarring display until a proper channel name resolver
is wired up.

* refactor(ui): extract PAGE_SIZE constant in conversations page (#5)

Replaces two hardcoded 25s with a single PAGE_SIZE = 25 constant.

* docs(migration): explain missing FK on conversation_first_id (#10)

conversation_first_id has no FK because conversations are not a separate
table with their own PK. They are virtual groups derived from message rows.
message_id already carries a FK for referential integrity.

* fix(lint): suppress pre-existing biome a11y errors in label component

* fix(conversations): stray $ in JSX channel display, increase query limit to 10k
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant