Incident and deploy types#359
Conversation
|
|
Resolves README.md conflict by regenerating counts via docs:generate (218 resource types, 38 toolsets after merging incidents + deploys on top of the new chaos resources). AI-Session-Id: 7619c357-b0da-4b97-b960-9cc632e86121 AI-Tool: claude-code AI-Model: unknown
|
@cursoragent can you run Sunil's Architecture Review on this? |
| operationPolicy: { risk: "read", retryPolicy: "safe" }, | ||
| pathParams: { incident_id: "incidentId" }, | ||
| responseExtractor: passthrough, | ||
| description: "Get incident details by ID", |
There was a problem hiding this comment.
Architecture violation: passthrough on real endpoints
All four incident operations (get, create, update, close) use responseExtractor: passthrough, which leaks raw backend envelope/debug/meta fields across the tool boundary.
CLAUDE.md explicitly prohibits this: "Never passthrough on a real endpoint. Backend envelope/debug/meta must not cross the tool boundary."
The deploys.ts in this same PR demonstrates the correct pattern with deployGetExtract. Incidents need an equivalent incidentGetExtract that projects a stable, documented shape (prettyId, title, status, severity, summary, impactedServices, environments, commanderHarnessUserId, createdAt, updatedAt).
| return { service: b.service, version: b.version }; | ||
| }); | ||
| } | ||
| if (item.deployTimestamp !== undefined) slim.deployTimestamp = item.deployTimestamp; |
There was a problem hiding this comment.
Inconsistent null handling for deployTimestamp between list and get
compactDeploy uses if (item.deployTimestamp !== undefined) which keeps null, but deployGetExtract uses if (typeof raw.deployTimestamp === 'number') which drops null.
An agent that lists deploys will see deployTimestamp: null and then fetch the detail view expecting the same field, only to find it absent. Recommend aligning both to typeof === 'number' since a real deploy timestamp is never null.
| } | ||
| if (Array.isArray(item.environments)) slim.environments = item.environments; | ||
| if (Array.isArray(item.buildVersions)) { | ||
| slim.services = item.buildVersions.map((bv) => { |
There was a problem hiding this comment.
compactDeploy lacks isRecord guard on buildVersions items
deployGetExtract (same file) correctly uses if (!isRecord(bv)) return bv before accessing bv.service / bv.version. compactDeploy instead uses (bv ?? {}) which coerces null to {} but leaves strings/numbers as-is, then casts to Record<string, unknown>.
If the API ever returns a non-object element in buildVersions, the result is { service: undefined, version: undefined } silently emitted into the list output. The isRecord guard from type-guards.ts is already imported in this file — use it here the same way deployGetExtract does.
| result.items = compactItems(items, compactFn); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
harness-search doesn't receive compactFn — per-resource compaction silently skipped for search results
harness-list.ts is updated to pass registry.getResource(resourceType).compactItem into compactItems. But harness-search.ts still calls compactItems(r.items) with the old single-arg signature.
This means searching for deploy resources returns the generic whitelist-compacted shape (missing services, untruncated summary), while listing returns the correct compactDeploy shape. An agent doing search + list for the same resource type will see two different response shapes for the same data.
| * projectIdentifier. The client still appends its default `accountIdentifier` | ||
| * query param; the Java side ignores unknown params. | ||
| */ | ||
| const DEPLOY_SCOPE = { account: "accountId", org: "orgId", project: "projectId" } as const; |
There was a problem hiding this comment.
DEPLOY_SCOPE and INCIDENT_SCOPE are identical — and duplicate STO_SCOPE
Both are { account: 'accountId', org: 'orgId', project: 'projectId' } as const, and this is already the third definition of this shape (STO has the same in sto.ts). The comment in both files even says "both route through /api/v1/mc/".
This should be a single shared constant (e.g. MC_SCOPE) exported from a shared location. If the Harness platform ever renames one of these query params (it has happened between API generations), only one of the three copies will get fixed.
| export function compactItems( | ||
| items: unknown[], | ||
| compactFn?: (item: Record<string, unknown>) => Record<string, unknown>, | ||
| ): unknown[] { |
There was a problem hiding this comment.
compactFn path bypasses the openInHarness → markdown hyperlink transform
When compactFn is provided, compactItems returns early from the custom function and never reaches the post-loop block that merges openInHarness into name as a markdown link (the slim.name = `[...](url) logic).
If a deploy item ever gains an openInHarness field, the link will either leak as a raw URL (if compactDeploy preserves it) or be silently dropped — neither is the intended behavior. The compactFn contract should document this gap, or the transform should be applied after compactFn returns.
…er into incident-and-deploy-types
4 participants
Description
Adds support for AI-SRE incident types and AI-SRE detected deployment types.
Type of Change
Checklist