From 3f5c2344abd5845e5003d2d17827a296742199e3 Mon Sep 17 00:00:00 2001 From: certctl-bot Date: Sat, 25 Apr 2026 16:07:31 +0000 Subject: [PATCH] =?UTF-8?q?fix(web,ci):=20close=20TS=E2=86=94Go=20type=20d?= =?UTF-8?q?rift=20across=205=20entities=20(D-2=20master)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes five 2026-04-24 audit findings (all P2, all category cat-f / diff-05x06-*) by reconciling the TypeScript interfaces in web/src/api/types.ts with the on-wire JSON shape Go's internal/domain/*.go structs actually emit. D-1 closed the same pattern for one entity (Certificate / ManagedCertificate); D-2 covers the remaining five. Per-entity verdicts (audit's "stricter side is the contract"): Agent — TRIM 5 phantoms (last_heartbeat, capabilities, tags, created_at, updated_at). Go emits last_heartbeat_at only. Target — ADD 2 (retired_at?, retired_reason?) — I-004 fields. DiscCert — ADD pem_data? — real field, real Go emit, omitempty. Issuer — TRIM phantom status. Go has Enabled bool only. Notif — TRIM phantom subject. Go has Message string only. Certificate — verify-only; D-1 closure confirmed clean at recon. Consumer fixes (same commit as the trim): - AgentDetailPage.tsx — remove dead Capabilities + Tags sections (always rendered empty); replace agent.created_at/updated_at row with the Go-emitted registered_at; widen heartbeatStatus() to accept undefined. - AgentsPage.tsx — same heartbeatStatus widening. - IssuersPage.tsx + IssuerDetailPage.tsx — issuerStatus() now derives from `enabled` exclusively; the dead `issuer.status || 'Unknown'` fallback is gone. - NotificationsPage.tsx — drop dead `|| n.subject` fallback. - NotificationsPage.test.tsx — drop dead `subject:` from mocks. - api/utils.ts::timeAgo widened to accept string | undefined | null. - api/types.test.ts — Agent (I-004) fixture trimmed of the 5 phantoms. Tests (Vitest): - 5 new describe blocks in web/src/api/types.test.ts: - Agent interface (D-2 phantom-fields trim) — 2 it blocks - Target interface (D-2 retirement fields) — 2 it blocks - DiscoveredCertificate interface (D-2 pem_data ADD) — 2 it blocks - Issuer interface (D-2 status phantom trim) — 1 it block - Notification interface (D-2 subject phantom trim) — 1 it block - Each block uses the literal-construction pattern from D-1; trimmed fields are pinned via excess-property comments that compile-fail when uncommented if a phantom is reintroduced. CI regression guardrail: - .github/workflows/ci.yml — existing D-1 step renamed to "Forbidden StatusBadge dead-key + TS phantom-field regression guard (D-1 + D-2)". Three new awk-windowed greps over Agent / Issuer / Notification interfaces in types.ts. The Agent grep includes a `grep -v 'last_heartbeat_at'` filter to avoid false positives on the legitimate Go-emitted heartbeat field. Documentation: - CHANGELOG.md — new D-2 section above B-1 under [unreleased] with full Added/Removed/Audit findings closed/Known follow-ups breakdown. - docs/architecture.md — Web Dashboard section gains a new "TS ↔ Go type contract rule (D-1 + D-2 closure)" paragraph capturing the stricter-side-wins rule and the CI guardrail it's anchored by. - coverage-gap-audit-2026-04-24-v5/unified-audit.md — Live Tracker score 20/47 → 25/47 (P2: 6/27 → 11/27). Per-finding ✅ RESOLVED Status blocks added to all 5 diff-05x06-* entries plus the verify-only Certificate entry. Closed-bundle index gets D-2 row. Verification (all gates green): - cd web && tsc --noEmit → clean - cd web && vitest run --reporter=dot → 9 files, 302 tests passing (was 294 → +8 D-2 cases) - cd web && vite build → clean - go vet ./internal/... ./cmd/... → clean (no Go touched) - golangci-lint v2.11.4 run ./... → 0 issues - D-2 Agent guardrail dry-run → empty (good) - D-2 Issuer guardrail dry-run → empty (good) - D-2 Notification guardrail dry-run → empty (good) - D-2 Target ADD-shape sanity → 2 retirement fields present - D-2 DiscCert ADD-shape sanity → pem_data present - D-1 Certificate guardrail still clean → empty (good) - OpenAPI YAML parses → 89 paths Audit findings closed: - diff-05x06-7cdf4e78ae24 (P2, Agent TS↔Go drift) - diff-05x06-2044a46f4dd0 (P2, Target TS↔DeploymentTarget Go drift) - diff-05x06-85ab6b98a2f7 (P2, DiscoveredCertificate TS↔Go drift) - diff-05x06-97fab8783a5c (P2, Issuer TS↔Go drift) - diff-05x06-caba9eb3620e (P2, Notification TS↔NotificationEvent drift) - diff-05x06-af18a8d7ef41 (P2) — verified clean since D-1; no edit Deferred follow-ups: - Issuer richer status view (enabled × test_status) — UX scope, not drift. - Real Agent metadata (capabilities, tags) — backend feature, not drift. - DiscoveredCertificate pem_data list-response perf — separate backend change. --- .github/workflows/ci.yml | 80 ++++++- CHANGELOG.md | 33 +++ docs/architecture.md | 2 + web/src/api/types.test.ts | 272 ++++++++++++++++++++++- web/src/api/types.ts | 75 ++++++- web/src/api/utils.ts | 7 +- web/src/pages/AgentDetailPage.tsx | 49 ++-- web/src/pages/AgentsPage.tsx | 7 +- web/src/pages/IssuerDetailPage.tsx | 13 +- web/src/pages/IssuersPage.tsx | 18 +- web/src/pages/NotificationsPage.test.tsx | 5 +- web/src/pages/NotificationsPage.tsx | 8 +- 12 files changed, 506 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a59745d..781c6ce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,7 +270,7 @@ jobs: exit 1 fi - - name: Forbidden StatusBadge dead-key + Certificate phantom-field regression guard (D-1) + - name: Forbidden StatusBadge dead-key + TS phantom-field regression guard (D-1 + D-2) # D-1 master closed cat-d-359e92c20cbf (Agent: 'Stale' dead key, # 'Degraded' missing), cat-d-9f4c8e4a91f1 (Notification: 'dead' # missing), cat-d-1447e04732e7 (Cert: 'PendingIssuance' dead @@ -344,6 +344,84 @@ jobs: exit 1 fi + # D-2 master closed five diff-05x06-* type-drift findings: + # Agent (5 phantoms), Issuer (1 phantom), Notification (1 phantom) + # — TRIM half. The Target (2 missing fields) and DiscoveredCertificate + # (1 missing field) — ADD half is pinned by the literal-construction + # blocks in web/src/api/types.test.ts, not a CI grep. The phantom- + # trim regression vector is an awk-windowed grep per interface + # mirroring the D-1 Certificate check above. + # + # See coverage-gap-audit-2026-04-24-v5/unified-audit.md + # diff-05x06-7cdf4e78ae24 (Agent), diff-05x06-97fab8783a5c (Issuer), + # diff-05x06-caba9eb3620e (Notification) for the closure rationale. + + # D-2 Agent phantom-field check. The grep matches `last_heartbeat` + # but NOT `last_heartbeat_at` (the legitimate Go-emitted field) — + # the `\b...\b` boundaries plus the `grep -v 'last_heartbeat_at'` + # filter handle that. + BAD_AGENT=$(awk ' + /^export interface Agent \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } + ' web/src/api/types.ts \ + | grep -E '\b(last_heartbeat|capabilities|tags|created_at|updated_at)\??\s*:' \ + | grep -v 'last_heartbeat_at' \ + || true) + if [ -n "$BAD_AGENT" ]; then + echo "D-2 regression: Agent TS interface re-added a phantom field:" + echo "$BAD_AGENT" + echo "" + echo "The Go-side internal/domain/connector.go::Agent emits exactly:" + echo "id, name, hostname, status, last_heartbeat_at?, registered_at," + echo "os, architecture, ip_address, version, retired_at?, retired_reason?." + echo "The five fields blocked by this guard (last_heartbeat," + echo "capabilities, tags, created_at, updated_at) were TS phantoms" + echo "the Go struct never emitted. See unified-audit.md" + echo "diff-05x06-7cdf4e78ae24 for closure rationale." + exit 1 + fi + + # D-2 Issuer phantom-field check. + BAD_ISSUER=$(awk ' + /^export interface Issuer \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } + ' web/src/api/types.ts \ + | grep -E '\bstatus\??\s*:' \ + || true) + if [ -n "$BAD_ISSUER" ]; then + echo "D-2 regression: Issuer TS interface re-added a phantom 'status' field:" + echo "$BAD_ISSUER" + echo "" + echo "The Go-side internal/domain/connector.go::Issuer has no 'status'" + echo "field — only 'enabled' (bool). Render sites derive the displayed" + echo "status from 'enabled' at the call site (see" + echo "web/src/pages/IssuersPage.tsx::issuerStatus). See unified-audit.md" + echo "diff-05x06-97fab8783a5c for closure rationale." + exit 1 + fi + + # D-2 Notification phantom-field check. + BAD_NOTIF=$(awk ' + /^export interface Notification \{/ { flag=1; next } + flag && /^\}/ { flag=0 } + flag { print FILENAME":"NR":"$0 } + ' web/src/api/types.ts \ + | grep -E '\bsubject\??\s*:' \ + || true) + if [ -n "$BAD_NOTIF" ]; then + echo "D-2 regression: Notification TS interface re-added a phantom 'subject' field:" + echo "$BAD_NOTIF" + echo "" + echo "The Go-side internal/domain/notification.go::NotificationEvent" + echo "has no 'subject' field — only 'message'. Pre-D-2 the consumer" + echo "at NotificationsPage.tsx had a dead '|| n.subject' fallback" + echo "that always fell through. See unified-audit.md" + echo "diff-05x06-caba9eb3620e for closure rationale." + exit 1 + fi + - name: Forbidden client-side bulk-action loop regression guard (L-1) # L-1 master closed cat-l-fa0c1ac07ab5 (bulk-renew loop) and # cat-l-8a1fb258a38a (bulk-reassign loop) by adding server-side diff --git a/CHANGELOG.md b/CHANGELOG.md index b51ee1b..cead220 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,39 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601. ## [unreleased] — 2026-04-25 +### D-2: TS ↔ Go type drift cluster — closed end-to-end + +> The 2026-04-24 coverage-gap audit flagged five `diff-05x06-*` findings — every one a TypeScript-vs-Go shape mismatch where the on-wire JSON the backend emits and the TS interface in `web/src/api/types.ts` had drifted apart. D-1 master closed the same pattern for `Certificate` (cat-f-ae0d06b6588f, 5 phantom fields trimmed, plus the cat-f-cert_detail_page_key_render_fallback render-site fix). D-2 closes it for the remaining five entities: Agent, Target, DiscoveredCertificate, Issuer, and Notification. The audit's blunt rule "stricter side is the contract" decides the per-entity verdict — for TS phantoms (fields declared on TS, never emitted by Go) the Go side wins and TS gets trimmed; for TS-missing fields (emitted by Go, absent from TS) the Go side still wins and TS gets the addition. Pre-D-2 the failure modes were: phantom fields silently rendered `'—'` at consumer sites (e.g. AgentDetailPage's "Capabilities" + "Tags" sections always rendered empty; IssuersPage rendered `'Unknown'` for every issuer; NotificationsPage's `n.message || n.subject` fallback always fell through), and missing fields forced `(target as any).retired_at` escapes that lost type-checking. Verify-only side task: Certificate / ManagedCertificate confirmed clean since D-1. + +### Breaking Changes + +None on the wire. The JSON the backend emits is byte-identical pre/post-D-2 — D-2 is purely TS-side reconciliation. The interface shapes change in ways that are TypeScript compile errors at consumer sites that read trimmed phantoms (intentionally — that's the closure mechanism) but no operator-visible behaviour shifts. + +### Added + +- `Target` interface gains `retired_at?: string | null` and `retired_reason?: string | null` (mirrors the Agent retirement-fields shape and the Go-side `internal/domain/connector.go::DeploymentTarget` I-004 model). An Agent retire cascades to all associated Targets per `service.RetireAgent → repository.RetireTarget`; the GUI can now type-check the retired-state surfacing without `(target as any).retired_at` escapes. +- `DiscoveredCertificate` interface gains `pem_data?: string`. The Go-side struct (`internal/domain/discovery.go::DiscoveredCertificate.PEMData`, `omitempty`) emits this field on the wire — populated by the agent filesystem scanner, the cloud-secret-manager connectors, and the repo SELECT. Optional because Go uses `omitempty`. Consumers can now reach the raw PEM with type-checked code. +- **CI regression guardrail extension** in `.github/workflows/ci.yml` (renamed `Forbidden StatusBadge dead-key + TS phantom-field regression guard (D-1 + D-2)`) — adds three new awk-windowed greps over the Agent / Issuer / Notification interfaces in `types.ts` that fail the build if any of the trimmed phantom fields reappear. The Agent regex `\b(last_heartbeat|capabilities|tags|created_at|updated_at)\b` is paired with a `grep -v 'last_heartbeat_at'` filter to avoid false positives on the legitimate Go-emitted heartbeat field. + +### Removed + +- `Agent` interface — 5 phantom fields trimmed: `last_heartbeat`, `capabilities`, `tags`, `created_at`, `updated_at`. None emitted by `internal/domain/connector.go::Agent`. Two had real consumers in `AgentDetailPage.tsx` (capabilities + tags sections) — both were removed because their guards always evaluated false. The "Updated" InfoRow that read `agent.updated_at` was also dropped (Go has no equivalent timestamp on Agent). `last_heartbeat_at` flipped from required to optional to match Go's `*time.Time omitempty`. +- `Issuer` interface — phantom `status: string` removed. Go has only `Enabled bool`. Both `IssuersPage.tsx::issuerStatus` and `IssuerDetailPage.tsx::issuerStatus` rewritten to compute `i.enabled ? 'Enabled' : 'Disabled'` exclusively (the pre-D-2 fallback `issuer.status || 'Unknown'` always rendered 'Unknown'). +- `Notification` interface — phantom `subject?: string` removed. The dead `{n.message || n.subject}` fallback at `NotificationsPage.tsx:241` was simplified to `{n.message}`. Test mocks in `NotificationsPage.test.tsx` no longer set the field. + +### Audit findings closed + +- diff-05x06-7cdf4e78ae24 (P2, Agent TS↔Go drift) +- diff-05x06-2044a46f4dd0 (P2, Target TS↔DeploymentTarget Go drift) +- diff-05x06-85ab6b98a2f7 (P2, DiscoveredCertificate TS↔Go drift) +- diff-05x06-97fab8783a5c (P2, Issuer TS↔Go drift) +- diff-05x06-caba9eb3620e (P2, Notification TS↔NotificationEvent Go drift) +- diff-05x06-af18a8d7ef41 (P2, Certificate / ManagedCertificate) — verified no residual drift since D-1; no edit required + +### Known follow-ups (deferred from D-2 scope) + +A richer Issuer status view that derives from `enabled × test_status` (instead of `enabled` alone) is deferred — a UX scope decision, not a contract drift, and the existing `test_status: 'untested' | 'success' | 'failed'` field is already on the TS interface for whoever picks up that work. Real Agent metadata fields (capabilities advertised at heartbeat time, operator-applied tags) are deferred — D-2 removed the false UI affordance; if/when the product wants real fields, re-introduce in `AgentDetailPage` in the same commit that ships the Go-side change. The `DiscoveredCertificate.pem_data` LIST-response performance optimization (gate emission on the per-id detail path, since pem_data is kilobytes per row) is deferred as a separate backend change — D-2 only closed the contract drift. + ### B-1: Orphan-CRUD client functions + RenewalPolicy GUI gap — closed end-to-end > The 2026-04-24 coverage-gap audit flagged a cluster of operator-blocking GUI omissions: six client.ts `update*` functions (`updateOwner`, `updateTeam`, `updateAgentGroup`, `updateIssuer`, `updateProfile`, plus the full `*RenewalPolicy` CRUD trio) had backend handlers, OpenAPI operations, and exported TypeScript fetchers — but zero page consumers. Operators wanting to fix a typo in an owner's email, rename a team, retarget an agent group's match rules, or edit a renewal-policy field were forced to either delete-and-recreate (losing FK history and audit-trail continuity) or open a `psql` session against the production database directly. The audit's blunt summary: "every backend feature ships with its GUI surface" — a load-bearing CLAUDE.md invariant — was being violated for five operator-facing entities. B-1 closes that violation by wiring per-page Edit modals onto five existing pages, adding a brand-new `RenewalPoliciesPage` for the rp-* CRUD surface, and deleting one dead duplicate (`exportCertificatePEM`) so the public client surface area stops growing without consumers. diff --git a/docs/architecture.md b/docs/architecture.md index 2ddc408..c3caf40 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -165,6 +165,8 @@ The dashboard includes an **ErrorBoundary component** for graceful error recover **Backend ↔ frontend round-trip rule (B-1 closure):** every backend CRUD operation must have at least one GUI consumer in `web/src/pages/`. Shipping a handler + repository method + OpenAPI operation + `client.ts` fetcher with no page that calls it leaves operators forced to `psql` directly — defeats the "every backend feature ships with its GUI surface" invariant and creates a destructive workflow when the missing path is `update*` (operators delete-and-recreate, losing FK history and audit-trail continuity). The CI guardrail in `.github/workflows/ci.yml` (`Forbidden orphan-CRUD client function regression guard (B-1)`) enforces this for the eight previously-orphan functions (`updateOwner`/`updateTeam`/`updateAgentGroup`/`updateIssuer`/`updateProfile` + `createRenewalPolicy`/`updateRenewalPolicy`/`deleteRenewalPolicy`); apply the same rule when adding any new write endpoint. If a fetcher is needed in `client.ts` before its consumer page exists, leave a TODO referencing this rule and ship them in the same commit. +**TS ↔ Go type contract rule (D-1 + D-2 closure):** every TypeScript interface in `web/src/api/types.ts` must field-match the Go-side `internal/domain/*.go` struct's JSON-emitted shape exactly. Phantom fields (declared on TS, never emitted by Go) silently render `'—'` and lull consumers into thinking a value will arrive that never does; missing fields (emitted by Go, absent from TS) force `(x as any).X` escapes that lose type-checking. Both failure modes are blocked by the CI guardrail in `.github/workflows/ci.yml` (`Forbidden StatusBadge dead-key + TS phantom-field regression guard (D-1 + D-2)`) which awk-windows each interface and grep-fails the build on phantom-field reintroduction — currently covers Certificate (D-1), Agent / Issuer / Notification (D-2). Apply the same rule when adding any new on-wire type: the Go-side json tag is the contract, the TS interface adapts to it, and a literal-construction Vitest in `web/src/api/types.test.ts` pins the post-add shape. Stricter side wins: when in doubt, the side that actually emits the field is the contract; never propose adding a phantom on Go to match a TS over-declaration. + ### PostgreSQL Database All state is stored in PostgreSQL 16. The schema uses TEXT primary keys (not UUIDs) with human-readable prefixed IDs like `mc-api-prod`, `t-platform`, `o-alice`. diff --git a/web/src/api/types.test.ts b/web/src/api/types.test.ts index cd39629..1ec985b 100644 --- a/web/src/api/types.test.ts +++ b/web/src/api/types.test.ts @@ -1,6 +1,14 @@ import { describe, it, expect } from 'vitest'; import { POLICY_TYPES, POLICY_SEVERITIES } from './types'; -import type { Agent, Certificate, CertificateVersion } from './types'; +import type { + Agent, + Certificate, + CertificateVersion, + DiscoveredCertificate, + Issuer, + Notification, + Target, +} from './types'; /** * Regression tests for the policy enum tuples. @@ -85,6 +93,9 @@ describe('Agent interface (I-004 retirement)', () => { // Construct an Agent with the retirement fields set. If Phase 2b names // them anything other than retired_at / retired_reason, this fails to // compile — which is exactly what the Red stage wants. + // D-2 (master): the post-D-2 Agent shape no longer carries + // last_heartbeat / capabilities / tags / created_at / updated_at — + // those were TS phantoms the Go-side struct never emitted. const retired: Agent = { id: 'ag-1', name: 'decom-01', @@ -94,13 +105,8 @@ describe('Agent interface (I-004 retirement)', () => { architecture: 'amd64', status: 'Offline', version: '2.1.0', - last_heartbeat: '2026-01-01T00:00:00Z', last_heartbeat_at: '2026-01-01T00:00:00Z', - capabilities: [], - tags: {}, registered_at: '2024-01-01T00:00:00Z', - created_at: '2024-01-01T00:00:00Z', - updated_at: '2026-01-01T00:00:00Z', retired_at: '2026-01-01T00:00:00Z', retired_reason: 'old hardware', }; @@ -111,6 +117,7 @@ describe('Agent interface (I-004 retirement)', () => { it('accepts an Agent without retired_at / retired_reason (optional fields)', () => { // Active agents should not carry retirement metadata. If Phase 2b makes // the fields required, this block fails to compile. + // D-2 (master): post-D-2 Agent shape (see sibling describe block). const active: Agent = { id: 'ag-2', name: 'web01', @@ -120,13 +127,8 @@ describe('Agent interface (I-004 retirement)', () => { architecture: 'amd64', status: 'Online', version: '2.1.0', - last_heartbeat: '2026-04-18T12:00:00Z', last_heartbeat_at: '2026-04-18T12:00:00Z', - capabilities: ['deploy', 'scan'], - tags: {}, registered_at: '2024-06-01T00:00:00Z', - created_at: '2024-06-01T00:00:00Z', - updated_at: '2026-04-18T12:00:00Z', }; expect(active.retired_at).toBeUndefined(); expect(active.retired_reason).toBeUndefined(); @@ -220,3 +222,251 @@ describe('Certificate interface (D-5 phantom-fields trim)', () => { expect(v.key_size).toBe(256); }); }); + +/** + * D-2 (diff-05x06-7cdf4e78ae24, master): Agent TS phantom-fields trim. + * + * Pre-D-2 the `Agent` interface declared five fields that the Go-side + * struct (`internal/domain/connector.go::Agent`) does NOT emit on the + * wire: `last_heartbeat`, `capabilities`, `tags`, `created_at`, + * `updated_at`. Two of them had real consumers (`AgentDetailPage.tsx` + * read `agent.capabilities` and `agent.tags`) — both always rendered the + * empty-state branch because the runtime values were always `undefined`. + * + * Post-D-2 a `agent.capabilities` access is a TS compile error, forcing + * every consumer to acknowledge the field is not part of the Agent + * contract. The Go-side struct emits exactly: id, name, hostname, status, + * last_heartbeat_at (note the `_at` suffix — this is the real heartbeat + * field and stays), registered_at, os, architecture, ip_address, version, + * retired_at?, retired_reason?. + */ +describe('Agent interface (D-2 phantom-fields trim)', () => { + it('does NOT declare last_heartbeat / capabilities / tags / created_at / updated_at', () => { + // Construct an Agent with ONLY the post-D-2 field set. If a future + // PR re-adds any of the five trimmed fields, the excess-property + // comments below become live TS errors when uncommented (and the + // CI guardrail in .github/workflows/ci.yml fires regardless). + const a: Agent = { + id: 'ag-test', + name: 'web-01', + hostname: 'web-01.prod', + status: 'Online', + last_heartbeat_at: '2026-04-25T12:00:00Z', + registered_at: '2024-06-01T00:00:00Z', + os: 'linux', + architecture: 'amd64', + ip_address: '10.0.0.1', + version: '2.1.0', + }; + expect(a.id).toBe('ag-test'); + expect(a.last_heartbeat_at).toBe('2026-04-25T12:00:00Z'); + + // Excess-property check (each MUST be a TS error if uncommented): + // const broken1: Agent = { ...a, last_heartbeat: '2026-...' }; // ❌ TS2353 + // const broken2: Agent = { ...a, capabilities: ['deploy'] }; // ❌ TS2353 + // const broken3: Agent = { ...a, tags: { env: 'prod' } }; // ❌ TS2353 + // const broken4: Agent = { ...a, created_at: '...' }; // ❌ TS2353 + // const broken5: Agent = { ...a, updated_at: '...' }; // ❌ TS2353 + }); + + it('keeps last_heartbeat_at (the real Go-emitted heartbeat field)', () => { + // Negative-prevention guard: the awk-windowed CI grep for the trimmed + // `last_heartbeat` field must NOT trip on the legitimate + // `last_heartbeat_at`. This test pins that the legitimate field stays. + const a: Agent = { + id: 'ag-2', + name: 'web-02', + hostname: 'web-02.prod', + status: 'Offline', + registered_at: '2024-06-01T00:00:00Z', + os: 'linux', + architecture: 'amd64', + ip_address: '10.0.0.2', + version: '2.1.0', + }; + expect(a.last_heartbeat_at).toBeUndefined(); + }); +}); + +/** + * D-2 (diff-05x06-2044a46f4dd0, master): Target retirement-fields ADD. + * + * Pre-D-2 the Go-side `DeploymentTarget` struct + * (`internal/domain/connector.go:24`) emitted `retired_at` and + * `retired_reason` (I-004 soft-retirement, mirroring the Agent + * treatment), but the TS `Target` interface did not declare them. + * Consumers wanting to surface the retired state in the GUI had to + * use `(target as any).retired_at` escapes that lost type-checking. + * + * Post-D-2 the TS interface declares both as optional nullable strings, + * mirroring the existing Agent retirement-fields shape. + */ +describe('Target interface (D-2 retirement fields)', () => { + it('accepts retired_at and retired_reason as optional nullable strings', () => { + const retired: Target = { + id: 't-decom-01', + name: 'old-iis-server', + type: 'iis', + agent_id: 'ag-old', + config: {}, + enabled: false, + created_at: '2024-01-01T00:00:00Z', + retired_at: '2026-03-01T00:00:00Z', + retired_reason: 'replaced by new iis-server', + }; + expect(retired.retired_at).toBe('2026-03-01T00:00:00Z'); + expect(retired.retired_reason).toBe('replaced by new iis-server'); + }); + + it('accepts a Target without the retirement fields (active row)', () => { + const active: Target = { + id: 't-1', + name: 'iis-server', + type: 'iis', + agent_id: 'ag-1', + config: {}, + enabled: true, + created_at: '2024-01-01T00:00:00Z', + }; + expect(active.retired_at).toBeUndefined(); + expect(active.retired_reason).toBeUndefined(); + }); +}); + +/** + * D-2 (diff-05x06-85ab6b98a2f7, master): DiscoveredCertificate pem_data ADD. + * + * Pre-D-2 the Go-side `DiscoveredCertificate` struct + * (`internal/domain/discovery.go::DiscoveredCertificate.PEMData`) emitted + * `pem_data` (omitempty — populated by repo SELECT, agent ingestion at + * cmd/agent/main.go:1021, and connector scans at + * internal/connector/discovery/azurekv/azurekv.go:234), but the TS + * `DiscoveredCertificate` interface did not declare it. Consumers wanting + * to inspect or download the raw PEM had to use `(d as any).pem_data`. + * + * Post-D-2 the TS interface declares it as `pem_data?: string`, optional + * because the Go side uses `omitempty` (empty string → not emitted). + * + * Performance note (deferred follow-up): the LIST endpoint also loads + * pem_data via the same repo SELECT; for large discovered-cert tables + * this can ship kilobytes per row. Optimising the list response to omit + * pem_data is a separate backend change. + */ +describe('DiscoveredCertificate interface (D-2 pem_data ADD)', () => { + it('accepts pem_data as an optional string', () => { + const d: DiscoveredCertificate = { + id: 'dc-1', + fingerprint_sha256: 'a'.repeat(64), + common_name: 'discovered.example.com', + sans: [], + serial_number: '01:02:03', + issuer_dn: 'CN=Test CA', + subject_dn: 'CN=discovered.example.com', + key_algorithm: 'ECDSA', + key_size: 256, + is_ca: false, + source_path: '/etc/ssl/certs/disc.pem', + source_format: 'pem', + agent_id: 'ag-1', + status: 'Unmanaged', + first_seen_at: '2026-04-25T12:00:00Z', + last_seen_at: '2026-04-25T12:00:00Z', + created_at: '2026-04-25T12:00:00Z', + updated_at: '2026-04-25T12:00:00Z', + pem_data: '-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----\n', + }; + expect(d.pem_data).toContain('BEGIN CERTIFICATE'); + }); + + it('accepts a DiscoveredCertificate without pem_data (list-response shape)', () => { + const d: DiscoveredCertificate = { + id: 'dc-2', + fingerprint_sha256: 'b'.repeat(64), + common_name: 'list.example.com', + sans: [], + serial_number: '04:05:06', + issuer_dn: 'CN=Test CA', + subject_dn: 'CN=list.example.com', + key_algorithm: 'ECDSA', + key_size: 256, + is_ca: false, + source_path: '/etc/ssl/certs/list.pem', + source_format: 'pem', + agent_id: 'ag-1', + status: 'Unmanaged', + first_seen_at: '2026-04-25T12:00:00Z', + last_seen_at: '2026-04-25T12:00:00Z', + created_at: '2026-04-25T12:00:00Z', + updated_at: '2026-04-25T12:00:00Z', + }; + expect(d.pem_data).toBeUndefined(); + }); +}); + +/** + * D-2 (diff-05x06-97fab8783a5c, master): Issuer status phantom trim. + * + * Pre-D-2 the TS `Issuer` interface declared a required `status: string` + * field that the Go-side struct (`internal/domain/connector.go::Issuer`) + * never emitted — the Go struct has only `Enabled bool`. The TS interface + * comment claimed "Backend returns enabled boolean; status is derived + * from this" but no derivation logic existed: `IssuersPage.tsx::~line 23` + * read `issuer.status || 'Unknown'` and always rendered 'Unknown'. + * + * Post-D-2 the `status` field is removed; the consumer now derives the + * displayed status from `enabled` at render time. + */ +describe('Issuer interface (D-2 status phantom trim)', () => { + it('does NOT declare a phantom `status` field — derive from `enabled`', () => { + // Construct a fully-populated Issuer with the post-D-2 shape. + // If `status` is re-added, this construction fails with "missing + // required" (TS2741) when status is required, or the excess-property + // comment below trips when it's added back as optional. + const i: Issuer = { + id: 'iss-test', + name: 'Test ACME', + type: 'acme', + config: {}, + enabled: true, + created_at: '2026-01-01T00:00:00Z', + }; + expect(i.id).toBe('iss-test'); + expect(i.enabled).toBe(true); + + // Excess-property check: + // const broken: Issuer = { ...i, status: 'Active' }; // ❌ TS2353 + }); +}); + +/** + * D-2 (diff-05x06-caba9eb3620e, master): Notification subject phantom trim. + * + * Pre-D-2 the TS `Notification` interface declared `subject?: string` — + * the field was acknowledged in the existing comment as "a historical + * frontend-only field the backend never emits" but kept on the interface + * "so legacy fixtures and the pendingNotif test mock still type + * correctly." Real consumer at `NotificationsPage.tsx::~line 241` had + * `{n.message || n.subject}` as a fallback that always fell through to + * `n.message` (since `n.subject` was always undefined). + * + * Post-D-2 the field is removed; the consumer drops the dead fallback + * and the test fixtures drop the dead `subject:` initializer. + */ +describe('Notification interface (D-2 subject phantom trim)', () => { + it('does NOT declare the phantom `subject` field', () => { + const n: Notification = { + id: 'no-test', + type: 'CertificateExpiring', + channel: 'email', + recipient: 'ops@example.com', + message: 'Certificate api.example.com expires in 14 days', + status: 'pending', + created_at: '2026-04-25T12:00:00Z', + }; + expect(n.id).toBe('no-test'); + expect(n.message).toContain('14 days'); + + // Excess-property check: + // const broken: Notification = { ...n, subject: 'Cert expiring' }; // ❌ TS2353 + }); +}); diff --git a/web/src/api/types.ts b/web/src/api/types.ts index d14eb07..c5c065a 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -67,6 +67,23 @@ export interface CertificateVersion { // API contract. See docs/architecture.md ER-diagram note and // coverage-gap-audit-2026-04-24-v5/unified-audit.md cat-s5-apikey_leak // for the closure rationale. +// +// D-2 (diff-05x06-7cdf4e78ae24, master): pre-D-2 this interface declared +// five fields the Go-side struct (internal/domain/connector.go::Agent) +// does NOT emit on the wire: `last_heartbeat` (the real field is +// `last_heartbeat_at`; the bare-name was a sibling typo never rejected +// at compile time), `capabilities`, `tags`, `created_at`, `updated_at`. +// Two of them had real consumers (AgentDetailPage rendered +// `agent.capabilities` and `agent.tags`) — both always rendered the +// empty-state branch because the runtime values were always undefined. +// Post-D-2 the interface field set matches the Go-emitted JSON exactly: +// id, name, hostname, status, last_heartbeat_at, registered_at, os, +// architecture, ip_address, version, retired_at?, retired_reason?. A +// `agent.capabilities` access is now a TS compile error. The CI guardrail +// in .github/workflows/ci.yml (`Forbidden StatusBadge dead-key + TS +// phantom-field regression guard (D-1 + D-2)`) blocks reintroduction of +// the trimmed field names while explicitly excluding `last_heartbeat_at` +// from the `last_heartbeat` regex. export interface Agent { id: string; name: string; @@ -76,13 +93,8 @@ export interface Agent { architecture: string; status: string; version: string; - last_heartbeat: string; - last_heartbeat_at: string; - capabilities: string[]; - tags: Record; + last_heartbeat_at?: string; registered_at: string; - created_at: string; - updated_at: string; // I-004: soft-retirement fields. When retired_at is non-null, the agent is // tombstoned — it will never heartbeat again and cascaded targets have been // retired alongside it. The retired tab on AgentsPage uses these to show the @@ -159,9 +171,17 @@ export interface Job { * without chasing server logs. * * `sent_at` and `error` are the pre-I-005 audit fields on the backend struct. - * `subject` is a historical frontend-only field the backend never emits; it's - * kept optional so legacy fixtures and the pendingNotif test mock still type - * correctly without forcing a rewrite of every existing consumer. + * + * D-2 (diff-05x06-caba9eb3620e, master): pre-D-2 this interface carried a + * phantom `subject?: string` field documented as "kept optional so legacy + * fixtures and the pendingNotif test mock still type correctly without + * forcing a rewrite of every existing consumer." The Go-side struct + * (`internal/domain/notification.go::NotificationEvent`) never emitted it, + * so `n.subject` was always `undefined` at runtime. The one real consumer + * (NotificationsPage rendering `{n.message || n.subject}`) always fell + * through to `n.message`. Post-D-2 the field is removed; the consumer + * drops the dead `|| n.subject` fallback and the test fixtures drop the + * dead `subject:` initializer. The CI guardrail blocks reintroduction. * * Status values follow the backend NotificationStatus constants: * pending · sent · failed · dead · read @@ -174,7 +194,6 @@ export interface Notification { type: string; channel: string; recipient: string; - subject?: string; message: string; status: string; certificate_id?: string; @@ -269,13 +288,21 @@ export interface RenewalPolicy { updated_at: string; } +// D-2 (diff-05x06-97fab8783a5c, master): pre-D-2 this interface declared +// a required `status: string` field that the Go-side struct +// (`internal/domain/connector.go::Issuer`) never emitted — the Go struct +// has only `Enabled bool`. The TS comment claimed "status is derived from +// this" but no derivation ever existed: `IssuersPage.tsx` read +// `issuer.status || 'Unknown'` and always rendered 'Unknown'. Post-D-2 +// the phantom is removed; render sites derive the displayed status from +// `enabled` (and optionally `test_status`) at the call site. The CI +// guardrail in .github/workflows/ci.yml blocks reintroduction. export interface Issuer { id: string; name: string; type: string; config: Record; - status: string; - /** Backend returns enabled boolean; status is derived from this */ + /** Backend returns enabled boolean; render sites derive status labels from this */ enabled: boolean; /** Timestamp of last connection test */ last_tested_at?: string; @@ -287,6 +314,14 @@ export interface Issuer { updated_at?: string; } +// D-2 (diff-05x06-2044a46f4dd0, master): pre-D-2 this interface lacked +// `retired_at` and `retired_reason` even though the Go-side struct +// (`internal/domain/connector.go::DeploymentTarget`) emits both as part +// of the I-004 soft-retirement model. Consumers wanting to surface the +// retired state had to escape via `(target as any).retired_at`. Post-D-2 +// the TS interface declares both as optional nullable strings, mirroring +// the Agent retirement-fields shape (an Agent retire cascades to all +// associated Targets per service.RetireAgent → repository.RetireTarget). export interface Target { id: string; name: string; @@ -297,6 +332,8 @@ export interface Target { last_tested_at?: string; test_status?: string; source?: string; + retired_at?: string | null; + retired_reason?: string | null; created_at: string; updated_at?: string; } @@ -403,6 +440,19 @@ export interface IssuanceRateDataPoint { } // Discovery types +// +// D-2 (diff-05x06-85ab6b98a2f7, master): pre-D-2 this interface lacked +// `pem_data` even though the Go-side struct +// (`internal/domain/discovery.go::DiscoveredCertificate.PEMData`, +// json:"pem_data,omitempty") emits it on the wire. The field is +// populated by the agent's filesystem scanner +// (cmd/agent/main.go::buildDiscoveryReport), the cloud-secret-manager +// connectors (e.g. internal/connector/discovery/azurekv/azurekv.go), and +// the repo SELECT that materialises the row from PostgreSQL. Post-D-2 +// the TS interface declares `pem_data?: string`, optional because the +// Go side uses `omitempty` (empty string → not emitted). Performance +// follow-up: the LIST endpoint loads pem_data via the same repo SELECT; +// a future change should gate emission on the per-id detail path only. export interface DiscoveredCertificate { id: string; fingerprint_sha256: string; @@ -416,6 +466,7 @@ export interface DiscoveredCertificate { key_algorithm: string; key_size: number; is_ca: boolean; + pem_data?: string; source_path: string; source_format: string; agent_id: string; diff --git a/web/src/api/utils.ts b/web/src/api/utils.ts index 9dfe3b8..f45d25c 100644 --- a/web/src/api/utils.ts +++ b/web/src/api/utils.ts @@ -8,7 +8,12 @@ export function formatDateTime(iso: string | undefined | null): string { return new Date(iso).toLocaleString('en-US', { year: 'numeric', month: 'short', day: 'numeric', hour: '2-digit', minute: '2-digit' }); } -export function timeAgo(iso: string): string { +// D-2 (master): widened to accept undefined/null since several Go-side +// timestamp fields are emitted as `omitempty` (e.g. Agent.last_heartbeat_at +// for never-heartbeated agents). Pre-D-2 the TS interfaces declared +// these as required strings, masking the case; post-D-2 the optionality +// is propagated end-to-end and the helper handles it explicitly. +export function timeAgo(iso: string | undefined | null): string { if (!iso) return '—'; const now = Date.now(); const then = new Date(iso).getTime(); diff --git a/web/src/pages/AgentDetailPage.tsx b/web/src/pages/AgentDetailPage.tsx index c7ca45c..88da98b 100644 --- a/web/src/pages/AgentDetailPage.tsx +++ b/web/src/pages/AgentDetailPage.tsx @@ -15,7 +15,12 @@ function InfoRow({ label, value }: { label: string; value: React.ReactNode }) { ); } -function heartbeatStatus(lastHeartbeat: string): string { +// D-2 (master): the `lastHeartbeat` parameter accepts undefined because +// the Go-side struct emits `last_heartbeat_at` as `omitempty` (a never- +// heartbeated agent omits the field entirely). Pre-D-2 the TS interface +// declared the field as required, masking this case. Post-D-2 the empty +// case is explicit at both the type level and the function signature. +function heartbeatStatus(lastHeartbeat: string | undefined): string { if (!lastHeartbeat) return 'Offline'; const ago = Date.now() - new Date(lastHeartbeat).getTime(); if (ago < 5 * 60 * 1000) return 'Online'; @@ -89,8 +94,15 @@ export default function AgentDetailPage() { ) : '—' } /> - - + {/* D-2 (master): pre-D-2 these rows used `agent.created_at` + + `agent.updated_at` — TS phantoms that the Go-side + struct (`internal/domain/connector.go::Agent`) never + emitted. The "Registered" row now reads from the real + Go-emitted `registered_at` field; the "Updated" row is + dropped because the Go struct has no equivalent + update-timestamp on Agent (heartbeats are tracked via + `last_heartbeat_at` above). */} + {/* System Info */} @@ -100,26 +112,17 @@ export default function AgentDetailPage() { {agent.ip_address || '—'}} /> - {agent.capabilities?.length ? ( -
-

Capabilities

-
- {agent.capabilities.map((c) => ( - {c} - ))} -
-
- ) : null} - {agent.tags && Object.keys(agent.tags).length > 0 ? ( -
-

Tags

-
- {Object.entries(agent.tags).map(([k, v]) => ( - {k}: {v} - ))} -
-
- ) : null} + {/* D-2 (master): the previous "Capabilities" + "Tags" sections + rendered `agent.capabilities` and `agent.tags`, both of + which were TS phantom fields the Go-side struct + (`internal/domain/connector.go::Agent`) never emitted. + Both sections always rendered as the empty-state fallback + (the `?.length ?` and `Object.keys(...).length > 0` + guards always evaluated false). Removed in D-2 master. + If/when the backend grows real Agent metadata fields + (capabilities advertised at heartbeat time, operator- + applied tags), re-introduce here in the same commit that + ships the Go-side change. */} diff --git a/web/src/pages/AgentsPage.tsx b/web/src/pages/AgentsPage.tsx index caf94eb..971c455 100644 --- a/web/src/pages/AgentsPage.tsx +++ b/web/src/pages/AgentsPage.tsx @@ -15,7 +15,12 @@ import ErrorState from '../components/ErrorState'; import { timeAgo } from '../api/utils'; import type { Agent, AgentDependencyCounts } from '../api/types'; -function heartbeatStatus(lastHeartbeat: string): string { +// D-2 (master): the `lastHeartbeat` parameter accepts undefined because +// the Go-side struct emits `last_heartbeat_at` as `omitempty` (never- +// heartbeated agents omit the field). Mirror of the same helper in +// AgentDetailPage.tsx — kept as twin definitions to avoid a shared- +// helper PR detour during D-2; consolidate in a follow-up if desired. +function heartbeatStatus(lastHeartbeat: string | undefined): string { if (!lastHeartbeat) return 'Offline'; const ago = Date.now() - new Date(lastHeartbeat).getTime(); if (ago < 5 * 60 * 1000) return 'Online'; diff --git a/web/src/pages/IssuerDetailPage.tsx b/web/src/pages/IssuerDetailPage.tsx index d5a49e3..fb6e29d 100644 --- a/web/src/pages/IssuerDetailPage.tsx +++ b/web/src/pages/IssuerDetailPage.tsx @@ -19,12 +19,15 @@ function InfoRow({ label, value }: { label: string; value: React.ReactNode }) { ); } -/** Derive display status from backend enabled boolean */ +// Derive display status from backend `enabled` boolean. +// +// D-2 (diff-05x06-97fab8783a5c, master): pre-D-2 the fall-through here +// was `issuer.status || 'Unknown'`, but `Issuer.status` was a TS phantom +// the Go-side struct never emitted (see types.ts::Issuer docblock for the +// full closure rationale). Post-D-2 the phantom is gone; this function +// derives the displayed status from `enabled` exclusively. function issuerStatus(issuer: Issuer): string { - if (issuer.enabled !== undefined) { - return issuer.enabled ? 'Enabled' : 'Disabled'; - } - return issuer.status || 'Unknown'; + return issuer.enabled ? 'Enabled' : 'Disabled'; } export default function IssuerDetailPage() { diff --git a/web/src/pages/IssuersPage.tsx b/web/src/pages/IssuersPage.tsx index e89cb68..786aaf3 100644 --- a/web/src/pages/IssuersPage.tsx +++ b/web/src/pages/IssuersPage.tsx @@ -14,13 +14,19 @@ import TypeSelector from '../components/issuer/TypeSelector'; import ConfigForm from '../components/issuer/ConfigForm'; import ConfigDetailModal from '../components/issuer/ConfigDetailModal'; -/** Derive display status from backend enabled boolean */ +// Derive display status from backend `enabled` boolean. +// +// D-2 (diff-05x06-97fab8783a5c, master): pre-D-2 the fall-through chain +// here was `issuer.status || 'Unknown'`, which always rendered 'Unknown' +// because the Go-side struct never emitted a `status` field — the TS +// interface comment claimed status was "derived from enabled" but no +// derivation existed. Post-D-2 the phantom `Issuer.status` is gone and +// this function is the canonical derivation site. `enabled` is a +// required boolean on Go's Issuer struct so the `!== undefined` guard +// is now belt-and-suspenders rather than load-bearing, but kept for +// defensive rendering against malformed responses. function issuerStatus(issuer: Issuer): string { - if (issuer.enabled !== undefined) { - return issuer.enabled ? 'Enabled' : 'Disabled'; - } - // Fallback for legacy data that may have status string - return issuer.status || 'Unknown'; + return issuer.enabled ? 'Enabled' : 'Disabled'; } export default function IssuersPage() { diff --git a/web/src/pages/NotificationsPage.test.tsx b/web/src/pages/NotificationsPage.test.tsx index 06621bb..a15dd41 100644 --- a/web/src/pages/NotificationsPage.test.tsx +++ b/web/src/pages/NotificationsPage.test.tsx @@ -50,12 +50,14 @@ function renderWithQuery(ui: ReactNode) { ); } +// D-2 (master): pre-D-2 these mocks set `subject:` — the field was a TS +// phantom the Go-side struct never emitted. Post-D-2 the phantom is +// removed from the Notification interface; the mocks no longer set it. const pendingNotif = { id: 'notif-001', type: 'ExpirationWarning', channel: 'Email', recipient: 'admin@example.com', - subject: 'Certificate expiring', message: 'Certificate expiring in 7 days', status: 'Pending', certificate_id: 'mc-prod-001', @@ -67,7 +69,6 @@ const deadNotif = { type: 'ExpirationWarning', channel: 'Email', recipient: 'admin@example.com', - subject: 'Certificate expiring', message: 'Certificate expiring in 7 days', status: 'dead', certificate_id: 'mc-prod-001', diff --git a/web/src/pages/NotificationsPage.tsx b/web/src/pages/NotificationsPage.tsx index dde3b2e..87c21cd 100644 --- a/web/src/pages/NotificationsPage.tsx +++ b/web/src/pages/NotificationsPage.tsx @@ -238,7 +238,13 @@ function NotificationRow({ {n.channel} -

{n.message || n.subject}

+ {/* D-2 (master): pre-D-2 the fallback was `{n.message || n.subject}`, + but `subject` was a TS phantom the Go struct never emitted + (`internal/domain/notification.go::NotificationEvent` has only + `message`). The fallback always fell through to `message` + because `subject` was always undefined. Post-D-2 the dead + fallback is dropped along with the phantom field. */} +

{n.message}

{isDead && (