From 5194853543437908f529b406141abafa0bb58ca8 Mon Sep 17 00:00:00 2001 From: Feng Qian Date: Mon, 10 Aug 2026 12:28:50 -0700 Subject: [PATCH 1/3] feat: remove the release-blocker concept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Release blockers" tile on the Explorer overview was structurally always 0. The `blocked` gap category it counted could only be produced by a standalone-word match on "blocked" in a narrow text blob (evidence type/path/url/command, residual risk, proof-gap next step), and its other trigger — `assessmentStatus === "blocked"` — was unreachable, because `deriveExpectationAssessment` only ever returns NOT COVERED / MANUAL / IMPLICIT / COVERED / PARTIAL. Nothing in the pipeline writes the word, so the tile read as "we checked and found none" while never having checked. Measured against the two projects that consume this engine: 0 of 326 real gap records across shipyard (54 targets) and monots (4 targets) would have been classified `blocked`. The category never fired anywhere. It also had teeth where it should not have. `rollupStatusFor` set an entire project's status to BLOCKED off that keyword match, and `riskIndicatorsFor` added `gapCounts.blocked` to `releaseRiskCounts.blockers` — the same records counted twice, so one blocked gap rendered as "2 blockers". Removed end to end: the `blocked` gap category and its classifier branches, the `blocked-gaps` analytics metric, `ReleaseRiskSummary.blockers`, `WorkspaceReleaseRiskCounts.blockers`, `WorkspaceAttentionCounts.blocked`, the BLOCKED rollup status, the "Blocked" owner risk badge, the dashboard tile, and the feature-table column. The task status `blocked` is a separate lifecycle vocabulary and is untouched. Fix a regression this removal introduced. Deleting the `status.includes("BLOCK")` branch from `riskBadgeFor` looked safe because the structured path can never produce that status — but the markdown-fallback path passes the Result cell verbatim from the scanned project, so a row reading `BLOCKED | HIGH | None` badged as Covered. The root cause was broader than the missing branch: a `evidenceConfidence === "HIGH"` claim promoted any unrecognized status to Covered, and the audited project writes that text. Confidence now settles only a row that states no result of its own, and a stated non-passing result maps to Gap. Bump the packages whose exported surface narrowed, matching the precedent in fada5bd: quality-map 0.1.0 -> 0.2.0 (GAP_CATEGORIES, GapCategory, the schema enum) and quality-core 0.2.0 -> 0.3.0 (the removed read-model fields and badge member). quality-tools stays at 0.3.2: it re-exports GAP_CATEGORIES, but moving it outside ^0.3.0 requires migrating the agent-skill pin in the same change, and scripts/check-quality-skill.sh npx-fetches the published package — so that pin cannot move before a release exists. Add the guards this change showed were missing: pin the literal published gap-category vocabulary, assert `gapCategoryOrder` covers `GAP_CATEGORIES` (an omission there silently drops a category from every gap record, and is not a type error), and pin the `riskBadgeFor` ladder over the statuses markdown reports actually carry. Co-Authored-By: Claude Opus 5 (1M context) --- .../004-quality-scoring/quality-map.yaml | 4 +- README.md | 2 +- .../assets/quality-map.template.yaml | 4 +- .../quality/references/map-feature/index.md | 4 +- .../quality-explorer/help/scoring/page.tsx | 5 -- docs/concepts/glossary.md | 2 +- docs/how-to/accept-a-known-gap.md | 4 +- packages/core/package.json | 2 +- .../src/analytics/compute-release-snapshot.ts | 11 ++-- .../core/src/analytics/metric-definitions.ts | 10 ---- packages/core/src/analytics/types.ts | 1 - packages/core/src/gap-triage/classify-gaps.ts | 9 ---- packages/core/src/owner-view/risk-summary.ts | 19 ++++--- packages/core/src/owner-view/types.ts | 2 +- .../core/src/quality-structure/assessment.ts | 2 +- packages/core/src/workspace/summaries.ts | 20 +------- packages/core/src/workspace/types.ts | 2 - packages/quality-map/package.json | 2 +- packages/quality-map/src/gap-categories.ts | 1 - .../src/quality-map-schema.test.ts | 16 ++++++ .../quality-map/src/quality-map.schema.json | 1 - .../ui/src/components/FeatureIndex.test.tsx | 2 +- packages/ui/src/components/FeatureIndex.tsx | 18 +------ packages/ui/src/components/OwnerDashboard.tsx | 11 +--- packages/ui/src/styles.css | 4 +- tests/contract/analytics.contract.test.ts | 6 +-- tests/contract/gap-triage.contract.test.ts | 13 +++-- .../modern-workspace.contract.test.ts | 6 +-- .../analytics/complete/quality-map.yaml | 10 ++-- .../gap-triage/complete/quality-map.yaml | 14 +++--- .../gap-triage/fallback/test-report.md | 2 +- tests/integration/analytics.test.ts | 4 +- tests/integration/gap-triage.test.ts | 10 ++-- tests/unit/gap-vocabulary.test.ts | 50 +++++++++++++++++++ 34 files changed, 137 insertions(+), 136 deletions(-) create mode 100644 tests/unit/gap-vocabulary.test.ts diff --git a/.quality/evidence/004-quality-scoring/quality-map.yaml b/.quality/evidence/004-quality-scoring/quality-map.yaml index a5431a6..8876b35 100644 --- a/.quality/evidence/004-quality-scoring/quality-map.yaml +++ b/.quality/evidence/004-quality-scoring/quality-map.yaml @@ -287,7 +287,7 @@ expectations: description: >- The release snapshot computes its metrics from published formulas with explicit guardrails, and counts how many checks sit in each state — stale, - manual-only, missing, blocked, accepted, deferred. Where no mapping exists + manual-only, missing, accepted, deferred. Where no mapping exists for a priority level, the figure is labelled unavailable instead of being approximated. source_type: "IMPLEMENTATION" @@ -305,7 +305,7 @@ expectations: - id: "metrics-state-counts" type: "contract" path: "tests/contract/analytics.contract.test.ts" - test_case: "counts stale, manual-only, missing, blocked, accepted, and deferred contexts" + test_case: "counts stale, manual-only, missing, accepted, and deferred contexts" contexts: ["local", "pr-ci"] - id: "metrics-unavailable-label" type: "contract" diff --git a/README.md b/README.md index 8209b8b..21409fa 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ details. ![Quality Explorer showing the overview for a project: a quality score of 100/100 alongside separate coverage, evidence confidence, and structure -confidence scores, the gaps and release blockers counts, and the runtime +confidence scores, the feature and gap counts, and the runtime observation set fetched from a CI workflow run.](docs/assets/quality-explorer-overview.png) ## What Quality does diff --git a/agent-skills/quality/references/map-feature/assets/quality-map.template.yaml b/agent-skills/quality/references/map-feature/assets/quality-map.template.yaml index 2ca2e80..71b0c39 100644 --- a/agent-skills/quality/references/map-feature/assets/quality-map.template.yaml +++ b/agent-skills/quality/references/map-feature/assets/quality-map.template.yaml @@ -59,9 +59,9 @@ expectations: # as tolerated risk for this check. An accepted gap stays visible but stops # counting as an open gap; accepting the category that drives the check's status # (missing / manual-only / weak) also lifts its quality/coverage score. Accepting - # a state gap (blocked/stale/deferred/unavailable/failing) is count-only. An agent + # a state gap (stale/deferred/unavailable/failing) is count-only. An agent # may PROPOSE an acceptance but must never accept a risk on the owner's behalf. - accepted_gaps: [] # Optional: subset of [missing, blocked, stale, deferred, manual-only, weak, failing, unavailable] + accepted_gaps: [] # Optional: subset of [missing, stale, deferred, manual-only, weak, failing, unavailable] # A proof definition is just the test TYPE (a fact about the artifact) at a # PATH. Evidence confidence is derived downstream from `type` via a diff --git a/agent-skills/quality/references/map-feature/index.md b/agent-skills/quality/references/map-feature/index.md index f8de574..1a8a91b 100644 --- a/agent-skills/quality/references/map-feature/index.md +++ b/agent-skills/quality/references/map-feature/index.md @@ -213,11 +213,11 @@ confidence, reported beside structure confidence and never substituting for it.) ### `accepted_gaps` — accepted risk (human-gated) A per-check list of gap **categories** a human has reviewed and accepted as tolerated -risk: a subset of `missing, blocked, stale, deferred, manual-only, weak, failing, +risk: a subset of `missing, stale, deferred, manual-only, weak, failing, unavailable`. An accepted gap stays visible but stops counting as an **open** gap; accepting the category that drives the check's status (`missing` / `manual-only` / `weak`) also lifts its quality/coverage score, while accepting a state category -(`blocked`/`stale`/`deferred`/`unavailable`/`failing`) is count-only. It never +(`stale`/`deferred`/`unavailable`/`failing`) is count-only. It never changes evidence confidence. Like the gates, it is **human-gated**: the agent may *propose* "accept this as tolerated risk" but must never write `accepted_gaps` for the owner. Remove the category to un-accept. diff --git a/apps/explorer/src/app/quality-explorer/help/scoring/page.tsx b/apps/explorer/src/app/quality-explorer/help/scoring/page.tsx index 1b60b65..e1cfc63 100644 --- a/apps/explorer/src/app/quality-explorer/help/scoring/page.tsx +++ b/apps/explorer/src/app/quality-explorer/help/scoring/page.tsx @@ -100,11 +100,6 @@ const gapTerms: readonly GlossaryItem[] = [ definition: "A gap where some evidence exists, but the proof is not automated enough, not gated, or not complete enough." }, - { - term: "Release blocker", - definition: - "A gap serious enough that the current quality model treats it as blocking release readiness." - }, { term: "Copy fix prompt", definition: diff --git a/docs/concepts/glossary.md b/docs/concepts/glossary.md index 3c8a685..de16879 100644 --- a/docs/concepts/glossary.md +++ b/docs/concepts/glossary.md @@ -87,7 +87,7 @@ aggregate score. Open gaps use these categories: -`missing`, `blocked`, `stale`, `deferred`, `manual-only`, `weak`, `failing`, and +`missing`, `stale`, `deferred`, `manual-only`, `weak`, `failing`, and `unavailable`. The distinction matters: diff --git a/docs/how-to/accept-a-known-gap.md b/docs/how-to/accept-a-known-gap.md index 37d7ac6..e4e29ce 100644 --- a/docs/how-to/accept-a-known-gap.md +++ b/docs/how-to/accept-a-known-gap.md @@ -40,7 +40,7 @@ rationale that the file cannot store. | Accepted category | Effect | | --- | --- | | `missing`, `manual-only`, or `weak` | The gap stops counting as open, and its structural coverage/static-quality penalty is removed. | -| `blocked`, `stale`, `deferred`, `failing`, or `unavailable` | The gap stops counting as open; the scores do not change. | +| `stale`, `deferred`, `failing`, or `unavailable` | The gap stops counting as open; the scores do not change. | In every case: @@ -81,5 +81,5 @@ whether proof investment is being postponed indefinitely, or whether accepted decisions need an expiry process outside Quality. **The score did not change.** That is expected for state categories such as -`blocked` or `failing`. Acceptance changes open-gap reporting, not the observed +`stale` or `failing`. Acceptance changes open-gap reporting, not the observed software result. diff --git a/packages/core/package.json b/packages/core/package.json index 0a56364..354cee4 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@shiplightai/quality-core", - "version": "0.2.0", + "version": "0.3.0", "type": "module", "description": "Deterministic analysis engine for evidence-backed software quality maps.", "license": "MIT", diff --git a/packages/core/src/analytics/compute-release-snapshot.ts b/packages/core/src/analytics/compute-release-snapshot.ts index f15c3d4..86391d5 100644 --- a/packages/core/src/analytics/compute-release-snapshot.ts +++ b/packages/core/src/analytics/compute-release-snapshot.ts @@ -156,16 +156,14 @@ function structuredMetrics(input: { availability: highPriorityAvailability, guardrail: "Only explicit gate-context evidence is counted as gated." }), - ...(["stale", "manual-only", "missing", "blocked"] as const).map((category) => + ...(["stale", "manual-only", "missing"] as const).map((category) => metric({ metricId: category === "stale" ? "stale-evidence" : category === "manual-only" ? "manual-only-exposure" - : category === "missing" - ? "missing-evidence" - : "blocked-gaps", + : "missing-evidence", numerator: gapRecords.filter((record) => record.category === category).length, denominator: gapRecords.length, records: gapRecords.filter((record) => record.category === category).map((record) => @@ -204,7 +202,7 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView { summary: missingSummary(input.targetId), filters: input.filters ?? {}, metrics: [], - riskSummary: { blockers: [], acceptedRisks: [], deferredRisks: [] }, + riskSummary: { acceptedRisks: [], deferredRisks: [] }, baselineComparison: buildBaselineComparison({ currentSnapshotId: input.targetId, records: [] }), guardrails: ["Release analytics require a selected feature."], missingSelection: { @@ -223,7 +221,7 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView { summary: missingSummary(input.targetId), filters: input.filters ?? {}, metrics: [], - riskSummary: { blockers: [], acceptedRisks: [], deferredRisks: [] }, + riskSummary: { acceptedRisks: [], deferredRisks: [] }, baselineComparison: buildBaselineComparison({ currentSnapshotId: input.targetId, records: [] }), guardrails: ["Selected feature is unavailable."], missingSelection: { @@ -282,7 +280,6 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView { metrics, filteredMetric, riskSummary: { - blockers: metrics.find((item) => item.metricId === "blocked-gaps")?.drilldownRecords ?? [], acceptedRisks: metrics.find((item) => item.metricId === "accepted-risks")?.drilldownRecords ?? [], deferredRisks: metrics.find((item) => item.metricId === "deferred-risks")?.drilldownRecords ?? [] }, diff --git a/packages/core/src/analytics/metric-definitions.ts b/packages/core/src/analytics/metric-definitions.ts index c7189b4..54e2206 100644 --- a/packages/core/src/analytics/metric-definitions.ts +++ b/packages/core/src/analytics/metric-definitions.ts @@ -51,16 +51,6 @@ export const metricDefinitions = [ excludedCriteria: "Partial evidence is not counted as missing.", limitations: ["Missing source fields are labeled unavailable."] }, - { - metricId: "blocked-gaps", - title: "Blocked Gaps", - formulaName: "blocked-gap-count", - numeratorDefinition: "Blocked gap records", - denominatorDefinition: "All source-backed gap records", - includedCriteria: "Gap category must be blocked.", - excludedCriteria: "Deferred-only gaps without blocked context are excluded.", - limitations: ["Blocker reasons are preserved from source text."] - }, { metricId: "accepted-risks", title: "Accepted Impact", diff --git a/packages/core/src/analytics/types.ts b/packages/core/src/analytics/types.ts index 4d4e2f6..3211bd6 100644 --- a/packages/core/src/analytics/types.ts +++ b/packages/core/src/analytics/types.ts @@ -64,7 +64,6 @@ export interface MetricResult { } export interface ReleaseRiskSummary { - readonly blockers: readonly MetricDrilldownRecord[]; readonly acceptedRisks: readonly MetricDrilldownRecord[]; readonly deferredRisks: readonly MetricDrilldownRecord[]; } diff --git a/packages/core/src/gap-triage/classify-gaps.ts b/packages/core/src/gap-triage/classify-gaps.ts index ccbc8d0..9c3d1fd 100644 --- a/packages/core/src/gap-triage/classify-gaps.ts +++ b/packages/core/src/gap-triage/classify-gaps.ts @@ -28,7 +28,6 @@ import type { const categoryLabels: Record = { missing: "Missing evidence", - blocked: "Blocked evidence", stale: "Stale evidence", deferred: "Deferred evidence", "manual-only": "Manual-only evidence", @@ -39,7 +38,6 @@ const categoryLabels: Record = { export const gapCategoryOrder: readonly GapCategory[] = [ "missing", - "blocked", "stale", "deferred", "manual-only", @@ -138,10 +136,6 @@ function categoriesFor(input: { categories.add("missing"); } - if (includesStandaloneAny(evidenceText, ["blocked"]) || input.assessmentStatus.toLowerCase() === "blocked") { - categories.add("blocked"); - } - if (includesStandaloneAny(evidenceText, ["deferred"])) { categories.add("deferred"); } @@ -264,9 +258,6 @@ function fallbackCategories(row: FallbackCoverageRow | undefined, hasEvidence: b if (!hasEvidence) { categories.add("missing"); } - if (includesStandaloneTerm(text, "blocked")) { - categories.add("blocked"); - } if (includesStandaloneTerm(text, "deferred")) { categories.add("deferred"); } diff --git a/packages/core/src/owner-view/risk-summary.ts b/packages/core/src/owner-view/risk-summary.ts index 91acf82..c71558a 100644 --- a/packages/core/src/owner-view/risk-summary.ts +++ b/packages/core/src/owner-view/risk-summary.ts @@ -8,11 +8,11 @@ export function riskBadgeFor(input: { }): OwnerRiskBadge { const status = input.status?.toUpperCase() ?? ""; - if (status.includes("BLOCK")) { - return "Blocked"; - } - - if (status.includes("FAIL")) { + // Markdown-fallback rows carry the Result cell verbatim from the scanned project, + // so any stated-but-not-passing result arrives here as a producer claim. "BLOCK" + // is still such a claim even though the blocked gap category is gone — it maps to + // the surviving Gap vocabulary rather than being dropped. + if (status.includes("FAIL") || status.includes("BLOCK")) { return "Gap"; } @@ -24,7 +24,14 @@ export function riskBadgeFor(input: { return "Missing"; } - if (status.includes("COVER") || status.includes("PASS") || input.evidenceConfidence?.toUpperCase() === "HIGH") { + if (status.includes("COVER") || status.includes("PASS")) { + return "Covered"; + } + + // Confidence may only settle a row that states no result of its own. The scanned + // project controls this text, so a high-confidence claim must never promote an + // unrecognized status to Covered. + if (status.length === 0 && input.evidenceConfidence?.toUpperCase() === "HIGH") { return "Covered"; } diff --git a/packages/core/src/owner-view/types.ts b/packages/core/src/owner-view/types.ts index 0f11fdd..bb480b4 100644 --- a/packages/core/src/owner-view/types.ts +++ b/packages/core/src/owner-view/types.ts @@ -1,7 +1,7 @@ import type { IndexSourceClassification, IndexSourceReference } from "../project-index/types"; export type OwnerViewState = "ready" | "missingTarget" | "directOpen"; -export type OwnerRiskBadge = "Covered" | "Partial" | "Gap" | "Blocked" | "Missing" | "Unknown"; +export type OwnerRiskBadge = "Covered" | "Partial" | "Gap" | "Missing" | "Unknown"; export interface OwnerTargetSummary { readonly targetId: string; diff --git a/packages/core/src/quality-structure/assessment.ts b/packages/core/src/quality-structure/assessment.ts index 42d7019..c0872a0 100644 --- a/packages/core/src/quality-structure/assessment.ts +++ b/packages/core/src/quality-structure/assessment.ts @@ -219,7 +219,7 @@ export function deriveExpectationAssessment( // Only the three EVIDENCE-STRENGTH categories drive `status` (and therefore the // score): no evidence → "missing", manual-only → "manual-only", otherwise "weak" // (IMPLICIT/PARTIAL). `status` is never a state category, so accepting one of the - // state/text categories (blocked/stale/deferred/unavailable, and fallback-only + // state/text categories (stale/deferred/unavailable, and fallback-only // "failing") is intentionally count-only — those never lower the score, so there // is nothing to lift here. This penalty-category mapping mirrors the status ladder // above and the categories gap-triage `categoriesFor` emits for the same status. diff --git a/packages/core/src/workspace/summaries.ts b/packages/core/src/workspace/summaries.ts index 4e633a9..1e73dbc 100644 --- a/packages/core/src/workspace/summaries.ts +++ b/packages/core/src/workspace/summaries.ts @@ -53,7 +53,6 @@ function riskCountsForTarget(result: ScanResult | undefined, targetId: string): const analytics = buildAnalyticsView({ result, targetId }); return { - blockers: analytics.riskSummary.blockers.length, accepted: analytics.riskSummary.acceptedRisks.length, deferred: analytics.riskSummary.deferredRisks.length }; @@ -74,7 +73,6 @@ function openRiskCount(gaps: Partial>): number { return ( (gaps.failing ?? 0) + (gaps.missing ?? 0) + - (gaps.blocked ?? 0) + (gaps.unavailable ?? 0) + (gaps.weak ?? 0) + (gaps["manual-only"] ?? 0) + @@ -145,10 +143,6 @@ function riskIndicatorsFor(input: { indicators.push(`${input.gapCounts.missing} missing`); } - if ((input.gapCounts.blocked ?? 0) > 0 || input.releaseRiskCounts.blockers > 0) { - indicators.push(`${(input.gapCounts.blocked ?? 0) + input.releaseRiskCounts.blockers} blockers`); - } - if ((input.gapCounts.weak ?? 0) > 0) { indicators.push(`${input.gapCounts.weak} weak`); } @@ -166,7 +160,7 @@ function riskIndicatorsFor(input: { } if (indicators.length === 0) { - indicators.push(input.status === "completed" && input.evidenceConfidence !== "LOW" ? "No immediate blockers" : "Review"); + indicators.push(input.status === "completed" && input.evidenceConfidence !== "LOW" ? "No open gaps" : "Review"); } return indicators; @@ -241,7 +235,6 @@ function attentionCountsFor(targets: readonly TargetSummary[]): WorkspaceAttenti (gaps.stale ?? 0) + (gaps.deferred ?? 0), atRisk: openRiskCount(gaps), - blocked: gaps.blocked ?? 0, missing: gaps.missing ?? 0, weak: gaps.weak ?? 0, manualOnly: gaps["manual-only"] ?? 0, @@ -260,12 +253,10 @@ export function buildWorkspaceSummary( const attentionCounts = attentionCountsFor(targets); const releaseRiskCounts = targets.reduce( (counts, target) => ({ - blockers: counts.blockers + target.releaseRiskCounts.blockers, accepted: counts.accepted + target.releaseRiskCounts.accepted, deferred: counts.deferred + target.releaseRiskCounts.deferred }), { - blockers: 0, accepted: 0, deferred: 0 } @@ -294,13 +285,6 @@ function rollupStatusFor(input: { return undefined; } - const blocked = input.targets.some((target) => - (target.gapCounts.blocked ?? 0) > 0 || target.releaseRiskCounts.blockers > 0 - ); - if (blocked) { - return "BLOCKED"; - } - const failing = input.targets.some((target) => (target.gapCounts.failing ?? 0) > 0); if (failing) { return "FAIL"; @@ -357,7 +341,7 @@ function featureTarget(input: { } function gapSeverity(category: GapCategory): WorkspaceActionItem["severity"] { - return category === "missing" || category === "blocked" || category === "failing" || category === "unavailable" + return category === "missing" || category === "failing" || category === "unavailable" ? "error" : "warning"; } diff --git a/packages/core/src/workspace/types.ts b/packages/core/src/workspace/types.ts index 13f36b9..e763e0e 100644 --- a/packages/core/src/workspace/types.ts +++ b/packages/core/src/workspace/types.ts @@ -24,7 +24,6 @@ export interface WorkspaceAttentionCounts { readonly covered: number; readonly partial: number; readonly atRisk: number; - readonly blocked: number; readonly missing: number; readonly weak: number; readonly manualOnly: number; @@ -35,7 +34,6 @@ export interface WorkspaceAttentionCounts { } export interface WorkspaceReleaseRiskCounts { - readonly blockers: number; readonly accepted: number; readonly deferred: number; } diff --git a/packages/quality-map/package.json b/packages/quality-map/package.json index fdcbf23..29f2b12 100644 --- a/packages/quality-map/package.json +++ b/packages/quality-map/package.json @@ -1,6 +1,6 @@ { "name": "@shiplightai/quality-map", - "version": "0.1.0", + "version": "0.2.0", "type": "module", "description": "Schema, parser, validation, normalization, and diagnostics for Shiplight quality maps.", "license": "MIT", diff --git a/packages/quality-map/src/gap-categories.ts b/packages/quality-map/src/gap-categories.ts index c12cad2..2c12d2e 100644 --- a/packages/quality-map/src/gap-categories.ts +++ b/packages/quality-map/src/gap-categories.ts @@ -10,7 +10,6 @@ // this from gap-triage/types.ts, so existing `.../gap-triage` importers are unaffected. export const GAP_CATEGORIES = [ "missing", - "blocked", "stale", "deferred", "manual-only", diff --git a/packages/quality-map/src/quality-map-schema.test.ts b/packages/quality-map/src/quality-map-schema.test.ts index 8b49d1b..8387c89 100644 --- a/packages/quality-map/src/quality-map-schema.test.ts +++ b/packages/quality-map/src/quality-map-schema.test.ts @@ -54,6 +54,22 @@ describe("quality-map JSON Schema (single source)", () => { expect(schema.$defs.gapCategory.enum).toEqual([...GAP_CATEGORIES]); }); + // GAP_CATEGORIES is published API (re-exported by quality-tools) and the docs and + // agent-skill assets hand-list it. The schema assertions above derive from the same + // tuple, so they pass for any edit of it; this pins the literal vocabulary so adding + // or removing a category has to be a deliberate, reviewed change. + it("pins the published gap-category vocabulary", () => { + expect([...GAP_CATEGORIES]).toEqual([ + "missing", + "stale", + "deferred", + "manual-only", + "weak", + "failing", + "unavailable" + ]); + }); + it("reflects the retired require_multi_layer and the live test_case fields", () => { const schema = buildQualityMapJsonSchema() as { $defs: { policyOverride: { properties: Record }; evidence: { properties: Record } }; diff --git a/packages/quality-map/src/quality-map.schema.json b/packages/quality-map/src/quality-map.schema.json index 7b85629..cc31033 100644 --- a/packages/quality-map/src/quality-map.schema.json +++ b/packages/quality-map/src/quality-map.schema.json @@ -76,7 +76,6 @@ "gapCategory": { "enum": [ "missing", - "blocked", "stale", "deferred", "manual-only", diff --git a/packages/ui/src/components/FeatureIndex.test.tsx b/packages/ui/src/components/FeatureIndex.test.tsx index 7346755..9728921 100644 --- a/packages/ui/src/components/FeatureIndex.test.tsx +++ b/packages/ui/src/components/FeatureIndex.test.tsx @@ -53,7 +53,7 @@ function makeTarget(): TargetSummary { evidenceConfidence: "MEDIUM", expectationCount: 3, gapCounts: { weak: 1 }, - releaseRiskCounts: { blockers: 0 } + releaseRiskCounts: { accepted: 0, deferred: 0 } } as unknown as TargetSummary; } diff --git a/packages/ui/src/components/FeatureIndex.tsx b/packages/ui/src/components/FeatureIndex.tsx index 227bd0a..74e0e35 100644 --- a/packages/ui/src/components/FeatureIndex.tsx +++ b/packages/ui/src/components/FeatureIndex.tsx @@ -4,7 +4,7 @@ import { useQcRoute } from "../host"; import Link from "next/link"; import { useMemo, useState } from "react"; -import { AlertTriangle, CheckCircle2, ChevronRight } from "lucide-react"; +import { CheckCircle2, ChevronRight } from "lucide-react"; import { Anchor, Badge, @@ -20,7 +20,7 @@ import type { HumanSource, ProjectMapFeature, ScanResult } from "@shiplightai/qu import type { TargetSummary } from "@shiplightai/quality-core/workspace"; // Explorer (spec 045): a flat, read-only feature index — the project's sources, then a scannable -// feature table carrying at-a-glance quality columns (confidence · checks · gaps · blockers). QC is +// feature table carrying at-a-glance quality columns (confidence · checks · gaps). QC is // view-only: curation (priority, confirm, sources, add-source) is authored in the repo by a coding // agent via the `quality` skill (PRs), not here. Per-feature depth lives on the feature page. @@ -114,7 +114,6 @@ function FeatureRow({ const hasDescription = feature.description !== undefined && feature.description.trim().length > 0; const confirmed = feature.status !== "candidate"; const gaps = target === undefined ? undefined : sumGaps(target.gapCounts); - const blockers = target?.releaseRiskCounts.blockers ?? 0; return (
@@ -180,18 +179,6 @@ function FeatureRow({ 0 ? "orange" : "dimmed"}> {gaps ?? "—"} - -
- {blockers > 0 ? ( - }> - {blockers} - - ) : ( - - {target === undefined ? "—" : 0} - - )} -
{hasDescription ? ( @@ -309,7 +296,6 @@ export function FeatureIndex({ Status Checks Gaps - Blockers {features.map((feature) => ( diff --git a/packages/ui/src/components/OwnerDashboard.tsx b/packages/ui/src/components/OwnerDashboard.tsx index 760a637..609681e 100644 --- a/packages/ui/src/components/OwnerDashboard.tsx +++ b/packages/ui/src/components/OwnerDashboard.tsx @@ -3,7 +3,7 @@ import { useQcRoute } from "../host"; import Link from "next/link"; -import { AlertTriangle, BarChart3, Boxes, CircleGauge, GitBranch, ShieldCheck } from "lucide-react"; +import { AlertTriangle, BarChart3, Boxes, CircleGauge, ShieldCheck } from "lucide-react"; import { Alert, Anchor, Box, Group, Paper, SimpleGrid, Stack, Text, Title } from "@mantine/core"; import type { SavedQcView } from "@shiplightai/quality-core"; import type { Workspace } from "@shiplightai/quality-core/workspace"; @@ -167,7 +167,7 @@ export function OwnerDashboard({ /> - + } @@ -185,13 +185,6 @@ export function OwnerDashboard({ : `${summary.attentionCounts.atRisk} open evidence ${summary.attentionCounts.atRisk === 1 ? "gap" : "gaps"} across ${totalCheckCount} quality ${totalCheckCount === 1 ? "check" : "checks"}.`} helpText="Gap records identify where quality checks still lack enough proof. One check can have more than one gap, so the gap count and check count measure different things." /> - } - label="Release blockers" - value={String(summary.releaseRiskCounts.blockers)} - ariaLabel={`${summary.releaseRiskCounts.blockers} release ${summary.releaseRiskCounts.blockers === 1 ? "blocker" : "blockers"} across the project.`} - helpText="Evidence gaps marked as release blockers by the underlying quality evidence and analytics model." - /> {project !== undefined ? ( diff --git a/packages/ui/src/styles.css b/packages/ui/src/styles.css index 786ec53..2117b75 100644 --- a/packages/ui/src/styles.css +++ b/packages/ui/src/styles.css @@ -233,11 +233,11 @@ .feature-index-row { display: grid; - grid-template-columns: 18px minmax(160px, 1fr) 96px 88px 104px 52px 44px 68px; + grid-template-columns: 18px minmax(160px, 1fr) 96px 88px 104px 52px 44px; align-items: center; gap: 12px; padding: 8px 14px; - min-width: 720px; + min-width: 640px; } /* Sources table: same headed-table style as the feature index (shares .feature-index-table wrapper, diff --git a/tests/contract/analytics.contract.test.ts b/tests/contract/analytics.contract.test.ts index c9b5a73..75a20db 100644 --- a/tests/contract/analytics.contract.test.ts +++ b/tests/contract/analytics.contract.test.ts @@ -16,8 +16,7 @@ describe("analytics release-confidence selectors", () => { expect(view.state).toBe("ready"); // hasAutomatedEvidence is now "has any automated evidence type" (depth removed). // Of the 7 P0/P1 expectations, four carry an automated type: p0-direct-gated - // (contract), p1-blocked (integration), p1-stale (e2e), p1-accepted (contract). - // p1-blocked previously had depth BLOCKED and was excluded; integration now counts. + // (contract), p1-ungated (integration), p1-stale (e2e), p1-accepted (contract). expect(direct).toMatchObject({ numerator: 4, denominator: 7, @@ -31,14 +30,13 @@ describe("analytics release-confidence selectors", () => { expect(view.guardrails.join(" ")).toContain("No single readiness score"); }); - it("counts stale, manual-only, missing, blocked, accepted, and deferred contexts", () => { + it("counts stale, manual-only, missing, accepted, and deferred contexts", () => { const view = buildAnalyticsView({ result: analyticsStructuredResult(), targetId }); const byId = new Map(view.metrics.map((metric) => [metric.metricId, metric])); expect(byId.get("stale-evidence")?.numerator).toBe(1); expect(byId.get("manual-only-exposure")?.numerator).toBe(2); expect(byId.get("missing-evidence")?.numerator).toBe(1); - expect(byId.get("blocked-gaps")?.numerator).toBe(1); expect(byId.get("accepted-risks")?.numerator).toBe(1); expect(byId.get("deferred-risks")?.numerator).toBeGreaterThanOrEqual(1); }); diff --git a/tests/contract/gap-triage.contract.test.ts b/tests/contract/gap-triage.contract.test.ts index 2339269..a2355db 100644 --- a/tests/contract/gap-triage.contract.test.ts +++ b/tests/contract/gap-triage.contract.test.ts @@ -22,7 +22,6 @@ describe("gap triage selectors", () => { expect(view.groups.map((group) => group.category)).toEqual( expect.arrayContaining([ "missing", - "blocked", "stale", "deferred", "manual-only", @@ -67,10 +66,10 @@ describe("gap triage selectors", () => { const records = view.records.filter((record) => record.expectationTitle === "Multi category expectation"); expect(records.map((record) => record.category)).toEqual( - expect.arrayContaining(["blocked", "stale", "deferred", "manual-only", "weak"]) + expect.arrayContaining(["stale", "deferred", "manual-only", "weak"]) ); expect(new Set(records.map((record) => record.expectationId)).size).toBe(1); - expect(records.every((record) => record.relatedCategoryIds.includes("blocked"))).toBe(true); + expect(records.every((record) => record.relatedCategoryIds.includes("stale"))).toBe(true); }); it("filters category, priority, source classification, and residual risk", () => { @@ -117,12 +116,12 @@ describe("gap triage selectors", () => { it("preserves structured evidence paths and commands for fix prompts", () => { const view = buildGapTriage({ result: gapStructuredResult(), targetId }); - const blocked = view.records.find((record) => record.expectationTitle === "Blocked expectation"); + const command = view.records.find((record) => record.expectationTitle === "Command evidence expectation"); const failing = view.records.find((record) => record.expectationTitle === "Failing expectation"); - expect(blocked?.evidence[0]).toMatchObject({ - command: "pnpm test -- blocked", - pathOrUrl: "pnpm test -- blocked" + expect(command?.evidence[0]).toMatchObject({ + command: "pnpm test -- command-evidence", + pathOrUrl: "pnpm test -- command-evidence" }); expect(failing?.evidence[0]).toMatchObject({ path: "tests/contract/failing.test.ts", diff --git a/tests/contract/modern-workspace.contract.test.ts b/tests/contract/modern-workspace.contract.test.ts index 425b04c..81df17f 100644 --- a/tests/contract/modern-workspace.contract.test.ts +++ b/tests/contract/modern-workspace.contract.test.ts @@ -73,7 +73,7 @@ function targetSummary(overrides: Partial & Pick { evidenceCount: 0, expectationCount: 0, diagnosticCounts: { error: 0, warning: 0, info: 0 }, - releaseRiskCounts: { blockers: 0, accepted: 0, deferred: 0 }, + releaseRiskCounts: { accepted: 0, deferred: 0 }, riskIndicators: [], sourceRefs: [ { label: "Project map", path: ".quality/project-map.yaml" }, @@ -682,7 +682,7 @@ describe("modern quality workspace", () => { evidenceCount: 0, expectationCount: 0, diagnosticCounts: { error: 0, warning: 0, info: 0 }, - releaseRiskCounts: { blockers: 0, accepted: 0, deferred: 0 }, + releaseRiskCounts: { accepted: 0, deferred: 0 }, riskIndicators: [], sourceRefs: [ { label: "Feature spec", path: "specs/030-agent-monitoring-runtime/spec.md" } diff --git a/tests/fixtures/analytics/complete/quality-map.yaml b/tests/fixtures/analytics/complete/quality-map.yaml index 3a3545b..35a9e3c 100644 --- a/tests/fixtures/analytics/complete/quality-map.yaml +++ b/tests/fixtures/analytics/complete/quality-map.yaml @@ -42,19 +42,19 @@ expectations: proof_gap: summary: "Manual-only exposure remains." next_step: "Add a repeatable automated UX check." - - id: "p1-blocked" - title: "P1 blocked expectation" + - id: "p1-ungated" + title: "P1 ungated expectation" source_type: "SOURCE" category: "payments" priority: "P1" evidence: - - id: "blocked-proof" + - id: "ungated-proof" type: "integration" - command: "pnpm test -- blocked" + command: "pnpm test -- ungated" contexts: - "local" proof_gap: - summary: "Blocked by external sandbox." + summary: "External sandbox access is pending." next_step: "Restore sandbox access or replace the dependency." - id: "p1-stale" title: "P1 stale expectation" diff --git a/tests/fixtures/gap-triage/complete/quality-map.yaml b/tests/fixtures/gap-triage/complete/quality-map.yaml index e9c123a..b2b0ce8 100644 --- a/tests/fixtures/gap-triage/complete/quality-map.yaml +++ b/tests/fixtures/gap-triage/complete/quality-map.yaml @@ -14,19 +14,19 @@ expectations: proof_gap: summary: "No executable proof exists." next_step: "Add a contract test." - - id: "blocked" - title: "Blocked expectation" + - id: "command-evidence" + title: "Command evidence expectation" source_type: "SOURCE" category: "billing" priority: "P1" evidence: - - id: "blocked-proof" + - id: "command-proof" type: "integration" - command: "pnpm test -- blocked" + command: "pnpm test -- command-evidence" contexts: - "local" proof_gap: - summary: "Blocked by unavailable staging credentials." + summary: "Staging credentials are unavailable." next_step: "Restore staging credentials." - id: "stale" title: "Stale expectation" @@ -175,5 +175,5 @@ expectations: contexts: - "manual-review" proof_gap: - summary: "Blocked, stale, and deferred context is source-provided." - next_step: "Unblock the deferred stale check." + summary: "Stale and deferred context is source-provided." + next_step: "Refresh the deferred stale check." diff --git a/tests/fixtures/gap-triage/fallback/test-report.md b/tests/fixtures/gap-triage/fallback/test-report.md index 6cb0faa..0113149 100644 --- a/tests/fixtures/gap-triage/fallback/test-report.md +++ b/tests/fixtures/gap-triage/fallback/test-report.md @@ -8,4 +8,4 @@ Fallback gap target is partial. | Testing What | Evidence | Result | Confidence | Residual Risk | | --- | --- | --- | --- | --- | -| Fallback gap expectation | docs/manual-gap.md | BLOCKED | LOW | Blocked fallback source | +| Fallback gap expectation | docs/manual-gap.md | NOT RUN | LOW | Unverified fallback source | diff --git a/tests/integration/analytics.test.ts b/tests/integration/analytics.test.ts index bdb29fe..71872fb 100644 --- a/tests/integration/analytics.test.ts +++ b/tests/integration/analytics.test.ts @@ -27,8 +27,8 @@ describe("analytics integration", () => { try { const view = buildAnalyticsView({ result: analyticsStructuredResult(), targetId }); - expect(view.metrics).toHaveLength(8); - expect(view.riskSummary.blockers).toHaveLength(1); + expect(view.metrics).toHaveLength(7); + expect(view.riskSummary.acceptedRisks).toHaveLength(1); } finally { globalThis.fetch = originalFetch; } diff --git a/tests/integration/gap-triage.test.ts b/tests/integration/gap-triage.test.ts index f5a1a01..34de1e3 100644 --- a/tests/integration/gap-triage.test.ts +++ b/tests/integration/gap-triage.test.ts @@ -26,7 +26,7 @@ describe("gap triage integration", () => { expect(view.records.length).toBeGreaterThan(10); expect(view.records.some((record) => record.nextProof.text === "No source-provided recommended action")).toBe(true); - expect(view.records.some((record) => record.residualRisk.includes("Blocked"))).toBe(true); + expect(view.records.some((record) => record.residualRisk.includes("Staging credentials"))).toBe(true); } finally { globalThis.fetch = originalFetch; } @@ -66,13 +66,13 @@ describe("gap triage integration", () => { it("keeps diagnostics and existing evidence context visible with gaps", () => { const view = buildGapTriage({ result: gapStructuredResult(), targetId }); - const blocked = view.records.find((record) => record.category === "blocked"); + const record = view.records.find((entry) => entry.expectationTitle === "Command evidence expectation"); // depth is the type-derived proof tier (integration -> "automated"). - expect(blocked?.evidence[0]).toMatchObject({ - label: "blocked-proof", + expect(record?.evidence[0]).toMatchObject({ + label: "command-proof", depth: "automated" }); - expect(blocked?.residualRisk).toBe("Blocked by unavailable staging credentials."); + expect(record?.residualRisk).toBe("Staging credentials are unavailable."); }); }); diff --git a/tests/unit/gap-vocabulary.test.ts b/tests/unit/gap-vocabulary.test.ts new file mode 100644 index 0000000..55eef5a --- /dev/null +++ b/tests/unit/gap-vocabulary.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { GAP_CATEGORIES, gapCategoryOrder, riskBadgeFor } from "@shiplightai/quality-core"; + +describe("gap category vocabulary", () => { + // `gapCategoryOrder` is typed `readonly GapCategory[]`, so an omission is not a type + // error. Both classifier paths end in `gapCategoryOrder.filter(...)`, so a category + // present in GAP_CATEGORIES but missing here is silently dropped from every gap + // record and every triage group — a whole class of gaps disappearing with no signal. + it("orders exactly the canonical gap categories", () => { + expect([...gapCategoryOrder].sort()).toEqual([...GAP_CATEGORIES].sort()); + }); +}); + +describe("owner risk badge", () => { + // Markdown-fallback rows copy the Result / Confidence / Residual Risk cells verbatim + // from the scanned project, so these inputs are attacker-controlled in the sense that + // matters here: the project being scored writes them. A stated non-passing result must + // never be promoted to Covered by a confidence claim sitting beside it. + it("does not let a confidence claim promote a stated non-passing result", () => { + expect(riskBadgeFor({ status: "BLOCKED", evidenceConfidence: "HIGH", residualRisk: "None", hasEvidence: true })) + .toBe("Gap"); + expect(riskBadgeFor({ status: "FAIL", evidenceConfidence: "HIGH", residualRisk: "None", hasEvidence: true })) + .toBe("Gap"); + expect(riskBadgeFor({ status: "SKIPPED", evidenceConfidence: "HIGH", residualRisk: "None", hasEvidence: true })) + .toBe("Unknown"); + }); + + it("keeps a blocked row a gap whatever confidence the report claims", () => { + for (const evidenceConfidence of ["HIGH", "MEDIUM", "LOW", undefined]) { + expect(riskBadgeFor({ status: "BLOCKED", evidenceConfidence, residualRisk: "None", hasEvidence: true })) + .toBe("Gap"); + } + }); + + it("still settles a row that states no result of its own by confidence", () => { + expect(riskBadgeFor({ status: undefined, evidenceConfidence: "HIGH", residualRisk: "None", hasEvidence: true })) + .toBe("Covered"); + expect(riskBadgeFor({ status: "", evidenceConfidence: "LOW", residualRisk: "None", hasEvidence: true })) + .toBe("Unknown"); + }); + + it("keeps the structured assessment statuses on their existing badges", () => { + expect(riskBadgeFor({ status: "COVERED", evidenceConfidence: "HIGH", residualRisk: "None", hasEvidence: true })) + .toBe("Covered"); + expect(riskBadgeFor({ status: "PARTIAL", evidenceConfidence: "MEDIUM", residualRisk: "None", hasEvidence: true })) + .toBe("Gap"); + expect(riskBadgeFor({ status: "NOT COVERED", evidenceConfidence: "LOW", residualRisk: "None", hasEvidence: false })) + .toBe("Missing"); + }); +}); From 073c400cc8a6307a5d76372ac2d37878bb53d74e Mon Sep 17 00:00:00 2001 From: Feng Qian Date: Mon, 10 Aug 2026 12:45:11 -0700 Subject: [PATCH 2/3] ci: inline the Claude review action into this repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review gate called out to ShiplightAI/internal-tools/claude-review@v1, a private action. That has been failing since sometime today with "Unable to resolve action `shiplightai/internal-tools`, not found", while the action, its v1 tag (d9bb901f2388), this workflow file, and both repos' Actions policies are all unchanged from this morning's passing run — so the break is an org-side access change this repo cannot see or fix. A public repo depending on a private action is the wrong shape regardless of that outage. Outside contributors cannot read the logic that gates their PR, it never runs for fork PRs because secrets are withheld there, and it fails in exactly this way when org settings move. Nothing in the shared action was secret: it is a checkout plus anthropics/claude-code-action plus a prompt, and the OAuth token is supplied by this repo. Both wrapped actions stay pinned at the same SHAs they had inside the composite. Verified the rendered prompt is byte-identical to the action's template with this repo's review-focus substituted in. The cost is that shared prompt edits no longer propagate here automatically, which is the right trade for the one public repo in the set — its review logic should be readable by the people it reviews. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/claude-code-review.yml | 54 +++++++++++++++++++++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index f550c30..cf33a59 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,10 +1,15 @@ name: Claude Code Review -# Thin caller: the prompt, severity/approval logic, checkout, and pinned action -# refs all live in the shared ShiplightAI/internal-tools/claude-review action. -# This file only owns the trigger, permissions, and repo-specific review focus -# (GitHub requires the trigger + permissions in the caller, and -# claude-code-action's tamper guard requires the token wired here). +# Self-contained: this repo is public, so its review gate is inlined rather than +# called out to the private ShiplightAI/internal-tools/claude-review action. A +# public repo whose CI depends on a private action cannot be read or reproduced +# by outside contributors, never runs for fork PRs (secrets are withheld there), +# and breaks whenever org-side action access changes. Nothing in the shared +# action was secret — the OAuth token is supplied here, from this repo's secrets +# — so inlining costs only the automatic propagation of shared prompt edits. +# +# The two actions below are the ones the shared action ran internally, at the +# same pinned SHAs. Keep them pinned by SHA. on: pull_request: types: [opened, synchronize] @@ -18,10 +23,29 @@ jobs: issues: read id-token: write # required: claude-code-action mints its App token via OIDC steps: - - uses: ShiplightAI/internal-tools/claude-review@v1 + - name: Checkout repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + fetch-depth: 1 + + - name: Run Claude Code Review + uses: anthropics/claude-code-action@51705da45eecce209d4700538bf8377d5b5fc695 # v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - review-focus: | + # The review (including approvals) is submitted by the Claude GitHub App + # token the action mints via OIDC — a distinct identity from the PR author, + # so it can approve without the self-approval restriction. + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Please review this pull request and provide feedback on: + - Code quality and best practices + - Potential bugs or issues + - Security concerns (secret handling, injection, least privilege) + - Performance considerations + - Test coverage + This repo is a quality oracle: it scores how well *other* projects are verified. Its credibility rests on scores nobody can fake. Weight these project invariants: @@ -61,3 +85,19 @@ jobs: - Docs must match reality: `README.md` documents the install path and the package layout, so a stale skill list, command set, or install ref there is a real defect, not a nit. + + Use the repository's CLAUDE.md for guidance on style and conventions. + Be constructive and specific; cite file and line. + + Classify every issue you find by severity: CRITICAL, HIGH, MEDIUM, or LOW. + + Then submit your review with your Bash tool, choosing the event by severity: + - If there are NO CRITICAL, NO HIGH and no MEDIUM issues, approve: + gh pr review ${{ github.event.pull_request.number }} --approve --body "" + - If there is any CRITICAL, HIGH or MEDIUM issue, request changes: + gh pr review ${{ github.event.pull_request.number }} --request-changes --body "" + - If you cannot confidently classify the severity, comment: + gh pr review ${{ github.event.pull_request.number }} --comment --body "" + + Always include the full severity-labelled findings in the review body. + claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr review:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)"' From c2cd002b587771f2e8b4b877019d29f48c42030e Mon Sep 17 00:00:00 2001 From: Feng Qian Date: Mon, 10 Aug 2026 12:47:40 -0700 Subject: [PATCH 3/3] ci: fail the review gate when its token is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inlined review job went green in 15s without posting a review. The run log shows why: claude_code_oauth_token resolved to "", and claude-code-action skips every step and exits 0 when it has no token. The secret is genuinely absent — repos/ShiplightAI/quality/actions/secrets and .../actions/organization-secrets both return total_count 0 — though reviews were posted from it earlier today, so it disappeared along with access to the private action. A gate that reports success without reviewing anything is the quiet failure CONTRIBUTING.md warns about: "a value that is dropped rather than rejected". Check the token before the run so a missing secret is a red check with an actionable message instead of a silent pass. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/claude-code-review.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index cf33a59..1e0f791 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -23,6 +23,21 @@ jobs: issues: read id-token: write # required: claude-code-action mints its App token via OIDC steps: + # Without a token, claude-code-action skips every step and exits 0 — the gate + # reports success having reviewed nothing. A review that silently passes is + # worse than one that visibly fails, so turn the silence into a red check. + - name: Verify review credentials are present + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + run: | + if [ -z "${CLAUDE_CODE_OAUTH_TOKEN}" ]; then + echo "::error::CLAUDE_CODE_OAUTH_TOKEN is empty or unset for this repository." + echo "claude-code-action no-ops without it, so this gate would report success" + echo "without reviewing the diff. Restore the secret at the repository or" + echo "organization level, then re-run." + exit 1 + fi + - name: Checkout repository uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 with: