Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
phase: design
title: Agent Registry Write Optimization Design
description: Conditional batch persistence and controlled passive pruning
---

# Design

## Data Flow

```mermaid
flowchart LR
Poll["listAgents refresh"] --> Detect["adapter detection"]
Detect --> Read["read registry snapshot"]
Read --> Batch["registerBatch"]
Batch -->|"all merged fields equal"| NoWrite["no write transaction"]
Batch -->|"new or changed"| Tx["single atomic write transaction"]
Read --> Cadence{"prune due?"}
Cadence -->|"no"| Skip["skip liveness scan"]
Cadence -->|"yes"| Scan["process.kill(pid, 0)"]
Scan -->|"no stale rows"| NoPruneWrite["no write transaction"]
Scan -->|"stale rows"| DeleteTx["single delete transaction"]
```

## Chosen Pruning Contract

- Add `AgentRegistry.pruneIfDue()` for passive refreshes.
- The first passive call scans immediately; later passive calls scan no more than once every 30 seconds per registry instance.
- Keep `AgentRegistry.prune()` as an immediate forced scan for compatibility and correctness-sensitive flows such as `agent start`.
- Both methods update the same last-pruned timestamp after a successful scan.
- A scan with no stale entries performs no write transaction.
- The clock and interval are constructor options with backward-compatible defaults, enabling deterministic tests without global timers.

Thirty seconds reduces a three-second console poll from ten scans to one. Stale names are still cleared immediately by start's forced prune, rename checks the conflicting row's liveness directly, registration checks only a relevant name conflict, and kill targets live adapter results. Passive rows disappear within 30 seconds.

Alternatives considered:

- Prune on every refresh but avoid an empty delete transaction: rejected because process liveness scans remain on the hot path.
- Prune only on mutations such as start/rename/kill: rejected because stale rows could remain indefinitely during read-only use.
- Persist the last-prune time in SQLite for a cross-process cadence: rejected because it adds migration and coordination writes to optimize a process-local console polling problem.
- Use a 60-second interval: workable, but 30 seconds gives faster passive cleanup while still removing 90% of scans at a three-second poll rate.

## Conditional Persistence

`registerBatch()` will merge incoming entries with current rows and compare every persisted field except `updated_at`. If all merged entries are identical, it returns before opening a transaction. If any may change, one transaction retains the existing atomic batch boundary. Each entry is re-read and re-merged inside that transaction so concurrent registry writers cannot make the preflight snapshot authoritative. An entry is upserted only when the transaction-time merged fields differ.

`updated_at` changes only on insert, actual persisted-field update, or explicit rename. Unchanged detection refreshes do not imply registry updates.

## Conflict and PID Semantics

- Dead rows owning an incoming name are deleted inside the same batch transaction.
- A conflicting row with the same PID but a different agent type is stale by definition: one OS PID cannot simultaneously be two provider processes. Delete it even though `kill(pid, 0)` reports the reused PID alive.
- A live different-PID name conflict remains a database constraint error, preserving current conflict visibility.
- Name overlays and existing-entry lookup use `type + pid`, not PID alone, preserving the documented cross-type PID-reuse guard.
- Same-type PID reuse remains accepted until reliable process-start identity is available.

## Public and Migration Compatibility

Existing method calls and return types remain valid. The constructor gains only an optional second options argument, and `pruneIfDue()` is additive. No table or migration changes are needed; comparison uses the existing columns.

## Overlap and Integration Order

`feature-console-main-thread-responsiveness` is expected to touch `AgentManager.ts` and its tests to share process snapshots across adapters. It must not absorb this registry change. Merge this branch first, then rebase the snapshot branch and compose its detection changes around the conditional registration and `pruneIfDue()` call. Registry SQL and cadence tests should remain owned here; snapshot enumeration tests remain owned there.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
phase: implementation
title: Agent Registry Write Optimization Implementation
description: Implementation record for conditional writes and passive prune cadence
---

# Implementation Record

## Status

Implementation and full-suite verification are complete; publication remains.

## Intended Files

- `packages/agent-manager/src/utils/AgentRegistry.ts`
- `packages/agent-manager/src/AgentManager.ts`
- `packages/agent-manager/src/database/connection.ts`
- `packages/agent-manager/src/index.ts`
- `packages/agent-manager/src/__tests__/utils/AgentRegistry.test.ts`
- `packages/agent-manager/src/__tests__/AgentManager.test.ts`

## Design Commitments

- Compare all persisted entry fields before writes.
- Retain a single transaction for changed batches.
- Revalidate inside the transaction.
- Keep immediate `prune()` and add 30-second `pruneIfDue()`.
- Do not change schema, migrations, process enumeration, or console polling.

## Implemented Behavior

- `AgentRegistry.registerBatch()` performs a read-only preflight and returns without a transaction when merged persisted fields are unchanged.
- Potentially changed batches retain one transaction, re-read each identity inside it, and issue at most one upsert per changed entry.
- `updated_at` uses the injected clock and advances only for a real insert/update or rename.
- `pruneIfDue()` scans immediately on its first call and then at a configurable interval defaulting to 30 seconds; `prune()` remains forced.
- Pruning opens a delete transaction only when stale rows exist.
- Cross-type rows sharing a reused PID are removed in the same registration transaction, and manager name overlays use `type + pid`.
- Optional constructor tracing records expanded SQLite operations for deterministic operation-count tests without changing default behavior.

## Design Alignment

The implementation matches the design without schema or migration changes. The only additive public surface is `AgentRegistryOptions` plus `pruneIfDue()`. Existing constructor and method calls remain valid.

## TDD Evidence

- Initial focused red: 7 failures, including redundant BEGIN/INSERT/COMMIT plus empty prune BEGIN/COMMIT on an unchanged refresh.
- Timestamp regression red: the changed-field test failed when `updated_at` used wall-clock time instead of the injected clock.
- Restored green: `AgentRegistry.test.ts` and `AgentManager.test.ts` passed 67/67, including atomic rollback coverage.
- Full agent-manager suite passed 509/509 with OS process visibility enabled for the existing print-agent integration.
- Full CLI suite passed 959/959 after the required workspace build.
- Full six-project build and lint completed successfully; lint reported six unrelated pre-existing warnings and zero errors.

## Integration Note

Land before `feature-console-main-thread-responsiveness`; that branch should rebase and resolve only the shared manager/test call-site overlap.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
phase: planning
title: Agent Registry Write Optimization Plan
description: TDD plan for conditional writes and controlled pruning
---

# Plan

## Task Queue

- [x] Add SQL operation tracing and fake-clock test fixtures.
- [x] Red: prove unchanged refreshes currently write and transact.
- [x] Green: skip unchanged upserts and the surrounding write transaction.
- [x] Red/green: persist changed fields exactly once and retain `updated_at` meaning.
- [x] Red/green: add 30-second passive prune cadence plus immediate forced prune.
- [x] Red/green: cover dead-name cleanup, cross-type PID reuse, and rename conflicts.
- [x] Update implementation/testing records and cross-check design alignment.
- [x] Run focused and full agent-manager/CLI tests, lint, typecheck, builds, and docs lint.
- [ ] Commit, rebase on `origin/main`, push, and open a PR without merging.

## Risks

- Preflight comparisons could race with another process. Mitigation: re-read and re-merge entries inside the single write transaction.
- Cadence could delay stale-name cleanup. Mitigation: keep `prune()` forced, preserve targeted conflict liveness checks, and bound passive delay to 30 seconds.
- Process-snapshot work could cause merge conflicts. Mitigation: land registry work first and rebase snapshot work afterward.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
phase: requirements
title: Agent Registry Write Optimization Requirements
description: Avoid redundant registry writes and bound passive stale-entry pruning
---

# Agent Registry Write Optimization

## Problem

`AgentManager.listAgents()` is called every three seconds by the console. Every refresh currently upserts every detected agent, advances `updated_at`, and opens a write transaction even when no persisted field changed. It also scans every registry row for process liveness and opens a prune transaction on every refresh.

## Goals

- Perform zero SQLite writes and no write transaction for an unchanged refresh.
- Persist each changed or new entry once, while retaining atomic batch behavior.
- Define `updated_at` as the time a persisted field actually changed.
- Put passive pruning on a deterministic cadence so console polling does not scan every three seconds.
- Keep an immediate forced prune for start and other correctness-sensitive callers.
- Preserve name conflict cleanup, rename behavior, live-process checks, PID reuse safeguards, and the existing SQLite schema/migrations.
- Preserve existing public calls to the `AgentRegistry` constructor, `register`, `registerBatch`, `prune`, `rename`, `lookup`, and `list`.

## Success Criteria

- SQL operation-count tests prove an unchanged refresh issues zero write statements.
- A changed field produces one row upsert in one batch transaction and advances `updated_at` once.
- A fake clock proves passive pruning runs immediately, is skipped before 30 seconds, and removes newly dead rows at 30 seconds.
- `prune()` remains an immediate forced operation independent of the passive cadence.
- Dead name conflicts, cross-type PID reuse, and rename conflicts retain their cleanup/error behavior.
- Focused and full agent-manager and CLI tests, lint, typecheck, and builds pass.

## Scope Boundaries

- Do not add or consume a shared process snapshot.
- Do not change adapter process enumeration or console refresh scheduling.
- Do not change the database schema or migration history.
- Same-type PID reuse remains the previously accepted limitation because reliable detection requires process-start metadata.

## Integration Constraint

The separate `feature-console-main-thread-responsiveness` work may replace repeated adapter process enumeration with a per-refresh process snapshot. This feature must land first because it changes only registry persistence and prune scheduling. The snapshot branch should then rebase on this work and preserve the write-elision/cadence tests while resolving any overlap in `AgentManager.ts` and `AgentManager.test.ts`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
phase: testing
title: Agent Registry Write Optimization Testing
description: Deterministic SQL operation-count and fake-clock coverage
---

# Testing Strategy

## Required Deterministic Cases

- [x] Unchanged `listAgents()` refresh performs zero INSERT/UPDATE/DELETE and opens no write transaction.
- [x] A changed persisted field produces exactly one upsert and advances `updated_at` once.
- [x] Passive prune scans immediately, skips before 30 seconds, and removes newly dead entries at the boundary.
- [x] Forced `prune()` removes newly dead entries even before the passive boundary.
- [x] Cross-type reuse of the same PID replaces stale identity without inheriting its name.
- [x] Dead name conflicts are cleaned up atomically.
- [x] Live rename conflicts still throw and dead rename conflicts are cleaned up.
- [x] A live name conflict rolls back all earlier writes in the same batch.

## Validation Commands

- `npm test --workspace @ai-devkit/agent-manager -- AgentRegistry.test.ts AgentManager.test.ts`
- `npm test --workspace @ai-devkit/agent-manager`
- `npm test --workspace ai-devkit`
- `npm run lint --workspace @ai-devkit/agent-manager`
- `npm run lint --workspace ai-devkit`
- `npm run typecheck --workspace @ai-devkit/agent-manager`
- `npm run build --workspace @ai-devkit/agent-manager`
- `npm run build --workspace ai-devkit`
- `npx ai-devkit@latest lint --feature agent-registry-write-optimization`

## Evidence

- Red run: focused suite failed 7 tests, and SQL trace showed an unchanged refresh issuing `BEGIN`, `INSERT`, `COMMIT`, `BEGIN`, `COMMIT`.
- Regression red: changed-field timestamp test failed after temporarily restoring wall-clock writes.
- Green run: focused suite passed 67 tests in 2 files.
- `npm run typecheck --workspace @ai-devkit/agent-manager`: exit 0.
- `npm run lint --workspace @ai-devkit/agent-manager`: exit 0.
- `npm test --workspace @ai-devkit/agent-manager`: 24 files, 509 tests passed (rerun with OS process visibility for the existing print-agent integration).
- `npm test --workspace ai-devkit`: 79 files, 959 tests passed after workspace packages were built.
- `npm run build`: all 6 projects built successfully.
- `npm run lint`: all 6 projects linted successfully; 0 errors and 6 unrelated pre-existing warnings.
- `npx ai-devkit@latest lint --feature agent-registry-write-optimization`: all feature checks passed.
11 changes: 7 additions & 4 deletions packages/agent-manager/src/AgentManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,18 @@ export class AgentManager {
});
}

const preExistingByPid = new Map(this.registry.list().map((e) => [e.pid, e]));
const identityKey = (type: string, pid: number): string => `${type}:${pid}`;
const preExistingByIdentity = new Map(
this.registry.list().map((entry) => [identityKey(entry.type, entry.pid), entry]),
);
const entries = allAgents.map((agent) =>
this.toRegistryEntry(agent, preExistingByPid.get(agent.pid)),
this.toRegistryEntry(agent, preExistingByIdentity.get(identityKey(agent.type, agent.pid))),
);
if (entries.length > 0) this.registry.registerBatch(entries);
this.registry.prune();
this.registry.pruneIfDue();

for (const agent of allAgents) {
const entry = preExistingByPid.get(agent.pid);
const entry = preExistingByIdentity.get(identityKey(agent.type, agent.pid));
if (entry) {
agent.name = entry.name;
}
Expand Down
Loading
Loading