Commit Graph
25 Commits
Author SHA1 Message Date
ZacharyZhang-NY f041336721 refactor: nuke dead exports, dead files, dead DB column, dead zero-value wrappers
Full-codebase ts-prune + manual audit sweep. Every change below is a pure
deletion (or a 2-line re-wire where a dead function was supposed to be
alive). No scope additions.

Dead wrappers / phantom features
  - shouldAutoApproveApproval: zero-value wrapper over shouldAutoApprove.
    Four call sites now import shouldAutoApprove from execution-policy
    directly.
  - evaluateRotation(session, connectorId, thresholdsOverride): the
    connectorId arg was never read (underscored), and thresholdsOverride
    was only exercised by the test suite — no production caller. Signature
    collapses to evaluateRotation(session). DEFAULT_THRESHOLDS /
    RotationThresholds exports and their tests go with it.
  - installLogRedactor: defensive console wrapper that was never wired
    from worker-entry. buildRedactor handles the known threat (env-value
    leaks); the pattern-based redactTokens stays for tests and future
    composition.

Dead files
  - resume-interrupted-tasks.ts: helper function never imported.
  - service-graph-orchestration-deps.ts: three factory closures
    superseded by the inline closures in create-orchestration-service.

Dead exports
  - listConnectorsByAuthKind (shared/connector-catalog)
  - getLocalePromptInstruction + whole localization.ts file
  - ImportantDocType
  - main-side ConnectorAuthKind (duplicated the shared one; not referenced)
  - CheckpointEngineService, RunHousekeepingService types
  - WorkspaceResolution type
  - cmdOrCtrlSymbol
  - AgentEventPush, CredentialPayload type aliases
  - parseApprovalPayloadJson, parseEnvValue, ProviderCard
  - getSOP — buildSOPSection reads the SOPS map directly; the individual
    getter had no callers. Deduped to a flat array.
  - renderWakeReason in wake-reasons/en.ts: byte-for-byte duplicate of the
    one in wake-reasons/index.ts.

Forgotten wiring (not cleanup, but adjacent)
  - registerAgentStandupRoute: the GET /api/agents/me/standup endpoint is
    advertised in every system prompt and wake-reason doc, but the route
    handler was never registered in api-server/index.ts. Agents have been
    hitting 404 since Pi Mono. Registered.

Dead DB column
  - v44 migration drops runs.context_snapshot_json. Column was added by
    v16-v25, never written or read, and v43's deliberate preservation
    was an interim measure. Same idempotent table-rebuild pattern as
    v41/v43.

Test tree
  - Phantom-feature threshold override tests removed (3). The rest of
    the rotation suite re-expressed against the real maxRuns=20 floor.

495/495 unit tests pass. typecheck + build clean.
2026-04-18 23:37:37 -04:00
ZacharyZhang-NY 7e1d4199ee refactor(schema): make runs.task_id nullable, delete heartbeat workaround
The NOT NULL constraint forced heartbeat runs to synthesize a fake task
row (`[Heartbeat] X — Check for work`) so the FK could resolve. Three
layers of code — acquireHeartbeatTask, findReusableHeartbeatTask,
cleanupDuplicateHeartbeatTasks, plus the is_heartbeat flag column + UI
filter — existed to paper over one bad constraint. Remove the
constraint and the whole workaround vanishes.

SQLite table rebuild with data preservation. Live production DB, no
backup — migration is atomic, idempotent, and verified before DROP.

## Migration v35 (rebuild runs with nullable task_id)

Uses `fn:` form so it can:
- Snapshot `COUNT(*) FROM runs` before copy
- Walk `PRAGMA table_info(runs)` to detect drift columns (v34 declared
  a column set; any additive ALTER since v14 shows up here)
- Assert `COUNT(*) FROM runs_new == beforeCount` before `DROP TABLE runs`
- `PRAGMA defer_foreign_keys = ON` so run_events(run_id) FK defers to
  commit time, by which runs has the same id set under the new name
- Recreate all 6 indexes on the new table

If the assertion fails, the migration throws inside the existing
`transaction()` wrapper. SQLite rolls back the whole migration, old
`runs` table is untouched, `schema_migrations` never records v35. Next
app start retries cleanly.

## Migration v36 (drop tasks.is_heartbeat + synthetic rows)

Runs now carry `task_id = NULL` for heartbeat dispatches. The
`is_heartbeat` flag is dead data going forward.

1. `UPDATE runs SET task_id = NULL WHERE task_id IN (SELECT id FROM tasks WHERE is_heartbeat = 1)`
2. `DELETE FROM tasks WHERE is_heartbeat = 1` (synthetic rows, not real work)
3. Rebuild `tasks` without the `is_heartbeat` column (same defer-FK +
   row-count-assert pattern as v35)

## Code cleanup

- `RunRecord.taskId: string | null`; `createRun` / `cancelRunForDeletion`
  / `finishRun` guard every `UPDATE tasks` path with a null-check.
- `TaskRecord.isHeartbeat` removed; every SELECT / filter / UI reference
  dropped.
- Delete `acquireHeartbeatTask` / `findReusableHeartbeatTask` /
  `cleanupDuplicateHeartbeatTasks` — dead after the constraint lifts.
- Delete `src/main/orchestration/heartbeat-task.ts` (pure workaround
  constant file).
- `heartbeat.ts` creates runs with `taskId: null` when the agent has no
  assigned work. Turn context already supports nullable task
  (heartbeatFallbackSection); prompt surface unchanged.
- `validateTaskCreation` drops the `isHeartbeat` parameter — user-
  authored + automation-authored tasks are never heartbeat.
- Renderer: `run.taskId ? taskMap.get(...) : undefined` in Dashboard /
  RunCenter / RunDetail / RunQueue.
- Delete `tests/unit/heartbeat-coalescing.test.ts` (12 tests that
  exercised the deleted DB methods).
- Drop 1 isHeartbeat-bypass test from `task-quality-gates.test.ts`.

## Grep verification

- `isHeartbeatTaskTitle src/` → 0
- `acquireHeartbeatTask src/` → migration comment only
- `is_heartbeat src/` → schema.ts migration history only
- `isHeartbeat src/` → 0

Tests: 578/578 (was 591 — delta is 13 tests that tested deleted code).
2026-04-17 16:16:08 -04:00
ZacharyZhang-NY 40040b3dd2 refactor(errors): surface 57 deferred catch-and-swallow sites via logger
The earlier sweep cleaned the hot path (worker/runtime/automation/api-
server/ipc core — 59 sites). This pass finishes the deferred long tail:
main/* services, database/* CRUD modules, api-server/* routes, ipc-*
handlers, thread-integration, automation helpers, browser, skills.

Default strategy: catch (error) { logger.warn("[module] context:", ...) }
— preserve current behavior, make the failure visible instead of
silently hidden. Widened 3 logger interface signatures to include warn
where callers only passed info/error previously (thread-helpers,
host-environment-service, ipc-domain-common).

Legitimate boundaries left intact: JSON.parse on untrusted input, DNS
resolver no-records paths, explicit safe* helpers (safeJson, parseJson,
parseTriggerDetail), URL-parse predicates, and `<unserialisable>`
fallbacks inside the prompt-builder summarisers. Each kept catch now
either has a reason in context or is a documented safe* helper.

Tests adjusted only where test-constructed no-op loggers needed a warn
stub (message-dispatch, company-mode-switch, ipc-agent-graph-handlers).

With the earlier pass, AgentCompany no longer has silently-swallowing
catch blocks in main-process or renderer business code.
2026-04-17 15:25:13 -04:00
ZacharyZhang-NY 5547c29491 update 2026-04-17 14:03:56 -04:00
ZacharyZhang-NY 1adfea55b0 chore(meetings): log skipped invalid action items; use requireParam for meetingId (P2-42, P3-48) 2026-04-16 23:08:06 -04:00
ZacharyZhang-NY c47050964a refactor: dedupe heartbeat-title check; JOIN thread visibility; typed skill service guard (P2-43, P2-44, P2-45) 2026-04-16 23:07:01 -04:00
ZacharyZhang-NY 3bdc8b4be1 refactor(api): extract authenticateRequest helper from handleRequest (P1-35) 2026-04-16 23:04:12 -04:00
ZacharyZhang-NY 2df921ca88 refactor(agents): share leadership role detector, use word-boundary match
P2-23: 'lead' in system-prompt.ts matched 'cleaner' / 'leaderboard' via
substring .includes(). Hoist LEADERSHIP_ROLE_KEYWORDS into a shared
agent-role-helpers.ts module; tokenize input on non-alphanum to require
whole-word match.

P3-54: Same keyword list was duplicated in automation-skills with drift
(missing 'ceo', different members). Both sites now call through the
shared helper.
2026-04-16 22:57:35 -04:00
ZacharyZhang-NY 43a03aa5e9 refactor(db): collapse three markRead variants into markMessageReadForReader (P1-16) 2026-04-16 22:50:40 -04:00
ZacharyZhang-NY 9874d825b6 refactor(api-server): move agents-standup SQL into db domain methods
Replace the six inline queryAll calls in routes-agents-standup.ts with
typed domain methods: db.agents gains listDirectReportsForStandup and
listReportStatusesForStandup (LEFT JOIN aggregation), db.tasks gains
listAssigneeTasksForStandup and listTasksForAssignees (safe on empty
assignee list), db.approvals gains listAgentPendingApprovals and
listAgentResolvedApprovals. Shared listRecentTaskComments is reused.
2026-04-16 22:39:43 -04:00
ZacharyZhang-NY 4342cf3e2f refactor(api-server): move approvals SQL into db.approvals domain methods
Replace the ad-hoc getApproval helper and the filterable list query
in routes-approvals.ts with db.approvals methods. Add findApprovalById
(non-throwing lookup returning null) and listApprovalsForApi (accepts
requestedByAgentId / relatedTaskId / statuses filters and builds the
prepared statement inside the domain layer).
2026-04-16 22:38:05 -04:00
ZacharyZhang-NY 28296b3d5f refactor(api-server): move agents SQL into db.agents/db.activity/db.approvals methods
Replace seven inline queryAll calls in routes-agents.ts with typed
domain methods. db.agents gains listAgentsForApi, listAgentsForStandup,
listExistingAgentsForHireCheck. db.tasks gains listTasksForStandup.
db.approvals gains listApprovalsForStandup and
listPendingHireApprovalPayloads. db.activity gains listRecentTaskComments
(safe on empty taskIds). All HTTP response projections stay identical.
2026-04-16 22:37:10 -04:00
ZacharyZhang-NY efb9fdb8cc refactor(api-server): move work-graph SQL into db domain methods
Replace six inline queryAll calls in routes-work-graph.ts with domain
methods. db.entityCrud gains listGoals, getGoalForUpdate, listProjects,
getProjectForUpdate. db.knowledge gains listSprints. db.company gains
getDashboardSnapshot, which returns the raw agent/task/activity rows
the dashboard handler aggregates. The route-level row shapes are
preserved in the new GoalListRow / ProjectListRow / SprintListRow
types so HTTP responses remain identical.
2026-04-16 22:34:46 -04:00
ZacharyZhang-NY 2be1bd1612 refactor(api-server): move misc SQL into db.knowledge domain method
Replace the inline knowledge_base queryAll in routes-misc.ts with
db.listKnowledgeEntries (added in the previous commit). Routes-misc
now holds no raw SQL.
2026-04-16 22:32:25 -04:00
ZacharyZhang-NY de02351168 refactor(api-server): move meetings SQL into db.knowledge domain methods
Replace inline queryAll calls in routes-meetings.ts with two new
domain methods on db.knowledge: listMeetings (powers
GET /api/companies/:companyId/meetings) and getMeeting (used by the
PATCH meeting handler). The MeetingListRow shape mirrors the historical
inline projection so HTTP responses stay byte-stable. Also adds
listKnowledgeEntries here for use by routes-misc.
2026-04-16 22:32:01 -04:00
ZacharyZhang-NY 79e57567b2 refactor(api-server): move files SQL into db.documents domain methods
Replace raw queryAll calls in routes-files.ts with two new domain
methods on db.documents: listDocuments (powers
GET /api/companies/:companyId/documents) and getDocumentForUpdate (used
by the PATCH document handler). The DocumentListRow / DocumentLookupRow
shapes preserve the historical inline-SQL projections so HTTP responses
remain identical.
2026-04-16 22:30:39 -04:00
ZacharyZhang-NY 7f097e3319 refactor(api-server): move tasks SQL into db.tasks/db.documents domain methods
Replace inline queryAll calls in routes-tasks.ts with two new domain
methods: tasks.listTasksByStatuses (used by GET /api/companies/:id/issues)
and documents.listImportantDocsForReview (used by the deliverable-review
gate). Routes now hold no SQL or column-mapping logic; the
TaskListByStatusesRow shape preserves the existing HTTP response fields
including requiresUserReview as a boolean.
2026-04-16 22:29:36 -04:00
ZacharyZhang-NY 18bcd0abfc refactor(api-server): extract IMPORTANT_DOC_TYPES constant
Move the duplicated important-document-type list out of routes-tasks.ts
and routes-files.ts into src/shared/document-categories.ts so the array
and lookup set live in one place.
2026-04-16 22:27:56 -04:00
ZacharyZhang-NY 466dac117d perf(api-server): scope agent standup route to caller's tasks and reports
Replace full-company SELECT * + JS filtering with targeted scoped queries:

  - direct reports = agents WHERE reports_to = me
  - my tasks = tasks WHERE assignee_agent_id = me
  - report tasks = tasks WHERE assignee_agent_id IN (...directReports)
  - report status counts = LEFT JOIN tasks GROUP BY report agent
  - approvals scoped to requested_by = me with separate pending /
    resolved queries (resolved capped at 5 in SQL)

JSON response shape unchanged.
2026-04-16 22:22:36 -04:00
ZacharyZhang-NY c81d72fb9c fix(api-server): SSE stream pulls incremental events via seq cursor
Replace per-poll 5000-row scan + JS filter with indexed WHERE seq > ?
query (uses idx_run_events_run). Batch up to 500 per call, drain loop
on reconnect. Poll interval raised 500ms -> 1000ms.
2026-04-16 21:57:35 -04:00
ZacharyZhang-NY f803b0b826 fix(api-server): require Origin header when no bearer token
Origin-less requests previously skipped the allowlist entirely, meaning
any process that omitted the Origin header (trivial for a non-browser
attacker) could reach the auth layer. Browsers always send Origin, so
rejecting Origin-less browser-style requests closes the bypass without
breaking agent CLIs, which authenticate with a bearer token instead.

New policy:
 - Requests with a Bearer Authorization header: Origin optional; if
   present it must be in the allowlist.
 - All other requests (including CORS preflight): Origin required and
   must be in the allowlist.
2026-04-16 21:42:04 -04:00
ZacharyZhang-NY 1b578c111a update 2026-04-16 14:48:51 -04:00
ZacharyZhang-NY 9145572422 update 2026-04-05 20:14:48 -04:00
ZacharyZhang-NY b8dda3c8d0 update 2026-04-04 17:01:26 -04:00
ZacharyZhang-NY 67c757f481 update 2026-04-04 12:03:50 -04:00