Merge pull request #1021 from corbitsdev/cl-7927-small-hardening-and-wording-follow-ups-from-0325-review

fix(hardening): small follow-ups from the 0.3.25 review

Author: TheGreatAxios

docs/PLUGINS.md

@@ -326,9 +326,10 @@ shape.
   `skill_search` / `use_skill` when they only set `user-invocable: false`
   (`git-rebase`, `linear-issue-workflow`, `style`, `philosophy`,
   `native-integration`, `typescript`, `ponytail`, `opsh`). Background libs
-  such as `git-worktrees` set both flags. Bake-only bodies such as
-  `idiot-proof` and `native-runtime` also set both flags (baked into workers;
-  not a slash and not listed for `use_skill`). The slash command is a direct
+  such as `git-worktrees` set both flags. Bodies that stay out of discovery
+  such as `idiot-proof` and `native-runtime` also set both flags (not a slash,
+  and hidden from the skill listing — though an explicit `use_skill` load by
+  name still resolves). The slash command is a direct
   user entry point on top.
 - **First-party catalog.** `plugins/corbits-skills/` (id `corbits-skills`,
   kind `command`, `defaultEnabled: true`) is the bundled skill catalog. Origin

docs/TELEMETRY.md

@@ -86,7 +86,8 @@ skill out of slash synthesis, not out of name reporting: eleven bundled
 skills carry the flag — eight stay listed and loadable (`git-rebase`,
 `linear-issue-workflow`, `opsh`, `philosophy`, `style`, `typescript`,
 `native-integration`, `ponytail`), `git-worktrees` resolves by explicit name
-only, and two stay hidden bake-only (`idiot-proof`, `native-runtime`). Of
+only, and two stay hidden from discovery (`idiot-proof`, `native-runtime`):
+still loadable by explicit name, but every load reports `custom`. Of
 the eleven, seven are reported by name and four stay `custom` (pinned;
 conservative under-reporting, never a leak). Unknown,
 project-local, and plugin-authored skill names are never transmitted —

src/permission/gate.test.ts

@@ -345,7 +345,7 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
     expect(verdict.allowed).toBe(false);
     expect(seen).toHaveLength(1);
     expect(seen[0]?.notice).toBe(
-      "A standing grant matches this command, but it uses --force, so it still needs approval.",
+      "A standing grant matches this command, but it uses --force=true, so it still needs approval.",
     );
   });
 
@@ -357,7 +357,7 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
     expect(verdict.allowed).toBe(false);
     expect(seen).toHaveLength(1);
     expect(seen[0]?.notice).toBe(
-      "A standing grant matches this command, but it uses --force, so it still needs approval.",
+      "A standing grant matches this command, but it uses -f=true, so it still needs approval.",
     );
   });
 
@@ -369,7 +369,7 @@ describe("grant-mismatch asks carry the guard reason as a notice (CL-6824)", ()
     expect(verdict.allowed).toBe(false);
     expect(seen).toHaveLength(1);
     expect(seen[0]?.notice).toBe(
-      "A standing grant matches this command, but it uses --force, so it still needs approval.",
+      "A standing grant matches this command, but it uses -ftrue, so it still needs approval.",
     );
   });
 

src/permission/gate.ts

@@ -148,16 +148,22 @@ function segmentGuard(
 // notice can name the operative reason: a force flag, or a destination the
 // containment authority did not approve. Returns undefined for anything else.
 // Display-only refinement — the guard decision itself is unchanged.
-function worktreeMismatchKind(
-  segment: string,
-): "force" | "destination" | "worktree" | undefined {
+type WorktreeMismatch =
+  | { kind: "force"; flag: string }
+  | { kind: "destination" }
+  | { kind: "worktree" };
+
+function worktreeMismatchKind(segment: string): WorktreeMismatch | undefined {
   const tokens = tokenize(segment);
   if (tokens[0] !== "git" || tokens[1] !== "worktree") return undefined;
   if (tokens[2] !== "add" && tokens[2] !== "remove") return undefined;
-  if (tokens.slice(3).some(isWorktreeForceFlag)) return "force";
+  const flag = tokens.slice(3).find(isWorktreeForceFlag);
+  if (flag !== undefined) return { kind: "force", flag };
   // `add` takes a destination for the new worktree; `remove` names an
   // existing worktree, so only `add` gets the destination noun.
-  return tokens[2] === "remove" ? "worktree" : "destination";
+  return tokens[2] === "remove"
+    ? { kind: "worktree" }
+    : { kind: "destination" };
 }
 
 // Explains a grant mismatch: a standing grant covers the segment, but the
@@ -171,13 +177,13 @@ function grantMismatchNotice(
     return "A standing grant matches this command, but it references a sensitive path, so it still needs approval.";
   }
   const worktreeKind = worktreeMismatchKind(segment);
-  if (worktreeKind === "force") {
-    return "A standing grant matches this command, but it uses --force, so it still needs approval.";
+  if (worktreeKind?.kind === "force") {
+    return `A standing grant matches this command, but it uses ${worktreeKind.flag}, so it still needs approval.`;
   }
-  if (worktreeKind === "destination") {
+  if (worktreeKind?.kind === "destination") {
     return "A standing grant matches this command, but the worktree destination is outside the approved locations, so it still needs approval.";
   }
-  if (worktreeKind === "worktree") {
+  if (worktreeKind?.kind === "worktree") {
     return "A standing grant matches this command, but the worktree is outside the approved locations, so it still needs approval.";
   }
   return "A standing grant matches this command, but it targets a path outside the workspace, so it still needs approval.";

src/provider/codex-responses.test.ts

@@ -1,12 +1,14 @@
 import { describe, expect, test } from "bun:test";
 import { BEARER_CREDENTIAL_SENTINEL } from "@intx/inference";
+import type { ProviderAdapter } from "@intx/inference";
 import type { ConversationTurn, LastCycleSource } from "@intx/types/runtime";
 import { ENVIRONMENT_TAG_NAME, PRODUCT_NAME } from "../branding.js";
 import {
   CODEX_ACCOUNT_ID_OPTION,
   CODEX_RESPONSES_PROVIDER,
   CODEX_SESSION_ID_OPTION,
   createCodexResponsesAdapter,
+  withParallelToolCalls,
 } from "./codex-responses.js";
 
 const source: LastCycleSource = {
@@ -74,6 +76,22 @@ describe("createCodexResponsesAdapter", () => {
     expect(body.parallel_tool_calls).toBe(true);
   });
 
+  test("passes a non-JSON body through unrewritten (CL-7927)", () => {
+    const stub = {
+      buildRequest: () => ({
+        url: "/codex/responses",
+        headers: {},
+        body: "not-json",
+      }),
+    } as unknown as ProviderAdapter;
+    const request = withParallelToolCalls(stub).buildRequest(
+      [userTurn("hello")],
+      "gpt-5.1-codex",
+      {},
+    );
+    expect(request.body).toBe("not-json");
+  });
+
   test("wraps the system prompt with host product identity as a developer item", () => {
     const adapter = createCodexResponsesAdapter(source);
     const request = adapter.buildRequest([userTurn("x")], "gpt-5.1-codex", {

src/provider/codex-responses.ts

@@ -55,12 +55,19 @@ function withHostReasoningEffort(adapter: ProviderAdapter): ProviderAdapter {
 // (HTTP 200 on POST /codex/responses, echoed true in response.created). The
 // packaged adapter still pins false, so the host rewrites the field on the
 // wire; the reactor already fans out a multi-call batch concurrently.
-function withParallelToolCalls(adapter: ProviderAdapter): ProviderAdapter {
+export function withParallelToolCalls(
+  adapter: ProviderAdapter,
+): ProviderAdapter {
   return {
     ...adapter,
     buildRequest: (messages, model, options) => {
       const request = adapter.buildRequest(messages, model, options);
-      const body = JSON.parse(request.body) as Record<string, unknown>;
+      let body: Record<string, unknown>;
+      try {
+        body = JSON.parse(request.body) as Record<string, unknown>;
+      } catch {
+        return request;
+      }
       body["parallel_tool_calls"] = true;
       return { ...request, body: JSON.stringify(body) };
     },

src/telemetry/classify.ts

@@ -80,8 +80,10 @@ const BUILT_IN_AGENT_NAMES: ReadonlySet<string> = new Set([
 // eleven bundled skills carry it — eight stay listed and loadable
 // (git-rebase, linear-issue-workflow, opsh, philosophy, style, typescript,
 // native-integration, ponytail), `git-worktrees` resolves by explicit name
-// only, and two stay hidden bake-only (idiot-proof, native-runtime), which
-// are baked into agent prompts rather than invoked as skills. Of the eleven,
+// only, and two stay hidden from listings (idiot-proof, native-runtime):
+// `disable-model-invocation` keeps them out of discovery, but an explicit
+// `use_skill` / `resolveSkillBody` load still resolves — and reports `custom`.
+// Of the eleven,
 // seven are reported by name and four stay `custom` (pinned; conservative
 // under-reporting, never a leak). Project- or plugin-authored skills are
 // never reported by name.

src/tui/stream.test.ts

@@ -11,6 +11,7 @@ import {
   streamRowGutter,
   toolRowLines,
   toolSentenceLines,
+  transcriptSyntaxStyle,
   type RowLayout,
   type StreamRow,
 } from "./stream";
@@ -533,3 +534,15 @@ describe("sub-agent dispatch row marks", () => {
     expect(streamRowGutter(merged, SOLO).content).toContain("✓");
   });
 });
+
+// TUI markdown links are click-only: there is no hover tracking, so no hover
+// state may add an affordance the idle render does not have. Pin
+// underline-absence on the link scopes so a future hover style cannot sneak
+// one in (CL-7927).
+describe("transcriptSyntaxStyle markdown links", () => {
+  test("link cells carry no underline", () => {
+    const styles = transcriptSyntaxStyle().getAllStyles();
+    expect(styles.get("markup.link")?.underline).not.toBe(true);
+    expect(styles.get("markup.link.url")?.underline).not.toBe(true);
+  });
+});

src/tui/stream.ts

@@ -953,9 +953,9 @@ const MARKDOWN_STYLES = {
   "markup.raw": { fg: UI.inFlight },
   "markup.list": { fg: UI.inFlightBright },
   "markup.quote": { fg: UI.textDim, italic: true },
-  "markup.link": { fg: UI.inFlightBright, underline: true },
+  "markup.link": { fg: UI.inFlightBright },
   "markup.link.label": { fg: UI.inFlightBright },
-  "markup.link.url": { fg: UI.inFlightBright, underline: true },
+  "markup.link.url": { fg: UI.inFlightBright },
   keyword: { fg: UI.inFlightBright },
   string: { fg: UI.done },
   number: { fg: UI.done },

tests/unit/telemetry-product-events.test.ts

@@ -245,9 +245,9 @@ test("first-party skill names are reported by name; everything else stays custom
   expect(classifySkillName("acme-internal-deploy")).toBe("custom");
   // Bundled catalog skills outside the closed allowlist are not reported by
   // name either — the allowlist is the closed set, not the skills directory.
-  // All four bake-only background skills (user-invocable: false, baked into
-  // agent prompts rather than invoked as skills) stay custom, while the
-  // seven flagged-but-allowlisted use_skill-only recipes assert by name above.
+  // All four hidden-from-discovery background skills (explicit loads still
+  // resolve) stay custom, while the seven flagged-but-allowlisted
+  // use_skill-only recipes assert by name above.
   expect(classifySkillName("idiot-proof")).toBe("custom");
   expect(classifySkillName("native-integration")).toBe("custom");
   expect(classifySkillName("native-runtime")).toBe("custom");