Skip to content
Closed
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
14 changes: 14 additions & 0 deletions agent_docs/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Defined in `src/utils/constants/endpoints/` with separate files per domain (e.g.
- All objects use `as const`
- **Group nested endpoints logically** (e.g., `ENTITY.ATTACHMENT.DOWNLOAD` not flat).
- **Use consistent param names** across endpoints. Avoid redundancy.
- **Prefix with service domain when names collide** — if `ATTACHMENT_ENDPOINTS` already exists in another service (e.g., `conversational-agent`), use `ORCHESTRATOR_ATTACHMENT_ENDPOINTS`. The domain prefix resolves cross-service ambiguity.

## Pagination

Expand Down Expand Up @@ -122,6 +123,19 @@ Naming: `{SERVICE}_PAGINATION` for response shape, `{SERVICE}_OFFSET_PARAMS` or
3. Type with conditional return: `Promise<T extends HasPaginationOptions<T> ? PaginatedResponse<R> : NonPaginatedResponse<R>>`
4. Update `docs/pagination.md` quick reference table

## Barrel exports

Use `export * from` for re-exporting types and services from barrel files (`index.ts`). Do not use explicit `export type { ... } from` when `export * from` covers the same exports — it creates duplication and maintenance burden.

```typescript
// CORRECT — clean, no duplication
export * from '@/models/conversational-agent';

// WRONG — redundant explicit re-export alongside wildcard
export * from '@/models/conversational-agent';
export type { FeedbackGetResponse } from '@/models/conversational-agent';
```

## Export naming

- Internal class: `{Entity}Service` (e.g., `EntityService`, `TaskService`)
Expand Down
10 changes: 10 additions & 0 deletions agent_docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
- **NEVER write `param || {}` for required parameters** — this hides bugs by silently accepting missing required data at call sites.
- **NEVER put required parameters in an Options object** — required values (IDs, keys, data) are always positional arguments. Options objects are reserved for optional parameters only, always the last argument, always marked `?`. E.g., `getOutput(jobKey: string)` not `getOutput(options: { jobKey: string })`, `close(instanceId, folderKey, options?)` not `close(options: { instanceId, folderKey })`. This is consistent across every service in the SDK.

- **NEVER assume enum wire format from Swagger alone** — verify against the live API. If Swagger says `FeedbackStatus` is `0, 1, 2` but the API actually returns `"Pending"`, `"Approved"`, `"Dismissed"` (strings), the enum must use string values. Wrong enum formats cause silent filter/comparison failures with no error.

### Transforms
- **NEVER add case-only entries to `{Entity}Map`** — field maps are for semantic renames only (`creationTime` → `createdTime`). Case conversion (`CreationTime` → `creationTime`) is handled by `pascalToCamelCaseKeys()`. Mixing them causes double-conversion bugs and makes the map lie about its purpose.
- **NEVER add transform steps without checking the actual API response first** — if the API already returns camelCase, don't add `pascalToCamelCaseKeys()`. If it doesn't return raw enum codes, don't add `applyDataTransforms()`. Each step must be justified by what the API actually sends.
Expand All @@ -30,6 +32,9 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
### Internal types
- **NEVER re-export `internal-types.ts` through index.ts** — these are private implementation details (raw API shapes, intermediate parsing types). Re-exporting pollutes the public API and creates breaking-change risk when internal formats change.

### Static mappings
- **NEVER define static mappings inside method bodies** — endpoint-to-type maps, status maps, and other constant lookup tables must be declared as module-level constants or in `internal-types.ts`. Rebuilding them on every call wastes memory and obscures that the data is fixed.

### Endpoints
- **NEVER bypass base service classes** — never directly instantiate HTTP clients; use `this.get()`, `this.post()` from BaseService.
- **NEVER hardcode HTTP method strings in service methods** — use existing constants. Hardcoded strings drift and miss centralized changes.
Expand All @@ -55,6 +60,9 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
- **NEVER create a separate `afterAll` per describe block** if the file already has one — reuse the existing cleanup block by pushing to the shared `createdRecordIds` array.
- **NEVER copy-paste JSDoc comments between endpoint groups** — each constant needs its own comment. A "Asset Service Endpoints" comment on `JOB_ENDPOINTS` is a review rejection.

### Versioning
- **NEVER bump `package.json` version in feature PRs** — version bumps are separate, dedicated PRs. Mixing version changes with feature work creates merge conflicts and makes release history unclear.

### Code hygiene
- **NEVER leave unused code** — unused imports, variables, redundant constructors that only call `super()`. Linter (oxlint) catches these.
- **NEVER add redundant constructors** — if the constructor only calls `super()`, delete it entirely.
Expand Down Expand Up @@ -94,6 +102,7 @@ Every new method must also have an integration test in `tests/integration/shared
- Use `generateRandomString()` from `tests/integration/utils/helpers.ts` for unique test data
- Tests run in both `v0` and `v1` init modes via `describe.each(modes)` — **only if the service is registered in both modes in `unified-setup.ts`**. New services that only support `v1` init should use `['v1']` only.
- **NEVER write redundant integration tests** — each test must cover a distinct code path, error scenario, or response shape aspect. A test that repeats an existing one with minor value differences (e.g., "should handle job with or without output" duplicating "should return parsed output for a completed job") adds no value and wastes CI time.
- **Verify integration tests actually exercise new code paths** — if adding a new endpoint (e.g., `getTaskDataById` for DocumentValidation), confirm the test uses appropriate test data for that endpoint. A test that calls a DocumentValidation endpoint with a Form task ID will "pass" by falling back to generic behavior, giving false confidence. The test should fail if the endpoint URL is wrong.
- **Include a transform validation test** for new methods with a transform pipeline. This test should verify: (a) transformed camelCase fields exist and have values (`job.createdTime`, `job.processName`), AND (b) original PascalCase API fields are absent (`(job as any).CreationTime` is `undefined`, `(job as any).ReleaseName` is `undefined`). This is a separate test from the basic "should retrieve by ID" test — it validates the SDK transform layer against the live API. Note: existing integration tests don't yet follow this pattern, but unit tests do (Assets, Queues, ChoiceSets). Extending it to integration tests catches mismatches between the Swagger spec assumptions and the live API response.

## Documentation
Expand Down Expand Up @@ -144,6 +153,7 @@ Documentation (the most commonly missed step):
- [ ] `docs/pagination.md` — quick reference table updated (if paginated method)
- [ ] `mkdocs.yml` — nav entry added (if new service, not needed for methods on existing services)
- [ ] `package.json` exports + `rollup.config.js` — subpath export added (if new service)
- [ ] Cloudflare Workers whitelist — new endpoints added in `apps-dev-tools` repo (`ApiCorsWorker/api-cors-worker.ts`)

## Skills for SDK development

Expand Down
Loading