Skip to content

[Graphite MQ] Draft PR GROUP:spec_832234 (PRs 4259, 4249)#4274

Closed
graphite-app[bot] wants to merge 2 commits intomainfrom
gtmq_spec_832234_1771903335727-fd95c9b4-1c42-4608-a48f-e3ae2dc51bdf
Closed

[Graphite MQ] Draft PR GROUP:spec_832234 (PRs 4259, 4249)#4274
graphite-app[bot] wants to merge 2 commits intomainfrom
gtmq_spec_832234_1771903335727-fd95c9b4-1c42-4608-a48f-e3ae2dc51bdf

Conversation

@graphite-app
Copy link
Contributor

@graphite-app graphite-app bot commented Feb 24, 2026

This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.

The following PRs are included in this draft PR:

NathanFlurry and others added 2 commits February 24, 2026 03:22
# Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
# Description

This change adds new tabs to the actor details interface, specifically adding Queue, Workflow, and Database tabs alongside the existing tabs. The implementation includes both the tab triggers in the navigation and their corresponding content components. Additionally, an unused import for `useQuery` from `@tanstack/react-query` and the `ActorTracesTab` import have been removed.

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
@railway-app
Copy link

railway-app bot commented Feb 24, 2026

This PR was not deployed automatically as @graphite-app[bot] does not have access to the Railway project.

In order to get automatic PR deploys, please add @graphite-app[bot] to your workspace on Railway.

@graphite-app graphite-app bot closed this Feb 24, 2026
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4050 February 24, 2026 03:23 Inactive
@graphite-app graphite-app bot deleted the gtmq_spec_832234_1771903335727-fd95c9b4-1c42-4608-a48f-e3ae2dc51bdf branch February 24, 2026 03:24
@claude
Copy link

claude bot commented Feb 24, 2026

PR Review

This is a merge queue PR combining two changes: #4249 (dashboard: bring new tabs back) and #4259 (fix SQLite teardown logic).


PR #4259 — fix(rivetkit): fix sqlite teardown logic

The core fix is solid: onDisconnect is now properly await-ed before the actor is allowed to sleep, preventing a race condition where the SQLite DB could be closed while an async onDisconnect callback was still writing to it.

What's fixed correctly:

  • connDisconnected changed from fire-and-forget to async/await, properly blocking the sleep path
  • resetSleepTimer() moved into the finally block so it only triggers after the callback completes
  • #pendingDisconnectCount counter + ActiveDisconnectCallbacks enum variant block sleep while disconnect callbacks are in-flight
  • try/finally in conn/mod.ts ensures CONN_DRIVER_SYMBOL is always cleared even if connDisconnected throws
  • closed flag + mutex-guarded close() in both db/mod.ts and db/drizzle/mod.ts prevents double-close and guards execute() calls after teardown
  • busy_timeout = 5000 and journal_mode = WAL pragmas are appropriate defaults for file-backed databases

Issues:

1. SQL string interpolation in test fixtures (minor)

In actor-db-drizzle.ts and actor-db-raw.ts, Date.now() is interpolated directly into a SQL string. It returns a number so there is no injection risk here, but interpolating values directly into SQL strings is a pattern worth avoiding in fixtures that others may copy. Consider using the parameterized form instead (consistent with other queries in these fixtures that already use c.db.execute(query, ...args)).

2. Test timing relies on implicit sleep behavior (minor)

test("completes onDisconnect DB writes before sleeping", async (c) => {
    await actor.configureDisconnectInsert(true, 250);
    await waitFor(driverTestConfig, SLEEP_WAIT_MS + 250);
    await actor.configureDisconnectInsert(false, 0);
    expect(await actor.getDisconnectInsertCount()).toBe(1);
});

The test implicitly relies on the actor having triggered at least one connection disconnect during the SLEEP_WAIT_MS window. A comment explaining why the count is expected to be exactly 1 (i.e., one client connection closes when the actor sleeps) would help future readers.


PR #4249 — feat(dashboard): bring new tabs back

Bug: database tab has content but no trigger

The diff adds <TabsContent value="database"> but does not add a corresponding <TabsTrigger value="database">. Looking at the current file, the tab bar renders: State, Connections, Queue, Workflow, Metadata — there is no "Database" entry. The ActorDatabaseTab component will never be reachable through the UI.

Either the trigger needs to be added alongside Queue and Workflow, or the TabsContent for database should be deferred until the trigger is also ready.

Minor: inconsistent className on the new TabsContent blocks

  • queue: "min-h-0 flex-1 mt-0"
  • workflow: "min-h-0 flex-1 mt-0 h-full"
  • database: "min-h-0 min-w-0 flex-1 mt-0 h-full"

The database content has an extra min-w-0 that queue and workflow don't. If intentional (e.g., the database component needs horizontal overflow handling), a comment would clarify why. If not, it should be made consistent.


Summary

Severity Location
Missing database TabsTrigger Bug actors-actor-details.tsx
SQL interpolation in test fixtures Minor actor-db-drizzle.ts, actor-db-raw.ts
Test comment clarity Nit actor-db.ts
Inconsistent min-w-0 on database TabsContent Nit actors-actor-details.tsx

The SQLite teardown fix (#4259) is correct and well-tested. The primary concern is the missing tab trigger for the database tab in #4249.

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.

2 participants