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,147 @@
# 2026-09-28 — fix-create-response-null-id (plan + diff)

- Worker: implementer
- Version: 0.1.0
- Node: `fix-create-response-null-id` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- Issue: [#157](https://github.com/datvt243/resume-nodejs-api/issues/157) — POST .../create responses always return `data._id: null`
- Branch: `157-post-create-responses` (base `staging`)
- Task (verbatim): "Fix issue #157 — POST .../create responses always return data._id: null. Root cause: src/candidate_profile/BaseService.ts handlerCreate's hookAfterSave reassigns the local destructured `data` variable, but that reassignment never propagates back through baseCreateDocument's return value. Fix so the caller (and API response) gets the real saved document including its real _id. Acceptance criteria: (1) a regression test on handlerCreate/baseCreateDocument asserts the returned _id matches the actually-persisted document's _id, not null; (2) spot-check at least one real CV section create endpoint end-to-end."

## Hub bytes before: 86675

## Node lookup

Matched the existing PENDING node `fix-create-response-null-id` directly
(task came from resolving GitHub issue #157, filed against this exact
node).

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

The production fix already exists on `staging` — same pattern as
`add-pagination-filtering-cv-sections`/#73, `add-logout-all-sessions`/#74,
and `fix-idor-broken-access-control`. `git log -- src/services/index.ts`
shows commit `f355e2f` ("fix: close broken access control, password leak,
and 3 other API bugs", 2026-08-21) already contains:

```diff
+ /**
+ * callback thực hiện sau khi thêm mới thành công. Nếu hook trả về
+ * (khác undefined), dùng giá trị đó thay _data — trước đây hook nhận
+ * `data` qua destructure-by-value nên gán lại bên trong hook không hề
+ * cập nhật _data ở đây, khiến response luôn trả nguyên kết quả thô của
+ * MODEL.create() ... thay vì list mới đã refetch.
+ */
if (props?.hookAfterSave) {
- await props?.hookAfterSave?.(document, { success: _success, message: _message, data: _data });
+ const replacement = await props.hookAfterSave(document, { success: _success, message: _message, data: _data });
+ if (replacement !== undefined) _data = replacement;
}
```
(`src/services/index.ts` `baseCreateDocument`, confirmed live on this
branch at lines 300-303 today.)

That same commit bundled 5 fixes into one, but only
`fix-candidate-password-leak` got its own evidence note at the time
(`evidence/implementer/2026-08-21/fix-candidate-password-leak-diff.md`,
itself still sitting at `sealed_pending_verifier` — never promoted). The
other 4 (IDOR, this one, TOKEN_EXP_IN, v2-register-await) never got a
per-node evidence note. IDOR was already backfilled later
(`fix-idor-broken-access-control`, SEALED 2026-09-08). This note performs
the same backfill for `fix-create-response-null-id`, plus closes the real
gap the issue's acceptance criteria pointed at: **no regression test
existed** for this behavior anywhere in `src/__tests__/`.

## Diff (smallest diff — no `src/` production code, only new tests)

`src/services/index.ts` and `src/candidate_profile/BaseService.ts` are
unchanged — the fix is already correct and live on `staging`. The only
diff is 2 new test files:

- `src/__tests__/services/baseCreateDocument.test.ts` — direct unit
coverage of `baseCreateDocument`'s `hookAfterSave` replacement
propagation (the exact root-cause line): asserts (a) a non-undefined
`hookAfterSave` return value becomes `result.data` instead of the raw
`MODEL.create()` result, (b) `undefined` falls back to the raw create
result, (c) no `hookAfterSave` at all leaves the raw result untouched.
- `src/__tests__/candidate_profile/BaseService.test.ts` — spot-check of a
real CV section's create flow: calls `createCrudService({ model, name:
'education' }).handlerCreate(...)` with the real, unmocked
`BaseService.ts` + `services/index.ts` code (only the Mongoose model
itself is faked), confirming the final response's `data._id` is the
real persisted id (`real-id-1`), not `null` — this is the exact code
path every CV section's real `POST .../create` endpoint uses.

## Command

```
npm test
```
Output (verbatim tail):
```
Test Suites: 26 passed, 26 total
Tests: 140 passed, 140 total
Snapshots: 0 total
Time: 7.101 s
Ran all test suites.
```
(140 = the 136 total recorded on the last SEALED node, `update-project-docs`,
+ 3 new tests in `baseCreateDocument.test.ts` + 1 new test in
`BaseService.test.ts`. A harness warning — "A worker process has failed to
exit gracefully... Active timers can also cause this" — printed above this
summary; pre-existing, unrelated to this diff (no timer/interval touched
here), and does not affect the pass/fail count.)

```
npm run build
```
Output: `tsc` clean, `cp -R ./src/views ./src/public ./dist/` (the `copy`
step) ran with no errors.

## Acceptance

| Criterion | Evidence |
|---|---|
| Trace to exactly one diagram node | `fix-create-response-null-id` |
| Smallest diff | 2 new test files only, 0 production `src/` changes (fix already live since `f355e2f`, 2026-08-21) |
| Regression test asserts `_id` is the real persisted id, not `null` | `src/__tests__/services/baseCreateDocument.test.ts` (root-cause level) + `src/__tests__/candidate_profile/BaseService.test.ts` (CV-section-flow level) |
| Spot-check a real CV section create endpoint end-to-end | See "Live end-to-end — not performed" below; done instead as an unmocked code-path spot-check through the real `createCrudService`/`BaseService.ts`/`services/index.ts`, per `BaseService.test.ts` |
| Exact test command run + output read back | `npm test` → `Tests: 140 passed, 140 total`; `npm run build` clean |
| Evidence note written | This file |

## Live end-to-end — not performed, said honestly

This session's sandbox has no `.env`, no local MongoDB (`mongod` not
running, no Mongo Docker container), and the Docker daemon itself is not
reachable (`docker info` fails) — no way to actually start `npm run dev`
against a real database to curl a live `POST /api/v1/education/create`.
Rather than fabricate a live-curl transcript, the "spot-check a real CV
section create endpoint end-to-end" criterion was satisfied instead by
exercising the real, unmocked code path (`BaseService.test.ts` above) —
only the Mongoose model itself is faked, everything else (`BaseService.ts`
handlerCreate → `services/index.ts` baseCreateDocument →
hookAfterSave → baseFindDocument refetch) runs for real. Flagging this gap
honestly rather than claiming a live server round-trip that didn't happen.

## Noticed, not done

- `haven/diagrams/dev-loop.prime-mermaid.md` is 39901B, over the 15KB
`/hub-tokens` archive threshold (checked this session, `hub_bytes_before`
above). Not archived here — out of scope for this node, flagged for a
future dedicated archive pass.
- The sibling `fix-candidate-password-leak` node is also already fixed
live on `staging` (same `f355e2f` commit) but still shows PENDING on the
diagram with a stale `sealed_pending_verifier` note from 2026-08-21 that
never got a verifier pass — same backfill pattern as this node, own
`/todo #152` pickup if wanted (issue #152 already filed).
- `TOKEN_EXP_IN`/v2-register-await (issues #155/#156) are also already
fixed in the same bundled commit — not touched here, out of scope for
this node.

## Seal gate

No outward-facing action taken (no commit/push). Only local file writes:
2 new test files under `src/__tests__/`. Pending verifier.

## Status

`sealed_pending_verifier`
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# 2026-09-28 — fix-create-response-null-id — verifier verdict

- Worker: verifier (subagent, dispatched via Agent tool)
- Node: `fix-create-response-null-id` (`haven/diagrams/dev-loop.prime-mermaid.md`)
- New PM status: SEALED

## Isolation proof

Dispatched as a fresh Agent-tool subagent with task description
"Independent verifier pass for fix-create-response-null-id" — no
conversation history from the implementer's session, no memory of writing
the diff under review. Only source read directly (per recipe step 2's
citation-confirmation exception): `src/services/index.ts` (current +
`git show f355e2f`), the two new test files, `git status`/`git log` on
branch `157-post-create-responses`. Never opened the implementer's
session/diff itself — only its evidence note, plus these independent
citation checks.

## Reasoning

- **Command matches doctrine**: `npm test` — matches
`doctrine/MEMORY.md`'s Test row exactly.
- **Output not truncated**: note's `npm test` tail
(`Test Suites: 26 passed, 26 total` / `Tests: 140 passed, 140 total`)
and `npm run build` output are verbatim, no `...`/truncation markers.
- **Criterion (a)** — regression test asserts real persisted `_id`, not
null: confirmed by reading
`src/__tests__/services/baseCreateDocument.test.ts` directly — 3 tests
cover hookAfterSave replacement propagation, undefined fallback, and
no-hook passthrough; the first asserts
`expect((result.data as any)._id).toBe('real-id-123')`.
- **Criterion (b)** — spot-check a real CV section create endpoint
end-to-end: note explicitly discloses the literal live-HTTP
interpretation was NOT performed (no local Mongo/Redis/Docker in that
sandbox — a real, stated constraint, not hidden) and substitutes
`src/__tests__/candidate_profile/BaseService.test.ts`, confirmed by
direct read: calls the real, unmocked `createCrudService(...).
handlerCreate` → real `BaseService.ts` → real `services/index.ts`
`baseCreateDocument` → real `hookAfterSave` refetch path, only the
Mongoose model itself faked. This exercises the exact production code
path every `POST .../create` endpoint uses, asserts
`data._id).toBe('real-id-1')` (not null). Judged: an honest, reasonable
substitution given the disclosed sandbox limit and the fact the
underlying fix has been live and unchanged on `staging` for 5+ weeks
(see next point) — not a REOPEN-worthy gap.
- **Bookkeeping-gap claim independently confirmed**: `git show f355e2f --
src/services/index.ts` really contains the cited hunk
(`const replacement = await props.hookAfterSave(...); if (replacement
!== undefined) _data = replacement;`), and current
`src/services/index.ts` lines 300-302 match it verbatim — the
production fix is genuinely already live, unchanged since 2026-08-21.
`BaseService.ts` confirmed unchanged (diff is test-only).
- **Proportion**: 2 new test files, 0 production `src/` changes —
proportionate; the production fix already exists, so a regression-test
diff is the smallest diff that closes the real gap (no prior test
coverage).
- **Seal gate**: `git status --short` on branch `157-post-create-responses`
shows only 3 untracked paths — the evidence note dir and the 2 new test
files — nothing staged/committed/pushed. `git log` on that branch shows
no new commit beyond the shared history with `staging`. Matches the
note's "no outward-facing action taken" claim.

## Forbidden states scan

- `ADHOC_WORK` — no: task traces to the pre-existing PENDING node
`fix-create-response-null-id` on the diagram (row confirmed before
edit).
- `NO_EVIDENCE` — no: implementer note exists at the cited path.
- `EDIT_UNVERIFIED` — no: `npm test` output is a real, non-truncated,
doctrine-matching command result, consistent with the described diff
(140 = prior 136 + 3 new `baseCreateDocument.test.ts` tests + 1 new
`BaseService.test.ts` test).
- `CODE_IN_HAVEN` — no: `find agent-hub/evidence/implementer/2026-09-28`
shows only the one `.md` note, no runnable code.
- `DIAGRAM_DRIFT` — resolved by this verdict: node row updated PENDING →
SEALED in place.

## Re-run

`none` — audit-only, per recipe default for a non-outward-facing, non-
release-gate bug/test-coverage fix. The note's command matched doctrine,
output was verbatim and covered every acceptance criterion, so no partial
or full re-run was warranted; independent checks were limited to
confirming the note's own citations (git history, current source, test
file contents) rather than regenerating its evidence.
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 @@ -60,7 +60,7 @@ flowchart TD
| `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. |
| `fix-v2-register-missing-await` | PENDING | `src/api/v1/auth/services/register.ts:44` — `bcryptGenerateSalt(password)` missing `await`, the Promise gets assigned straight into the Mongoose model's password field → every `POST /api/v2/auth/register` fails with a Promise→string cast error. |
| `fix-create-response-null-id` | PENDING | Minor. `BaseService.ts` `handlerCreate`'s `hookAfterSave` reassigns the local destructured `data` variable, never actually updating what `baseCreateDocument` returns → every `POST .../create` response has `data._id: null` instead of the real new ID. |
| `fix-create-response-null-id` | SEALED | Minor. `BaseService.ts` `handlerCreate`'s `hookAfterSave` reassigns the local destructured `data` variable, never actually updating what `baseCreateDocument` returns → every `POST .../create` response has `data._id: null` instead of the real new ID. Backfill, same pattern as `fix-idor-broken-access-control`: the fix itself was already live since commit `f355e2f` (2026-08-21, `src/services/index.ts` lines ~300-302, `const replacement = await props.hookAfterSave(...); if (replacement !== undefined) _data = replacement;`) — independently confirmed via `git show f355e2f -- src/services/index.ts` and the current file, both match. This node closed the real gap: no regression test existed. 2 new test files added (`src/__tests__/services/baseCreateDocument.test.ts` — root-cause unit coverage; `src/__tests__/candidate_profile/BaseService.test.ts` — unmocked `createCrudService`/`BaseService.ts`/`services/index.ts` spot-check, only the Mongoose model faked), 0 production changes — proportionate. `npm test`: 140/140 passed. Live HTTP end-to-end was not performed (no local Mongo/Redis/Docker in the implementer's sandbox, disclosed honestly, not hidden) — accepted the unmocked code-path spot-check as a reasonable substitute given the fix has been live and stable for 5+ weeks. 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`; confirmed via `git status --short` on `157-post-create-responses` that nothing was committed/pushed). Evidence: `evidence/verifier/2026-09-28/fix-create-response-null-id-seal.md`. |
| `add-candidate-self-delete` | PENDING | Feature (not a bug). No endpoint lets a candidate delete their own account — needed to clean up 2 test accounts created during live-verification of the 5 bug fixes above on production (`livecheck+...@example.com`, `livecheckB+...@example.com`). Requirement: `DELETE /api/v1/candidate`, using only `req.user._id` (never an id from the client — follows the IDOR-safe pattern from `fix-idor-broken-access-control`), cascade-deletes data across all 7 CV section models by `candidateId`. |
| `fix-candidate-me-candidateid-not-string` | PENDING | **Critical, found by accident while testing #79.** `candidate_me/index.ts` `handlerGetAboutMe` — `_id` from the raw Mongoose document is an ObjectId instance, passed straight into `idQuerySafe.safeQuery({}, { candidateId: _id })` — `QuerySafe.safeQuery` only accepts `typeof value === 'string'`, so an ObjectId silently fails that check and `candidateId` gets dropped from the filter → `model.find({})` returns CV data (education/experience/award/certificate/project/generalInformation) for **every candidate mixed together**, on every `GET /api/me/:email` request (public, no auth) and `/download-pdf`. Live-tested confirmed: a brand-new candidate profile returned real data belonging to `votan.it@gmail.com`. Fix: `.toString()` on `_id` before passing it in. |
| `feat-i18n-api-messages-auth` | PENDING | Feature, GitHub issue #78 (phase 1 of several). i18n infrastructure (hand-rolled `t(key, lang)`, reads `locales/vi.json`/`en.json`, middleware detects `Accept-Language`, defaults `vi`) + fully migrates the auth flow (register/login/logout/refresh). Does NOT migrate Joi validation messages (different architecture — Joi schemas are built once at module load with no request context; needs error TYPE → i18n key mapping, left as a follow-up). Does NOT touch candidate/CV section messages (separate follow-up). |
Expand Down
42 changes: 42 additions & 0 deletions src/__tests__/candidate_profile/BaseService.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Spot-check for issue #157 — exercises createCrudService's real
* handlerCreate (BaseService.ts) end-to-end through the real, unmocked
* services/index.ts (baseCreateDocument + its hookAfterSave refetch), the
* same code path every CV section's real POST .../create endpoint uses
* (e.g. education). Only the Mongoose model itself is faked. Confirms the
* response's data._id is the real persisted id, not null.
*/
import { createCrudService } from '@/candidate_profile/BaseService';

function createFakeEducationModel() {
const docs: Record<string, any>[] = [];
return {
validate: jest.fn().mockResolvedValue(undefined),
create: jest.fn(async (doc: Record<string, any>): Promise<Record<string, any>> => {
// Mirrors real Mongoose behavior for MODEL.create({ _id: null, ... }):
// an explicit `_id: null` is NOT kept as null — Mongoose still
// assigns a real ObjectId-shaped id.
const saved: Record<string, any> = { ...doc, _id: `real-id-${docs.length + 1}` };
docs.push(saved);
return saved;
}),
find: jest.fn((query: Record<string, any>) => ({
exec: jest.fn().mockResolvedValue(docs.filter((d) => d.candidateId === query.candidateId)),
})),
};
}

describe('createCrudService handlerCreate — real CV section create flow (issue #157)', () => {
it('returns the real persisted _id in data, not null, for a section like education', async () => {
const model = createFakeEducationModel();
const { handlerCreate } = createCrudService({ model, name: 'education' });

const result: any = await handlerCreate({ candidateId: 'c1', school: 'MIT' });

expect(result.success).toBe(true);
const data = Array.isArray(result.data) ? result.data[0] : result.data;
expect(data._id).toBeDefined();
expect(data._id).not.toBeNull();
expect(data._id).toBe('real-id-1');
});
});
Loading
Loading