feat(db): add harness ingress grants and retirement fences - #5655
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| }, | ||
| table => [ | ||
| index('IDX_quick_chat_messages_thread_created_at').on(table.thread_id, table.created_at), | ||
| uniqueIndex('quick_chat_messages_server_projection_uidx').on(table.server_projection_key), |
There was a problem hiding this comment.
WARNING: Unique index on nullable server_projection_key is not partial
quick_chat_messages_server_projection_uidx indexes every row, including the NULL keys required for legacy messages. After this additive migration every existing row is legacy with a NULL key, so the unique index is a full-table btree that enforces nothing until harness projections exist. Follow the existing nullable unique-index pattern (for example UQ_kilocode_users_openrouter_upstream_safety_identifier) and restrict it to non-null keys. Declare .concurrently() on the schema index, regenerate, then wrap the generated CREATE UNIQUE INDEX CONCURRENTLY with the COMMIT; / BEGIN; statement-breakpoint pair from packages/db/AGENTS.md.
| uniqueIndex('quick_chat_messages_server_projection_uidx').on(table.server_projection_key), | |
| uniqueIndex('quick_chat_messages_server_projection_uidx').on(table.server_projection_key).where(sql`${table.server_projection_key} IS NOT NULL`).concurrently(), |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // A lease never acknowledges import. No timestamp watermark can exclude a late commit. | ||
| index('IDX_quick_chat_messages_pending_ingress') | ||
| .on(table.thread_id, table.created_at, table.id) | ||
| .where(sql`${table.provenance} = 'legacy' AND ${table.ingress_acknowledged_at} IS NULL`), |
There was a problem hiding this comment.
WARNING: Indexes on existing quick_chat_messages are not concurrent
IDX_quick_chat_messages_pending_ingress and IDX_quick_chat_messages_ingress_lease are built on an already-created table. Their predicates match every current row (provenance = 'legacy' AND ingress_acknowledged_at IS NULL), so a non-concurrent CREATE INDEX takes a write lock for the full build. Add .concurrently() here and on the lease index .where(...) below, regenerate, then insert the COMMIT; / BEGIN; wrappers required by packages/db/AGENTS.md.
| .where(sql`${table.provenance} = 'legacy' AND ${table.ingress_acknowledged_at} IS NULL`), | |
| .where(sql`${table.provenance} = 'legacy' AND ${table.ingress_acknowledged_at} IS NULL`).concurrently(), |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Reviewed by grok-4.6 · Input: 295.4K · Output: 27.5K · Cached: 1.1M Review guidance: REVIEW.md from base branch |
1 participant
No new behavior — This change prepares later chat features without enabling them.
Summary
quick_chat_messagesaddsprovenance,server_projection_key,ingress_acknowledged_at,ingress_lease_token, andingress_lease_expires_at; historical rows and old writers default to unacknowledgedlegacymessages.agent_harness_clients,agent_harness_conversation_grants, andagent_harness_invitation_resultsadd revocable access records and conversation-lifetime invitation replay;agent_harness_conversation_registryandagent_harness_retirementspreserve orphan discovery and permanent fences. Migration 0234 prepares this PostgreSQL storage without activating runtime delivery; old message identity, text, timestamps, and nullable, nonuniqueclient_idremain unchanged.Files
packages/db/src/schema.ts— Source, modified (+142 lines): adds partial indexes for unacknowledged legacy messages and lease expiry, without timestamp cutoffs. An index makes projection keys globally unique; a check requires keys for harness messages and forbids keys for legacy messages. Ingress and retirement leases require a token and expiry together. Browser/mobile registrations store server-derivedsession_bindingreferences and versionedsupported_tools, never bearer or refresh tokens. Grants link the user, client, thread, generation, expiry, and revocation; user IDs remain arbitrary text. Invitation results use(thread_id, operation_id)and storeinput_digest,invitation_id, andcanonical_resultcontaininginvitationId,acceptInviteUrl, and pendingemailStatus. Results have no invitation foreign key or expiry and cascade only with their thread. The registry stores nullable user/organization links for discovery, not authority. Retirements use(thread_id, generation),account_deletedorcontext_retiredreasons, and an index for pending delivery. Generations default to zero and reject negatives. The registry and retirement records have no foreign keys, so cascades cannot remove them. Retirement acknowledgment ends retries but retains the fence.anonymizeCloudUserData, used bysoftDeleteUser, now writesaccount_deletedretirement fences before removing chat payloads, including registry entries whose threads already disappeared. It uses the registered generation or zero for old threads, preserves existing fences on retry, and clears registry ownership links. It deletes clients, grants, messages, threads, and invitation results while retaining discovery identifiers; existing callers, deletion responses, and subscription checks stay unchanged.Files
apps/web/src/lib/user/index.ts— Source, modified (+39/-11 lines): extends transactional cleanup and documents retained identifiers. It clears both user and organization links for registry rows matched by user ownership or thread ownership.Jest
rootsnow includes database tests alongside web tests, so continuous integration (CI) discovers both. The existing transforms, mappings, test selection, and setup hooks remain unchanged; this expands discovery without replacing the web suite.Files
apps/web/jest.config.ts— Source, modified (+1 line): adds the database source root and explicitly retains the existing web root.Tests: 2 files changed, 669 lines added.
apps/web/src/lib/user/index.test.ts(modified, +239) andpackages/db/src/agent-harness-schema.test.ts(added, +430) add account cleanup and ingress regression coverage.Generated: 3 files changed, 40,479 lines added.
packages/db/src/migrations/0234_agent_harness_ingress.sql(migration, added, +79),packages/db/src/migrations/meta/0234_snapshot.json(snapshot, added, +40,393), andpackages/db/src/migrations/meta/_journal.json(journal, modified, +7).Verification
No manual tests ran for this level. This level does not activate the runtime, and no isolated PostgreSQL test connection is approved.
Visual Changes
Visual Changes: N/A
Reviewer Notes
Human steps
This level requires no new environment values, secrets, or flag changes.
Scope and evidence
Kilo-Org/cloud./Users/igor/Projects/.worktrees/shared-agent-harness-3bb0.shared-agent-harness-3bb0-s5...shared-agent-harness-3bb0-s6(level 6 only).Notes
No manual runtime verification ran for this level. Full backend, browser, iOS, and Android verification remains required on the completed stack tip.
Real PostgreSQL tests, account cleanup tests, and migration application remain required in CI; local artifact tests do not prove database behavior.
Stacked PRs — merge bottom to top. Each level shows only its own diff.
Runtime verification (E2E, user advocacy, simplify) runs on the tip PR over every level.
Every level keeps its own checks, its own bot review, and its own threads; each one is answered on its own PR.
Each level is its own deliverable: it builds and passes its own checks alone.
A finding on a level is repaired on that level, then carried upward with stack.sh forward.
shared-agent-harness-3bb0— chore(agent-harness): register workspaces and enforce CI boundaries #5632shared-agent-harness-3bb0-s2— feat(agent-harness): define portable domain and snapshots #5637shared-agent-harness-3bb0-s3— feat(agent-harness): define commands tools and permission policy #5639shared-agent-harness-3bb0-s4— feat(agent-harness): share client state and cursor recovery #5643shared-agent-harness-3bb0-s5— feat(agent-harness): persist command intents and execution receipts #5647shared-agent-harness-3bb0-s6— feat(db): add harness ingress grants and retirement fences #5655 ← this PRshared-agent-harness-3bb0-s7— feat(agent-harness): deliver legacy history and project durable text #5659shared-agent-harness-3bb0-s8— feat(agent-harness): authorize durable grants and registered clients #5662shared-agent-harness-3bb0-s9— feat(agent-harness): fence retirement and retry payload cleanup #5667shared-agent-harness-3bb0-s10— feat(agent-harness): persist authoritative state in SQLite #5675shared-agent-harness-3bb0-s11— feat(agent-harness): admit durable runs and revisioned commands #5678shared-agent-harness-3bb0-s12— feat(agent-harness): recover queued runs and stream checkpointed steps #5688shared-agent-harness-3bb0-s13— feat(agent-harness): resolve interactions and dispatch tools sequentially #5693 (tip)