fix(ci): frontmatter gate, correct review phase numbers, refresh rubric dimension 8

.claude/skills/pr-review/rubric.md

@@ -1,12 +1,22 @@
 # Orbit Review Rubric
 
-The single source of truth for what a review checks, shared verbatim by two skills:
-`/pr-review` walks it dimension-by-dimension over a **diff**, and `/audit-code-quality`
-(#228) walks it over the **whole repo**. Both read this one file
-(`.claude/skills/pr-review/rubric.md`) — there is no second copy, so the two can never
-drift. It is command-agnostic on purpose: it contains **dimensions, severities, and
-templates** — no orchestration, no scope resolution, no GitHub mechanics. Those live in
-the consuming skill.
+The single source of truth for what a review checks **in this repo**. Exactly two things
+read it: `/pr-review` (`.claude/skills/pr-review/SKILL.md`), which walks it
+dimension-by-dimension over a **diff**, and `.github/workflows/claude-review.yml`, which
+invokes that same skill in CI and maintains no criteria of its own. Unlike
+orbit-ui-mobile, this repo ships no `/audit-code-quality` skill, so nothing here walks the
+rubric over the whole repo.
+
+**A second copy exists**, at `orbit-ui-mobile/.claude/skills/pr-review/rubric.md`, where
+`/audit-code-quality` also walks it. Two repos and two CIs mean the file cannot be
+deduped, so the copies are lockstep twins kept aligned by hand, exactly like the two
+`pr-review/SKILL.md` copies. Sanctioned divergences are backend-only material, such as
+dimension 13's transaction-teardown bullet (`ORBIT0002`). Change a dimension here and
+mirror it there in the same task; a drift between the two is a defect, not a variant.
+
+It is command-agnostic on purpose: it contains **dimensions, severities, and templates**,
+no orchestration, no scope resolution, no GitHub mechanics. Those live in the consuming
+skill.
 
 Every finding cites the rule it came from (a `CLAUDE.md` rule number, `no-comments.cjs`,
 a `DESIGN.md` line, an orbit-api hard rule, or a security category) so the author can
@@ -64,7 +74,7 @@ Every finding, every dimension, the same shape:
 · issue: <1-2 sentences — what is wrong>
 · risk: <1-2 sentences — what goes wrong if it ships>
 · fix: <the concrete change, or a corrected snippet>
-· reference: <CLAUDE.md rule N | no-comments.cjs | DESIGN.md:181 | orbit-api hard rule | OWASP | security category>
+· reference: <CLAUDE.md rule N | no-comments.cjs | DESIGN.md "Bans" | orbit-api hard rule | OWASP | security category>
 ```
 
 ---
@@ -164,30 +174,71 @@ well-named function** so the code reads without prose.
 
 ### 8. DESIGN.md / AI-slop
 
-> Reference: DESIGN.md:181 (AI-slop tells), DESIGN.md:185 (scene-sentence test),
-> DESIGN.md:159-169 (bans). **Gated: only when the diff touches `apps/*` UI files.**
-
-Scan for the AI-slop tells from `DESIGN.md:181`:
-
-- Gradients used as decoration outside the sanctioned gradient-header.
-- Cards-in-cards (opaque card-on-card on dark).
-- Gray text on colored backgrounds.
-- Rounded-square icon tiles above headings.
-- Semantic-red destructive fills where the artboard shows a text pill.
-- Oversized centered H1 outside hero contexts; decorative gradient borders.
-
-Token / ban checks (`DESIGN.md:159-169`):
-
-- No raw `--slate-*` references or hardcoded violet rgba — semantic tokens only
-  (`--primary` / `--primary-rgb` / `tintFromPrimary`).
-- No `transition-all` (animate `transform` / `opacity`, named); no `h-screen` (use
+> Reference: `DESIGN.md` at the orbit-ui-mobile repo root, sections **Identity & anchor
+> (locked)** (:18), **Bans** (:469), **AI-slop test** (:505), **Scene-sentence test**
+> (:525). Cite the section name, not the line alone: the line numbers move, the section
+> names do not. **Gated: only when the diff touches `apps/*` UI files.** An
+> orbit-api-only diff marks this N/A, and in CI the file is not checked out at all, so it
+> is "not verifiable in CI".
+
+The anchor is the **de-decorated navy-violet orbital** (the #539 freeze, 2026-07-17).
+Identity comes from three carriers and nothing else: the **orbital logo mark**, the
+**Astra orbital glyph**, and **ring-shaped status and progress indicators**. It never
+comes from a background gradient, a glow, decorative background orbit arcs, or texture.
+**Quiet decoration is still decoration**: a softened glow, a 0.03-opacity texture, a
+"subtle" mesh is the same finding as the loud version. The freeze removed the layer, it
+did not dim it.
+
+Scan for the AI-slop tells:
+
+- Decoration used as hierarchy: any glow, gradient wash, gradient border, gradient text
+  (`bg-clip-text` over a gradient), mesh, bloom, texture, or "quiet" background effect.
+  There is no sanctioned gradient and no sanctioned glow left: `--gradient-header`,
+  `GradientTop`, and the primary-glow shadow token are **deleted**.
+- Cards in cards (opaque card-on-card on dark), and cards used where spacing would have
+  grouped.
+- A coloured side-stripe border on a row, card, callout, or alert.
+- Connector or tree lines in a hierarchy.
+- Grey text on coloured backgrounds; rounded-square icon tiles above headings.
+- Semantic-red destructive fills where the spec shows a text pill.
+- An oversized centered H1 outside a hero context.
+- The hero-metric template used as decoration, or any invented precise-looking number.
+- A whole-section fade-and-rise scroll reveal, or any page-load choreography.
+- An animation whose purpose cannot be named from the closed list.
+- A heading and the intro beneath it saying the same thing; an eyebrow that enumerates
+  rather than labels.
+
+Token checks that need judgment:
+
+- **`--primary` is fill and graphic only; `--primary-soft` is the accent text token.**
+  Accent-coloured small text on the canvas is a finding, not a preference. On light both
+  resolve to the same value, so the split only bites on dark.
+- **Accent rationing**: the accent appears on the active tab, progress and ring
+  indicators, done dots, the primary CTA, the FAB, and active nav. That is the whole
+  list. Accent on a card, a row, a border, a heading, or an icon not communicating state
+  is decoration.
+- No raw `--slate-*` reference and no hardcoded violet rgba. Semantic tokens only; tints
+  come from `--primary-rgb` (web) or `tintFromPrimary` (mobile).
+- No hand-rolled `box-shadow` heavier than `--shadow-1/2/3`. Shadows model occlusion
+  under a lifted surface; they never carry the accent hue.
+- No `transition-all` (animate `transform` and `opacity`, named); no `h-screen` (use
   `min-h-dvh`); no new font families, radii, or colors outside the spec.
-- No per-component scheme branches — schemes resolve through tokens.
-- No em dashes in user-facing copy.
-
-Then the **scene-sentence test** (`:185`): describe the rendered screen in one sentence.
-If it reads like every other SaaS app, it is generic — flag it to rework until the
-sentence names Orbit's navy-cosmic, violet-glow character.
+- No per-component scheme branch. Schemes resolve through tokens.
+
+**Do not hand-flag what an ESLint `local/*` rule already fails on** in orbit-ui-mobile:
+decorative glow, raw gradient and gradient text, side-stripe borders, off-scale spacing,
+`space-x-*` / `space-y-*`, overshoot easing, arbitrary z-index, full-bleed pill CTAs, and
+em dashes in copy each have a gate. Report the gate's verdict; re-flagging it by hand is
+noise. `DESIGN.md`'s **Enforcement** section is the authoritative gate-versus-reviewer
+split.
+
+Then the **scene-sentence test**: describe the rendered screen in one sentence, as if
+narrating a film scene. If it reads like every other SaaS app ("a clean modern dashboard
+with cards"), it is generic. The sentence must name Orbit's character: a near-black
+neutral canvas, quiet tonal panels separated by hairlines, one violet reserved for what is
+done and what is next, and the orbital ring language carrying the identity. **If the only
+way to make the sentence specific is to describe decoration, the design has failed and the
+decoration is not the fix.**
 
 ### 9. Parity (web ↔ mobile)
 

.github/workflows/arch-map.yml

@@ -5,6 +5,9 @@ on:
   push:
     branches: [main]
 
+permissions:
+  contents: read
+
 jobs:
   drift:
     runs-on: ubuntu-latest

.github/workflows/guards.yml

@@ -1,9 +1,15 @@
 name: Guards
 
-# Vendored from orbit-ui-mobile (REBUILD.md 6.1.1): the cross-repo dash ban.
-# The old session-only hook matched /Email/ and ResendEmailService.cs only, so
-# 71 tracked .cs files carried dashes ungated (a confirmed audit gap). This job
-# holds for every tool because CI is the one layer nothing bypasses.
+# Cheap repo-hygiene gates vendored from orbit-ui-mobile, one job each. They
+# hold for every tool because CI is the one layer nothing bypasses.
+#
+# Dash Ban (REBUILD.md 6.1.1): the old session-only hook matched /Email/ and
+# ResendEmailService.cs only, so 71 tracked .cs files carried dashes ungated (a
+# confirmed audit gap).
+#
+# Skill and Agent Frontmatter (REBUILD.md section 14, item 1): an unquoted YAML
+# value containing ": " drops the skill or agent silently, which is how this
+# repo's security-reviewer stopped loading.
 
 on:
   pull_request:
@@ -34,8 +40,24 @@ jobs:
       - name: Baseline only shrinks
         run: node tools/check-dashes.mjs --check-baseline
       - name: PR title and body carry no dashes
-        if: github.event_name == 'pull_request'
+        # Human-authored prose only. A bot PR body is machine-copied upstream text
+        # (Dependabot quotes the dependency's own changelog verbatim), which Orbit
+        # neither wrote nor can edit, and which Dependabot regenerates on every
+        # rebase. The file-level dash check above still runs on bot PRs, because
+        # the files they change ARE Orbit's. See orbit-api#425, where a Hangfire
+        # release note's en dashes blocked a security-relevant NuGet bump.
+        if: github.event_name == 'pull_request' && github.event.pull_request.user.type != 'Bot'
         env:
           PR_TITLE: ${{ github.event.pull_request.title }}
           PR_BODY: ${{ github.event.pull_request.body }}
         run: node tools/check-dashes.mjs --text "$PR_TITLE $PR_BODY"
+
+  frontmatter:
+    name: Skill and Agent Frontmatter
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: actions/setup-node@v7
+        with:
+          node-version: 22
+      - run: node tools/check-frontmatter.mjs

src/Orbit.Api/Controllers/OAuthController.cs

@@ -280,14 +280,25 @@ await ConcurrencyRetry.SaveWithRetryAsync(
         return Result.Success(user);
     }
 
+    /// <summary>
+    /// Exchanges an authorization code for an Orbit API key.
+    /// </summary>
+    /// <remarks>
+    /// A <c>client_id</c> form field is accepted by the protocol and ignored here. This server is a
+    /// public-client authorization server (<c>token_endpoint_auth_method: none</c>) whose
+    /// <c>/oauth/register</c> hands out an unpersisted random identifier, so a <c>client_id</c> presented
+    /// at this endpoint is an unverified, client-asserted string and matching it against the equally
+    /// client-asserted value captured at authorize time proves nothing about the caller. Client binding
+    /// comes from PKCE S256, the exact redirect_uri match, single-use codes, and the 5-minute expiry, all
+    /// enforced in <see cref="OAuthAuthorizationStore.ExchangeCode"/>.
+    /// </remarks>
     [HttpPost("/oauth/token")]
     [DistributedRateLimit("auth")]
     [Consumes("application/x-www-form-urlencoded")]
     public async Task<IActionResult> Token(
         [FromForm] string grant_type,
         [FromForm] string code,
         [FromForm] string code_verifier,
-        [FromForm] string? client_id,
         [FromForm] string redirect_uri,
         CancellationToken ct)
     {

src/Orbit.Api/openapi.json

@@ -1,3 +1,3 @@
 {
   "openapi": "3.1.1",
   "info": {
@@ -8814,14 +8814,6 @@
                       }
                     }
                   },
-                  {
-                    "type": "object",
-                    "properties": {
-                      "client_id": {
-                        "type": "string"
-                      }
-                    }
-                  },
                   {
                     "type": "object",
                     "properties": {

tests/Orbit.Infrastructure.Tests/Controllers/OAuthControllerTests.cs

@@ -537,7 +537,7 @@ public async Task Token_UnsupportedGrantType_ReturnsBadRequest()
     {
         var result = await _controller.Token(
             "client_credentials", "code-abc", "verifier-xyz",
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
         var json = JsonSerializer.Serialize(bad.Value);
@@ -549,7 +549,7 @@ public async Task Token_InvalidCode_ReturnsBadRequest()
     {
         var result = await _controller.Token(
             "authorization_code", "nonexistent-code", "verifier-xyz",
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
         var json = JsonSerializer.Serialize(bad.Value);
@@ -561,7 +561,7 @@ public async Task Token_InvalidRedirectUri_ReturnsBadRequest()
     {
         var result = await _controller.Token(
             "authorization_code", "nonexistent-code", "verifier-xyz",
-            "client-123", "https://evil.com/callback", CancellationToken.None);
+            "https://evil.com/callback", CancellationToken.None);
 
         var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
         JsonSerializer.Serialize(bad.Value).Should().Contain("invalid_redirect_uri");
@@ -580,7 +580,7 @@ public async Task Token_ValidCodeExchange_ReturnsAccessToken()
 
         var result = await _controller.Token(
             "authorization_code", authCode, codeVerifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var ok = result.Should().BeOfType<OkObjectResult>().Subject;
         var json = JsonSerializer.Serialize(ok.Value);
@@ -589,6 +589,39 @@ public async Task Token_ValidCodeExchange_ReturnsAccessToken()
         await _apiKeyRepo.Received(1).AddAsync(Arg.Any<ApiKey>(), Arg.Any<CancellationToken>());
     }
 
+    [Fact]
+    public async Task Token_TakesNoClientId_AndBindsOnPkceAndRedirectUriInstead()
+    {
+        var (verifier, challenge) = GeneratePkce();
+        var code = _authStore.CreateCode(UserId, challenge, "https://claude.ai/callback", "client-registered-at-authorize");
+
+        var result = await _controller.Token(
+            "authorization_code", code, verifier,
+            "https://claude.ai/callback", CancellationToken.None);
+
+        result.Should().BeOfType<OkObjectResult>();
+
+        typeof(OAuthController).GetMethod(nameof(OAuthController.Token))!
+            .GetParameters().Select(p => p.Name)
+            .Should().NotContain("client_id");
+    }
+
+    [Fact]
+    public async Task Token_WrongCodeVerifier_ReturnsInvalidGrant()
+    {
+        var (_, challenge) = GeneratePkce();
+        var (otherVerifier, _) = GeneratePkce();
+        var code = _authStore.CreateCode(UserId, challenge, "https://claude.ai/callback", "client-123");
+
+        var result = await _controller.Token(
+            "authorization_code", code, otherVerifier,
+            "https://claude.ai/callback", CancellationToken.None);
+
+        var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
+        JsonSerializer.Serialize(bad.Value).Should().Contain("invalid_grant");
+        await _apiKeyRepo.DidNotReceive().AddAsync(Arg.Any<ApiKey>(), Arg.Any<CancellationToken>());
+    }
+
     [Fact]
     public async Task Token_ValidExchange_CreatesReadWriteClaudeKeyWithDefaultScopes()
     {
@@ -601,7 +634,7 @@ public async Task Token_ValidExchange_CreatesReadWriteClaudeKeyWithDefaultScopes
 
         var result = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         result.Should().BeOfType<OkObjectResult>();
         created.Should().NotBeNull();
@@ -632,7 +665,7 @@ public async Task Token_ValidExchange_RevokesPriorClaudeKeysBeforeIssuingNew()
 
         var result = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         result.Should().BeOfType<OkObjectResult>();
         priorKey.IsRevoked.Should().BeTrue();
@@ -712,7 +745,7 @@ public async Task Token_WithAttackerDomainRedirectUri_ReturnsBadRequest()
     {
         var result = await _controller.Token(
             "authorization_code", "any-code", "verifier-xyz",
-            "client-123", "https://attacker.com/callback", CancellationToken.None);
+            "https://attacker.com/callback", CancellationToken.None);
 
         var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
         var json = JsonSerializer.Serialize(bad.Value);
@@ -865,7 +898,7 @@ public async Task Token_WithNonce_EchoesNonceInResponse()
 
         var result = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var ok = result.Should().BeOfType<OkObjectResult>().Subject;
         var json = JsonSerializer.Serialize(ok.Value);
@@ -882,7 +915,7 @@ public async Task Token_WithoutNonce_OmitsNonceFromResponse()
 
         var result = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var ok = result.Should().BeOfType<OkObjectResult>().Subject;
         JsonSerializer.Serialize(ok.Value).Should().NotContain("nonce");
@@ -904,7 +937,7 @@ public async Task VerifyCode_WithNonce_BindsNonceRetrievableAtTokenExchange()
 
         var tokenResult = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var ok = tokenResult.Should().BeOfType<OkObjectResult>().Subject;
         JsonSerializer.Serialize(ok.Value).Should().Contain("verify-nonce-7");
@@ -931,7 +964,7 @@ public async Task GoogleAuth_WithNonce_BindsNonceRetrievableAtTokenExchange()
 
         var tokenResult = await _controller.Token(
             "authorization_code", code, verifier,
-            "client-123", "https://claude.ai/callback", CancellationToken.None);
+            "https://claude.ai/callback", CancellationToken.None);
 
         var ok = tokenResult.Should().BeOfType<OkObjectResult>().Subject;
         JsonSerializer.Serialize(ok.Value).Should().Contain("google-nonce-42");

tools/arch-map.mjs

@@ -10,11 +10,14 @@ import { fileURLToPath } from "node:url";
 
 const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "..");
 
+const BUILD_OUTPUT_DIRS = new Set(["bin", "obj"]);
+
 function walk(dir, out = []) {
   for (const name of readdirSync(dir).sort()) {
     const full = join(dir, name);
-    if (statSync(full).isDirectory()) walk(full, out);
-    else if (name.endsWith(".cs")) out.push(full);
+    if (statSync(full).isDirectory()) {
+      if (!BUILD_OUTPUT_DIRS.has(name)) walk(full, out);
+    } else if (name.endsWith(".cs")) out.push(full);
   }
   return out;
 }