Commit Graph
19 Commits
Author SHA1 Message Date
ZacharyZhang-NY 7b54fe54a8 fix(security): materialize complete skill packages 2026-05-15 01:36:16 -04:00
ZacharyZhang-NY f5b3de438e fix(security): limit skill URL import reads 2026-05-15 01:18:44 -04:00
ZacharyZhang-NY 0cbaabf937 fix(security): bound skill URL redirects 2026-05-15 01:07:43 -04:00
ZacharyZhang-NY 6ae506bf73 fix(security): preserve SNI for skill URL fetches 2026-05-15 01:04:41 -04:00
ZacharyZhang-NY d1a3d23c70 fix(security): scope skill assignments by company 2026-05-14 23:37:58 -04:00
ZacharyZhang-NY 6da0a58769 refactor(db): group database domain modules 2026-04-22 19:24:25 -04:00
ZacharyZhang-NY 118e3d8795 Remove final AppDatabase pass-through methods with ESLint guard (C1 part 3/3)
Migrate all remaining pass-through call sites (social, skills, budget,
entityCrud, documents, knowledge, automation, wakeup, backup, threads,
settings, snapshot, connectors, workspaces, activity) to their owning
domain modules. `AppDatabase` now carries only three methods:

  - createHireRequest (multi-module composite: agents + approvalsMod)
  - rollbackConfigRevision (multi-module composite: documents + entityCrud)
  - queryAll (raw-SQL escape hatch, @internal)

The vault/env-binding infrastructure previously passed argument-by-
argument into `entityCrud.saveSecret`/`resolveSecretEnv` now binds at
module construction, letting callers invoke `db.entityCrud.saveSecret({...})`
without carrying vault plumbing.

Structural types that narrowed AppDatabase to a bag of flat methods
(`ConnectorModelPolicyDeps` callers, `CapabilityMatchDb`, test mocks in
budget-enforcement / budget-incidents / task-quality-gates /
failure-escalation / task-quality-gates-sweep) are reshaped to the new
namespace. `connectorPolicyDepsFromDb` replaces the old pattern of
handing the whole AppDatabase to `validateConnectorModelSelection`.

A new ESLint rule in eslint.config.mjs blocks new top-level methods on
AppDatabase so the pass-through layer cannot silently grow back.
`AppDatabase.ts` dropped from 193 lines to 58; `AppDatabaseBase` lost
the 270-line trailing pass-through block.
2026-04-20 13:31:39 -04:00
ZacharyZhang-NY 0db6e4261f Migrate top-20 high-frequency catch sites to AppError taxonomy (C6.1 part 2/2)
Each of these catches already logged via an accessible `logger` or `log`.
The migration wraps the thrown value through `classifyUnknownError(raw,
fallback)` and emits the log line with the structured `[code]` prefix.
Observable behaviour — return values, fail strings, DB writes — is
unchanged; the only addition is a machine-readable `code` on every log
line so future telemetry can bucket by category.

Sites migrated (11 files, 20 catch sites):

  src/main/browser-action-service.ts             (2) approval lookup, payload JSON
  src/main/ipc-credentials-handlers.ts           (4) save, remove, test lookup, test resolve
  src/main/hire-hook.ts                          (3) getAgent, connector def, outer exception
  src/main/checkpoint-engine.ts                  (2) onTaskStarted, onDocumentDrafted
  src/main/document-lifecycle.ts                 (1) getRun while deriving taskId
  src/main/skills/runtime.ts                     (2) builtin statSync, builtin readdir
  src/main/worker/handlers/run-status-handler.ts (1) run not found during status
  src/main/worker/handlers/run-log-handler.ts    (1) lease renewal lookup
  src/main/notification-service.ts               (1) notification.close during destroy
  src/main/updater-service.ts                    (1) autoUpdater.checkForUpdates
  src/main/ipc-support.ts                        (2) ipc handler throw, emitEvent send

Why only 20 tonight: 248 catches exist in src/main/. Rewriting them all
in one commit is high-diff-risk (collides with Linus's in-flight
database/backup/migration work on database/*, connector-definitions-*,
message-* and scripts/*) and low-value (pure-library catches with no
accessible logger produce dead `void classified` constructs). The
taxonomy module is committed separately so the remaining 228 sites can
land incrementally via the C6.2 proposal.

All 11 files selected sit outside Linus's working set. Verified:
  - pnpm typecheck   clean
  - pnpm test:unit   498 passed
2026-04-20 12:22:52 -04:00
ZacharyZhang-NY a3006f0b38 Assign default skills when a hire is approved
The CEO's operating prompt instructed the model to inspect each direct
report every wake, compare assigned skills to role, and POST missing
ones — business logic routed through the LLM at O(reports) API calls
per heartbeat. The guarantee was unreliable and expensive.

Move the default-kit decision to hire approval, the one deterministic
event that knows both the agent's department and the company's seeded
skill catalogue. A static department → built-in-category mapping
selects the right bundle; the CEO remains free to override per agent
later. Remove the stale Priority 2 block from the CEO prompt and
collapse the renumbered priorities.
2026-04-19 19:18:01 -04:00
ZacharyZhang-NY 00b0e0aaed refactor(ui+skills): split import.ts + primitives.tsx god files
Two more > 400 LOC modules split along responsibility seams. Zero
behavior change — public APIs re-export from the original file paths,
so every consumer import stays working.

| Original                              | Split into                    | LOC each |
|---------------------------------------|-------------------------------|----------|
| src/main/skills/import.ts (458)       | import (orchestrator) /       | 132      |
|                                       | discovery (findSkillMdFiles,  | 153      |
|                                       | buildLocalInventory)          |          |
|                                       | materialize                   | 24       |
|                                       | import-local                  | 147      |
|                                       | import-url                    | 85       |
| src/renderer/components/ui/           | primitives (barrel re-export) | 11       |
| primitives.tsx (419)                  | Card / Eyebrow / Badge /      | 9–59     |
|                                       | Spinner / Panel / Field /     | each     |
|                                       | Input / Select / TextArea /   |          |
|                                       | ActionButton / SectionButton  |          |

`createSkillImporter` still returns `{ importFromSource, importFromGitHub,
importFromLocal, importFromUrl, cleanupSkillFiles, parseGitHubUrl,
findSkillMdFiles, buildLocalInventory }` — every existing consumer
resolves unchanged.

`primitives.tsx` is now a pure barrel re-exporting the 11 primitives.
Consumers can migrate to direct imports (`../ui/Card`) opportunistically
but the barrel path keeps working.
2026-04-17 16:35:30 -04:00
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 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 883fd8e118 fix(skills): honor agent_skill_assignments in runtime skill projection
After Wave D1 switched to pi-coding-agent's loadSkills, the runtime
ignored the per-agent assignment table entirely — every agent saw the
full company catalog. The UI (SkillAgentToggles) still let users curate
which skills each agent should see, so user decisions silently did
nothing.

Pi's progressive-disclosure model accepts a skillPaths list at load
time. Use that to pass only assigned skills, preserving the user's
curation while keeping the description-only prompt footprint.

- listRuntimeSkillEntries reads db.listAgentSkillAssignments(agentId),
  filters company skills to the assigned set, passes only those paths
  to loadSkills.
- Empty assignment set short-circuits (no SKILL.md scan, no prompt
  section).
- connectorId parameter stays for signature compatibility; no current
  connector-specific policy applies.

UI contract unchanged. Runtime now obeys it.
2026-04-17 15:05:20 -04:00
ZacharyZhang-NY 5547c29491 update 2026-04-17 14:03:56 -04:00
ZacharyZhang-NY 5ce3c8ff76 refactor(skills): hoist ROLE_SKILL_CATEGORIES to module constant
P2-43: The role → skill-category map was declared inside
assignRoleSkillsToAgent, making every call reallocate the object and
hiding the coupling between these strings and on-disk category names.
Move to a module-level constant with a docblock noting that mixed-case
names (iOS-Dev, Video-maker) must match the filesystem layout.
2026-04-16 23:03:44 -04:00
ZacharyZhang-NY 8cf6be05bb fix(skills): enforce per-file and aggregate size caps for imported SKILL.md content 2026-04-16 21:58:32 -04:00
ZacharyZhang-NY c5c3919c78 fix(skills): bound local skill walk with depth, file count, and symlink loop protection 2026-04-16 21:57:29 -04:00
ZacharyZhang-NY 9145572422 update 2026-04-05 20:14:48 -04:00
ZacharyZhang-NY 67c757f481 update 2026-04-04 12:03:50 -04:00