test(agent-service): cover the auth, workflow, and backend API clients - #7384
test(agent-service): cover the auth, workflow, and backend API clients#7384aglinxinyuan wants to merge 2 commits into
Conversation
Three of the four API clients under src/api had no spec. auth-api is the one that decides whether a request is authenticated at all, and none of its decisions were pinned. Adds 30 tests: - auth-api: JWT decoding and its segment-count check, the REGULAR role default, expiry in seconds, the never-expires case of a token without exp, and Bearer header parsing. - workflow-api: the nested JSON-string encoding of workflow content in both directions, the empty-description default, and the error text on a refused save or a missing workflow. - backend-api: the endpoint set, the defensive copy of the shared config, and the metadata fetch's failure paths. No production file is touched.
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7384 +/- ##
============================================
+ Coverage 84.24% 84.45% +0.20%
Complexity 4142 4142
============================================
Files 1169 1169
Lines 46745 46734 -11
Branches 5201 5201
============================================
+ Hits 39381 39469 +88
+ Misses 5663 5564 -99
Partials 1701 1701
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds Bun test coverage for the agent-service API client layer under agent-service/src/api, pinning current behavior for auth token handling, workflow persistence/retrieval (including nested JSON string encoding of workflow content), and backend operator-metadata fetching/config exposure.
Changes:
- Added new specs for
auth-api, covering JWT decoding/validation policy decisions and header parsing/creation. - Added new specs for
workflow-api, covering persist/retrieve request shapes, content stringification/parsing, and error propagation. - Added new specs for
backend-api, covering config copying semantics and operator-metadata fetch success/failure paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agent-service/src/api/workflow-api.spec.ts | Adds tests for workflow persist/retrieve request/response encoding and error handling. |
| agent-service/src/api/backend-api.spec.ts | Adds tests for backend config copying and operator-metadata fetch failure modes. |
| agent-service/src/api/auth-api.spec.ts | Adds tests pinning JWT decode/expiry policy and Bearer header handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { describe, expect, test } from "bun:test"; | ||
| import { createAuthHeaders, extractBearerToken, extractUserFromToken, validateToken } from "./auth-api"; | ||
|
|
||
| /** Builds a token with the given payload. Only the payload segment is ever read back. */ |
There was a problem hiding this comment.
Good catch — reworded. The helper now says all three segments are required and that only the payload one is decoded, so the header and signature just have to be present.
| return `header.${encoded}.signature`; | ||
| } | ||
|
|
||
| const SECONDS = 1000; |
There was a problem hiding this comment.
Agreed — dropped the constant instead of renaming it. exp is a UNIX second count, so the expiries are now built in seconds (nowInSeconds() + 60) and there is no ms/s conversion left to reason about. Re-ran the two mutations that cover these lines (relax the three-segment check, compare exp as milliseconds) — both still red.
Address review comments on the auth-api spec. The `tokenWith` doc said only the payload segment is ever read back, which reads as if a two-segment token would do; `decodeJWT` requires exactly three. `SECONDS = 1000` was a millisecond conversion factor under a name that reads as a second count. `exp` is a UNIX second count, so build the expiries in seconds via a `nowInSeconds` helper and drop the conversion entirely.
What changes were proposed in this PR?
Three of the four API clients under
agent-service/src/apihad no spec.auth-api.tsis the one that decides whether a request is authenticated at all, and none of its decisions were pinned.Adds 30 tests across three spec files, following the
fetch-spy pattern already established bycompile-api.spec.ts.auth-api — several of these are policy choices that read like oversights, so the tests state the intent rather than just the behaviour:
exproleREGULAR, so absent means least privilegebearer/BEARERworkflow-api — the workflow
contentround-trips as a nested JSON string: the request sendsJSON.stringify(content)and the response is re-parsed when it comes back as a string. Sending the object directly is the obvious-looking mistake and the backend rejects it, so both directions are pinned, along with the empty-description default and the error text on a refused save or a missing workflow.backend-api — the endpoint set, the defensive copy of the module-level config, and the two failure paths of the metadata fetch.
Verified by mutation, all reverted (production diff empty):
ADMINexpas expiredexpas milliseconds instead of secondscontentas a nested objectcontentThe three-segment mutation initially survived: the test used
"only.two", whose payload failsJSON.parseregardless, so the segment check was never actually exercised. Replaced with a two-segment token carrying a valid payload — an unsigned token — which is the case the check exists for.No production file is touched.
Any related issues, documentation, discussions?
Closes #7381
How was this PR tested?
bun run typecheckandbun run format:checkboth pass.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)