You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 4184273
Browse filesBrowse the repository at this point in the historyBrowse files
chore: retire hackathon framing — reposition repo as a product
- delete DELIVERY.md (hackathon submission playbook; preserved in history)
- AUDIT.md: reframe review-window/reviewer language as hosted-deploy
requirements; keep findings and dated record intact
- README/FORGE.md: release gate wording, drop walkthrough/reviewer refs
- code comments/test names: remove "judges"/"demo" phrasing
- package.json: drop DELIVERY.md from published files and format globs
- .gitignore: guard personal CV pdf from ever being committed
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The codebase is **functionally strong and well-tested**. This audit is about the gap between "tests pass" and "production-grade hosted submission that survives reviewers on a remote box." Findings are ranked by what actually loses points or breaks in the review window (Jul 14 – Aug 6).
17
+
The codebase is **functionally strong and well-tested**. This audit is about the gap between "tests pass" and "production-grade hosted deployment that survives on a locked-down remote box." Findings are ranked by what actually breaks in unattended hosted operation.
18
18
19
19
---
20
20
@@ -46,7 +46,7 @@ Every finding F1–F12 is fixed and verified. Suite grew **282 → 300 tests** (
46
46
## Resolved this session (2026-06-12)
47
47
48
48
-**Architecture / MCP story (was the open question behind F8):** The Slack agent now drives the DocForge MCP server over a stdio child process (`src/forge/mcp-client.ts`); `producePdf` routes create → compile → lint → export through real MCP tool calls, with the in-process path kept as a transport fallback (`FORGE_MCP=off` forces it). Proven by `tests/forge-mcp-pipeline.test.ts` (spawns the server, yields a real PDF, confirms the export tool returns the sanitized `output.pdf` basename). The compile tool was split: plain `docforge_compile_document` (bot path) + `docforge_compile_document_async` (task-based, for the MCP-Tasks operation) — because a plain `callTool` on the task tool returns *"No task store provided for task-capable tool."* (verified). **283/283 tests pass.**
49
-
-**F8 (AI fiction):** Removed `ANTHROPIC_API_KEY` from deploy env (DELIVERY.md §9) and corrected the "LLM may refine" claim (FORGE.md §2). Eligible technology is now stated as **MCP server integration**; determinism is framed as the differentiator. No LLM — and the docs no longer pretend otherwise.
49
+
-**F8 (AI fiction):** Removed `ANTHROPIC_API_KEY` from the documented deploy env and corrected the "LLM may refine" claim (FORGE.md §2). The integration surface is stated as **MCP server integration**; determinism is framed as the differentiator. No LLM — and the docs no longer pretend otherwise.
**Still open (unchanged, do not lose these):** F1 (offline packages), F2 (Dockerfile), F3 (HTTP manifest), F4 (event dedup), F5 (double-confirm race), F6 (download host pin), F7 (CSV cap on Slack path), F10 (snippet allowlist), F11 (linter), F12 (download test). The MCP work **adds** a small dependency to F2's deploy bundle: the container must ship `dist/index.js` (the bot spawns it) and the bot + server must share `DOCFORGE_DATA_ROOT` (the PDF path is resolved by `document_id` from that shared volume).
@@ -55,17 +55,17 @@ Every finding F1–F12 is fixed and verified. Suite grew **282 → 300 tests** (
55
55
56
56
| # | Finding | Severity | Area |
57
57
|---|---------|----------|------|
58
-
| F1 | Typst compile **requires network** to fetch `@preview/*` packages — contradicts "offline cache" non-negotiable; **breaks on a locked-down reviewer container**|**Critical**| Deploy / Security |
59
-
| F2 |**No Dockerfile / deploy artifact** despite DELIVERY.md §9 requiring one for the review window|**Critical**| Deploy |
60
-
| F3 | Manifest is **socket-mode only**; DELIVERY.md §9 + §13 require HTTP (`socket_mode_enabled: false`) for the unattended review window|**High**| Deploy |
58
+
| F1 | Typst compile **requires network** to fetch `@preview/*` packages — contradicts "offline cache" non-negotiable; **breaks in a locked-down container**|**Critical**| Deploy / Security |
59
+
| F2 |**No Dockerfile / deploy artifact** despite the delivery plan requiring one for hosted operation|**Critical**| Deploy |
| F6 | Slack file download **does not validate `url_private_download` host**; bot token sent as `Authorization: Bearer` to an event-supplied URL (defense-in-depth — events are signature-verified) |**Medium**| Security |
| F8 |**No LLM anywhere**, yet submission copy says "agent / LLM may refine" and deploy env requires`ANTHROPIC_API_KEY` — reality/narrative mismatch reviewers will probe|**Medium**| Positioning |
65
+
| F8 |**No LLM anywhere**, yet product copy said "agent / LLM may refine" and the deploy env required`ANTHROPIC_API_KEY` — reality/narrative mismatch |**Medium**| Positioning |
66
66
| F9 |`.slack/`, `.residual-sessions/`, `.env.sample`**not in `.gitignore`** — one `git add -A` from leaking CLI state |**Medium**| Security / Hygiene |
67
67
| F10 |`typst_snippets` validator is a **denylist**; `#calc.fact(99999)`-style compute injection passes (bounded only by timeout) |**Low**| Security |
68
-
| F11 |**No linter/formatter** (no ESLint/Prettier config) — the "amateurish" surface the brief calls out|**Low**| Polish |
68
+
| F11 |**No linter/formatter** (no ESLint/Prettier config) — an avoidably unpolished surface|**Low**| Polish |
69
69
| F12 | File-download path has **zero test coverage**|**Low**| Test gap |
70
70
71
71
---
@@ -88,15 +88,15 @@ error: failed to download package (… Connection refused) → no PDF produced
88
88
```
89
89
Note: `--package-path <dir>` does **not** fix this — it *adds* a search path; Typst still falls back to the network/default cache for `@preview/*`. The fix must vendor packages into a path Typst treats as the source *and* block the network fallback (e.g. `--package-cache-path` pre-populated, plus container egress rules).
90
90
91
-
DOCFORGE.md §15 lists as a **non-negotiable**: *"No network access during compile (offline package cache)"* and *"Package allowlist only."* Neither is enforced. HARDENING.md already flags this as "Medium / unaddressed" — it is actually **Critical for hosted review** because:
92
-
- A reviewer running in a container with no warm cache and egress restrictions gets **compile failures on first use** of any template that imports a package (research_report, all chart/markdown paths).
91
+
DOCFORGE.md §15 lists as a **non-negotiable**: *"No network access during compile (offline package cache)"* and *"Package allowlist only."* Neither is enforced. HARDENING.md already flags this as "Medium / unaddressed" — it is actually **Critical for hosted deploys** because:
92
+
- A container with no warm cache and egress restrictions gets **compile failures on first use** of any template that imports a package (research_report, all chart/markdown paths).
93
93
- The board-pack (`kpi_report`) and any charted template depend on `primaviz`.
94
94
95
95
**Fix.** Vendor the three pinned packages into the repo (e.g. `vendor/typst-packages/preview/...`) and pass `--package-path vendor/typst-packages` (or set `TYPST_PACKAGE_PATH`) in `runTypst`. Add a build/CI step that asserts the vendored versions match `template.json` pins. This simultaneously closes the "offline cache" and "allowlist only" gaps.
96
96
97
97
### F2 — No deployment artifact
98
98
99
-
`find . -name "Dockerfile*"` → none. DELIVERY.md §9 explicitly requires a container with Node 20+, Typst 0.14.2, build artifacts, and `GET /health`. The `/health` route exists in `src/slack/app.ts` (HTTP mode only), but there is nothing to actually deploy. Without F1 + F2 fixed, the "Deploy uptime 24h+" pre-submit gate (§13) cannot be met.
99
+
`find . -name "Dockerfile*"` → none. The delivery plan explicitly required a container with Node 20+, Typst 0.14.2, build artifacts, and `GET /health`. The `/health` route exists in `src/slack/app.ts` (HTTP mode only), but there is nothing to actually deploy. Without F1 + F2 fixed, the "Deploy uptime 24h+" gate cannot be met.
100
100
101
101
**Fix.** Add a `Dockerfile` (Node 22 base, install pinned Typst, `npm ci && npm run build`, vendored packages from F1, `CMD node dist/slack/app.js`) and a smoke step. Wire `SLACK_SOCKET_MODE=false`.
102
102
@@ -105,12 +105,12 @@ DOCFORGE.md §15 lists as a **non-negotiable**: *"No network access during compi
105
105
## High findings
106
106
107
107
### F3 — Manifest is socket-mode only
108
-
`slack/manifest.json` has `socket_mode_enabled: true` and no `request_url`. Socket mode needs a live `slack run` / long-lived process with `SLACK_APP_TOKEN`; DELIVERY.md §9 says *"Laptop `slack run` is insufficient"* for the Jul 14–Aug 6 window and requires HTTP (`POST /slack/events`). The code **supports** HTTP (`ExpressReceiver` branch in `app.ts`, `detectMode()` in `config.ts`) — only the manifest and deploy config are missing. Ship an HTTP manifest variant.
108
+
`slack/manifest.json` has `socket_mode_enabled: true` and no `request_url`. Socket mode needs a live `slack run` / long-lived process with `SLACK_APP_TOKEN`; a laptop `slack run` is insufficient for unattended hosting, which requires HTTP (`POST /slack/events`). The code **supports** HTTP (`ExpressReceiver` branch in `app.ts`, `detectMode()` in `config.ts`) — only the manifest and deploy config are missing. Ship an HTTP manifest variant.
109
109
110
110
### F4 — No Slack event de-duplication
111
111
`grep` for `retry_num` / `x-slack-retry` / dedup → **nothing**. Slack redelivers an event if not acked within 3s, up to 3×. The workflows ack fast (Bolt auto-ack) but the *expensive* work (gather → compile → upload) runs after, and `app_mention` handlers have no idempotency key. Two real consequences:
112
112
- A slow gather/compile that pushes the handler past 3s on the *first*`app_mention` can trigger a retry that starts a **second** workflow run.
113
-
- Combined with F5, this is the most likely way a reviewer sees duplicate output.
113
+
- Combined with F5, this is the most likely way a user sees duplicate output.
114
114
115
115
**Fix.** Drop events whose `x-slack-retry-num` header is set (Bolt exposes it on the receiver), or keep a short-TTL `Set<event_ts>` and no-op on repeats.
... await compileAndUploadX(...) // seconds of work
122
122
deletePendingX(id); // only now removed
123
123
```
124
-
The pending entry is **not claimed before the await**. A reviewer double-clicking "Approve" (likely — the button gives no disabled-state feedback while compiling) passes `getPendingX` twice → two compiles → two `files.uploadV2`. Also true if F4 fires.
124
+
The pending entry is **not claimed before the await**. A user double-clicking "Approve" (likely — the button gives no disabled-state feedback while compiling) passes `getPendingX` twice → two compiles → two `files.uploadV2`. Also true if F4 fires.
125
125
126
126
**Fix.** Atomically claim-and-delete *before* compiling: `const pending = takePendingX(id); if (!pending) return;` where `takePendingX` deletes on read. Re-store only if you need it for an error path.
### F8 — "Agent / LLM" narrative vs. deterministic reality
148
148
`grep` for `@anthropic`/`claude-agent-sdk`/`openai`/`generateText` across `src/` → **nothing**. Forge is keyword routing (`router.ts`) + deterministic helpers (`workflow-mappers/workflows.ts`). That is *more reliable* and defensible — but:
149
-
-submission copy (DELIVERY.md §2) and FORGE.md §2 say*"LLM may refine after helper"* — it doesn't.
150
-
-DELIVERY.md §9 lists`ANTHROPIC_API_KEY` as a required deploy env var that nothing reads.
149
+
-product copy (FORGE.md §2) said*"LLM may refine after helper"* — it doesn't.
150
+
-deploy docs listed`ANTHROPIC_API_KEY` as a required env var that nothing reads.
151
151
152
152
Reviewers in "New Slack Agent" track will ask "where's the agent intelligence?" Either (a) **lean into determinism** as the differentiator ("schema-validated, no hallucinated money/KPIs — by construction") and drop the LLM/`ANTHROPIC_API_KEY` claims, or (b) add a genuine LLM refinement step (Claude via `@anthropic-ai/sdk`) on the *prose* fields only, never pricing/KPIs. Pick one; the current half-claim is the weakest spot for the pitch.
153
153
@@ -183,7 +183,7 @@ No ESLint/Prettier config. For a "looks amateurish" brief this is the concrete l
183
183
184
184
## Recommended fix order (highest delivery ROI first)
185
185
186
-
1.**F1 + F2 + F3** (deploy bundle): vendor packages offline, Dockerfile, HTTP manifest. Without these the submission can't survive the review window. *(~half a day)*
186
+
1.**F1 + F2 + F3** (deploy bundle): vendor packages offline, Dockerfile, HTTP manifest. Without these the app can't survive a locked-down hosted environment. *(~half a day)*
187
187
2.**F4 + F5** (dedup + claim-before-compile): kills duplicate-PDF, the most visible live defect. *(~1–2h)*
0 commit comments