Skip to content

Commit 763f568

Browse files
committed
docs: update squad records for compare instance field
Capture the compare instance-field session in squad decisions, archives, orchestration logs, and agent histories.
1 parent 1e85dcc commit 763f568

7 files changed

Lines changed: 126 additions & 65 deletions

File tree

.squad/agents/codereviewer/history.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,15 @@
169169
**Verdict:** ✅ Charter now serves as authoritative reference for code review standards across all project phases. Tech-specific checks operationalize historical review findings (2026-04-06 through 2026-04-14) into explicit, repeatable criteria.
170170

171171
**Pattern to remember:** The charter evolves as patterns emerge from actual reviews. When CodeReviewer flags something in a review that isn't yet in the charter, ApiOpsLead may enhance the charter to make the pattern explicit for future reviews.
172+
173+
### 2026-05-26: Compare instance-field review
174+
175+
**Context:** Reviewed the compare JSON instance-metadata change that tags one-sided diffs with `instance: 'source' | 'target'`.
176+
177+
**Verdict:** ✅ APPROVED — No blocking issues found
178+
179+
**Key Findings:**
180+
181+
1. The change is at the correct seam: compare-service output, which updates the machine-readable contract without perturbing text or table output.
182+
2. Focused unit coverage is sufficient for the code path changed and verifies both instance assignments while preserving `property-diff` behavior.
183+
3. Remaining gap is non-blocking: add a future CLI-level JSON contract test to catch regressions in serialized output shape.

.squad/agents/typescriptdev/history.md

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,24 @@ Same rule applies to `expect(fs.copyFile).toHaveBeenCalledWith(...)` assertions
249249

250250
**Commit:** (pending)
251251

252-
### 2026-05-23: Build failure due to incomplete dependency install (`tsc` not found)
252+
### 2026-05-26: Compare JSON instance metadata
253253

254-
**Context:** `npm run build` failed immediately with `sh: 1: tsc: not found` even though `typescript` is declared in `devDependencies`.
254+
**Context:** Added instance ownership metadata to compare JSON results so source-only and target-only resources are distinguishable without inferring semantics from `diffType`.
255255

256-
**Root cause:** The workspace had a partial `node_modules` install that did not include full dev tooling and was missing `node_modules/.bin/tsc`.
256+
**Key Decisions:**
257+
- Extended `ComparisonDifference` with optional `instance: 'source' | 'target'`
258+
- Set `instance: 'source'` for `missing` results and `instance: 'target'` for `extra` results at the compare service output seam
259+
- Left `property-diff` unchanged because it already describes a bilateral comparison, not one-sided ownership
260+
261+
**Pattern:** When structured CLI output is ambiguous, fix the result model at the service boundary rather than encoding meaning in presentation-specific formatting.
262+
263+
**Validation:** `npx vitest run tests/unit/services/compare-service.test.ts`
264+
265+
### 2026-05-26: Compare JSON output now identifies source vs target ownership
266+
267+
**Context:** `apiops compare --format json` exposed `diffType: 'missing' | 'extra'`, but that was ambiguous without the table/text formatter's wording.
257268

258-
**Resolution:** Ran `npm ci` from repository root to restore lockfile-consistent dependencies, then re-ran `npm run build` successfully.
269+
**Pattern:** When human-readable output already interprets a structural enum, add the missing semantic data to the shared result model instead of post-processing CLI strings. For compare results, source-only resources should carry `instance: 'source'` and target-only resources should carry `instance: 'target'` directly on `ComparisonDifference`.
259270

260-
**Pattern:** If TypeScript is declared but `tsc` is missing at runtime, verify dependency install integrity before changing code or build scripts. For this repo, use `npm ci` as the first recovery step.
271+
**Guardrail:** Keep `property-diff` shape unchanged unless the extra metadata is semantically meaningful. Focused service-level tests are enough here because the CLI JSON path is a direct `JSON.stringify(result)` passthrough.
261272

.squad/decisions-archive.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Squad Decisions Archive
2+
3+
Archived from `.squad/decisions.md` on 2026-05-26T21:18:34Z. Entries older than 30 days were moved here after the active decisions file exceeded 20 KB.
4+
5+
### 2026-04-21T19:35:00Z: SOAP/WADL spec extraction prefers link format with inline XML fallback
6+
**By:** ApimExpert (via Squad session with a user)
7+
**Status:** Implemented
8+
**What:** For soap-type APIs, `getApiSpecification` requests `format=wsdl-link` first. On HTTP 5xx, it falls back to the inline (non-link) `format=wsdl` export which returns raw WSDL XML in `properties.value`. WADL follows the same pattern (`wadl-link``format=wadl` fallback). The XML fallback content is saved as `specification.wsdl` / `specification.wadl` and is re-importable via PUT `?import=true&format=wsdl` (or `wadl-xml`).
9+
**Why:** User requires full round-trip fidelity — SOAP APIs must be re-importable to a new APIM instance. APIM's `wsdl-link` emitter deterministically returns HTTP 500 on many real-world SOAP APIs. Azure/apiops reference tool skips XML specs on 500 with comment "non-link exports cannot be reimported" — this is inaccurate; the inline form IS re-importable. Converting SOAP → OpenAPI via `openapi-link` works but loses SOAP semantics on round-trip.
10+
11+
### 2026-04-21T19:34:00Z: Synthetic GraphQL APIs skip the graphql-link export call
12+
**By:** ApimExpert (via Squad session with a user)
13+
**Status:** Implemented
14+
**What:** Before calling `graphql-link` export, `api-extractor.ts` probes ApiSchema children via `hasGraphQLSchemaResource` and checks for `contentType` containing 'graphql'. If found (synthetic GraphQL — SDL stored as an ApiSchema resource), the export call is skipped. If not found (pass-through GraphQL), `graphql-link` is called normally.
15+
**Why:** APIM returns HTTP 406 on `graphql-link` export for synthetic GraphQL APIs because there is nothing to export — the SDL is already held as an ApiSchema child resource and is captured by standard ApiSchema extraction. Skipping the redundant call avoids the error without losing fidelity.
16+
17+
### 2026-04-21T19:33:00Z: XML export fallback bypasses the default 5xx retry loop
18+
**By:** ApimExpert (via Squad session with user)
19+
**Status:** Implemented
20+
**What:** `getApiSpecification` passes `noRetryOn5xx=true` to `request()` when exporting `wsdl-link` or `wadl-link`. The fallback to inline format runs immediately on HTTP 5xx rather than after three retries.
21+
**Why:** APIM's wsdl-link/wadl-link 500 errors are deterministic failures in APIM's XML emitter, not transient. Retrying wastes time and delays the fallback. The inline format path is fast and reliable.
22+
23+
### 2026-04-14T21:37:55Z: Resource Path Labels for Log Output
24+
**By:** CodeReviewer
25+
**Status:** Approved
26+
**What:** Implemented `buildResourceLabel()` utility to generate human-readable hierarchical resource paths in logs. Format: serviceName/grandparent/parent/name (e.g., "apim-1/petstore/get-user" instead of just "get-user"). Applied across resource-extractor.ts, api-extractor.ts, and extract-service.ts.
27+
**Why:** Improves observability by providing full context in log messages; aids debugging and tracing. Tested comprehensively (8 unit tests, all 467 integration tests passing). Compliant with Constitution §I-§VIII; no secret safety risks (only metadata logged).
28+
29+
### 2026-04-13T23:35:35Z: Replace --verbose with --log-level Option
30+
**By:** TypeScriptDev
31+
**Status:** Implemented
32+
**What:** Replaced boolean `--verbose` flag with `--log-level <level>` supporting debug, info, warn, error (default: info). Logger updated with `LOG_LEVEL_PRIORITY` numeric filtering; all 432 tests pass.
33+
**Why:** Granular log control (4 levels vs. binary), production-friendly (suppress INFO noise), industry standard alignment (kubectl, docker, terraform), explicit semantics. Breaking change; users update `--verbose` to `--log-level debug`.
34+
35+
### 2026-04-13T18:50:54Z: Comprehensive test coverage for API publisher and rate limiting
36+
**By:** TestEngineer
37+
**What:** Created `tests/unit/services/api-publisher.test.ts` (20 tests) covering all aspects of API publisher service, and enhanced `tests/unit/clients/apim-client.test.ts` with 4 rate-limiting tests for HTTP 429 handling.
38+
**Why:** Critical gap: api-publisher.ts had no dedicated test file despite being central to T032 (revision handling). Additionally, HTTP 429 rate limiting (FR-015) was untested. Both pose production risks. Solution maintains Constitution §VI (unit tests only, no external deps), full mock coverage, and edge case testing.
39+
40+
### 2026-04-10T18:14:39Z: Text-first XML parsing in ApimClient.getResource
41+
**By:** TypeScriptDev
42+
**What:** Modified `getResource` to handle raw XML responses from APIM policy endpoints by reading response as text first, detecting XML via Content-Type header or body sniffing, then wrapping in ARM envelope.
43+
**Why:** APIM policy endpoints (ServicePolicy, ApiPolicy, etc.) return raw XML instead of JSON-wrapped XML. Previous implementation crashed on `response.json()`. Text-first approach is defensive, maintains backward compatibility (no interface changes), and handles both explicit and implicit XML detection.
44+
45+
### 2025-04-09T05:34:00Z: Formalized commit message convention
46+
**By:** ApiOpsLead
47+
**What:** Codified the commit convention (include `Closes #N` or `Fixes #N` when resolving issues) into CONTRIBUTING.md and PR template. Previously this existed only in agent memory.
48+
**Why:** Conventions that only live in agent memory are invisible to human contributors and new AI agents. Formalizing in repo files ensures all contributors follow the same process.
49+
50+
### 2025-05-18: GitHub Agentic Workflows (gh-aw) Adoption Strategy
51+
**By:** GitHubExpert
52+
**Status:** Proposed
53+
**Scope:** Branch maintenance workflows
54+
55+
**Context:** Whether or not to use gh-aw or hand-rolled YAML implementations.
56+
57+
**Decision:**
58+
- Use gh-aw LabelOps pattern, event pattern for advising.
59+
- Use hand-rolled yaml for deterministic outcode, like CI gates.
60+
61+
**Impact:** Reduces maintenance burden for advisory workflows; eliminates keyword-matching brittleness in triage; no change to security posture.

.squad/decisions.md

Lines changed: 20 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,24 @@
22

33
## Active Decisions
44

5+
### 2026-05-26T21:18:34Z: Compare JSON results include source or target instance metadata
6+
**By:** TypeScriptDev
7+
**Status:** Implemented
8+
**What:** Added an optional `instance: 'source' | 'target'` field to `ComparisonDifference` so structured compare output explicitly identifies whether a non-matching resource exists only in the source or only in the target APIM instance.
9+
10+
**Implementation:**
11+
- `missing` diffs now emit `instance: 'source'`
12+
- `extra` diffs now emit `instance: 'target'`
13+
- `property-diff` entries remain unchanged
14+
- Focused unit coverage verifies both instance values and confirms property diffs are unaffected
15+
16+
**Why:** `diffType` alone was ambiguous in JSON mode because it did not say which APIM instance owned a resource that appeared only on one side. The compare service is the right seam to make the machine-readable contract self-describing without changing table or text output.
17+
18+
**Validation:**
19+
- `npx vitest run tests/unit/services/compare-service.test.ts`
20+
21+
---
22+
523
### 2026-05-22T08:08:23Z: apiops compare Command — Cloud-to-Cloud Comparison
624
**By:** ApimExpert
725
**Status:** Completed (lint errors pending)
@@ -146,66 +164,6 @@
146164
**What:** Made `--cli-package` optional in `apiops init`. The command now supports two package consumption modes: (1) **Public npm mode** (default, when `--cli-package` NOT provided): generates package.json with `"@peterhauge/apiops-cli": "latest"`, no local tarball copy, no `.apiops/` directory created, standard consumption pattern after npm publish. (2) **Local tarball mode** (when `--cli-package <path>` provided): copies tarball to `.apiops/` directory, generates package.json with `"apiops": "file:.apiops/{tarball}"`, preserves existing behavior for local development/testing.
147165
**Why:** After publishing to npm as `@peterhauge/apiops-cli`, requiring users to download the package and run `apiops init --cli-package ./tarball.tgz` added unnecessary friction. Most users want to reference the public package directly. The change is backward compatible — existing workflows with `--cli-package` continue to work unchanged. Improves user experience with simpler onboarding.
148166

149-
### 2026-04-21T19:35:00Z: SOAP/WADL spec extraction prefers link format with inline XML fallback
150-
**By:** ApimExpert (via Squad session with a user)
151-
**Status:** Implemented
152-
**What:** For soap-type APIs, `getApiSpecification` requests `format=wsdl-link` first. On HTTP 5xx, it falls back to the inline (non-link) `format=wsdl` export which returns raw WSDL XML in `properties.value`. WADL follows the same pattern (`wadl-link``format=wadl` fallback). The XML fallback content is saved as `specification.wsdl` / `specification.wadl` and is re-importable via PUT `?import=true&format=wsdl` (or `wadl-xml`).
153-
**Why:** User requires full round-trip fidelity — SOAP APIs must be re-importable to a new APIM instance. APIM's `wsdl-link` emitter deterministically returns HTTP 500 on many real-world SOAP APIs. Azure/apiops reference tool skips XML specs on 500 with comment "non-link exports cannot be reimported" — this is inaccurate; the inline form IS re-importable. Converting SOAP → OpenAPI via `openapi-link` works but loses SOAP semantics on round-trip.
154-
155-
### 2026-04-21T19:34:00Z: Synthetic GraphQL APIs skip the graphql-link export call
156-
**By:** ApimExpert (via Squad session with a user)
157-
**Status:** Implemented
158-
**What:** Before calling `graphql-link` export, `api-extractor.ts` probes ApiSchema children via `hasGraphQLSchemaResource` and checks for `contentType` containing 'graphql'. If found (synthetic GraphQL — SDL stored as an ApiSchema resource), the export call is skipped. If not found (pass-through GraphQL), `graphql-link` is called normally.
159-
**Why:** APIM returns HTTP 406 on `graphql-link` export for synthetic GraphQL APIs because there is nothing to export — the SDL is already held as an ApiSchema child resource and is captured by standard ApiSchema extraction. Skipping the redundant call avoids the error without losing fidelity.
160-
161-
### 2026-04-21T19:33:00Z: XML export fallback bypasses the default 5xx retry loop
162-
**By:** ApimExpert (via Squad session with user)
163-
**Status:** Implemented
164-
**What:** `getApiSpecification` passes `noRetryOn5xx=true` to `request()` when exporting `wsdl-link` or `wadl-link`. The fallback to inline format runs immediately on HTTP 5xx rather than after three retries.
165-
**Why:** APIM's wsdl-link/wadl-link 500 errors are deterministic failures in APIM's XML emitter, not transient. Retrying wastes time and delays the fallback. The inline format path is fast and reliable.
166-
167-
### 2026-04-14T21:37:55Z: Resource Path Labels for Log Output
168-
**By:** CodeReviewer
169-
**Status:** Approved
170-
**What:** Implemented `buildResourceLabel()` utility to generate human-readable hierarchical resource paths in logs. Format: serviceName/grandparent/parent/name (e.g., "apim-1/petstore/get-user" instead of just "get-user"). Applied across resource-extractor.ts, api-extractor.ts, and extract-service.ts.
171-
**Why:** Improves observability by providing full context in log messages; aids debugging and tracing. Tested comprehensively (8 unit tests, all 467 integration tests passing). Compliant with Constitution §I-§VIII; no secret safety risks (only metadata logged).
172-
173-
### 2026-04-13T23:35:35Z: Replace --verbose with --log-level Option
174-
**By:** TypeScriptDev
175-
**Status:** Implemented
176-
**What:** Replaced boolean `--verbose` flag with `--log-level <level>` supporting debug, info, warn, error (default: info). Logger updated with `LOG_LEVEL_PRIORITY` numeric filtering; all 432 tests pass.
177-
**Why:** Granular log control (4 levels vs. binary), production-friendly (suppress INFO noise), industry standard alignment (kubectl, docker, terraform), explicit semantics. Breaking change; users update `--verbose` to `--log-level debug`.
178-
179-
### 2026-04-13T18:50:54Z: Comprehensive test coverage for API publisher and rate limiting
180-
**By:** TestEngineer
181-
**What:** Created `tests/unit/services/api-publisher.test.ts` (20 tests) covering all aspects of API publisher service, and enhanced `tests/unit/clients/apim-client.test.ts` with 4 rate-limiting tests for HTTP 429 handling.
182-
**Why:** Critical gap: api-publisher.ts had no dedicated test file despite being central to T032 (revision handling). Additionally, HTTP 429 rate limiting (FR-015) was untested. Both pose production risks. Solution maintains Constitution §VI (unit tests only, no external deps), full mock coverage, and edge case testing.
183-
184-
### 2026-04-10T18:14:39Z: Text-first XML parsing in ApimClient.getResource
185-
**By:** TypeScriptDev
186-
**What:** Modified `getResource` to handle raw XML responses from APIM policy endpoints by reading response as text first, detecting XML via Content-Type header or body sniffing, then wrapping in ARM envelope.
187-
**Why:** APIM policy endpoints (ServicePolicy, ApiPolicy, etc.) return raw XML instead of JSON-wrapped XML. Previous implementation crashed on `response.json()`. Text-first approach is defensive, maintains backward compatibility (no interface changes), and handles both explicit and implicit XML detection.
188-
189-
### 2025-04-09T05:34:00Z: Formalized commit message convention
190-
**By:** ApiOpsLead
191-
**What:** Codified the commit convention (include `Closes #N` or `Fixes #N` when resolving issues) into CONTRIBUTING.md and PR template. Previously this existed only in agent memory.
192-
**Why:** Conventions that only live in agent memory are invisible to human contributors and new AI agents. Formalizing in repo files ensures all contributors follow the same process.
193-
194-
### 2025-05-18: GitHub Agentic Workflows (gh-aw) Adoption Strategy
195-
**By:** GitHubExpert
196-
**Status:** Proposed
197-
**Scope:** Branch maintenance workflows
198-
199-
**Context:** Whether or not to use gh-aw or hand-rolled YAML implementations.
200-
201-
**Decision:**
202-
- Use gh-aw LabelOps pattern, event pattern for advising.
203-
- Use hand-rolled yaml for deterministic outcode, like CI gates.
204-
205-
**Impact:** Reduces maintenance burden for advisory workflows; eliminates keyword-matching brittleness in triage; no change to security posture.
206-
207-
---
208-
209167
### 2026-06-11: GitHub Agentic Workflows (gh-aw) Security Assessment
210168
**By:** SecurityExpert
211169
**Status:** Proposed
@@ -249,6 +207,8 @@
249207

250208
**Net Assessment:** gh-aw improves least-privilege, blast radius, auditability, and separation of concerns vs. current model. Weakens determinism and introduces prompt injection surface (mitigated by constraints). Net security improvement for advisory workflows when guardrails are in place.
251209

210+
Archived entries older than 30 days are stored in `.squad/decisions-archive.md`.
211+
252212
---
253213

254214
## Governance
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Session Log
2+
3+
- Timestamp: 2026-05-26T21:18:34Z
4+
- Topic: compare instance field
5+
- Summary: TypeScriptDev made compare JSON output self-describing by tagging source-only and target-only differences with an `instance` field. CodeReviewer approved and noted only a missing CLI-level JSON contract test as follow-up.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Orchestration Log
2+
3+
- Timestamp: 2026-05-26T21:18:34Z
4+
- Agent: CodeReviewer
5+
- Work summary: Reviewed the compare instance-field change, found no blocking issues, and approved the diff with one non-blocking note about adding a future CLI-level JSON contract test.
6+
- Outcome: Approved
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Orchestration Log
2+
3+
- Timestamp: 2026-05-26T21:18:34Z
4+
- Agent: TypeScriptDev
5+
- Work summary: Added `instance: 'source' | 'target'` to the compare JSON result model, updated compare result production in the compare service, added focused unit coverage, and ran targeted validation.
6+
- Validation: `npx vitest run tests/unit/services/compare-service.test.ts`

0 commit comments

Comments
 (0)