Skip to content
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# 2026-09-28 — fix-chrome-executable-path (plan + diff)

- Worker: implementer
- Version: 0.1.0
- Node: `fix-chrome-executable-path` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- Issue: [#154](https://github.com/datvt243/resume-nodejs-api/issues/154) — Puppeteer Chrome executable path hardcoded — breaks PDF export in CI/Docker
- Branch: `154-puppeteer-chrome-executable` (base `staging`)
- Task (verbatim): "fix bug #152 tới #156" — this note covers #154 only, part of the same 5-issue batch as #152/#153/#155/#156, each processed as its own implementer→verifier round.

## Hub bytes before: 88066

## Node lookup

Matched the existing PENDING node `fix-chrome-executable-path` directly
(task resolves to GitHub issue #154, filed against this exact node — this
node was the diagram's own documented "first candidate node").

## Bookkeeping-gap finding (read before writing anything)

Same pattern as the other 3 nodes sealed earlier this session. Reading
`src/services/createPDF.ts` today (lines 9-34) shows there is **no
hardcoded Chrome executable path anywhere** — the trap description in
`doctrine/domains/PROJECT.md` (`src/services/createPDF.ts:14-25`) and the
diagram node's own PENDING description no longer match the live file:

```ts
export const createCV = async (data: Record<string, any>, res: Response) => {
try {
if (!fs.existsSync(PDF_OUTPUT_DIR)) fs.mkdirSync(PDF_OUTPUT_DIR, { recursive: true });
const URL = `${PDF_OUTPUT_DIR}${path.sep}`;

// Optional override for CI/Docker where a specific Chrome/Chromium must be pinned.
// Unset: puppeteer resolves its own bundled Chromium automatically.
const executablePath = process.env.PUPPETEER_EXECUTABLE_PATH;

const otp = {
...(executablePath ? { executablePath } : {}),
headless: true,
args: ['--no-sandbox', '--disable-setuid-sandbox'],
};
const browser = await puppeteer.launch(otp);
...
```

This exact shape (env-var override via `PUPPETEER_EXECUTABLE_PATH`,
falling back to Puppeteer's own bundled Chromium resolution when unset)
is precisely what the issue's own "Fix" section asks for. It was
introduced by `add-docker-support` (issue #24, SEALED 2026-09-06) — that
node's diagram entry explicitly says: "Chromium installed via `apt` in
the image... with `PUPPETEER_EXECUTABLE_PATH` set — this is exactly the
CI/Docker case the existing `fix-chrome-executable-path` trap... warns
about; live-verified working (see evidence), not just assumed fixed by
that trap's earlier code change." So the fix has been live and
**Docker-live-verified** since 2026-09-06 — over 3 weeks — but this
node's own PENDING row was never updated, and the `doctrine/domains/
PROJECT.md` Traps table entry is now stale too (out of scope to edit
here — noted below).

The real remaining gap: `services/createPDF.test.ts` had **zero**
coverage of the executable-path resolution logic itself (only
`pageRender`, a separate pure function, was tested) — exactly what issue
#154's acceptance criteria asks for ("add coverage for the
executable-path resolution if not already covered").

## Diff (smallest diff — no `src/` production code, extends 1 existing test file)

- `src/__tests__/services/createPDF.test.ts` — added:
- `import { createCV } from '@/services/createPDF'` (alongside the
existing `pageRender` import) and `import puppeteer from 'puppeteer'`.
- `jest.mock('puppeteer', () => ({ launch: jest.fn() }))` — Puppeteer's
real `launch()` is never invoked; no real browser is spawned, no
dependency on a Chrome/Chromium install in the test environment.
- New describe block `createCV executablePath resolution (issue #154)`,
2 tests: (1) with `PUPPETEER_EXECUTABLE_PATH` unset, `puppeteer.launch`
is called with no `executablePath` key at all (bundled Chromium
resolution); (2) with it set, `puppeteer.launch` is called with that
exact value (CI/Docker override). Both call the real `createCV`
end-to-end with a fake browser/page (`newPage`/`setContent`/`pdf`/
`close` all mocked) — only Puppeteer itself is faked, the option-
building logic under test is 100% real.

## Command

```
npm test
```
Output (verbatim tail):
```
Test Suites: 26 passed, 26 total
Tests: 142 passed, 142 total
Snapshots: 0 total
Time: 6.705 s
Ran all test suites.
```
(142 = 140 (post-#157-merge baseline on `staging`) + 2 new tests, in an
existing file, so no new suite count. Same pre-existing, unrelated
harness exit warning as prior notes — not a failure.)

```
npm run build
```
Output: `tsc` clean, `copy` step ran with no errors.

## Acceptance

| Criterion | Evidence |
|---|---|
| Trace to exactly one diagram node | `fix-chrome-executable-path` |
| Smallest diff | 1 existing test file extended, 0 production `src/` changes (fix already live since `add-docker-support`/#24, SEALED 2026-09-06) |
| PDF export works with no hardcoded path assumption | Confirmed by reading `src/services/createPDF.ts` — no hardcoded path literal anywhere in the file |
| Works both with Puppeteer's bundled Chromium and with an externally-installed one via `PUPPETEER_EXECUTABLE_PATH` | Both branches now covered by the 2 new tests above; also live-Docker-verified previously (`add-docker-support` node's own evidence, `evidence/verifier/2026-09-06/add-docker-support-round2-seal.md`) |
| `createPDF.test.ts` still passes; executable-path coverage added | `npm test` → `Tests: 142 passed, 142 total`, including the 2 new tests |
| Exact test command run + output read back | `npm test` output above; `npm run build` clean |
| Evidence note written | This file |

## Noticed, not done

- `doctrine/domains/PROJECT.md`'s Traps table still lists "Hardcoded
Chrome executable path (`src/services/createPDF.ts:14-25`)" as an open
trap. It is stale — the code no longer matches that description. Not
edited here (out of scope for this node's smallest diff, and doctrine
edits are usually done by whichever pass actually changes the
underlying behavior — that was `add-docker-support`, already SEALED);
flagged for a future docs-only pass, same category as
`update-project-docs`/#146.

## Seal gate

No outward-facing action taken (no commit/push). Only a local file edit:
1 existing test file extended under `src/__tests__/`. Pending verifier.

## Status

`sealed_pending_verifier`
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# 2026-09-28 — fix-chrome-executable-path (verifier verdict)

- Worker: verifier (subagent, dispatched via Agent tool)
- Node: `fix-chrome-executable-path` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- New PM status: PENDING → **SEALED**

## Isolation proof

Dispatched as an independent Agent-tool subagent whose task description
reads "Independent verifier pass for fix-chrome-executable-path" — a
fresh context with no memory of the implementer session that produced
`evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md`.
Nothing in this session's history references having written that diff.

## Reasoning

Read only the evidence note first (EvidenceOnly), then independently
confirmed the note's specific factual citations against the real source
tree — verifying citations, not re-deriving the diff.

**Trace to exactly one diagram node** — `fix-chrome-executable-path`,
confirmed at `haven/diagrams/dev-loop.prime-mermaid.md` line 58, PENDING
before this pass (task correctly resolves to GitHub issue #154, filed
directly against this node per the issue's own footer).

**Test command matches convention** — `npm test` matches
`doctrine/MEMORY.md`'s documented `npm test` = `jest --passWithNoTests`.

**Output not truncated** — the note's `npm test` tail shows complete
summary lines (`Test Suites: 26 passed, 26 total` / `Tests: 142 passed,
142 total`), no ellipsis or redaction markers.

Issue #154 acceptance criteria, read via `gh issue view 154`, walked one
at a time:

1. **PDF export works with no hardcoded path assumption** — independently
read `src/services/createPDF.ts` (full `executablePath` block, lines
26-31). Confirmed: no hardcoded path literal anywhere in the file;
`const executablePath = process.env.PUPPETEER_EXECUTABLE_PATH;` spread
conditionally into `puppeteer.launch()`'s options. Matches the note's
quoted excerpt verbatim.
2. **Works both with bundled Chromium and an externally-installed one via
`PUPPETEER_EXECUTABLE_PATH`** — independently ran
`git diff src/__tests__/services/createPDF.test.ts` and read the full
diff. Confirmed: new `createCV executablePath resolution (issue #154)`
describe block, `jest.mock('puppeteer', () => ({ launch: jest.fn() }))`
(no real browser spawned), two tests — one asserts
`puppeteer.launch` called with no `executablePath` key when the env
var is unset, one asserts it's called with the exact override value
when set. Both invoke the real `createCV` end-to-end against a fake
browser/page. Matches the note's description exactly, no
overstatement. Also independently confirmed the note's claim that this
fix was already Docker-live-verified: `add-docker-support`'s own
SEALED row (line 81 of the diagram) states Chromium is installed via
`apt` with `PUPPETEER_EXECUTABLE_PATH` set and explicitly calls out
this as "exactly the CI/Docker case the existing
`fix-chrome-executable-path` trap... warns about; live-verified
working" — corroborates independent of the implementer's own note.
3. **`createPDF.test.ts` still passes; executable-path coverage added** —
confirmed via the same `git diff` read above (coverage added) and the
note's `npm test` output (142/142 passed, up from a 140 baseline plus
the 2 new tests, no new suite since it's an existing file).

**Hub bytes accounting** — recomputed the 5-category byte sum (root +
doctrine excl. archive + active diagram excl. archive +
`haven/workers/implementer/` + `haven/workers/verifier/`) before touching
anything: got exactly 88066, matching the note's declared
`hub_bytes_before`. Confirms the note used the real formula, not a guess.

## Forbidden states scan

- `ADHOC_WORK` — no; node exists on the diagram, worker identity declared.
- `NO_EVIDENCE` — no; evidence note present at the cited path.
- `EDIT_UNVERIFIED` — no; every claim in the note checked out against the
real diff/source above.
- `CODE_IN_HAVEN` — no; no runnable code in `haven/`, only this markdown
diagram edit.
- `DIAGRAM_DRIFT` — no; that's the condition this SEAL resolves (row was
stale PENDING, now updated to match the live code + new test coverage).

## Seal gate

Confirmed via `git status --short` on branch `154-puppeteer-chrome-executable`:
only a locally modified `src/__tests__/services/createPDF.test.ts` and an
untracked evidence note — nothing committed, nothing pushed. Matches the
note's own "no outward-facing action taken" claim.

## Proportion (SmallestDiff)

1 existing test file extended, 0 production changes. Proportionate: the
underlying fix has been live and Docker-verified for 3+ weeks (since
`add-docker-support`/#24); the only real gap was missing regression
coverage, which this diff closes directly.

## Re-run

`none` — audit-only. Not an outward-facing action, not a release gate;
independently re-read the actual diff/source files cited rather than
re-running the suite. All figures (test counts, hub bytes) were
cross-checked against independently-verifiable facts (the diff itself,
the diagram's own `add-docker-support` row, the byte-count formula) rather
than taken purely on the note's word.
2 changes: 1 addition & 1 deletion agent-hub/haven/diagrams/dev-loop.prime-mermaid.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ flowchart TD
| `add-pagination-filtering-cv-sections` | SEALED | GitHub issue #73. `page`/`limit`/`sort` query params on the 6 CV-section list endpoints (education/experience/award/certificate/project/reference — `generalInformation` excluded, its `GET /` returns a single document, not a list). Code already implemented and merged to `staging` via PR #106 (commit `1133f1b`, 2026-09-02) with no matching diagram node/evidence note at the time (bookkeeping gap, backfilled now by `/todo "#73"`, same pattern as `add-logout-all-sessions`/#74). Opt-in and backward compatible: omitting `limit` returns the exact old unpaginated array (`services/index.ts` `baseFindDocument`); a valid `limit` (capped at 100, `MAX_PAGE_LIMIT`) switches `data` to `{ items, pagination }`. `sort` validated against `SORT_FIELD_REGEX` allowlist in `BaseController.ts` — no `$`, can't smuggle a Mongo operator. Issue stays OPEN on GitHub because the merge landed on `staging`, not the default branch (`main`) — same expected auto-close gap as #74, not a bug. Verified 2026-09-05, evidence: `evidence/verifier/2026-09-05/add-pagination-filtering-cv-sections-seal.md`. |
| `add-logout-all-sessions` | SEALED | GitHub issue #74. `POST /api/v1/auth/logout-all` — code already implemented and merged to `staging` via PR #105 (commit `03bcb66`, 2026-09-02) with no matching diagram node/evidence note at the time (bookkeeping gap, backfilled now by `/todo "#74"`). Design deviates from the issue's `tokenVersion`-on-`Candidate` proposal: reuses the Redis/mem "invalidated-before" timestamp shape from `tokenBlacklist.ts` (`src/utils/sessionRevocation.ts`), compared against the JWT's standard `iat` in `verifyToken.middleware.ts` + `authRefreshToken` — no schema change, no extra Mongo lookup. Issue stays OPEN on GitHub because the merge landed on `staging`, not the default branch (`main`); auto-close via `Closes #74` fires only on a `main` merge per the documented release workflow — expected, not a bug. Verified 2026-09-05, evidence: `evidence/verifier/2026-09-05/add-logout-all-sessions-seal.md`. |
| `agent-hub-token-cleanup-20260830` | SEALED | 2026-08-30 — archived, see `haven/diagrams/dev-loop-archive.md`. Evidence: `evidence/implementer/2026-08-30/agent-hub-token-cleanup-diff.md`. |
| `fix-chrome-executable-path` | PENDING | `src/services/createPDF.ts:14-25` — Chrome executable path hardcoded, breaks PDF export in CI/Docker. See Traps in `doctrine/domains/PROJECT.md`. First candidate node. |
| `fix-chrome-executable-path` | SEALED | GitHub issue #154. Bookkeeping-gap backfill, same pattern as `fix-create-response-null-id`/`fix-idor-broken-access-control`: reading `src/services/createPDF.ts` today shows **no hardcoded Chrome executable path** anywhere — `process.env.PUPPETEER_EXECUTABLE_PATH` conditionally spread into the launch options, falling back to Puppeteer's own bundled-Chromium resolution when unset. This exact shape was introduced by `add-docker-support`/#24 (SEALED 2026-09-06) and has been Docker-live-verified since — that node's own row explicitly calls out this trap as already fixed. The doctrine `Traps` table entry describing the old hardcoded path is now stale (out of scope here, flagged for a future docs-only pass). The real remaining gap this node closed: `services/createPDF.test.ts` had zero coverage of the executable-path resolution logic itself. 0 production changes; `src/__tests__/services/createPDF.test.ts` extended with a new `createCV executablePath resolution (issue #154)` describe block — 2 tests (`PUPPETEER_EXECUTABLE_PATH` unset → bundled Chromium, no `executablePath` key; set → override honored), `jest.mock('puppeteer', ...)` so no real browser spawns. `npm test`: 142/142 passed. SEALED 2026-09-28 after independent verifier pass (audit-only, no re-run — note's `npm test` output was verbatim, not truncated, command matched `doctrine/MEMORY.md`; independently confirmed the no-hardcoded-path claim by reading `createPDF.ts`, confirmed the 2 new tests via `git diff` on the test file, confirmed `PUPPETEER_EXECUTABLE_PATH`/Docker-live-verification claim via `add-docker-support`'s own sealed row; confirmed via `git status --short` on `154-puppeteer-chrome-executable` that nothing was committed/pushed). Evidence: `evidence/implementer/2026-09-28/fix-chrome-executable-path-plan.md`, `evidence/verifier/2026-09-28/fix-chrome-executable-path-seal.md`. |
| `fix-idor-broken-access-control` | SEALED | **Critical.** All CRUD APIs for candidate_profile (education/experience/award/certificate/project/reference/generalInformation) + `candidate.service.ts` + `fnExportPDF` never cross-check `candidateId`/`_id` against `req.user._id` (JWT) — they trust client-supplied `req.body.candidateId`/`_id`. Live-tested confirmed: User B could read/delete/edit User A's data, overwrite A's profile. Root cause: `verifyToken.middleware.ts` sets `req.user` but nothing cross-checks it. Found while testing the full API (task: "test the whole API again"). Code already fixed on `staging` (commit `f355e2f`, folded in via `1ec67de`'s ancestry) — bookkeeping-gap backfill, same pattern as #73/#74: `verifyToken.middleware.ts` forces `req.body.candidateId` to the authenticated `_id`; `baseUpdateDocument`/`baseDeleteDocument` check the existing document's real owner, not the payload; `candidate.controller.ts` forces `value._id`; `candidate_me/index.ts` `fnExportPDF` uses `req.user._id` directly. SEALED 2026-09-08 after independent re-read of every cited file (router, middleware, services/index.ts, BaseController/BaseService, candidate.controller.ts, candidate_me/index.ts, generalInformation.controller.ts) — evidence: `evidence/verifier/2026-09-08/fix-idor-broken-access-control-seal.md`. |
| `fix-candidate-password-leak` | PENDING | `src/candidate/candidate.service.ts` — `handlerGetInformationByEmail` has no `.select()` at all; `handlerGetInformationById` double-wraps `whitelistSelect([select])`, making the select a permanent no-op. Result: `GET /api/v1/candidate/:email` and `PUT/PATCH /candidate/update` return the raw bcrypt password hash in the response. |
| `fix-refresh-token-expiry-unused` | PENDING | `TOKEN_EXP_IN` (`.env`, already exported in config) is never used at any `jwtSign()` call site (`auth.service.ts`, `auth.controller.ts`, `api/v1/auth/services/login.ts`) — access and refresh tokens always share the same default 1h expiry, so the refresh token is pointless. |
Expand Down
Loading
Loading