Uh oh!
There was an error while loading. Please reload this page.
chore: add covering index on Lease(agentName, expiredAt) - #258
Conversation
Remove local unsafe timing comparison from middleware and import the timing-safe version from @/lib/dispatch-env instead. This eliminates the timing leak from the early length-return and removes code duplication.
…ork queries Fixes#250 Add @@index([agentName, expiredAt]) to the Lease model so that resolveActiveWork queries can use an index-only scan instead of falling back to a full table scan as the Lease table grows.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) PR Review: chore: add covering index on Lease(agentName, expiredAt)SummaryThe core change (adding Change-by-Change Findings
Linked Issue FitIssue #250 Acceptance Criteria:
The primary deliverable is correct. Two criteria require post-merge verification. Standards Compliance
Evidence Provider FindingsNo evidence providers configured. Tool Harness FindingsNo tool harness output available (planning warning: could not parse response as JSON). Unknowns / Needs Verification
RecommendationRequest changes to: (1) remove the accidental |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Jory Irving <46251616+joryirving@users.noreply.github.com>
There was a problem hiding this comment.
AI Automated Review
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)
PR Review: chore: add covering index on Lease(agentName, expiredAt)
Recommendation
Approve — The primary schema change satisfies the linked issue's acceptance criteria. The additional refactoring (safeEqual extraction, ESLint ignore) appears related to ongoing maintenance but introduces no risk.
Change-by-Change Findings
1. prisma/schema.prisma — Primary Change ✅
- Added
@@index([agentName, expiredAt])to theLeasemodel - Positioned correctly after
@@unique([agentName, issueId])and before existing@@index([expiredAt]) - Fits issue #250: Directly implements the "Recommended Fix" from the linked issue
2. src/lib/dispatch-env.ts — Refactoring
- Changed
safeEqualfrom module-private toexported - No functional change to the function body (timing-safe Buffer comparison preserved)
3. src/middleware.ts — Refactoring
- Removed inline
safeEqualfunction - Now imports from
@/lib/dispatch-env— deduplication is valid cleanup
4. eslint.config.mjs — Maintenance
- Added
vitest/dist/**to ESLint ignores - Consistent with fix introduced in PR #247
Standards Compliance
| Area | Status | Notes |
|---|---|---|
| Schema conventions | ✅ Pass | Prisma schema uses proper @@index syntax; validates with npx prisma validate |
| No hardcoded agent names | ✅ Pass | Change is schema-level, not agent-specific |
| Secrets handling | ✅ Pass | No secrets introduced or modified |
| API contracts | ✅ Pass | Only refactoring existing code; no breaking changes |
Linked Issue Fit
Issue #250 — "P2 — Add covering index on Lease(agentName, expiredAt) for active-work query performance"
| Acceptance Criterion | Status | Evidence |
|---|---|---|
@@index([agentName, expiredAt]) exists on Lease model | ✅ Satisfied | Patch shows index added at line 489 of schema.prisma |
| Query plan shows index usage for active-work lookups | Runtime verification required; index structure is correct | |
| Schema migration applied (or db push for dev) | PR body states npx prisma validate passes; no migration file in diff |
Note: The PR adds the index but does not include a migration file. Per AGENTS.md conventions, local dev uses db push and production uses prisma migrate deploy. The index change requires either running npm run db:push locally or generating/merging a migration before deployment. This is standard practice and not a blocker.
Evidence Provider Findings
No evidence providers configured — no scan output available.
Tool Harness Findings
No tool harness results available (planning failed for 0 planned requests).
Unknowns / Needs Verification
Schema migration: No migration file was included in the diff. Deployment will require either generating a migration (
npx prisma migrate dev --name add_lease_agentname_expiredat_index) or usingnpm run db:pushfor dev environments. This is typical for Prisma schema changes and not a blocker.Query plan verification: The second acceptance criterion (verifying index usage in query plans) requires runtime observation against a populated
Leasetable. The structural change is correct.
Summary
The PR correctly adds the requested composite covering index to the Lease model, satisfying the primary intent of issue #250. The refactoring of safeEqual into a shared module and the ESLint ignore update are orthogonal improvements with no risk. The work follows repository conventions (Prisma schema validation, appropriate file changes) and introduces no breaking changes.
Fixes#250
Add
@@index([agentName, expiredAt])to the Lease model so thatresolveActiveWorkqueries can use an index-only scan instead of falling back to a full table scan as the Lease table grows.Changes:
Lease(agentName, expiredAt)inprisma/schema.prismaSchema validation:
npx prisma validatepasses ✅