Skip to content
Open
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
192 changes: 192 additions & 0 deletions GRAPHIFY_FINDINGS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
# Graphify codebase audit — findings

A one-off structural audit of the monorepo, generated by building a
[graphify](https://github.com/Graphify-Labs/graphify) knowledge graph of the
code and sending targeted audit agents into the hotspots the graph surfaced.

> **Method.** The code graph is deterministic (tree-sitter AST, no LLM,
> nothing left the machine): 2,764 files → 16,839 nodes / 44,925 edges, 99% of
> edges `EXTRACTED` (read directly from source) vs 1% `INFERRED`. The graph's
> `affected` / `path` / `explain` traversals identified the load-bearing hubs
> and complexity hotspots; those exact files were then read and audited. Every
> finding below cites `file:line`. Findings marked *graph-derived* were flagged
> by the graph and spot-verified; the rest come from reading the code.
>
> The doc/PDF/image semantic layer (249 docs + 96 images) was **not** ingested —
> the graphify `claude` extraction backend requires `ANTHROPIC_API_KEY` and only
> community *labeling* uses the local CLI. Re-run with a key set to include it.

## Architecture at a glance

The graph recovered Studio's shape from imports/calls alone. Largest labeled
communities: `MCP UI Resources & Client Hooks` (248), `Dialog UI Components`
(214), `Tool IO Types` (209), `Tool Definition & OAuth Providers` (166), `MCP
Proxy Factory` (161), `Agent Loop State`, `Registry Public API`. The
load-bearing hubs are exactly the documented ones — `StudioContext` (blast
radius 152), `defineTool()` (143 tool files import it), `cn()` / `Button()`
(UI). **No direct circular imports.** The codebase is heavily UI-weighted — most
of the top-20 god files/functions are React components, not backend.

---

## 🔴 High — real bug risk

### 1. Synchronous network git on the sandbox daemon request path
`packages/sandbox/daemon/git/rebase-onto-base.ts:283,297,331,357`

`rebaseOntoBase()` (reached via `routes/git.ts:693`) runs `fetch` / `push
--force` / `rebase` through `spawnSync` (`git-sync.ts:33`). This is the exact
scenario CONTRIBUTING rule #1 forbids: a network git call freezes the single Bun
event loop for seconds→30s, the `/health` probe misses, and Studio tears the
sandbox down **mid-rebase**. `gitAsync` already exists (`git/git-async.ts`) —
rebase just never got migrated. Sharpest edge in the codebase.

**Fix:** convert `rebase-onto-base.ts` to `gitAsync` and `await` each step (it's
already a linear sequence). Move `syncOriginRemote`
(`git/sync-origin-remote.ts:15`, also on the publish path) too.

### 2. Uncapped synchronous file read in the daemon fs route
`packages/sandbox/daemon/routes/fs.ts:173-174`

`fs.readFileSync(filePath,"utf-8")` then `.split("\n")` with **no size ceiling**
(images are capped at 5MB, text isn't). One `read` of a big log blocks the loop
twice (sync I/O + CPU split) → missed health probe + memory blow-up. The whole
`branch-status.ts` monitor (`:215,244`, sync `git` + `readFileSync` +
`createHash` per dirty file, on the 250ms `fs.watch` / 3s poll path) and the
glob walk (`:751` `readdirSync`, `:910` `statSync`) are sync on the same loop.
The upload/download handlers in that same file were made async — these core
handlers were left behind, as were write/edit/rename/mkdir/unlink
(`:199,237,278,315,365`).

**Fix:** switch to `node:fs/promises` + `await`; `gitAsync` for the git calls.

### 3. Permanent setup failures thrown as plain `Error` → automations re-fire forever
`apps/mesh/src/api/routes/decopilot/dispatch-run.ts:974,1073,1092`

"Model not allowed for your role" (`:974`), "not allowed to write to this
thread" (`:1073`), and async-research-model-in-wrong-slot (`:1092`) throw a plain
`Error`, not `PermanentRunError`. The automation workflow
(`automations/dbos-workflow.ts:463`) only calls `deactivateAutomationStep` on
`isPermanentRunError(err)`. So a scheduled automation pinned to a now-disallowed
model **fails identically on every cron tick forever**, burning a run each time
with no operator signal — exactly what `agent_not_found` (already permanent) was
meant to prevent. These configs can never succeed.

**Fix:** throw `PermanentRunError` with new codes (`model_not_allowed`,
`thread_forbidden`, `model_not_streamable`).

---

## 🟠 Medium — security-adjacent & correctness

### 4. Full-access (`*:*`) MCP key leaked on aborted dispatch
`apps/mesh/src/api/routes/decopilot/dispatch-run.ts:347,358` — leak paths `:1953,2013`

`mintMcpEndpoint` creates a Better Auth API key with `permissions: {"*":["*"]}`
and a 1h TTL, minted eagerly inside `prepareRun` *before* the payload-size check
(`:1953`) and org-slug resolution (`:2013`). On either failure — or any throw in
`prepareRun` after minting — the wildcard key is never revoked and stays valid
for up to an hour.

**Fix:** revoke the minted key in `failPreparedRun` / the `prepareRun` catch, or
defer minting until after those gates pass.

### 5. SSO check does an uncached DB query on every authenticated request
`apps/mesh/src/api/app.ts:1723` → `apps/mesh/src/storage/org-sso-config.ts:11`

`orgSsoConfig.getByOrgId` is an uncached `selectFrom("org_sso_config")` run for
*every* request with an org + user — including the common case where SSO is not
enforced (returns null). Adds a serial DB round-trip to the latency of every
API/MCP request; enforced orgs pay for a second query.

**Fix:** back `getByOrgId` with a short-TTL cache keyed by orgId, like the
adjacent `memberRoleCache` (`app.ts:1349`). Invalidate on SSO-config writes.

### 6. `resolveOrgFromPath` registered twice for the same hot route
`apps/mesh/src/api/app.ts:1698` **and** `app.ts:1906`

Both register `app.use("/api/:org/files/*", resolveOrgFromPath)`. The middleware
has no early-return when the org is already resolved, so every `/api/:org/files/*`
request (image serving, iframed thread outputs) does the org lookup +
`rebindOrgScope` **twice**.

**Fix:** delete the redundant registration (L1906).

---

## 🟡 Complexity / simplification

### 7. Hand-rolled async primitives — 7 confirmed `@decocms/std` violations
The repo mandates `@decocms/std` for all sleep/retry/backoff. Confirmed hits in
production source (each also needs the import added):

| # | Location | Issue | Replace with |
|---|----------|-------|--------------|
| 1 | `apps/mesh/src/web/components/sandbox/hooks/sandbox-events-context.tsx:488` | `Math.min(BASE * 2 ** attempt, MAX)` SSE reconnect backoff, **no jitter** | `exponentialBackoffWithJitter` (CLAUDE.md names SSE reconnect as *the* use case) |
| 2 | `apps/mesh/src/cli/lib/port-wait.ts:32` | Promise-wrapped `setTimeout` poll delay | `sleep` |
| 3 | `packages/sandbox/server/provider/agent-sandbox/client.ts:410` | same | `sleep` |
| 4 | `packages/sandbox/server/provider/agent-sandbox/client.ts:512` | same | `sleep` |
| 5 | `apps/mesh/src/harnesses/decopilot/built-in-tools/cluster-research-job.ts:272` | `setTimeout` race-timeout | `sleep` |
| 6 | `apps/mesh/src/web/components/monaco/index.ts:71` | `setTimeout(…,100)` settle delay | `sleep` |
| 7 | `apps/mesh/src/web/components/sandbox/preview/preview.tsx:777` | `setTimeout` dev-server settle | `sleep` |

Borderline (not clear-cut): `web/hooks/use-merged-store-discovery.ts:117`
(delay-less manual retry loop → could be `retry`), and
`packages/sandbox/daemon/process/pty-spawn.ts:91` (sync `Bun.sleepSync` retry —
std has no sync equivalent, unfixable without making the fn async).

### 8. `mountLegacy` boilerplate copy-pasted 9×
`apps/mesh/src/api/app.ts` — L1747, 1811, 1817, 1824, 1912, 1928, 1954, 1973, 1989

Identical "new Hono + `createLogDeprecatedRoute({ mountPath })` + `.route`" block,
each with a hand-typed `mountPath` string that must match the mount arg. A
copy-paste with a stale `mountPath` silently mislabels deprecation logs.

**Fix:** a `mountLegacy(app, mountPath, routes)` helper collapses all nine to one
line each and removes the string-matching footgun.

### 9. Two un-cleared timers
- `apps/mesh/src/api/app.ts:1644` — `setInterval(cleanupExpiredApiKeys, 24h)` is
never cleared or tracked in `shutdown`; in dev HMR/tests each `createApp()`
leaks a 24h interval whose closure pins the old `database.db`. The sibling
`asyncResearchJobSweeper` (`:1041`) *is* disposed — so this is an inconsistency.
- `apps/mesh/src/api/app.ts:216` — `rejectAfter()`'s 5s `setTimeout` in the health
check is never cleared on the winning path; K8s probes leave dangling timers
and shutdown can be held up to 5s.

**Fix:** capture the handles and `clearInterval`/`clearTimeout` in the cleanup
closure / a `finally`.

---

## 🟢 Dead code
- `createDesktopSubtaskDispatcher`
(`packages/harness/src/decopilot/built-in-tools/desktop-subtask-dispatcher.ts:48`)
— zero inbound edges in the graph; grep confirms **1 occurrence total** (the
definition). Genuinely dead. *(graph-derived, verified)*

The graph flagged 214 zero-inbound functions, but most are cross-package exports
the intra-file AST can't resolve; `knip` already covers this class, so only
grep-verified entries are reported here.

---

## Suggested order
1. Migrate `rebase-onto-base.ts` + `routes/fs.ts` + `branch-status.ts` off sync
fs/spawn (#1, #2) — can silently kill live sandboxes.
2. Make the three setup failures `PermanentRunError` (#3) — cheap; stops infinite
cron failure loops.
3. Revoke the leaked `*:*` key (#4).
4. Batch the rest (SSO cache, dup middleware, std-primitive sweep, `mountLegacy`,
timer leaks) into a cleanup PR.

## Reproducing the graph
```bash
uv tool install graphifyy
graphify extract . --code-only # deterministic AST graph, no API key
graphify label graphify-out --backend claude-cli # name communities via Claude CLI
graphify query "how does authentication work" # traverse instead of grep
graphify affected "define-tool.ts" # blast radius of a change
graphify path "defineTool()" "StudioContext" # trace the link between two nodes
```
Add `ANTHROPIC_API_KEY` and drop `--code-only` to also ingest docs/PDFs/images.