Skip to content

Commit 3143c6b

Browse files
fix: address comments
1 parent a0a2ef8 commit 3143c6b

5 files changed

Lines changed: 18 additions & 7 deletions

File tree

agent_docs/conventions.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,15 @@ OData APIs require `$` prefix on query params. The SDK accepts clean camelCase k
147147

148148
**Apply manually in:** `getById()` methods accepting `BaseOptions``const apiOptions = addPrefixToKeys(options, ODATA_PREFIX, Object.keys(options))`.
149149

150+
### Case for OData query values
151+
152+
OData is case-insensitive, so the server accepts either case. Convention: `filter`, `select`, `expand`, `orderby` values use the field case shown in the SDK response (camelCase for services that transform, else as-is) — not the raw API. Applies within JSDoc examples, tests, and internal calls.
153+
154+
```ts
155+
filter: "state eq 'Running'" // not "State eq ..."
156+
select: "outputArguments,outputFile" // not "OutputArguments,..."
157+
```
158+
150159
## Headers utility
151160

152161
`createHeaders()` from `src/utils/http/headers.ts` builds headers from key-value pairs, filtering undefined.

agent_docs/rules.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
1313
- **NEVER use `as unknown as` type casts** — refactor to make types flow naturally. Casts hide real type errors and break when upstream types change.
1414
- **NEVER make all fields required** if the API sometimes omits them — mark optional fields as optional. Over-requiring causes runtime `undefined` access on fields the API didn't return.
1515
- **NEVER leave raw strings/numbers for fixed value sets** — use enums. Raw values lose type safety and autocomplete. If the API returns `1`, `2`, `3` for status, map them to a `Status` enum.
16-
- **NEVER duplicate fields across option types** — extend existing ones. If `CaseInstanceOperationOptions` already has `comment`, extend it instead of re-declaring. When the shape is identical, use `extends` (e.g., `export interface EntityUpdateRecordByIdOptions extends EntityGetRecordByIdOptions {}`) instead of a type alias, as type aliases break TypeDoc documentation generation.
17-
- **NEVER use type aliases for response types** — even when the shape matches an existing one, use an `extends` interface. Type aliases (e.g., `export type EntityUpdateRecordResponse = EntityRecord`) break TypeDoc docs generation by not rendering as standalone types. Use `export interface EntityUpdateRecordResponse extends EntityRecord {}` instead.
16+
- **NEVER duplicate fields across option types** — extend existing ones. If `CaseInstanceOperationOptions` already has `comment`, extend it instead of re-declaring.
17+
- **NEVER use a single-type alias when the shape matches one existing type** — single-type aliases like `export type EntityUpdateRecordResponse = EntityRecord` break TypeDoc docs generation by not rendering as standalone types. Use `export interface EntityUpdateRecordResponse extends EntityRecord {}` instead. **This rule applies only to single-type aliases.** For intersections of multiple types (e.g., composed response types like `export type {Entity}GetResponse = Raw{Entity}GetResponse & {Entity}Methods`), keep the `type` alias — TypeDoc renders intersection aliases correctly. **Rule of thumb: `interface extends` for a single parent; `type =` for intersections or anything else.**
1818
- **NEVER write `param || {}` for required parameters** — this hides bugs by silently accepting missing required data at call sites.
1919
- **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.
2020

@@ -50,7 +50,7 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
5050
- **NEVER write test descriptions that don't match the code**`'should call entity.insert'` is wrong if testing `insertRecord()`. Mismatched descriptions make failures misleading.
5151
- **NEVER hardcode test values** — use existing constants from `tests/utils/constants/`. Hardcoded values drift and hide which test is using which fixture.
5252
- **NEVER leave unused mock methods in mock objects** — dead mocks obscure what the test actually exercises and accumulate as the API evolves.
53-
- **NEVER use `console.log` + `return` in integration test guards** — use `throw new Error()`. Silent skips hide missing test configuration and make CI green when tests aren't actually running.
53+
- **NEVER silently skip integration tests when test data or config is missing**always use `throw new Error()`, never `console.log`/`console.warn` + `return`. A skipped test stays green in CI while not actually exercising anything, which defeats the purpose of the test. If no jobs/entities/tasks exist in the environment, the test is not doing its job and must fail loudly. This applies in both `beforeAll` setup and test body guards.
5454
- **NEVER wrap integration test API calls in try/catch** — let errors propagate naturally. Silent catches mask real failures and make tests pass when they should fail.
5555
- **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.
5656
- **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.
@@ -66,6 +66,7 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
6666
- **NEVER skip `mkdocs.yml` when adding a new service** — the new service page won't appear in the docs site navigation without a nav entry. Existing services adding methods don't need this.
6767
- **NEVER skip `@track()` decorator** on public service methods — telemetry gaps are invisible until production debugging, when they're expensive.
6868
- **NEVER use PascalCase in JSDoc examples** — write `id` not `Id`. Examples must match the SDK's post-transform response format or users will write broken code.
69+
- **NEVER use raw API field names in OData query values**`filter`, `select`, `expand`, `orderby` strings must match the case shown in the SDK response object, not the underlying API. See "Case for OData query values" in `agent_docs/conventions.md`. Applies **within JSDoc examples**, integration tests, unit tests, and internal service-to-service calls.
6970

7071
## Testing guidelines
7172

@@ -88,7 +89,6 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st
8889

8990
Every new method must also have an integration test in `tests/integration/shared/{domain}/`. These run against a live API and catch issues unit tests miss — wrong endpoints, broken transforms, auth/header problems.
9091

91-
- Use `console.warn()` + skip (not `throw`) for `beforeAll` setup preconditions that are outside the test's control (e.g., no test data available). `throw` is for test body guards where missing config means the test can't run at all.
9292
- Use `getServices()` and `getTestConfig()` from `tests/integration/config/unified-setup.ts`
9393
- Use `registerResource()` from `tests/integration/utils/cleanup.ts` for cleanup tracking
9494
- Use `generateRandomString()` from `tests/integration/utils/helpers.ts` for unique test data

src/models/orchestrator/jobs.models.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { JobGetAllOptions, JobGetByIdOptions, RawJobGetResponse } from './jobs.t
22
import { PaginatedResponse, NonPaginatedResponse, HasPaginationOptions } from '../../utils/pagination';
33

44
/** Combined response type for job data with bound methods. */
5-
export interface JobGetResponse extends RawJobGetResponse, JobMethods {}
5+
export type JobGetResponse = RawJobGetResponse & JobMethods;
66

77
/**
88
* Service for managing UiPath Orchestrator Jobs.

src/services/orchestrator/jobs/jobs.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ export class JobService extends FolderScopedService implements JobServiceModel {
136136
if (!id) {
137137
throw new ValidationError({ message: 'id is required for getById' });
138138
}
139+
if (!folderId) {
140+
throw new ValidationError({ message: 'folderId is required for getById' });
141+
}
139142

140143
const headers = createHeaders({ [FOLDER_ID]: folderId });
141144
const keysToPrefix = Object.keys(options);

tests/integration/shared/orchestrator/jobs.integration.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,7 @@ describe.each(modes)('Orchestrator Jobs - Integration Tests [%s]', (mode) => {
179179
});
180180

181181
if (result.items.length === 0) {
182-
console.warn('No jobs available to validate structure — skipping.');
183-
return;
182+
throw new Error('No jobs available to validate structure.');
184183
}
185184

186185
const job = result.items[0];

0 commit comments

Comments
 (0)