Skip to content

refactor(workflow-executor): own record not-found handling in the port [PRD-450]#1636

Open
matthv wants to merge 2 commits into
feat/prd-214-server-step-mapperfrom
refactor/prd-450-getone-thin-not-found-in-port
Open

refactor(workflow-executor): own record not-found handling in the port [PRD-450]#1636
matthv wants to merge 2 commits into
feat/prd-214-server-step-mapperfrom
refactor/prd-450-getone-thin-not-found-in-port

Conversation

@matthv
Copy link
Copy Markdown
Member

@matthv matthv commented Jun 8, 2026

What

Follow-up refactor on top of #1629. Keeps @forestadmin/agent-client's Collection#getOne a thin by-id passthrough and moves the not-found semantics into AgentClientAgentPort (the layer that owns RecordNotFoundError).

Why

The generic agent-client shouldn't decide that a 404 / empty body means "record not found" — that's a domain concern of the executor. Centralizing it in the port also makes the behavior explicit and testable.

Changes

  • agent-client/collection.tsgetOne no longer has a try/catch, doesn't swallow 404s, doesn't normalize empty bodies. It just GETs by id and returns the result.
  • workflow-executor/agent-client-agent-port.tsgetRecord now owns the not-found handling:
    • a 404RecordNotFoundError;
    • a 200 + empty body (some agents answer this for a missing composite-key record) → RecordNotFoundError;
    • any other error rethrown — no masking a 500 / auth / network failure as "record not found".
  • Tests: mock HttpRequester.is404Error; added cases for 404 → RNF, non-404 → rethrown (not masked), and 200-empty → RNF.

Testing

  • Full workflow-executor suite: 936 passing, lint clean.

Essentially a refactor (no behavior change for callers) — relates to PRD-450.

🤖 Generated with Claude Code

Note

Move RecordNotFoundError handling from Collection.getOne into AgentClientAgentPort.getRecord

  • Moves 404-to-null conversion out of Collection.getOne so HTTP errors propagate directly to callers.
  • AgentClientAgentPort.getRecord now catches 404s via HttpRequester.is404Error and throws RecordNotFoundError; non-404 errors are rethrown. A 200 with an empty body is also treated as a missing record.
  • RecordNotFoundError constructor now accepts a string | (string | number)[] id, joining arrays with | for the error message.
  • Behavioral Change: Collection.getOne no longer swallows 404s or empty responses — any caller outside AgentClientAgentPort that relied on the null return value will now receive thrown errors instead.

Macroscope summarized ce724f4.

…t [PRD-450]

Keep @forestadmin/agent-client's Collection#getOne a thin by-id passthrough and
move the not-found semantics into AgentClientAgentPort, which owns
RecordNotFoundError:

- getOne no longer swallows errors nor normalizes empty bodies — it just GETs by id.
- getRecord converts a 404 to RecordNotFoundError, treats a 200 + empty body as
  not-found, and rethrows any other error (no masking of real failures as
  "record not found").

Follow-up refactor on top of #1629. Relates to PRD-450.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 8, 2026

PRD-450

@qltysh
Copy link
Copy Markdown

qltysh Bot commented Jun 8, 2026

Qlty


Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (2)

RatingFile% DiffUncovered Line #s
New Coverage rating: A
packages/agent-client/src/domains/collection.ts0.0%175
New Coverage rating: A
...ages/workflow-executor/src/adapters/agent-client-agent-port.ts100.0%
Total87.5%
🤖 Increase coverage with AI coding...
In the `refactor/prd-450-getone-thin-not-found-in-port` branch, add test coverage for this new code:

- `packages/agent-client/src/domains/collection.ts` -- Line 175

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

…dError

Move the `id.join('|')` formatting into RecordNotFoundError so call sites pass
the recordId as-is. The constructor accepts a string or a (composite) id array
and joins it, removing the repeated join at every throw site.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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