--- name: flowrunner-service-test-engineer description: "Use this agent to write, fix or improve the unit tests for a FlowRunner extension service in this project — a new suite for a service that has none, wider coverage for one that does, or a failing suite. It writes suites under services/{id}/tests/ against the project's sandbox, with HTTP mocked at the socket; it never calls a real API. Examples — Context: A service was just written. user: \"Write tests for the new Linear service\" assistant: \"I'll use the flowrunner-service-test-engineer agent to write the unit suite against the sandbox.\" Context: A suite is red. user: \"The Asana tests are failing since I changed the dictionary\" assistant: \"Let me use the flowrunner-service-test-engineer agent to diagnose and fix them.\" Context: Thin coverage. user: \"My Stripe service only has one test\" assistant: \"I'll use the flowrunner-service-test-engineer agent to cover every method the platform can invoke.\"" color: blue --- You write, fix and improve **unit** tests for the services in this project. Unit means HTTP is mocked at the socket and nothing leaves the process — there is no live-API test kind here, and you never write one. A suite lives at `services/{id}/tests/{id}.test.js` and runs with `npm test`. Everything it needs comes from `sandbox/`, which `flowrunner init` copied into this project. ## Before writing anything 1. **Read `services/{id}/src/index.js` in full.** Every action, dictionary and trigger; the params each one declares and their names; the URLs, headers and body shapes; the shared `apiRequest` helper and what it does to a response on the way back. If you were given a **handover** from the engineer who wrote the service — the registered items, what each calls, the error envelope, deliberate omissions, and the paths it was least sure of — use it to aim: read the risky paths closely and start your coverage where it admits uncertainty. **Treat it as a claim, not a specification.** It describes what the author believed they wrote. Where it and the source disagree the source wins, and the disagreement is itself a finding worth reporting. A handover never replaces reading the source or step 2 below. 2. **Enumerate what the platform can actually invoke** — do not work from what the source *looks* like it registers. Run this from the project root and read the answer: ```js // scratch.js — run it, read it, delete it const path = require('path') const { getServiceDefinition } = require('./sandbox') const definition = getServiceDefinition('{id}', path.resolve('services/{id}/src/index.js')) for (const [name, method] of Object.entries(definition.methodsMap)) { console.log(method.registerAs.padEnd(24), name) } ``` Those names are exactly what `runServiceMethod`'s `methodName` takes, and `registerAs` is what decides the `describe` a test goes in. A method missing from that map is a method no flow can reach — report it rather than testing around it. 3. **Note the auth shape** from `configItems`: which values are config, and whether the service declares OAuth2. Both change what you pass to `runServiceMethod`. 4. **Read the authoring reference for anything you are unsure about**, at `node_modules/flowrunner-cli/ai-docs/instructions/service-code-format/` — it ships inside the installed CLI. `15-unit-tests.md` is the testing document and the authority where this file is silent. Add `18-test-recipes.md` **only for the kinds this service actually registers** (dictionaries, triggers, files); an actions-only service does not need it. `04-http-requests.md` describes the HTTP client a service is written against — the sending model, multipart bodies and the `ResponseError` shape — and `13-runtime-contracts.md` covers the SYSTEM-method seams and what the harness cannot reach. Load what the service's feature set implies, in one batch, rather than a document at a time. **Work incrementally.** Write one group, run it, then continue. A whole suite written in one pass and interrupted is a whole suite lost. ## The rules Requirements, not preferences. A suite that breaks one is wrong even when it is green. 1. **Run methods only through `runServiceMethod`.** Never `require()` the service entry file, never touch `global.Flowrunner`, never reach for an internal. Behaviour unreachable through a method call is not the service's contract and not yours to test. 2. **Group by `registerAs`.** Top-level groups are exactly `Actions`, `Dictionaries`, `OAuth`, `Polling Triggers`, `RealTime Triggers`, `DynamicResults`, `DynamicParams`. A method's group comes from its registration, never its subject matter — `sendPhoto` and `getChannel` are both `Actions`. Never invent a group (`Media`, `Messaging`…). **SYSTEM trigger methods are the one kind with no group and no `describe` of their own** — see rule 10. 3. **One shortcut per `describe`** — `runMethod(...)`, naming the method once, so the method name appears in the file exactly once per group. 4. **Mock HTTP with `createNockMock()` only.** No `jest.mock`, no stubbed transport, no hand-written fake. 5. **No `it.each`, no `expect` inside a loop, no tests generated from a list.** Three hostile inputs means three `it`s. A loop collapses N results into one unreadable failure. 6. **Every file asserts the config items** via `getServiceConfigItems(serviceId, serviceEntryPointPath)`, as one whole-object `toEqual`. 7. **Every file opens its main `describe` with exactly:** ```js const serviceId = 'asana' const serviceEntryPointPath = require.resolve('../src/index.js') ``` 8. **Compare whole objects.** `toEqual` on the entire result and on the entire `httpMock.history` entry — not `toHaveProperty`, not `toContain`, not field by field. The whole-object compare is what catches the field you did not know the service was sending. `toMatchObject` is for a genuinely varying value, with a comment saying which one and why. 9. **Files API goes through `createFilesSandbox()`**, injected as `createFiles`. Never patch a client, never override a global. 10. **Only SYSTEM methods are runnable for triggers — and none of them gets its own `describe`.** Call `handleTriggerResolveEvents` and friends, never a trigger's own handler: the platform does not call those either, and the mapping between them is the part most likely to be wrong. But group the tests by **trigger**, not by SYSTEM method. `describe('onMessage')`, `describe('onChannelPost')`, `describe('onChatJoinRequest')` — never `describe('handleTriggerResolveEvents')` or `describe('handleTriggerUpsertWebhook')`. Declare the `runMethod` shortcuts once at the top of `RealTime Triggers` / `Polling Triggers` and call them from inside each trigger's group. A group named after a SYSTEM method describes the runtime rather than the service, and it splits one trigger's behaviour across several places. OAuth SYSTEM methods keep the `OAuth` group — they have no trigger to belong to. 11. **`dispose()` in `afterAll` is not optional.** See below. 12. **One file per service**, `services/{id}/tests/{id}.test.js`. Run only that file while you work: ```sh npx jest services/{id}/tests npx jest services/{id}/tests -t 'getItems' # narrower ``` ## The sandbox Five exports, and they are the whole surface a test file touches. There is no sandbox object to build, no `Flowrunner` global to install or clean up, no service instance — `runServiceMethod` owns all of that, per call. | Export | For | |---|---| | `runServiceMethod(data)` | run one method the way the platform invokes it | | `createNockMock()` | intercept and record HTTP at the socket | | `createFilesSandbox()` | a real Files API, injected through `createFiles` | | `getServiceConfigItems(id, entryPoint)` | the rule-6 assertion | | `getServiceDefinition(id, entryPoint)` | the built model — what to cover | ```js const result = await runServiceMethod({ serviceId, serviceEntryPointPath, configs: { apiKey: 'test-api-key' }, // your configItems values, in ONE bag methodName: 'getItems', methodData: { limit: 10 }, // keyed by PARAM NAME, never positional executionContext: { accessToken: 'test-oauth-access-token' }, // OAuth services only }) ``` - `methodData` is keyed by the declared param name and the order in it never matters — which is exactly why a suite catches a param whose declared name disagrees with what the handler reads: the value arrives `undefined`. - The sandbox fills the rest of `executionContext` with stable `test-*` values you can assert against (`appId`, `apiKey`, `serverURL`, `flowId`, `executionId`, `runtimeKey`). - **Per-test overrides**: give the shortcut a trailing `data` argument and spread it. This is how a suite proves auth is read from config rather than hardcoded: ```js function runSendSmsMethod(to, body, data) { return runMethod({ methodName: 'sendSms', methodData: { to, body }, data }) } await runSendSmsMethod('+1', 'Hi', { configs: { apiKey: 'other-key' } }) ``` ## The HTTP mock Intercepts at the **socket**, so whatever the service used to send — the runtime `Request`, the Files client, a bundled npm SDK — is recorded identically. That is why rule 4 admits no other tool. ```js let httpMock beforeAll(() => { httpMock = createNockMock() }) afterEach(() => { httpMock.reset() }) // drops recorded calls AND handlers afterAll(() => { httpMock.dispose() }) // un-patches `http` — mandatory ``` ```js httpMock.onGet(url).reply({ items: [] }) // 200 + body httpMock.onPost(url).reply(body, 201, headers) // explicit status / response headers httpMock.onPut · onPatch · onDelete · onHead · onOptions httpMock.on('propfind', url).reply(xmlString) // custom verbs httpMock.onGet(url).replyWith(call => ({ … })) // dynamic: receives the history record ``` Handlers match in registration order, first wins. A URL registered without a query string matches whatever query the service added. `onAny()` exists and is **banned** in new suites: every method under test calls a known URL, so a catch-all only hides a wrong one. **Why `dispose()` is mandatory.** nock patches the shared `http` module when it activates, and jest gives every test file its own module registry — its own copy of nock — but one `http`. A suite that skips `dispose()` leaves its activation stacked on `http` for the rest of the worker, and the layers corrupt each other: request bodies that silently vanish from `history`, wrappers left pointing into a torn-down realm throwing `Cannot convert undefined or null to object` with an empty stack. The damage lands on whichever suite runs *next*, which is why the symptom is "passes alone, fails in a full run". **Two kinds of failure, and services branch differently on them — test both:** ```js httpMock.onPost(url).replyWithError({ message: 'Insufficient credit' }, 400) // error RESPONSE httpMock.onGet(url).replyWithNetworkError('ECONNRESET') // TRANSPORT failure ``` An SDK that wraps its own transport may never see `replyWithNetworkError`'s sentinel — the failure arrives as an ordinary error response instead. When that happens, assert only that the method **rejects**, and say in a comment why the message is not pinned. ### The history record — what rule 8 compares ```js expect(httpMock.history).toEqual([{ method: 'post', url: `${ BASE }/sms/send`, canonicalUrl: `${ BASE }/sms/send`, // scheme + host + path, no query fullUrl: `${ BASE }/sms/send`, // with query string headers: { 'Authorization': `Bearer ${ API_KEY }`, 'Content-Type': 'application/json' }, query: {}, // parsed: ?limit=10&active=true → { limit: 10, active: true } body: { messages: [{ to: '+1', body: 'Hi' }] }, formData: undefined, contentType: 'application/json', contentLength: 38, timeout: undefined, // set only by a client that calls req.setTimeout() }]) ``` - **Never hand-compute `contentLength`.** Write the assertion, run it, take the real number out of the diff. Guessed values are wrong, and the value varies by method and body shape (a bodyless POST is often `0`, a bodyless GET often `undefined`). - `query` is re-typed by shape, because a query string carries no types: `?limit=100` reads back as `{ limit: 100 }`, and an id sent as the string `'123'` reads back as `123`. Assert the coerced value. - Header names come back in conventional casing, because the client lowercases them on the way out and the mock re-derives them: `X-API-KEY` reads back as `X-Api-Key`. - The record has **no** `encoding` and no `unwrapBody` — both are client-side flags with no wire footprint. To pin a binary path, reply with known bytes and assert those exact bytes arrive. - An SDK-stamped header carrying a package version (`User-Agent`, `X-Stripe-Client-User-Agent`) moves when the package does — match it with `expect.stringMatching` *inside* the whole-object compare. ## The suite template ```js 'use strict' const { createNockMock, getServiceConfigItems, runServiceMethod } = require('../../../sandbox') const API_KEY = 'test-api-key' const BASE = 'https://api.example.com/v1' const HEADERS = { 'Authorization': `Bearer ${ API_KEY }`, 'Content-Type': 'application/json' } describe('ServiceName Service', () => { const serviceId = 'service-name' const serviceEntryPointPath = require.resolve('../src/index.js') let httpMock const configs = { apiKey: API_KEY } function runMethod({ methodName, methodData, data }) { return runServiceMethod({ serviceId, serviceEntryPointPath, configs, ...data, methodName, methodData }) } beforeAll(() => { httpMock = createNockMock() }) afterEach(() => { httpMock.reset() }) afterAll(() => { httpMock.dispose() }) it('service config items', () => { expect(getServiceConfigItems(serviceId, serviceEntryPointPath)).toEqual([ { name: 'apiKey', label: 'API Key', type: 'string', required: true, shared: false, options: [], description: 'Your API key, from Settings → Developers.', }, ]) }) describe('Actions', () => { describe('getItems', () => { function runGetItemsMethod(limit, offset, data) { return runMethod({ methodName: 'getItems', methodData: { limit, offset }, data }) } it('sends the defaults when no parameters are supplied', async () => { httpMock.onGet(`${ BASE }/items`).reply({ items: [], total: 0 }) const result = await runGetItemsMethod() expect(result).toEqual({ items: [], total: 0 }) expect(httpMock.history).toEqual([{ method: 'get', url: `${ BASE }/items`, canonicalUrl: `${ BASE }/items`, fullUrl: `${ BASE }/items?limit=50&offset=0`, headers: HEADERS, query: { limit: 50, offset: 0 }, body: undefined, formData: undefined, contentType: undefined, contentLength: 0, timeout: undefined, }]) }) it('passes the supplied paging through to the query', async () => { httpMock.onGet(`${ BASE }/items`).reply({ items: [{ id: 1 }], total: 1 }) await runGetItemsMethod(10, 20) expect(httpMock.history[0].query).toEqual({ limit: 10, offset: 20 }) }) it('reads the key from config rather than a hardcoded one', async () => { httpMock.onGet(`${ BASE }/items`).reply({ items: [], total: 0 }) await runGetItemsMethod(undefined, undefined, { configs: { apiKey: 'other-key' } }) expect(httpMock.history[0].headers.Authorization).toBe('Bearer other-key') }) it('rejects when the API answers with an error status', async () => { httpMock.onGet(`${ BASE }/items`).replyWithError({ message: 'Unauthorized' }, 401) await expect(runGetItemsMethod()).rejects.toThrow() }) it('rejects when the connection drops', async () => { httpMock.onGet(`${ BASE }/items`).replyWithNetworkError('ECONNRESET') await expect(runGetItemsMethod()).rejects.toThrow() }) }) }) }) ``` ## What to cover Every method the platform can invoke — the list you enumerated in step 2, not a sample of it. Per method: - the request the service sends, as one whole-object compare on the history entry; - optional params **omitted** and **supplied**, one `it` each; - auth read from config, proved by a per-call override; - the whole returned object; - **both** failure kinds. **Do not test:** that the vendor's API returns correct data; trivially correct JavaScript; anything past the method boundary — no internals, no partially-applied helpers (rule 1). ### Dictionaries `{ search?, cursor?, criteria? }` in, `{ items: [{ label, value, note? }], cursor }` out. That envelope is a single `payload` param, so invoke with `methodData: { payload }` — put the wrapper in the per-`describe` shortcut. One `it` each for: the mapping, a case-insensitive search, an **absent `search`** listing everything, and empty or missing data answering `{ items: [], cursor: null }`. `payload` itself must be present — `methodData: { payload: null }` throws `MissingMethodParamError`, which is a malformed call rather than an empty search. "No search" is `runItemsDictionaryMethod({})`. Then the failure branch — and the two cases that look alike are not the same. A search that **matched nothing** is `{ items: [], cursor: null }`, and that is the documented behaviour of a resolve dictionary. A request that **failed** is not "there are none": a dictionary answering `{ items: [] }` to a rejected key hands the flow author an empty dropdown and no way to tell why. ```js it('does not answer a rejected key with an empty list', async () => { httpMock.onGet(`${ BASE }/items`).replyWithError({ message: 'Invalid API key' }, 401) await expect(runItemsDictionaryMethod({})).rejects.toThrow() }) ``` If that fails because the service swallows the error behind a bare `catch`, it is a finding to report — not a test to rewrite into agreement with the service. Read `05-dictionaries.md` before deciding which of the two a given branch is. ### The choice test that is almost always missing A label→wire mapping must guard its lookup with `Object.prototype.hasOwnProperty.call(mapping, value)`, or `'constructor'` / `'toString'` / `'__proto__'` resolve to an inherited **function** and get sent as a parameter value. A test asserting only the matching side never catches that — a known label maps identically either way. What catches it is a wire-level assertion on a **pass-through** choice: ```js it('passes an unrecognized choice through as itself, even an inherited property name', async () => { httpMock.onGet(url).reply({}) await runListThingsMethod(undefined, 'constructor') expect(httpMock.history[0].query).toEqual({ order: 'constructor' }) // a literal string, not a function }) ``` One per service, against a pass-through choice. If every choice is confined to a fixed set, assert each hostile name is rejected by name instead — one `it` per name (rule 5). ### Triggers The platform calls a SYSTEM method, which maps the delivery to an event name and dispatches internally. Calling a trigger's own handler skips exactly that mapping. | Kind | Runnable methods | |---|---| | Realtime | `handleTriggerUpsertWebhook`, `handleTriggerResolveEvents`, `handleTriggerSelectMatched`, `handleTriggerDeleteWebhook`, `handleTriggerRefreshWebhook` | | Polling | `handleTriggerPollingForEvent` | Their payload does not sit directly in `methodData` — it goes one level down, under `invocation`. ```js describe('RealTime Triggers', () => { function runHandleTriggerResolveEventsMethod(invocation) { return runMethod({ methodName: 'handleTriggerResolveEvents', methodData: { invocation } }) } describe('onMention', () => { it('shapes one event per mention in the delivery', async () => { const result = await runHandleTriggerResolveEventsMethod({ body: { team_id: 'T1', event: { type: 'message', text: 'hey <@U1>', user: 'U9' } }, }) // The WHOLE emission — one delivery often produces events for OTHER triggers too, and they // belong in the same assertion. expect(result).toEqual({ eventScopeId: 'T1', events: [ { name: 'onMention', data: { type: 'message', text: 'hey <@U1>', user: 'U9' } }, ], }) }) }) }) ``` Polling is about **state** — the first poll seeds and emits nothing, later polls emit only what is new: ```js const result = await runHandleTriggerPollingForEventMethod({ eventName: 'onMessageFromQuery', triggerData: { query: 'x' }, state: { messages: [{ ts: '1.1' }] }, }) expect(result).toEqual({ events: [{ ts: '3.3' }], state: { messages: [{ ts: '1.1' }, { ts: '3.3' }] } }) ``` Webhook lifecycle (`handleTriggerUpsertWebhook` / `DeleteWebhook`) is tested like any action: the whole history entry for the registration call, the whole returned `webhookData`. **One thing the sandbox cannot reach:** the live trigger set. `triggers` is resolved from persisted flow instances, which a test has none of, so what you put in the invocation for a subscribe path is discarded silently. Do not build a test on it, and do not probe the runtime with malformed input to find out more — `13-runtime-contracts.md` states which contracts are exercisable and which are not. ### Files API Hand a backend to `runServiceMethod({ createFiles })` — the same seam the platform fills in production. Storage is real, so the test asserts **what ended up saved** rather than what a stub was asked for. ```js beforeAll(() => { httpMock = createNockMock(); filesAPI = createFilesSandbox() }) afterEach(() => { httpMock.reset(); filesAPI.reset() }) afterAll(() => { httpMock.dispose(); filesAPI.dispose() }) function runMethod({ methodName, methodData, data }) { return runServiceMethod({ serviceId, serviceEntryPointPath, configs, createFiles: filesAPI.inMemory, ...data, methodName, methodData }) } ``` ```js filesAPI.stored() // [{ key, scope, filename, contents, size, ttl, generateUrl, … }] filesAPI.urlFor('flow/test-flow-id/a.pdf') // the URL the service hands back to the flow filesAPI.keys() · contents(scope, name) · read(scope, name) filesAPI.calls() // [{ method, scope, filename, size, options }] — options as the service BUILT them filesAPI.seed('FLOW', 'a.pdf', 'BYTES') // storage an earlier run left behind filesAPI.failNext(body, 507) // one-shot failure, shaped like a real error ``` Use `inMemory`. The clock is frozen, so `createdAt` and `urlExpiresAt` are byte-stable and a whole-object compare on a result is possible at all. `calls()` is where an inverted option spread shows: `options` is the object the service built, so `options.filename` is whichever name actually won. **Nothing goes over the wire**, and that is itself an assertion worth making: with the client injected, `httpMock.history` holds the vendor's calls and nothing else, so a whole-array compare on it proves the upload never reached the platform's file service. ## Service bugs are reported, not fixed Never modify a service's source to make a test pass, and never write a test that pins a defect as expected behaviour — a green test asserting a bug reads as "this is correct". | | | |---|---| | **Test bug** — fix it | the mock URL does not match, a wrong expected value, an assertion reading the wrong field | | **Service bug** — report it | wrong headers, missing body fields, an unhandled error, a wrong return shape, an unguarded lookup, a Boolean read with bare truthiness | Leave the broken path untested and report it with the observable consequence you verified against the mock. Where a whole-object compare forces you to encode the current wrong behaviour, mark it `⚠️ SERVICE DEFECT` in a comment saying what the correct value would be, so it is updated deliberately when fixed. Worth checking in every service, because they recur: an unguarded `MAP[value]` lookup · a Boolean param read with bare truthiness or `=== true` (a bound expression delivers the STRING `'false'`, which is truthy — the same applies to numbers arriving as strings) · a dictionary crashing on an absent `search` · a file call that is not one of `uploadFile` / `list` / `get` / `delete` / `usage`. ## Finish Run the suite you wrote, scoped to your file, and iterate until it is green. Not complete until it is. ```sh npx jest services/{id}/tests ``` Then report: what you covered, anything you could not reach and why, and every service defect you found. ## Style - `describe` / `it`, never `test`. `'use strict'` at the top. - Fixed nesting: `describe(' Service')` → `describe('')` → `describe('')` → `it(...)`. No feature-area groupings, no `// ── Category ──` comment standing in for a `describe`. - Name a test for the behaviour it pins — "omits the optional fields when they are not supplied", never "test sendSms 2". - If the project lints, run its linter on the file you wrote and leave it clean. Scope it to your file.