Skip to content

test(server): resolution capture unit + integration coverage (CRU-130) - #373

Merged
selfcontained merged 2 commits into
mainfrom
agt_0123942fc9f5/agent-2fc9f5
Apr 21, 2026
Merged

test(server): resolution capture unit + integration coverage (CRU-130)#373
selfcontained merged 2 commits into
mainfrom
agt_0123942fc9f5/agent-2fc9f5

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

Summary

Phase 1 test coverage for the review round-trip flow (parent of CRU-127). Backs the resolution-capture implementation landed in #372 / CRU-128 with unit + HTTP integration tests so every new guard has a regression net.

No production code changed — tests only.

Coverage

Unit (apps/server/test/db/agent-manager.test.ts, additive):

  • updateFeedbackStatus / updateFeedbackStatusByParent
    • ignored rejects without a reason (empty, whitespace-only)
    • fixed accepted with or without a reason
    • resolution_reason, resolution_commit, resolved_at persisted
    • Benign re-resolve preserves the original audit trail (COALESCE)
    • resolved_at does not drift on repeat calls; cleared on revert to open
  • submitReviewResolution
    • Empty / whitespace / over-length (10k) summary
    • Review not in complete state
    • Still-open items surface the offending IDs
    • Ignored-without-reason items surface the offending IDs
    • Missing review returns 404
    • Happy path: summary + resolution commit + round_number=1
    • Idempotent upsert replaces summary + commit on repeat submit
    • Summary is trimmed on persist
    • Failing preconditions leave no partial rows
  • persona_reviews.last_reviewed_commit
    • Set on createPersonaReview; defaults to null
    • Updated on completePersonaReview; COALESCE preserves when omitted

HTTP integration (apps/server/test/resolution-capture-integration.test.ts, new file):

  • PATCH /api/v1/agents/:id/feedback/:feedbackId
    • Reason must be a string; 10k cap
    • ignored without reason → 400
    • Happy paths for fixed / ignored record reason + HEAD sha
    • Reverting to open does not recompute resolution commit
  • POST /api/v1/agents/:id/persona-reviews/:personaAgentId/resolution
    • Summary validation (missing / whitespace / non-string) → 400
    • Still-open items → 409 with IDs
    • Ignored items missing a reason → 409 with IDs
    • Review not complete → 409
    • Parent not found → 404
    • Happy path persists summary + HEAD sha, row readable via DB
    • Unauthenticated requests → 401

Follows the mcp-auth-integration.test.ts pattern: inert runtime, isolated test DB, runCommand mocked so resolveHeadSha returns a controlled value.

Test plan

  • pnpm run check (backend + web tsc)
  • pnpm run test — 366 passed, 11 skipped (pre-existing)
  • New suite alone: 39 unit + integration tests all green
  • pnpm run test:e2e — not re-run; no production code changed, no E2E assertions touched

selfcontained and others added 2 commits April 20, 2026 21:54
Covers Phase 1 of the round-trip review flow (CRU-128). Pairs well with
the implementation PR so reviewers can see what each new guard protects.

Unit tests (apps/server/test/db/agent-manager.test.ts):
- updateFeedbackStatus / ...ByParent: ignored rejects without reason,
  accepts fixed with or without reason, records resolution_reason /
  resolution_commit / resolved_at, and preserves the audit trail on
  benign re-resolves (COALESCE + first-transition semantics).
- submitReviewResolution: empty / whitespace / over-length summary,
  non-'complete' review status, still-open items, ignored-without-reason,
  missing review, happy path (summary + HEAD sha + round 1), upsert on
  repeat submit, summary trimming, no partial persistence on failure.
- persona review last_reviewed_commit: set on create, default null,
  updated on completePersonaReview, COALESCE preserves it when omitted.

HTTP integration tests (resolution-capture-integration.test.ts):
- PATCH feedback endpoint: reason type/size validation, ignored-without
  -reason, happy paths for fixed/ignored, resolution commit not recorded
  on revert to 'open'.
- POST resolution endpoint: summary validation, 409 for open items,
  ignored-missing-reason, wrong review status, 404 missing parent,
  happy path persists summary + HEAD + round_number, auth required.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…130)

Addresses three low-severity findings from the persona review on PR #373:

- Add a 401 test for PATCH /feedback/:feedbackId so an auth-hook
  regression that accidentally excludes the PATCH route fails loudly
  instead of relying on the sibling POST /resolution test.
- Exercise the manager-level 10,000-character summary cap through
  handleAgentError on POST /resolution, so HTTP-layer status-code
  translation for that specific throw path is covered end-to-end.
- Add a 404 test for the "both agents exist, no persona_reviews row"
  branch inside submitReviewResolution's transaction — complements
  the existing "parent missing" 404 test which short-circuits before
  the manager runs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@selfcontained
selfcontained marked this pull request as ready for review April 21, 2026 04:01
@selfcontained
selfcontained merged commit 6507fc1 into main Apr 21, 2026
1 check passed
@selfcontained
selfcontained deleted the agt_0123942fc9f5/agent-2fc9f5 branch April 21, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant