Commit Graph
20 Commits
Author SHA1 Message Date
ZacharyZhang-NY f012f8e880 refactor(skills): drop company_skills.markdown column, read from disk via IPC
`company_skills.markdown` stored the full SKILL.md body. The on-disk
file under `profile/skills/{companyId}/{slug}/SKILL.md` is what pi-
coding-agent actually reads via `loadSkills` at runtime. The DB column
was a duplicate — import wrote to both, renderer read from DB. Any
discrepancy between disk and DB was invisible until the agent behaved
differently from what the user saw in SkillDetail.

Disk becomes the single source of truth. DB keeps name / description /
metadata / source info / file inventory — the cheap lookup row. UI reads
SKILL.md content live via a new IPC channel.

## Migration v37 (rebuild company_skills without markdown)

Table-rebuild pattern matching v35/v36: `fn:` form, PRAGMA walk for
drift columns, row-count assertion before DROP, `defer_foreign_keys`
for the `agent_skill_assignments(skill_id)` reference. Indexes
recreated. Idempotent via `schema_migrations` ledger.

Existing DB rows' markdown content is discarded — but the on-disk
SKILL.md files are the authoritative copy, so no user-visible data
loss. Any agent using those skills keeps working; the UI just starts
reading from disk instead of the DB cache.

## Code changes

- `src/main/database/skills.ts`: drop `markdown` from read mapping,
  upsert input type, INSERT column/bind list, ON CONFLICT DO UPDATE.
- `src/shared/types/records-core.ts`: `SkillRecord.markdown` removed.
- `src/main/skills/import.ts` + `import-github.ts`: drop `markdown:`
  from 4 `saveCompanySkill` call sites. Disk-write via
  `materializeSkillFiles` unchanged (it's what the file agent reads).
- `src/main/skills/index.ts`: expose `skillFilesDir` for the new IPC
  handler to resolve `${profile}/skills/${companyId}/${slug}/SKILL.md`.
- `src/shared/contracts/channels.ts` + `schemas-execution.ts` +
  `desktop-api/skills.ts`: new `getSkillMarkdown` channel
  `{ companyId, slug } → DesktopResult<string>`.
- `src/preload/index.ts`: expose `getSkillMarkdown(companyId, slug)`.
- `src/main/ipc-skill-handlers.ts`: handler validates schema, ensures
  company, resolves path, `readFile`, returns content. Fails closed
  with `SKILL_NOT_FOUND` on ENOENT, `SKILL_READ_FAILED` on other
  read errors.
- `src/renderer/components/skills/SkillDetail.tsx`: `useEffect` fetches
  markdown via IPC when the skill detail loads; shows "Loading…" until
  resolved, surfaces read errors. Preserves `stripFrontmatter`.
- Tests: drop `markdown:` from 2 fixture sites
  (`skill-types.test.ts`, `skill-aware-assignment.spec.ts`).

## Verification

- typecheck / lint / 578 tests green
- `grep 'skill\.markdown' src/` → 0 DB-record reads
- `grep '\.markdown\b' src/main/database/skills.ts` → 0
- Remaining `.markdown` references are all `ghSkill.markdown` (GitHub
  fetcher's intermediate struct feeding `materializeSkillFiles` to
  disk) and parameter names in `stripFrontmatter(markdown: string)` /
  fs-writer helpers — none read from a DB row.
2026-04-17 16:22:31 -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 14dc9a4897 refactor(cost): single writer for cost_entries, finishRun reads SUM
Cost was written through three independent paths: worker-runner shipped
`finalUsage.costUsd` in run-finished -> finishRun wrote `runs.cost_usd`,
`tasks.cost_usd += costUsd`, `agents.spent_monthly_usd += costUsd`, and
inserted a per-run aggregate row in cost_entries. CostWriter
additionally inserted per-message rows in cost_entries from main. Any
CostWriter failure created silent drift between `SUM(cost_entries)`
(read by budget enforcement) and `runs.cost_usd` (read by the UI).

Make CostWriter the single writer, finishRun the single reader.

- Add `db.recordCostEntry(...)` as the first-class insert path for
  per-message cost rows. CostWriter calls it; tests call it directly.
- Remove the aggregate INSERT from finishRun. finishRun now reads
  `SELECT COALESCE(SUM(amount_usd), 0) FROM cost_entries WHERE run_id=?
  AND unattributed=0` and writes that as the authoritative run total to
  runs.cost_usd, tasks.cost_usd, and agents.spent_monthly_usd.
- Drop `costUsd` from `finishRun` input, the worker `run-finished`
  ChildMessage shape, the automation `RunFinishedEvent` type, and every
  caller + test fixture. Cost is owned by the per-message path.
- worker-runner stops extracting `usage.cost.total` — token counts stay
  for session accumulation, cost comes from pi-ai via CostWriter.
- run-finished-helpers now gates budget incident checks on
  `event.status === "succeeded"` instead of a cost field that no longer
  exists on the event.
- automation-run-completion fires `budget_threshold` on every succeeded
  run (post-run spent_monthly_usd already reflects the SUM path).
- Update budget tests to write a `recordCostEntry` before `finishRun`
  to simulate the per-message cost path; drop `costUsd` from cross-
  subsystem, edge-case-budget, database, and related fixtures.

One writer, one reader, one source of truth. No drift.
2026-04-17 15:04:22 -04:00
ZacharyZhang-NY 5547c29491 update 2026-04-17 14:03:56 -04:00
ZacharyZhang-NY e94939a562 test(connector-id): migrate fixtures and assertions to pi-mono ids
Replaces legacy adapter ids (codex_local / claude_local / gemini_local /
claude_compatible) across ~42 test files with pi-mono equivalents:
- codex_local → openai-codex
- claude_local → claude-pro
- gemini_local → gemini-cli
- claude_compatible → anthropic

Hand-rewrites where fixture semantics changed:
- connector-matching: TASK_TYPE_PREFERENCE ordering rebased (claude-pro
  first for code/content/management; google first for research)
- connector-model-policy: getModels(provider) from pi-ai instead of the
  deleted adapter.module.models array
- session-management + edge-case-session: adapter exemption removed;
  rotation now uniform under pi-mono
- ipc-core-admin-handlers: CSV compat model replaced with pi-ai anthropic
- connectors.spec: lookup by openai-codex id (catalog now starts with claude-pro)
- onboarding-model-pool: listProviderModels(provider) helper; label text
  changed to "ChatGPT Plus/Pro (Codex)"
- database integration: added vault insecure-fallback env + electron mock
- command-paths: uses new @main/connector-helpers normalizeCommandValue
- pi-mono-migrations: added `as string` casts for 3 negative-existence
  assertions verifying legacy ids were removed

Drops:
- pnpm-workspace.yaml packages: [] (was "packages/*"; directory empty)
- obsolete CONNECTOR_STRENGTHS import; .module.models probing

Verification:
- pnpm typecheck → 0 errors
- pnpm lint → 0 errors / 0 warnings
- pnpm build:verify → 0 errors / 0 warnings
- pnpm test:unit → 508/508 pass
- pnpm test:integration → 96/96 pass
- All 6 adapter hard gates pass (NOT FOUND / 0 matches / empty)
- pi-ai / pi-agent-core / pi-coding-agent all resolved @0.67.6
2026-04-17 02:36:53 -04:00
ZacharyZhang-NY 0d044aeee8 chore(lint): replace any with unknown or typed shapes in tests
- DB introspection tests: introduce local RawDb alias for casts
  that poke into the private .db handle.
- IPC handler tests: introduce IpcHandler alias matching the
  registerHandle signature (event + unknown args).
- selective-ipc / session-management: narrow handler and data
  params to unknown or the minimum shape the code reads.
2026-04-17 01:45:00 -04:00
ZacharyZhang-NY 0f1b7c0b33 test(integration): split autonomous-chain.spec.ts into per-scenario files
The original tests/integration/autonomous-chain.spec.ts was ~1400 lines
and 50 KB, covering 19 describe blocks across 10 orthogonal scenarios.
Physically extract each describe (or closely related group) into its
own file inside tests/integration/autonomous-chain/ so each file stays
under the project's 500-line ceiling and failures point at the
scenario that regressed.

Shared beforeEach / afterEach / createAgent / buildCtx helpers move to
autonomous-chain/shared-setup.ts, which exposes installSharedSetup() as
a factory that each spec calls at module load. The tests themselves
are moved byte-for-byte; only module-level references (`db`,
`companyId`, `workspaceId`, `tempDirs`) are rewritten to go through
the shared handle. No assertions, it() calls, or behavior are changed.

Split layout:
  role-detection.spec.ts        — Step 1: CEO role detection
  ceo-phased-prompts.spec.ts    — Step 2: phased prompts + approvals
  wakeup-queue.spec.ts          — Step 3: deferred wake queue
  approval-decisions.spec.ts    — Step 4: hire approval decisions
  task-propagation.spec.ts      — Step 5: task/comment wake propagation
  run-failure.spec.ts           — Step 6: run failure escalation
  crash-recovery.spec.ts        — Step 7: worker crash recovery
  wake-reason.spec.ts           — Step 8: wake reason injection
  e2e-flows.spec.ts             — full-chain + review-fix regressions
  automation-messages.spec.ts   — automation/messages/assignment/workflows

Test count stays at 363 passing; typecheck stays at 156 errors.
2026-04-16 23:34:54 -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 1b578c111a update 2026-04-16 14:48:51 -04:00
ZacharyZhang-NY 67c757f481 update 2026-04-04 12:03:50 -04:00
ZacharyZhang-NY 65e85135c1 large update 2026-03-29 14:08:15 -04:00
ZacharyZhang-NY 047b09eae4 test(10-01): add cross-subsystem pipeline integration tests
- 6 tests exercising full 5-subsystem pipeline (D-04)
- Quality gate -> CAS checkout -> budget gate -> session -> completion
- Budget exhaustion blocks subsequent runs correctly
- Session token accumulation across multiple runs verified
- CAS double-checkout prevention in pipeline context
- Uses real AppDatabase, budget enforcement, session management services
2026-03-27 20:32:54 -04:00
ZacharyZhang-NY 4f5a14671d test(10-01): add process recovery integration tests
- 10 tests covering worker crash detection, lock release, agent reset
- Session handoff markdown generation with real DB
- Context preservation (D-02): agent assignment retained after crash
- DB consistency verification: no running runs/agents, no orphaned locks
- Multiple concurrent runs all interrupted on crash
- Uses real AppDatabase with temp dirs, gemini_local connector
2026-03-27 20:31:17 -04:00
ZacharyZhang-NY 4d47712669 fix(04-01): resolve type errors and update integration tests for wakeup API
- Fix SQLInputValue type in listWakeupRequests params
- Fix leftover pendingWakes reference in worker-service.ts
- Fix RunStatus 'completed' -> 'succeeded' in atomic-checkout test
- Update autonomous-chain.spec.ts to use new wakeup lifecycle API
2026-03-26 19:00:51 -04:00
ZacharyZhang-NY 0cd65521b8 update 2026-03-18 14:42:14 -04:00
ZacharyZhang-NY 1ba3147ed2 update 2026-03-18 01:15:58 -04:00
ZacharyZhang-NY 7a3db27ffa update 2026-03-17 12:31:06 -04:00
ZacharyZhang-NY 54bcc77ef7 update 2026-03-16 13:07:14 -04:00
ZacharyZhang-NY d067589276 update 2026-03-15 23:49:46 -04:00
ZacharyZhang-NY 2af82b6bb3 Initialize project and ignore local references 2026-03-15 22:17:07 -04:00