docs(linkedin-studio): cold-review R3 (Engage surfaces) — independent kald-review of 4 surfaces
Reproduces the S1/R2a/R2b non-fabricating method: 2 blind cold Opus lenses for the round (intent + correctness), each covering all 4 surfaces, no cross-feed, every mechanical claim tool-grounded. Surfaces: firsthour, calendar, headless-review, pivot. Verdict REWORK — 1 of 4 surfaces, 1 MAJOR; 0 BLOCKER: - calendar: the queue load (queueFormatSummary) surfaces none of the id/draft_path/character_count that publish/reschedule/cancel require, and the reschedule step's "carry from the entry shown in Step 2" is a direct contradiction (those fields are never shown) -> MAJOR - firsthour: ALLOW (1 MINOR bare ref paths; clipboard printf pointer) - headless-review: ALLOW (2 MINOR: SendUserFile absent from allowed-tools on the primary surfacing path; dead v3.1.0 reload anchor post version-reset) - pivot: ALLOW (clean — heuristic, worked example, off-by-one phase map all reconcile) Independence axis earned its keep again: convergence on headless SendUserFile (both lenses) + one divergence resolved by main grounding (calendar - intent-lens flagged the data-gap MAJOR, correctness-lens passed it on structural arithmetic; main grounded queueFormatSummary's output and confirmed the MAJOR, same shape as R2b's newsletter resumption table). Connects to existing systemic findings (no new cross-cutting): clipboard printf (firsthour confirmed, folds into R2a's 10-file finding) and bare reference paths (firsthour adds 3 sites to R2b's pattern). New SUGGESTION-class pattern: allowed-tools over-declaration on 3 of 4 surfaces. Counts: 0 BLOCKER, 1 MAJOR, 3 MINOR, 5 SUGGESTION. Cumulative cold-review coverage 17/29. Review finds; changes no code. test-runner 138/0 unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GqEHp4uDiivfrAUjw4BdE
This commit is contained in:
parent
2b706609bd
commit
69f37ba2b3
1 changed files with 218 additions and 0 deletions
218
docs/hardening/review-R3.md
Normal file
218
docs/hardening/review-R3.md
Normal file
|
|
@ -0,0 +1,218 @@
|
|||
---
|
||||
type: cold-review
|
||||
batch: R3
|
||||
journey: "Engage — post-publish & longform-support surfaces"
|
||||
scope: "FROZEN committed files at HEAD 2b70660 (clean tree; post-hardening cold pass)"
|
||||
method: "2 independent cold Opus reviewers for the round (intent + correctness), each covering all 4 surfaces, no cross-feed; every mechanical claim tool-grounded (anti-fabrication mandate); reviewers carry NO drafting-session context"
|
||||
surfaces: [firsthour, calendar, headless-review, pivot]
|
||||
reviewers:
|
||||
- "intent-lens (conformance: intent delivery + cross-ref resolution + class predicates + graceful degradation + terminology)"
|
||||
- "correctness-lens (internal consistency + bound-vs-canonical + checklist/phase arithmetic + allowed-tools completeness)"
|
||||
status: "COMPLETE — all 4 surfaces reviewed (firsthour, calendar, headless-review, pivot)"
|
||||
verdict: REWORK
|
||||
counts: { BLOCKER: 0, MAJOR: 1, MINOR: 3, SUGGESTION: 5 }
|
||||
---
|
||||
|
||||
# Cold review — R3 (Engage · post-publish & longform-support)
|
||||
|
||||
Independent post-hoc cold review of the 4 Engage-journey surfaces, on the FROZEN committed files
|
||||
(HEAD `2b70660`). Mirrors the S1 `review.md` + R2a + R2b 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 — both outcomes recurred this batch:**
|
||||
1. **Convergence:** both blind lenses independently surfaced the same real defect on `headless-review`
|
||||
(`SendUserFile` invoked on the primary surfacing path but absent from `allowed-tools`).
|
||||
2. **Divergence resolved by grounding** (the strongest argument FOR the two-lens method, recurring from
|
||||
R2b's newsletter): on `calendar` the intent-lens flagged a **MAJOR** (the publish/reschedule/cancel
|
||||
actions key off `id`/`draft_path`/`character_count` that the queue load never surfaces), while the
|
||||
correctness-lens passed the surface as ALLOW — its structural pass found the step/option arithmetic
|
||||
reconciled but did **not** trace the data-flow from load → display → action placeholders. Main
|
||||
re-grounded `queueFormatSummary`'s actual output independently (below) → the intent-lens is right. The
|
||||
independence axis caught a real MAJOR one lens never probed.
|
||||
|
||||
---
|
||||
|
||||
## Connections to existing systemic findings (no NEW ★ cross-cutting this batch)
|
||||
|
||||
R3 surfaces **connect to** the two systemic patterns already recorded in R2a/R2b rather than adding new
|
||||
ones. Both connections were re-grounded by main on the R3 files:
|
||||
|
||||
- **★ cross-cutting #1 (clipboard `printf '%s'`, R2a, 10 files) — firsthour confirmed present.**
|
||||
`firsthour.md:72` `printf '%s' '<DRAFT_COMMENTS_BLOCK>' | node …/clipboard-helper.mjs` + `:75` the
|
||||
unconditional "Copied your draft comments to clipboard." This is the exact systemic pattern: a
|
||||
single-quoted shell string corrupts any draft text containing an apostrophe (`it's`, `don't` — common
|
||||
in natural comment copy), and the "Copied" confirmation is unconditional. firsthour is one of the 10
|
||||
files STATE already lists; **no new derivation — folds into the R2a ★ #1 consolidated fix.**
|
||||
- **★ cross-cutting #2 (bare relative reference paths vs `${CLAUDE_PLUGIN_ROOT}/`, R2b) — firsthour adds
|
||||
3 sites.** `firsthour.md:110` (prose parenthetical), `:118`, `:119` (Reference-Files pointer list) are
|
||||
bare `references/…` while the same file's **executable** blocks correctly prefix `${CLAUDE_PLUGIN_ROOT}/`
|
||||
(`:72`, `:84`) and sibling commands prefix their Reference-Files lists too (`calendar.md:206-207`).
|
||||
**Lower impact than the R2b instances** (firsthour's bare paths are in a pointer list + one prose
|
||||
mention, not inside an executable `Read`), so latent rather than active — but a real parity break worth
|
||||
catching in the same repo-wide grep pass. Counted as a per-surface MINOR below.
|
||||
|
||||
**Recurring (SUGGESTION-class, NOT elevated to ★) — `allowed-tools` over-declaration.** Three of the four
|
||||
surfaces declare a tool the body never invokes: `firsthour` (`Glob`/`Grep`), `calendar` (`Write`/`Edit`),
|
||||
`pivot` (`Grep`). Harmless (over-declaration widens permission surface but breaks nothing;
|
||||
*under*-declaration would be the real risk and there is none). Noted per-surface; optional minimal-surface
|
||||
trim, fold into the consolidated fix if touched.
|
||||
|
||||
---
|
||||
|
||||
## firsthour.md — VERDICT: ALLOW (0 BLOCKER/MAJOR · 1 MINOR · 2 SUGGESTION + clipboard pointer)
|
||||
|
||||
Class: **guided/stateful + subagent orchestration** ("post-publish first-hour / reply-loop sprint").
|
||||
Intent delivered: Step 0 load → Step 1 identify post → Step 2 delegate to `engagement-coach` → Step 3
|
||||
present (timeline / targets / drafts / velocity) → Step 4 `recordFirstHourPlan` persist → Step 5
|
||||
`post-feedback-monitor` handoff. Both subagent targets carry the `linkedin-studio:` namespace and resolve
|
||||
(`agents/engagement-coach.md`, `agents/post-feedback-monitor.md`); `recordFirstHourPlan` signature
|
||||
(`planDate, postTopic, targets, draftComments, plan`) matches the call (`:85-91` vs `state-updater.mjs:235`);
|
||||
6 steps (`grep -cE '^## Step'` = 6), sequential, no orphan markers; Step 2→3 value-flow reconciles (coach
|
||||
asked for target-list / self-comments / timeline / velocity, Step 3 presents exactly those four); no
|
||||
"thought leadership". Empty-state degradation present.
|
||||
|
||||
### MINOR — bare reference paths → see Connections (★ #2) (`firsthour.md:110, :118, :119` vs prefixed `:72, :84`). Intent-lens.
|
||||
|
||||
### SUGGESTION — Step 0 voice-samples read has no stated fallback
|
||||
`firsthour.md:29` reads voice-samples "so every draft comment is in the user's voice," but no path is
|
||||
specified when the file is absent (progressive onboarding suppresses voice until 5+ samples, CLAUDE.md rule
|
||||
7). Non-breaking (the coach can still draft), but the degradation is unstated. Add "if absent, draft in a
|
||||
neutral first-person register and skip voice-matching."
|
||||
|
||||
### SUGGESTION — `allowed-tools` over-declares `Glob`/`Grep` (`:12-13`); body invokes neither. Correctness-lens. See Recurring note.
|
||||
|
||||
### (pointer) clipboard `printf '%s'` systemic bug — `firsthour.md:72, :75` confirmed present → folds into ★ #1 (R2a). No new derivation.
|
||||
|
||||
---
|
||||
|
||||
## calendar.md — VERDICT: REWORK (0 BLOCKER · 1 MAJOR · 0 MINOR · 1 SUGGESTION)
|
||||
|
||||
Class: **guided/stateful + routing** ("view/manage scheduling queue + publish action"). The **view** side
|
||||
(14-day calendar, format mix, pillar balance) delivers; the **action** side has a load-bearing data gap.
|
||||
Step/sub-step/option arithmetic all reconcile (correctness-lens: Steps 1–4 sequential; sub-markers 3a–3f
|
||||
present and ordered; 5 options offered with 4 handlers + explicit no-op, no dangling branch; Quick-Routing
|
||||
anchor `:89` exists); empty/missing-queue degradation present and correct (`queue-manager.mjs:12-27`
|
||||
returns `[]`; body 3a routes "no posts" → `/linkedin:batch`/`quick`); no "thought leadership".
|
||||
|
||||
### MAJOR — the queue load surfaces none of the `id`/`draft_path`/`character_count` the publish/reschedule/cancel actions require (`calendar.md:31-43, :117, :169-174, :185` vs `queue-manager.mjs:112-122`)
|
||||
- **Verified by main (independent re-grounding of the lens divergence):** Step 1 loads the queue
|
||||
**exclusively** through `queueFormatSummary` (`:31-43`). `queueFormatSummary` (`queue-manager.mjs:112-122`,
|
||||
read in full) emits only ` {date} {time} | {hook…} | {pillar} ({fmt}) [{status}]` — it exposes **no**
|
||||
`id`, **no** `draft_path`, **no** `character_count`. The Step 2 display (`:52-71`) mirrors that field set.
|
||||
- **Three concrete breakages on the action paths:**
|
||||
1. **Mark-as-published (the PRIMARY route — Quick-Routing `:25` jumps straight here)** calls
|
||||
`queueUpdateStatus('[post-id]', 'published')` (`:117`) — `[post-id]` was never surfaced. Step 3d
|
||||
also needs `charCount: NNNN` (`:129`), likewise un-surfaced.
|
||||
2. **Reschedule** (`:174`) calls `queueAdd('[post-id]','[draft_path]', …, [charCount])` (8-arg signature
|
||||
confirmed `queue-manager.mjs:63`) and is **explicitly told** to "carry the unchanged fields
|
||||
(draft_path, pillar, format, hook preview, char count) from **the entry shown in Step 2**"
|
||||
(`:169-172`) — but Step 2 provably shows none of `draft_path`/`char count`/`id`. A direct
|
||||
contradiction: the instruction points at a view that lacks the fields it says to carry.
|
||||
3. **Cancel** (`:185`) likewise needs the un-surfaced `[post-id]`.
|
||||
- **Self-recovery caveat (honest):** `queue.json` is in Reference Files (`:208`) and `Read` is allowed, so
|
||||
a capable agent *could* read raw entries to recover `id`/`draft_path`/`char_count`. But the body never
|
||||
instructs that, and the reschedule text actively **mis-directs** to Step 2. Latent-but-real on the
|
||||
primary route → MAJOR, not MINOR.
|
||||
- **Independence note:** intent-lens flagged MAJOR; correctness-lens passed the surface ALLOW (its
|
||||
arithmetic/structure pass reconciled but did not trace load→display→action data-flow). Main grounded
|
||||
`queueFormatSummary`'s output → intent-lens confirmed. Two-lens method earned its keep (same shape as
|
||||
R2b's newsletter resumption table).
|
||||
- **disposition:** in Step 1 also dump raw entries (e.g. `console.log(JSON.stringify(queueUpcoming(14)))`,
|
||||
or a `queueRead()` dump exposing `id`/`draft_path`/`character_count`), and re-point the reschedule text
|
||||
from "the entry shown in Step 2" to "the raw queue entry loaded in Step 1." Surface the display ordinal →
|
||||
queue-`id` mapping so 3b/reschedule/cancel can fill `[post-id]`.
|
||||
|
||||
### SUGGESTION — `allowed-tools` over-declares `Write`/`Edit` (`:13-14`); every mutation routes through `Bash` node one-liners, "View draft" uses `Read`. Correctness-lens. See Recurring note.
|
||||
|
||||
---
|
||||
|
||||
## headless-review.md — VERDICT: ALLOW (0 BLOCKER/MAJOR · 2 MINOR · 1 SUGGESTION)
|
||||
|
||||
Class: **guided/stateful + heavy subagent orchestration + routing** (cold 5-archetype package on a frozen
|
||||
draft → one operator-gated report). Intent delivered: Step 1 resolve-from-disk → Step 2 freeze (`cp`
|
||||
snapshot) → Step 3 parallel fan-out (the `--type`→`subagent_type` table `:141-145` maps to the 5 real cold
|
||||
review modes: content / language / fact / persona-resonance / persona-conversion) → Step 4 consolidate →
|
||||
Step 5 surface + optional `edition-state.json` persist. All reviewer agents resolve; the writing-contract
|
||||
fallback chain terminates in `references/longform-quality-rules.md` (present); degradation well-handled
|
||||
(missing `--draft` → edition-state or ask; `cp` unavailable → live draft + note; degraded reviewer
|
||||
re-runs). "five archetypes" reconciles with the 5-row `--type` table; 5 flags all consumed, no orphan; no
|
||||
"thought leadership".
|
||||
|
||||
### MINOR — `SendUserFile` invoked on the primary surfacing path but absent from `allowed-tools` (`:208, :221` vs `:19-25`). BOTH lenses.
|
||||
- `allowed-tools` (`:19-25`) = Read, Glob, Grep, Bash, AskUserQuestion, Task, Write — no `SendUserFile`;
|
||||
body uses it 2× (`grep -c` = 2), as the documented **primary** operator-gated delivery ("operator-gated
|
||||
via SendUserFile"). Held at MINOR (not MAJOR) by two guards: `:208` carries an in-text fallback ("else a
|
||||
markdown `file://` link") and the report is independently persisted via the declared `Write` (`:206`), so
|
||||
surfacing degrades rather than breaks.
|
||||
- **disposition:** add `SendUserFile` to `allowed-tools` (if a real tool in the target harness), or soften
|
||||
the body wording from "primary/operator-gated via SendUserFile" to "surface via a `file://` link (or
|
||||
`SendUserFile` if available)."
|
||||
|
||||
### MINOR — `v3.1.0` reload anchor misleads on the post-reset version line (`:81-82`)
|
||||
- **Verified by main:** `:81-82` says the three cold archetypes "were added in **v3.1.0** — if the session
|
||||
predates them, reload." Current `plugin.json` version = **0.5.3** (`:3`); CHANGELOG `[0.4.0]` (2026-05-31)
|
||||
records the **honest version reset 4.1.0 → 0.4.0**, so `v3.1.0` is a *pre-reset* tag no longer on the
|
||||
current line. A reader on 0.5.3 comparing numerically (0.5.3 < 3.1.0) would wrongly conclude they
|
||||
"predate" the agents and must reload — when 0.5.3 is post-reset and already ships all three (they are in
|
||||
CLAUDE.md's 19-agent list). Harmless if followed (an unnecessary reload), but the version anchor misleads.
|
||||
- **disposition:** anchor by event/date, not the dead tag — e.g. "added with the cold-review package
|
||||
(CHANGELOG 3.1.0, pre-reset); reload if your session predates those agents."
|
||||
|
||||
### SUGGESTION — fan-out N-count unit left implicit: `persona-resonance` issues "one call per active persona" (`:144`) while the header counts "<N> archetypes" / "<N> run in parallel" (`:172, :219`). Pin whether N counts review-modes (5) or Task-calls (≥5). Non-breaking. Correctness-lens.
|
||||
|
||||
---
|
||||
|
||||
## pivot.md — VERDICT: ALLOW (0 BLOCKER/MAJOR · 0 MINOR · 1 SUGGESTION)
|
||||
|
||||
Class: **guided/stateful + routing (no subagent orchestration by design)** ("re-open a long-form edition so
|
||||
cleared gates re-run before lock"). Intent delivered: Step 1 load+locate (stops if `articles.NN` absent) →
|
||||
Step 2 measure scope + classify → Step 3 append `pivots[]`, reset `currentPhase`, un-lock, invalidate
|
||||
downstream verdicts → Step 4 write `STATE.md` + point at `/linkedin:newsletter`. **Unusually
|
||||
well-reconciled** (correctness-lens, all main-checkable): 4 steps sequential; the >20%/>2-sections
|
||||
heuristic stated identically in 3 places (`:8-9, :54-55, :92`); the worked example's arithmetic checks out
|
||||
(+42% = (1992−1400)/1400 ✓; "added 2 sections … at the boundary of '>2'" correctly attributes the trigger
|
||||
to the 20% arm since `2` is not `>2`); the off-by-one phase map is explicitly reconciled (`:102-108`,
|
||||
`to-phase` = last *completed* phase, newsletter resumes at the step after); `gatesToRerun` (4 entries `:123`)
|
||||
matches the summary + STATE line (`:163, :149-151`). `allowed-tools` correctly **omits `Task`** (delegates
|
||||
gate-running to `/linkedin:newsletter`, never spawns). All 3 Reference-File targets resolve; degradation
|
||||
present (Step 1 stop-on-missing-article; Step 2.2 absent-baseline → ask operator). No "thought leadership".
|
||||
|
||||
### SUGGESTION — `allowed-tools` over-declares `Grep` (`:18`); the only grep in the body is a *shell* `grep -c '^## '` inside a `Bash` block (`:89`), not the `Grep` tool. `Glob` plausibly resolves the series root — keep it. Correctness-lens. See Recurring note.
|
||||
|
||||
---
|
||||
|
||||
## Gate decision — R3 COMPLETE (4 surfaces)
|
||||
|
||||
| Surface | Verdict | BLOCKER | MAJOR | MINOR | SUGGESTION |
|
||||
|---|---|---|---|---|---|
|
||||
| firsthour | ALLOW | 0 | 0 | 1 | 2 |
|
||||
| calendar | REWORK | 0 | 1 | 0 | 1 |
|
||||
| headless-review | ALLOW | 0 | 0 | 2 | 1 |
|
||||
| pivot | ALLOW | 0 | 0 | 0 | 1 |
|
||||
| **★ #1 clipboard** (pointer, firsthour) | — | — | — | (folds into R2a) | — |
|
||||
| **★ #2 bare paths** (firsthour, 3 sites) | — | 0 | 0 | (counted in firsthour) | — |
|
||||
|
||||
**Batch verdict: REWORK** — 1 of 4 surfaces (calendar) carries **1 MAJOR**: the queue load surfaces none of
|
||||
the `id`/`draft_path`/`character_count` that publish/reschedule/cancel require, and the reschedule step's
|
||||
"carry from the entry shown in Step 2" is a direct contradiction. **0 BLOCKER anywhere.** firsthour /
|
||||
headless-review / pivot are ALLOW (pivot notably clean — every count, the heuristic boundary case, and the
|
||||
off-by-one phase map reconcile).
|
||||
|
||||
**Independence verdict:** convergence on headless-review `SendUserFile` (both lenses) + one **divergence
|
||||
resolved by main's grounding** (calendar: intent-lens flagged the data-gap MAJOR, correctness-lens passed
|
||||
it on structural arithmetic, main grounded `queueFormatSummary`'s output and confirmed the MAJOR). Lens-B
|
||||
also uniquely caught the `v3.1.0` dead anchor + the over-declaration pattern; Lens-A uniquely caught the
|
||||
bare paths + the clipboard pointer. Both lenses earned their keep.
|
||||
|
||||
**Systemic findings now span R2a+R2b+R3:** clipboard `printf` (R2a, 10 files incl. firsthour) · component
|
||||
scaffold (R2b, 3 files) · bare reference paths (R2b+R3, now 3 files incl. firsthour's 3 sites). **New this
|
||||
batch (SUGGESTION-class, not ★):** `allowed-tools` over-declaration on 3 of 4 R3 surfaces. Cold review
|
||||
**finds**; it changes no code. Each fix is its own operator-gated decision. Recommended consolidated-fix
|
||||
order unchanged: (1) clipboard `printf` [R2a, 10 files, highest blast radius], (2) component scaffold [3
|
||||
files], (3) bare reference paths [grep-driven, now incl. firsthour], then the per-surface items (calendar
|
||||
queue-data MAJOR, headless `SendUserFile`/`v3.1.0`, over-declaration trims). Local-only (hardening-class),
|
||||
pushed per the 2026-06-30 operator delegation (public catalog, no secrets).
|
||||
|
||||
**Cumulative cold-review coverage: 17/29** (review.md S1=4 · R2a=5 · R2b=4 · R3=4).
|
||||
Loading…
Add table
Add a link
Reference in a new issue