test(candidate): add regression coverage for self-delete cascade (#158)

agent-hub/evidence/implementer/2026-09-29/add-candidate-self-delete-plan.md

@@ -0,0 +1,136 @@
+# 2026-09-29 — add-candidate-self-delete (implementer plan)
+
+- Worker: implementer
+- Version: 0.1.0
+- Node: `add-candidate-self-delete` (`haven/diagrams/dev-loop.prime-mermaid.md`)
+- Task (verbatim): Candidate self-delete (issue #158): DELETE
+  /api/v1/candidate, authenticated, using only req.user._id (never a
+  client-supplied id, same IDOR-safe pattern as fix-idor-broken-access-control).
+  Must cascade-delete the candidate's data across all CV section models
+  (education/experience/award/certificate/project/reference/generalInformation,
+  plus application/profile since those now exist) by candidateId, and clean
+  up any uploaded files (CV PDF, section images) tied to that candidate.
+  Acceptance criteria: DELETE /api/v1/candidate removes the candidate
+  document and every CV-section document owned by that candidate; uploaded
+  files (resume PDF, section images) are removed from disk, not just
+  orphaned; a candidate cannot delete another candidate's account under any
+  input (id always comes from req.user._id); regression test covers the
+  cascade across all section models. Diagram node: add-candidate-self-delete
+  (currently PENDING).
+
+## Hub bytes before: 100436
+
+## Finding: already fully implemented, bookkeeping-gap backfill
+Same pattern as `fix-idor-broken-access-control`/`fix-candidate-password-leak`/
+`fix-create-response-null-id`: reading the real code shows the feature is
+already 100% live and correct, just never backfilled with a diagram node
+transition or evidence.
+
+- `DELETE /api/v1/candidate` -> `fnDelete` (`src/candidate/candidate.controller.ts:157-168`)
+  -> `handlerDelete` (`src/candidate/candidate.service.ts:118-148`). Introduced
+  by commit `32953ed` ("feat: add DELETE /api/v1/candidate for self-service
+  account deletion"), predating this hub's own history (before commit
+  `f355e2f`'s bug-fix batch, 2026-08-21) — this node has simply sat PENDING
+  on the diagram since the hub started tracking it.
+- `fnDelete` reads NO client-supplied id anywhere — it calls
+  `handlerDelete((req as any).user?._id, (req as any).lang)` directly, no
+  body/params/query id read at all. Structurally IDOR-safe by construction,
+  not just by convention (nothing to override even if a client tried).
+- `routers/api/v1/index.ts:27` — `router.use('/candidate', verifyToken,
+  routeCandidate)` — confirms the whole `/candidate` router (including this
+  route) sits behind `verifyToken`, so `req.user._id` is genuinely populated
+  by the time `fnDelete` runs.
+- `handlerDelete` (`candidate.service.ts:20-30`) already has a
+  `CV_SECTION_MODELS` array covering ALL 9 models: `generalInformation`,
+  `Experience`, `Education`, `Reference`, `Project`, `Certificate`, `Award`,
+  `Application`, `Profile` — the last 2 were added later, 1-line-each, by
+  `add-application-tracker`/#132 and `add-cv-profile-selection`/#133
+  specifically so this cascade wouldn't orphan those newer sections (both
+  those nodes' own evidence/diagram rows call this out explicitly). Confirms
+  the "plus application/profile since those now exist" part of the task is
+  already satisfied.
+- File cleanup on disk already implemented: `CV_UPLOAD_DIR`/`{_id}-cv.pdf`
+  resume PDF (`candidate.service.ts:136-139`), plus every
+  project/certificate/award image (`IMAGE_SECTION_MODELS`, images collected
+  via `.find({candidateId}, {images:1})` BEFORE those documents are deleted,
+  then `fs.unlinkSync` per file, `candidate.service.ts:123-131,141-145`).
+
+The real remaining gap this node closes: **zero regression tests existed**
+for `handlerDelete` (confirmed via `grep -rn "handlerDelete" src/__tests__`
+before this diff — no hits) — exactly the issue's own 4th acceptance
+criterion ("Regression test covers the cascade across all section models").
+
+## Diff
+| File | Why |
+|---|---|
+| `src/__tests__/candidate/candidate.service.test.ts` | New `handlerDelete` describe block (5 tests), appended to the existing file (already covers `candidate.service.ts`'s other handlers for issue #152). Extended the shared `jest.mock('@/models', ...)` factory to give `deleteMany`/`deleteOne`/`find` real `jest.fn()`s instead of `{}` placeholders — needed by `handlerDelete`, doesn't affect the pre-existing #152 tests (they only ever touch `Candidate.findOne`/`findById`). 0 production changes — the feature was already correct. |
+
+## Bug found and fixed during implementation (self-caught, in the test file only)
+First attempt used `jest.mock('fs')` at the top of `candidate.service.test.ts`
+to control `existsSync`/`unlinkSync`. `jest.mock()` calls are hoisted by
+ts-jest/babel to run BEFORE the file's own `import` statements — so the
+auto-mocked `fs` was already in place when `candidate.service.ts`'s import
+chain (`utils/index.ts` -> `utils/bcrypt.ts` -> `bcrypt`) transitively
+loaded the real `bcrypt` package, whose `@mapbox/node-pre-gyp` native-binding
+resolver calls a REAL `fs.existsSync()` to find its own `package.json` at
+import time. With `fs` auto-mocked, that call returned `undefined`, and the
+whole suite failed to load: `node_modules/bcrypt/package.jsondoes not exist`.
+Confirmed real (not a fluke): reproduced consistently, confirmed `bcrypt`
+loads fine standalone via plain `node -e "require('bcrypt')"`, and traced
+the stack trace to `pre-binding.js` inside `candidate.service.ts`'s own
+import chain. Fixed by switching to `jest.spyOn(fs, 'existsSync')`/
+`jest.spyOn(fs, 'unlinkSync')` in `beforeEach` instead — `jest.spyOn` is a
+normal runtime statement, not hoisted, so it only takes effect after every
+import (including bcrypt's) has already resolved for real. `afterEach(() =>
+jest.restoreAllMocks())` added to fully restore `fs` after this describe
+block. Not added to `doctrine/domains/PROJECT.md`'s Traps table — this is a
+test-authoring footgun specific to `jest.mock('fs')` + any module in the
+same file's import graph that needs real `fs` at load time (bcrypt here),
+not a `src/` runtime bug; noted here for whoever writes the next fs-mocking
+test in this codebase.
+
+## Command
+`npm test` (from `/Users/_david/Workspace/Project/resume/resume-nodejs-api`)
+
+## Output (verbatim, tail)
+```
+A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this, ensure that .unref() was called on them.
+Test Suites: 29 passed, 29 total
+Tests:       160 passed, 160 total
+Snapshots:   0 total
+Time:        7.634 s, estimated 12 s
+Ran all test suites.
+```
+(The "worker process failed to exit gracefully" warning is the same
+pre-existing Jest/open-handle notice seen on prior nodes this session,
+unrelated to this diff — every suite still passed, 0 failures. 160 = 155
+baseline, from the immediately preceding sealed node `add-bulk-import-cv-sections`
+this session, + 5 new `handlerDelete` tests.)
+
+Also ran `npm run build` (tsc && copy) — clean, no typecheck errors, no
+output beyond the copy step.
+
+## Acceptance
+| Criterion | Evidence |
+|---|---|
+| `DELETE /api/v1/candidate` removes the candidate document and every CV-section document owned by that candidate | `handlerDelete` (`candidate.service.ts:133-134`) — `Promise.all(CV_SECTION_MODELS.map(...deleteMany({candidateId:_id})))` then `MODEL.deleteOne({_id})`. Regression test "cascades deleteMany({candidateId}) across every CV section model, then deletes the candidate document itself" asserts all 9 models' `deleteMany` called with `{candidateId: 'cand1'}` AND `Candidate.deleteOne` called with `{_id: 'cand1'}` — part of `Tests: 160 passed, 160 total` |
+| Uploaded files (résumé PDF, section images) are removed from disk, not just orphaned | `candidate.service.ts:136-145` — `fs.unlinkSync` on the CV file path and every collected image path, gated by `fs.existsSync`. 3 regression tests: "removes the candidate's uploaded CV file from disk when one exists", "never calls unlinkSync for a CV file that does not exist on disk", "removes every project/certificate/award image file from disk, collected before those documents are deleted" — all pass |
+| A candidate cannot delete another candidate's account under any input (id always comes from req.user._id) | `fnDelete` (`candidate.controller.ts:157-168`) never reads any id from `req.body`/`req.params`/`req.query` — only `(req as any).user?._id`, populated exclusively by `verifyToken` (confirmed via `routers/api/v1/index.ts:27` mounting `/candidate` behind `verifyToken`). Structural guarantee, not test-covered at the controller layer per this codebase's own established convention (see `candidate.controller.test.ts`'s file header: "the rest of candidate.controller.ts is thin wiring already covered indirectly elsewhere, same precedent as every other controller in this codebase") — `fnDelete` has zero branching/logic to unit-test beyond what's already visible by inspection |
+| Regression test covers the cascade across all section models | 5 new tests in `src/__tests__/candidate/candidate.service.test.ts`'s new `handlerDelete (issue #158)` describe block, part of `Tests: 160 passed, 160 total` |
+
+## Noticed, not done
+- No controller-level test added for `fnDelete` itself — see the 3rd
+  acceptance-criterion row above for why (zero logic to test beyond what
+  the service-level tests + code inspection already prove; matches this
+  codebase's own stated convention for thin controller wiring).
+- The `jest.mock('fs')` -> `jest.spyOn(fs, ...)` footgun (see "Bug found and
+  fixed" above) is worth a one-line addition to a future test-authoring
+  note in `haven/workers/implementer/MEMORY.md`'s "Patterns that work here"
+  (currently `<<FILL>>`) — not added in this diff, flagged for whoever
+  next fills that section.
+
+## Seal gate
+Not applicable to this recipe step — no outward-facing action (commit,
+push, delete, external API call) has happened yet. The single test-file
+diff was shown in full in the session transcript. Commit/push waits for
+`/ship` (not invoked this round; `/todo` was run without `--ship`).

agent-hub/evidence/verifier/2026-09-29/add-candidate-self-delete-seal.md

@@ -0,0 +1,150 @@
+# 2026-09-29 — add-candidate-self-delete (verifier verdict)
+
+- Worker: verifier (subagent, dispatched via Agent tool)
+- Node: `add-candidate-self-delete`
+- New PM status: SEALED
+
+## Isolation proof
+Dispatched fresh via the Agent tool with the task string "Run verify_seal
+for node `add-candidate-self-delete`, judging the evidence note at
+`agent-hub/evidence/implementer/2026-09-29/add-candidate-self-delete-plan.md`"
+— no memory of any implementer session, everything below was re-derived
+independently from the repo (reading real files, running `git log`/`git
+diff`/`git status`, re-running `npm test`/`npm run build` myself) rather
+than trusted from the note's prose.
+
+## Reasoning
+
+1. **Diff matches the note's claim, genuinely minimal.** `git status
+   --short` on `158-candidate-self-delete-allow`: only `M
+   src/__tests__/candidate/candidate.service.test.ts` and the new
+   untracked evidence-note directory. `git diff staging --stat -- src/`:
+   1 file, `+122/-11`. No change to `candidate.service.ts`,
+   `candidate.controller.ts`, or any route file — confirmed by their
+   absence from the diff stat, not inferred.
+
+2. **`candidate.service.ts` read in full.** `CV_SECTION_MODELS`
+   (lines 20-30) lists all 9: generalInformation, Experience, Education,
+   Reference, Project, Certificate, Award, Application, Profile.
+   `handlerDelete` (118-148): `if (!(await MODEL.findById(_id))) return
+   { success: false, ... }` before any delete call — confirmed no
+   `deleteMany`/`deleteOne` is reachable on that branch. Then
+   `Promise.all(CV_SECTION_MODELS.map(model => model.deleteMany({
+   candidateId: _id })))` followed by `MODEL.deleteOne({ _id })`. Image
+   filenames are collected via `IMAGE_SECTION_MODELS` (`[Project,
+   Certificate, Award]`) `.find({candidateId}, {images:1})` BEFORE the
+   cascade delete runs (lines 127-131, ordered strictly before line 133),
+   then each image and the CV PDF (`CV_UPLOAD_DIR/{_id}-cv.pdf`) is
+   `fs.unlinkSync`'d gated by `fs.existsSync` (136-145).
+
+3. **`candidate.controller.ts`'s `fnDelete` read in full** (157-168): the
+   only identifier used is `(req as any).user?._id`; the function body
+   contains no reference to `req.body`, `req.params`, or `req.query`
+   anywhere. Structural, not conventional.
+
+4. **Routing read in full.** `routers/api/v1/index.ts:27` — `router.use(
+   '/candidate', verifyToken, routeCandidate)` mounts the entire
+   candidate router (including `DELETE /`) behind `verifyToken`.
+   `candidate.route.ts:261` — `router.delete('/', fnDelete)`, no
+   alternate/unauthenticated route to the same handler found anywhere in
+   the file.
+
+5. **Git history independently confirmed.** `git log --oneline -- src/
+   candidate/candidate.service.ts | tail -20` shows `32953ed feat: add
+   DELETE /api/v1/candidate for self-service account deletion`,
+   chronologically before `f355e2f fix: close broken access control,
+   password leak, and 3 other API bugs` in the same log — matches the
+   note's claim exactly.
+
+6. **Test diff read in full via `git diff staging`.** New `describe`
+   block `candidate.service.ts handlerDelete (issue #158)` has exactly 5
+   `it(...)` tests: not-found short-circuit (asserts `deleteOne` and
+   every section's `deleteMany` NOT called), full 9-model cascade
+   (asserts `deleteMany({candidateId:'cand1'})` on all 9 keys +
+   `Candidate.deleteOne({_id:'cand1'})`), CV-file-exists-so-unlink,
+   CV-file-missing-so-no-unlink, and project/certificate/award image
+   cleanup collected before deletion. `grep -n "jest.mock('fs')"` in the
+   file matches ONLY the explanatory code comment (line 92 context),
+   never an actual `jest.mock('fs')` call; the file does use
+   `jest.spyOn(fs, 'existsSync'/'unlinkSync')` in `beforeEach` and
+   `jest.restoreAllMocks()` in `afterEach` — the self-reported bug/fix
+   narrative is real, not just narrated. The shared `jest.mock('@/models',
+   ...)` factory was extended in place (placeholders `{}` → real
+   `{deleteMany: jest.fn()}` etc., `Project`/`Certificate`/`Award` also
+   gained `find: jest.fn()`) — confirmed the pre-existing `describe(
+   'candidate.service.ts password exclusion (issue #152)')` block (lines
+   1-64) is byte-for-byte unmodified in the diff context and its 3 tests
+   (`handlerGetInformationByEmail`, `handlerGetInformationById` x2) still
+   read from the file untouched.
+
+7. **`npm test` independently re-run** (not audit-only — self-selected
+   given the unusual "0 production diff" claim, an explicit re-run
+   scenario worth the cost here). Verbatim tail of my own run:
+   ```
+   Test Suites: 29 passed, 29 total
+   Tests:       160 passed, 160 total
+   Snapshots:   0 total
+   Time:        8.24 s
+   Ran all test suites.
+   ```
+   Matches the note's claimed `29 passed, 29 total` / `160 passed, 160
+   total` exactly. Same pre-existing "worker process failed to exit
+   gracefully" notice, unrelated to this diff (present on prior SEALED
+   nodes too).
+
+8. **`npm run build` independently re-run.** Output: `tsc && npm run
+   copy` → `cp -R ./src/views ./src/public ./dist/`, no typecheck errors,
+   clean. `git status --short` after build shows no new/stray changes
+   (dist/ is gitignored, as expected).
+
+9. **`doctrine/domains/PROJECT.md` Traps table checked** — none of the 7
+   listed traps (Chrome path, CORS wildcard, body-size limit, auth
+   validation skip, lint script, public/ static exposure, prod port)
+   relate to this diff; nothing here reintroduces any of them.
+
+10. **Controller-not-unit-tested reasoning checked directly.**
+    `candidate.controller.test.ts`'s file header reads: "The rest of
+    candidate.controller.ts is thin wiring already covered indirectly
+    elsewhere, same precedent as every other controller in this
+    codebase (not unit-tested per-function)." `fnDelete` itself is 12
+    lines (157-168), a single try/catch, zero conditionals — a
+    reasonable call given the codebase's own stated precedent, and the
+    IDOR-safety claim (criterion 3) is structural (verified directly
+    above), not something a controller-level mock test would add
+    confidence to.
+
+**Proportion**: diff is exactly 1 test file, 0 production code — smaller
+than the node's own scope could have required (a naive implementer could
+have "fixed" already-correct code); appropriately minimal, matches
+`SmallestDiff`.
+
+**Forbidden states scan**: `ADHOC_WORK` — no, real diagram node exists and
+this ran through the verify_seal recipe. `NO_EVIDENCE` — no, implementer
+note + this verdict note both exist. `EDIT_UNVERIFIED` — no, `npm test`/
+`npm run build` were independently re-run and read back verbatim above,
+not inferred. `CODE_IN_HAVEN` — no `.ts`/`.js` files were added to
+`haven/`. `DIAGRAM_DRIFT` — being corrected by this SEAL (PM status now
+matches the confirmed-live code state).
+
+## Re-run
+`full` — re-ran `npm test` (29/29 suites, 160/160 tests, matched exactly)
+and `npm run build` (clean) myself from the repo root, plus independently
+read every file the note cited (`candidate.service.ts`,
+`candidate.controller.ts`, `routers/api/v1/index.ts`,
+`candidate.route.ts`, the full test diff, `candidate.controller.test.ts`'s
+header) rather than auditing the note's prose alone, and ran `git log`/
+`git diff`/`git status` myself to confirm the minimal-diff and git-history
+claims. Justification: this node's central claim ("0 production code
+changed, feature already fully live") is unusual enough, and the
+coordinating task explicitly requested this depth of independent
+confirmation, to warrant paying the re-run cost rather than defaulting to
+audit-only.
+
+## Hub bytes before
+100436 (per implementer's note)
+
+## Hub bytes after
+103617 (measured after updating diagram PM status, same 5-category
+`/hub-tokens` per-session-total formula: root .md files + doctrine/ + the
+active non-archive haven/diagrams/ file + implementer worker bundle +
+verifier worker bundle, raw byte counts summed)