diff --git a/docs/hardening/review-R2b.md b/docs/hardening/review-R2b.md new file mode 100644 index 0000000..d79b954 --- /dev/null +++ b/docs/hardening/review-R2b.md @@ -0,0 +1,281 @@ +--- +type: cold-review +batch: R2b +journey: "Create — orchestrators & front-door" +scope: "FROZEN committed files at HEAD 5474df5 (clean tree; post-hardening cold pass)" +method: "2 independent cold Opus reviewers per surface (intent + correctness), no cross-feed; every mechanical claim tool-grounded (anti-fabrication mandate); reviewers carry NO drafting-session context" +surfaces: [create, batch, pipeline, newsletter] +reviewers: + - "intent-lens (conformance: intent delivery + cross-ref resolution + class predicates + terminology)" + - "correctness-lens (internal consistency + bound-vs-canonical + checklist arithmetic + structure)" +status: "COMPLETE — all 4 surfaces reviewed (create, batch, pipeline, newsletter)" +verdict: REWORK +counts: { BLOCKER: 0, MAJOR: 3, MINOR: 10, SUGGESTION: 6, systemic_patterns: 2 } +--- + +# Cold review — R2b (Create · orchestrators & front-door) + +Independent post-hoc cold review of the 4 Create-journey orchestrators/front-door, on the FROZEN +committed files (HEAD `5474df5`). Mirrors the S1 `review.md` + R2a model (the cold-review method that +did **not** fabricate): read-and-show before assert, every `file:line` tool-confirmed, reviewers carry +no drafting-session context. The per-command interactive gate (`log.md`) already passed these; this +pass adds the **independent** axis that gate never had. + +**Independence cross-check — two outcomes worth noting this batch:** +1. **Convergence** (the R2a pattern): both blind lenses independently surfaced the same real defect on + several surfaces — batch's bare-vs-prefixed reference path, batch's 3a/3b component-band tension, + newsletter's `allowed-tools` omission, create's 8-option `AskUserQuestion`. +2. **Divergence resolved by grounding** (new this batch, the strongest argument FOR the two-lens + method): on `newsletter` the intent-lens **asserted** the resumption table "maps every currentPhase + to the correct resume step"; the correctness-lens **counted** the rows (17) against the canonical + `_doc.phases` (18) and found the `contract-gate` row missing. Main re-grounded it independently + (below) → the correctness-lens is right. The independence axis caught a real MAJOR that one lens had + asserted away. + +--- + +## ★ Cross-cutting finding #1 (systemic) — 5-component draft scaffold (960–1,640) cannot satisfy the 1,200–1,800 standard band it is gated against + +- **Pattern (verified by main, independent of reviewers — `grep` blast radius):** the identical + 5-component "standard post" breakdown — `Hook 110-140` + `Context 200-300` + `Insight 400-800` + + `Implication 200-300` + `CTA 50-100` — appears in **exactly 3 files**: `post.md:100-104`, + `batch.md:88-92`, `pipeline.md:58-62`. (`grep -rln "Insight.*400" commands/` → those three only; + `quick`/`first-post`/`react` carry the lone `Hook 110-140` line, NOT the full scaffold, so are + unaffected.) +- **Grounded arithmetic:** component **min-sum = 110+200+400+200+50 = 960** · **max-sum = + 140+300+800+300+100 = 1,640**. Each file then gates the SAME post against the canonical standard band + **1,200–1,800** (`content-quality-gate.md:17`; `post.md:128`, `batch.md:95`, `pipeline.md:77`). +- **Mechanism (two-sided):** (a) a draft built at the component minimums is **960 chars — 240 below** + the 1,200 floor enforced by the next step AND the live PreToolUse `content-quality-gate` hook; + (b) the component maximums sum to **1,640 — 160 below** the 1,800 ceiling, so the scaffold literally + cannot reach the upper half of its own target band. +- **Why it matters:** on the most common path (drafting a standard text post) the drafting recipe and + the acceptance test are mutually incompatible, with no transition/connective slack to close the + 960→1,200 gap. In `batch` the defect is sharper: the 3a scaffold is **format-blind** (always the + 5-component standard layout) while the 3b gate is **format-aware** (`batch.md:114` `format: + [text/carousel/video]`, rotation `:76`), so a `quick`-format post built from the scaffold (960–1,640) + also blows the quick band (150–500). In `pipeline` the contradiction is between two **adjacent** + steps (Step 2 draft → Step 3 scorecard). +- **Severity:** recorded **MAJOR** in `batch` and `pipeline` (real adjacent-step / gate contradiction + on every standard post). **Reconciliation note for R2a:** R2a recorded the same scaffold in `post.md` + at **MINOR** ("component minimums sum below the 1,200 optimal floor") and flagged only the min-side. + The batch/pipeline analysis shows it is a two-sided, gate-hitting contradiction, not merely loose + guidance — the post.md instance is arguably under-rated and should be reconciled in the same fix. +- **Disposition (NOT fixed here — review finds, operator decides; 3-file change = own go):** one + consolidated fix across all 3 files — raise the component floors so min-sum ≥ 1,200 (e.g. widen + Context/Insight) AND either lift the ceiling or accept ~1,640, AND scope the standard 5-component + scaffold explicitly to standard-format posts where the command is format-aware (`batch`), adding + per-format draft guidance for quick/carousel/video. **Recommend treating this as the second + consolidated fix out of the sweep, after the R2a clipboard fix.** + +## ★ Cross-cutting finding #2 (recurring) — bare relative reference paths vs `${CLAUDE_PLUGIN_ROOT}/` + +- **Pattern:** several `Read`/reference paths are written **bare** (resolved against the runtime cwd) + while the same file's appendix and most other sites prefix `${CLAUDE_PLUGIN_ROOT}/`. `batch.md:43` + (`references/content-angles.md`) vs `batch.md:206` (prefixed, same file) — flagged by **both** batch + lenses. `pipeline.md:31,55,70,75` bare vs `pipeline.md:28,64,107` + appendix `:204-209` prefixed — + `content-angles.md` is bare at `:55` but prefixed at `:204`. +- **Impact:** the files exist (not dead refs), but a bare path fails the `Read` when cwd ≠ plugin root, + on real paths run every invocation (angle-select, optimize). Self-recoverable via Glob, latent. +- **Disposition:** normalize all bare reference/asset paths to `${CLAUDE_PLUGIN_ROOT}/`. Cheap, + mechanical; fold into the consolidated fix pass. (Worth a repo-wide grep for the same pattern in the + other 25 commands during the eventual fix.) + +--- + +## create.md — VERDICT: ALLOW (0 BLOCKER/MAJOR · 1 MINOR · 0 SUGGESTION) + +Class: **routing** (pure delegating front-door). Both lenses confirm intent delivered: Step 0 context → +Step 1 intent-ID → Step 2 route, with explicit "you do not draft here / do NOT inline the target's +steps" (`create.md:49-51,64-65`) — delegation purity intact, single source of truth preserved. All 8 +routed targets resolve (`post/quick/react/carousel/video/multiplatform/batch/newsletter`, `:37-44` + +`:55-62`, `ls`-confirmed); the three enumerations (description `:7`, menu `:37-44`, route table +`:55-62`) are mutually consistent (8/8/8, same order); no `subagent_type` refs (correct — it routes to +commands); no "thought leadership". Correctness-lens: **0 findings**. + +### MINOR — Step 1 directs one `AskUserQuestion` carrying 8 options; documented support is 2–4 +- `create.md:35-44` — "use `AskUserQuestion`" immediately followed by 8 numbered options (`grep -cE + "^[0-9]+\. \*\*"` → 8). Grounded against the plugin-dev reference + `command-development/.../interactive-commands.md:469` ("2-4 options per question") + `:906`. +- **impact:** on the PRIMARY interactive path (user names no format) the front-door instructs a single + question with double the documented option range. +- **anti-fabrication caveat (carried from the reviewer, honestly):** grounded = (a) 8 options + instructed, (b) the documented 2–4 range. NOT grounded = whether the live `AskUserQuestion` runtime + hard-rejects >4 vs silently truncates/degrades. **If the runtime hard-rejects, this escalates to + MAJOR/BLOCKER** on the no-format-named path; if it only degrades, the picker is over-long. Worth a + runtime check before the fix. +- **disposition:** group the 8 intents into ≤4 options (e.g. Short-form / Reaction / Visual / Long-form + & batch) with a drill-down, or split into two questions. + +--- + +## batch.md — VERDICT: REWORK (0 BLOCKER · 1 MAJOR · 3 MINOR · 2 SUGGESTION) + +Class: **guided/stateful + routing** ("create a full week of content"). Intent delivered: the Step 0→5 +flow traces the frontmatter promise; all cross-refs resolve (`trend-spotter`, `content-planner` → +`agents/`; `/linkedin:calendar` → `commands/`; `queue-manager.mjs` + `ical-generator.mjs` exports + +the 8-arg `queueAdd` call/signature match; all 6 reference/asset paths + `SKILL.md`); graceful +degradation present; no "thought leadership". + +### MAJOR — Step 3a component scaffold contradicts the Step 3b band gate → see ★ cross-cutting #1 (`batch.md:88-92` vs `:95`) +Sharper here than elsewhere: 3a is **format-blind** (always the 5-component standard layout, 960–1,640) +while 3b is **format-aware** (`:114` `format:[text/carousel/video]`, rotation `:76`), so a `quick`-format +post built from 3a also blows the quick band 150–500. **Both lenses flagged this** (intent-lens MINOR, +correctness-lens MAJOR — recorded at the higher severity: it hits a real gate on every standard post). + +### MINOR — bare reference path → see ★ cross-cutting #2 (`batch.md:43` vs `:206`). Both lenses. + +### MINOR — `weekly_goal` cadence decoupled from the fixed "3–5 posts" headline +- `batch.md:5,65` fix the output at "3–5 posts"; `batch.md:52` schedules against `weekly_goal` slot + templates (2x/3x/4x/5x). At `weekly_goal=2x`, 3–5 posts against 2 weekly slots overflow into the next + week (`:53` "next available slot after today") — ~2.5 weeks of content under a "full week" label. +- **impact:** non-breaking (scheduling rolls forward), but "full week" + "3–5" is internally + inconsistent with the 2x cadence. +- **disposition:** tie post count to `weekly_goal`, or note that overflow rolls into following weeks. + +### MINOR — orphan sub-step marker `5b` with no `5a` +- `batch.md:172` `### 5b. Generate Calendar File`; `grep -n "5a" batch.md` → no match (exit 1). Step 3 + has 3a/3b/3c/3d; Step 5 jumps straight to 5b. +- **disposition:** renumber to `5a`, or drop the letter. + +### SUGGESTION — `weekly_goal` default (3x) lives only in the referenced `scheduling-strategy.md:15`, never stated in `batch.md`. Optional one-line "default 3x" for self-evident degradation. + +### SUGGESTION — `planned_date` metadata never computed +- `batch.md:109` writes `planned_date: YYYY-MM-DD` into each draft header, but Step 2 only computes + `scheduled_date`/`scheduled_time` (`:54`); `planned_date` is introduced nowhere upstream. +- **disposition:** drop `planned_date`, or define where it is derived. + +--- + +## pipeline.md — VERDICT: REWORK (0 BLOCKER · 1 MAJOR · 4 MINOR · 2 SUGGESTION + clipboard pointer) + +Class: **post-emitting + guided/stateful + routing** ("full end-to-end pipeline"). Intent delivered: +Steps 0–8 map to every named stage. All post-emitting predicates present (hook 110–140 `:58,:76` · +length band `:77` · no-body-link `:78` · buzzword check `:79` · topic→expertise `:49,:55,:80`); all +cross-refs + function signatures resolve (`content-planner`, `trend-spotter`; `/linkedin:calendar`, +`/linkedin:analyze`; `queueAdd` 8-arg call/signature; `writeState`/`updatePostTracking`); no "thought +leadership". + +### MAJOR — Step 2 component scaffold cannot satisfy the Step 3 total-length gate → see ★ cross-cutting #1 (`pipeline.md:58-62` vs `:77`) +Adjacent-step contradiction: Step 2 partitions into 960–1,640; the very next step's scorecard asserts +"Total 1,200–1,800". Correctness-lens, grounded arithmetic. + +### MINOR — inline buzzword checklist enumerates 8, canonical gate enumerates 10 +- `pipeline.md:79` lists 8 terms (= CLAUDE.md rule 4); SSOT `content-quality-gate.md:13` adds + 'actionable insights' + 'best practices' = 10. A draft passing the inline list can still trip the + Write hook. (Gate-vs-rule divergence, not unique to this file — also noted on carousel in R2a.) +- **disposition:** align to the 10-term canonical list, or reference the gate instead of duplicating. + +### MINOR — bare reference paths → see ★ cross-cutting #2 (`pipeline.md:31,55,70,75`). Intent-lens. + +### MINOR — Step 4 deferred/queued path falls through into the immediate Publish steps +- `pipeline.md:97-101` offers "Schedule / Add to queue / Save as draft (no schedule)"; Steps 5–8 + (`:120` "15-20 min BEFORE posting", `:135` Publish, `:158` first-hour, `:171` post-analysis) then run + with **no branch**. A user who queued/deferred is marched through Pre-Engagement → Publish → + Monitoring, contradicting the just-made defer decision. +- **disposition:** add an early-exit after Step 4 for options 2–4 ("if scheduled/queued, end here; Steps + 5–8 run at publish time"). + +### MINOR — Step 7 inlines a static first-hour checklist instead of routing to the stateful surface +- `pipeline.md:158-169` inlines a 5-item plan; the dedicated `/linkedin:firsthour` delegates to + `engagement-coach`, persists via `recordFirstHourPlan`, hands off to `post-feedback-monitor` — strictly + richer (Step 8 already routes to `/linkedin:analyze`, so the inline first-hour is the inconsistent one). +- **disposition:** route to `/linkedin:firsthour`. + +### SUGGESTION — over-provisioned `allowed-tools`: `:13` declares `WebFetch` but no body step fetches (trend-spotter does its own). Drop unless a URL-ingest step is intended. + +### SUGGESTION — hardcoded Norwegian peak times: `:92-95` bakes "European/Norwegian audience" peak windows into the body while `scheduling-strategy.md` (read at `:107`) is the SSOT for slots; conflicts with the domain/audience-general principle. Source from the reference/config. + +### (pointer) clipboard `printf '%s'` systemic bug — `pipeline.md:139` confirmed present (the only R2b surface in the 10-content-command set). Folds into the R2a ★ cross-cutting clipboard finding; no new derivation. + +--- + +## newsletter.md — VERDICT: REWORK (0 BLOCKER · 1 MAJOR · 2 MINOR · 2 SUGGESTION) + +Class: **guided/stateful + routing + heavy subagent orchestration** (long-form 18-phase pipeline, +~110 KB). Intent delivered: all 18 phases present, ordered, `[GATE]`/`[OPERATØR]`-marked. **18-phase +count confirmed by both lenses** (`0,1,1.5,2,2.5,3a,3b,4,4.5,5,5.5,6,6.5,7,7.5,8,9,10`; headline `:25` +matches body + template `_doc.phases` + build-status). All 7 longform agents (fact-checker, +editorial-reviewer, persona-reviewer, voice-scrubber, content-reviewer, language-reviewer, fact-reviewer) +present in `agents/` AND invoked; gate sequence ordered before lock (`:1570`): skeleton 2.5 → spine 3a → +fact-check 5 → editorial 5.5 → persona 6 → headless 6.5 → visual 7.5 → LOCK 8 → hook 9. All `subagent_type` +carry the `linkedin-studio:` namespace (the 5 prefix-less grep hits are line-wraps). All ~25 cross-refs +(agents, commands, scripts, configs, render, docs) resolve. Pivot heuristic, flag caps, step-label +5.5/6.5 consistency all clean. No "thought leadership". + +### MAJOR — deterministic resumption table omits the contract-gate phase (Step 4.5) → breaks resume between Step 4 and Step 5 +- **Verified by main (independent re-grounding of a lens disagreement):** the resumption table + `newsletter.md:209-228` has **no `contract-gate` row** (`grep contract-gate` over the table region → + none). The canonical `_doc.phases` it claims to mirror (`:230-231`) **does** define it — + `config/edition-state.template.json` lists `"contract-gate — … (Step 4.5)"` between + `consistency-quality` (Step 4) and `factcheck-sweep` (Step 5). Step 4.5 actually writes it: + `newsletter.md:988` "Set `currentPhase: "contract-gate"`". +- **Two concrete breakages on the multi-session resume path (the file's core premise, `:200-204`):** + 1. **Gate skipped on resume.** The rule (`:203-204`) is "run the step AFTER the recorded phase." Row + `:219` maps `consistency-quality → Step 5` (Fact-check), but the step after Step 4 is Step 4.5 + (contract-gate), not Step 5. A session aborting after Step 4 resumes **past** the deterministic + contract-gate, never running it. + 2. **Unrecognized phase on resume.** A session aborting after Step 4.5 has `currentPhase: + "contract-gate"`, absent from the table → falls into the `:232-234` fallback ("missing or + unrecognized → do NOT guess … confirm with the operator"), defeating the deterministic-resumption + guarantee the section is built on. +- **Note:** the linear next-pointers are correct (`:918` "next: contract-gate", `:989` "next: + fact-check"); only the resume **table** is short one row — the defect surfaces solely on abort/resume + between Step 4 and Step 5. +- **Independence note:** the intent-lens asserted this table "maps every currentPhase to the correct + resume step"; the correctness-lens counted (17 rows vs 18 phases) and found the gap. Main confirmed + the correctness-lens. Two-lens method earned its keep here. +- **disposition:** insert a `contract-gate → Step 5 — Fact-check sweep` row, and repoint + `consistency-quality → Step 4.5 — Contract-gate`. + +### MINOR — Step 1 says the brief is first persisted "in Step 2"; the rest of the file says Step 1.5 +- `newsletter.md:303` "Record the resolved brief inline (you will persist it to edition-state in **Step + 2**)" contradicts `:287-289`, `:412-418`, `:494-496` (all: first durable write is the **Step 1.5** + checkpoint). Stale "Step 2" — almost certainly predates the Fix #2 Step 1.5 insertion; non-breaking + (Step 1 only records inline either way). +- **disposition:** change `:303` "in Step 2" → "at the Step 1.5 checkpoint". + +### MINOR — `allowed-tools` omits `SendUserFile` (body-primary operator gate) + `mcp__mcp-image__generate_image` (default image route) +- `newsletter.md:11-19` declares `Read, Glob, Grep, WebFetch, Bash, AskUserQuestion, Task, Write`. The + body names `SendUserFile` as the **primary** operator gate at Steps 5.5/6.5/7.5 (13 uses, e.g. `:1138`) + and mcp-image as the **default** image route (`:1471`); neither is declared. **Both lenses flagged + this** (intent-lens MINOR, correctness-lens SUGGESTION — recorded at the higher: the declared "default" + path can't execute under the frontmatter as written). Every use guards with a fallback ("`SendUserFile` + if available, else a markdown `file://` link"), so it degrades gracefully → not load-bearing. +- **disposition:** add `SendUserFile` (+ optionally mcp-image) to `allowed-tools`, or downgrade the body + wording from "default/primary" to "if permitted". + +### SUGGESTION — undefined "LTL plugin" acronym: `newsletter.md:36,725` ("the LTL plugin" / "the LTL rule"); repo-wide the bare phrase appears only here, no expansion; the plugin is canonically "LinkedIn Studio". (The env vars `LTL_SERIES_ROOT`/`LTL_BRAND` `:48,154-156` ARE a legit convention consumed by `render/build-*.mjs` — not a defect.) Rename to "LinkedIn Studio plugin", or define once. + +### SUGGESTION — "leveraged" in doc prose (`newsletter.md:1785`, note-only): ordinary verb in the command's own explanatory prose, not generated post content; CLAUDE.md rule 4 targets generated posts. Not a real violation; optionally swap to "drew on / built on". + +--- + +## Gate decision — R2b COMPLETE (4 surfaces) + +| Surface | Verdict | BLOCKER | MAJOR | MINOR | SUGGESTION | +|---|---|---|---|---|---| +| create | ALLOW | 0 | 0 | 1 | 0 | +| batch | REWORK | 0 | 1 | 3 | 2 | +| pipeline | REWORK | 0 | 1 | 4 | 2 | +| newsletter | REWORK | 0 | 1 | 2 | 2 | +| **★ cross-cutting #1** (scaffold, 3 files) | — | 0 | (counted in batch + pipeline; spans post.md from R2a) | — | — | +| **★ cross-cutting #2** (bare paths) | — | 0 | 0 | (counted in batch + pipeline) | — | + +**Batch verdict: REWORK** — 3 of 4 surfaces (batch, pipeline, newsletter), each with **1 MAJOR**: +(1) the systemic 5-component scaffold contradicting the 1,200–1,800 band (batch + pipeline; spans +post.md from R2a), and (2) newsletter's resumption table missing the contract-gate phase. **0 BLOCKER +anywhere.** `create` is ALLOW (one option-count MINOR with a runtime caveat). + +**Independence verdict:** convergence on batch path-prefix / batch 3a-3b band / newsletter allowed-tools +/ create 8-option (both lenses) — plus one **divergence resolved by main's grounding** (newsletter +resumption table: intent-lens asserted complete, correctness-lens counted the gap, main confirmed). Both +the convergence and the resolved divergence are signals these are real defects, not single-reviewer +framing. + +**Systemic findings now span R2a+R2b:** clipboard `printf` (R2a, 10 files) · component scaffold (R2b, 3 +files incl. post.md from R2a) · bare reference paths (R2b, 2 files, worth a repo-wide grep). Cold review +**finds**; it changes no code. Each fix is its own operator-gated decision. Recommended consolidated-fix +order out of the sweep so far: (1) clipboard `printf` [R2a, 10 files, highest blast radius], (2) +component scaffold [3 files], (3) bare reference paths [grep-driven], then the per-surface items. +Local-only (hardening-class), not pushed.