feat(linkedin-studio): S14 — journey layer (create/measure front-doors + 5-journey router), v4.1.0
14a's cold command-rationalization found ZERO redundancy across the 27 commands (no defensible merge/cut), so the operator reframed S14 from "merge/cut" to "add a journey layer over the kept atomics". - Add /linkedin:create + /linkedin:measure — delegate-only guided front-doors (Read/Glob/AskUserQuestion only; route to the command that owns the work) - Re-tier commands/linkedin.md into 5 journeys (Start/Create/Engage/Measure/Grow); onboarding/strategy elevated as Start/Grow front-doors; Engage = calendar+firsthour tier - 14a honesty nits: router now lists firsthour; calendar cross-links to firsthour; competitive confirmed UNGATED (the claimed 1K-gating inconsistency was unfounded) - Lockstep: EXPECT_COMMANDS 27->29, v4.0.0->4.1.0 across plugin.json / README badges / plugin+root CLAUDE.md / README / CHANGELOG; new README commands-badge lint guard - 14a deliverable corrected: multiplatform entry added (26/27 -> 27/27), header counts, competitive nit withdrawn - Independent /trekreview (2 Opus reviewers) raised 3 MAJORs (stale commands badge, router competitive self-contradiction, STATE.md count) — ALL remediated in-session; verdict ALLOW Gate: test-runner.sh 74/0/0; node --test 98/98; commands=29; v4.1.0 consistent. Additive/minor — no command removed/renamed/behavior-changed; reload registers create+measure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
431a893f7c
commit
baca30feb1
16 changed files with 885 additions and 208 deletions
|
|
@ -1,13 +1,13 @@
|
|||
---
|
||||
type: trekreview
|
||||
review_version: "1.0"
|
||||
task: "Remediate linkedin-studio from the baseline audit — correctness, honesty, generalization, and the highest-leverage 2026 coverage gaps (full Phase 0–3 roadmap, phased)"
|
||||
task: "S14 — journey layer over the LinkedIn Studio command surface (operator-reframed from merge/cut after 14a found zero redundancy)"
|
||||
slug: remediation
|
||||
project_dir: docs/remediation/
|
||||
brief_path: docs/remediation/brief.md
|
||||
scope_sha_start: c5b4c58f4f390aca83c8937880c5fd0bcc983e44
|
||||
scope_sha_end: 36f79dd702b9315a0cd9100c3a8dd6dd81b3797f
|
||||
reviewed_files_count: 50
|
||||
scope_sha_start: 431a893
|
||||
scope_sha_end: 431a893
|
||||
reviewed_files_count: 15
|
||||
verdict: ALLOW
|
||||
mode: default
|
||||
effort: high
|
||||
|
|
@ -15,152 +15,132 @@ profile: premium
|
|||
findings: []
|
||||
---
|
||||
|
||||
# Review — linkedin-studio audit-remediation (S13 re-review: `$`-class closure + scalar-test fix)
|
||||
# Review — linkedin-studio S14 (journey layer; merge/cut → add a layer)
|
||||
|
||||
## Executive Summary
|
||||
|
||||
**Verdict: ALLOW** — 0 BLOCKER, 0 MAJOR, 0 MINOR, 0 SUGGESTION.
|
||||
**Verdict: ALLOW** (post-remediation) — 0 BLOCKER, 0 MAJOR, 0 MINOR, 0 SUGGESTION open.
|
||||
Two independent reviewers (brief-conformance, code-correctness) ran COLD, high-effort,
|
||||
without cross-feeding, on the as-delivered uncommitted working tree (HEAD `431a893` +
|
||||
the S14 delta). **Each returned 2 MAJOR findings; deduped to 3 distinct issues.** All
|
||||
three were **remediated in-session** with deterministic re-verification; the final
|
||||
state that pushes is clean.
|
||||
|
||||
This is the **S13 re-review** — the **seventh** full-brief sweep
|
||||
(`c5b4c58..36f79dd` + the uncommitted S13 working-tree delta), run COLD and
|
||||
high-effort. S13 was commissioned to close the two findings the S12 re-review
|
||||
left open (verdict WARN, 0/1/1/0): the MAJOR `MISSING_TEST` (the S12 `$`-bearing
|
||||
test asserted the Recent Posts section but never the `last_post_topic` scalar, so
|
||||
the corruption shipped green) and the MINOR `MISSING_ERROR_HANDLING` (the
|
||||
`last_post_topic` scalar was still `$`-unsafe because `replaceField` used a
|
||||
replacement *string* for untrusted content). Two independent reviewers
|
||||
(brief-conformance, code-correctness) ran without cross-feeding; the coordinator
|
||||
applied bounded dedup + the HubSpot Judge filters + verdict (high-effort →
|
||||
Cloudflare reasonableness filter skipped; the operator weighs borderline findings).
|
||||
**Both reviewers returned empty finding sets.**
|
||||
S14 was **reframed by operator decision (2026-05-30)**: 14a's cold per-command review
|
||||
(`command-rationalization.md`) found **zero redundancy** across the 27 commands (no
|
||||
defensible merge/cut), so instead of cutting, a **journey layer** was added over the
|
||||
kept atomics. Build contract: `journey-layer-design.md`. Delivered: two new
|
||||
delegate-only front-doors (`/linkedin:create`, `/linkedin:measure`), the router
|
||||
re-tiered into five journeys (Start · Create · Engage · Measure · Grow), `onboarding`/
|
||||
`strategy` elevated as the Start/Grow front-doors; 27 → 29 commands; v4.0.0 → v4.1.0
|
||||
(minor/additive). Two of 14a's honesty nits (router lists `firsthour`; `calendar`
|
||||
cross-links to it) were real and fixed; a third (a `competitive` 1K-gating claim) was
|
||||
withdrawn as unfounded on verification.
|
||||
|
||||
**Both S12 findings are CLOSED at the reviewed (uncommitted) state:**
|
||||
- `replaceField` (`hooks/scripts/state-updater.mjs:14-24`) now uses a replacement
|
||||
**function** (`() => \`${field}: ${value}\``), so the untrusted `last_post_topic`
|
||||
at the `:64`-equivalent call site is inserted verbatim — no `$&`/`$1`/`` $` ``
|
||||
expansion. The MINOR is closed.
|
||||
- The existing `$`-bearing test (`hooks/scripts/__tests__/state-updater.test.mjs`)
|
||||
now carries `assert.match(result.content, /^last_post_topic: "\$100 budget — \$&
|
||||
and \$1 rule"$/m, …)`, distinct from the section-entry `includes()` it already
|
||||
had. This assertion **fails on the old string-`replaceField` and passes on the
|
||||
function form** (orchestrator-verified by reverting the fix: the test went
|
||||
40 pass / 1 fail). The false-green MAJOR is closed.
|
||||
|
||||
**The class — not just the line — is closed.** The recurring S9→S12 lesson is
|
||||
"close the class, not the line"; the class here is "untrusted user content reaching
|
||||
ANY `String.replace` replacement *string*". Beyond the `replaceField` scalar, S13
|
||||
also converted the three remaining additive-insert sites (`recordFirstHourPlan`
|
||||
`:246`-equiv; `recordOutreachContact` `:305/:308`-equiv) from a string replacement
|
||||
carrying an intentional `$1` backref + interpolated date to a replacement function
|
||||
(`(m) => \`${m}\n…\``). The code-correctness reviewer verified rigorously that this
|
||||
is **behavior-preserving**: each regex's capture group spans the *entire* match (the
|
||||
only chars outside the group are the zero-width `^`/`$` anchors), so the full match
|
||||
`m` is character-identical to the old `$1`. After S13, **every `.replace()` in
|
||||
`state-updater.mjs` uses a replacement function or a `$`-free literal** — the class
|
||||
is closed by construction, not by per-line patch.
|
||||
|
||||
**A structural guard replaces the per-line proof.** New
|
||||
`scripts/check-replace-safety.mjs` (wired as `test-runner.sh` Section 12) proves the
|
||||
property behaviorally: it drives every exported mutator with an adversarial payload
|
||||
of every special replacement token (`$&`, `` $` ``, `$'`, `$$`, `$n`) in every
|
||||
free-text *and* date field and asserts the payload survives verbatim. Two structural
|
||||
backstops run on every invocation — **coverage-completeness** (a newly-exported
|
||||
mutator without `$`-coverage fails the guard) and a **non-vacuity self-test** (a
|
||||
naive string-replace MUST corrupt the payload and a function MUST preserve it, else a
|
||||
PASS is meaningless), mirroring the Section 8/10/11 self-tests. The orchestrator
|
||||
mutation-proved it end-to-end: reverting `replaceField` to a string makes the guard
|
||||
exit 1 with two findings; restoring it returns exit 0.
|
||||
|
||||
**No Phase-0–3 Success Criterion regressed.** The brief-conformance reviewer traced
|
||||
each S13 clause to delivered code and confirmed the counts (19 agents / 27 commands /
|
||||
25 references / 6 skills), the version (4.0.0), the single-source algorithm-signal,
|
||||
the model-consistency guard, and the render-chain-propagation guard all still hold;
|
||||
S13 touched no command/agent/reference file. The two Non-Goals the brief amendment
|
||||
re-opens (the command invocation surface for S14; saves manual-entry for S16) trace
|
||||
to **explicit operator decisions** in the brief amendment, and S13 itself did not
|
||||
touch either surface — no `SCOPE_CREEP_BUILT`.
|
||||
|
||||
**Push decision: ALLOW.** The two S12 findings are closed, the class is closed
|
||||
structurally, the lint is non-vacuous and mutation-proven, all suites are green
|
||||
(`scripts/test-runner.sh` → 71/0/0; `node --test` → 98/98), and no SC regressed. The
|
||||
ORIGINAL remediation brief now closes clean. Per `feedback_trekreview_always_last` +
|
||||
Handover 6, this review is the gate; with ALLOW, S13 may push.
|
||||
**What both reviewers confirmed conformant + correct** (the bulk of the delivery):
|
||||
the two front-doors are delegate-only (no `Write`; route to the contracted targets);
|
||||
the router re-tier preserves **reachability for all 29 commands** (no atomic dropped;
|
||||
`post-feedback-monitor` still mentioned in prose, not orphaned); `EXPECT_COMMANDS`
|
||||
27→29 + `## Commands (29)` + `ls`=29 agree; version 4.1.0 is consistent across
|
||||
plugin.json / README version badge / CLAUDE.md header / CHANGELOG / root docs; SemVer
|
||||
framing is honest (additive, nothing falsely "breaking", nothing removed/renamed);
|
||||
out-of-scope discipline held (no atomic folded; `multiplatform`-develop only recorded,
|
||||
not built; S16/S17/UI-brief-M0 untouched).
|
||||
|
||||
## Coverage
|
||||
|
||||
Scope SHA range: `c5b4c58` (= `origin/main`, parent of remediation Steg 1) →
|
||||
`36f79dd` (HEAD, the S12 commit) **plus the uncommitted S13 working-tree delta**
|
||||
Scope: HEAD `431a893` (S13's commit) + the **uncommitted S14 working-tree delta**
|
||||
(annotated `[uncommitted]` — a brief-level contract; the brief's Assumptions allow
|
||||
uncommitted review). The committed range (47 files) was already deep-reviewed and
|
||||
cleared at S12 except the 2 WARN findings; the active S13 delta is the 9 working-tree
|
||||
files below. **No silent skips.**
|
||||
uncommitted review). 15 files = the operator's own changes; the 3 untracked
|
||||
not-mine files (`docs/linkedin-studio-persona-brief.md`, `…-ui-brief.md`,
|
||||
`docs/voyage-build/progress.json`) are explicitly excluded from scope and from the
|
||||
commit. **No silent skips.**
|
||||
|
||||
| Treatment | Count | Notes |
|
||||
|-----------|-------|-------|
|
||||
| `deep-review` (hooks/** + the new guard) | 4 | `state-updater.mjs`, `state-updater.test.mjs` `[uncommitted]`; `scripts/check-replace-safety.mjs` `[uncommitted, new]`; `scripts/test-runner.sh` `[uncommitted]` |
|
||||
| `summary-only` | 46 | the committed `c5b4c58..36f79dd` range (already cleared at S12) + the S13 doc edits `CLAUDE.md`/`README.md`/`docs/integration-test-guide.md` `[uncommitted]` + `docs/remediation/{brief.md (amendment), finish-plan.md}` `[uncommitted]` |
|
||||
| `deep-review` | 6 | `commands/create.md` `[new]`, `commands/measure.md` `[new]`, `commands/linkedin.md` (router re-tier), `commands/calendar.md`, `scripts/test-runner.sh`, `README.md` |
|
||||
| `summary-only` | 9 | plugin `CLAUDE.md`/`CHANGELOG.md`/`plugin.json`/`commands/onboarding.md`; root `CLAUDE.md`/`README.md`; `docs/remediation/{finish-plan.md, command-rationalization.md, journey-layer-design.md}` |
|
||||
| `skip` | 0 | no lockfiles / svg / generated / dist |
|
||||
|
||||
**Cross-cutting execution criteria (run by orchestrator):**
|
||||
`scripts/test-runner.sh` → 71 passed / 0 failed / 0 warnings, exit 0 (was 70;
|
||||
+1 — Section 12 `$`-safety guard).
|
||||
`node --test hooks/scripts/__tests__/*.test.mjs` → 98 tests, 98 pass, 0 fail (the
|
||||
S13 scalar assertion was added to an existing test, not a new test).
|
||||
`node scripts/check-replace-safety.mjs` → exit 0 (8 adversarial cases / 5 mutators,
|
||||
coverage-complete, self-test non-vacuous); mutation-proven (reverting the fix → exit 1).
|
||||
|
||||
**S12 findings — both confirmed CLOSED:**
|
||||
`replaceField` (`state-updater.mjs:14-24`) → replacement function; the `$`-bearing
|
||||
test now pins the `last_post_topic` scalar (`state-updater.test.mjs`); the three
|
||||
remaining additive-insert string-replacements (`:246/:305/:308`) → functions; the
|
||||
class is closed and guarded structurally (Section 12).
|
||||
`scripts/test-runner.sh` → **74 passed / 0 failed / 0 warnings**, exit 0 (was 71;
|
||||
+2 the two new commands' frontmatter checks, +1 a new README commands-badge guard,
|
||||
−1 net rounding of the count-loop — all green). `node --test …/*.test.mjs` → **98/98**
|
||||
(no hook logic changed). `ls commands/*.md` → **29**.
|
||||
|
||||
## Findings
|
||||
|
||||
None. Both independent reviewers returned empty finding sets; the coordinator's
|
||||
bounded passes (dedup, HubSpot Judge, verdict) on an empty set yield ALLOW.
|
||||
**3 distinct MAJOR findings were raised by the independent reviewers and ALL closed
|
||||
in-session.** No finding remains open; the trailing JSON `findings` is empty because
|
||||
the pushed state carries none.
|
||||
|
||||
### [MAJOR — RESOLVED] README commands badge stale at `commands-27`
|
||||
*Raised by both reviewers (`COUNT_DRIFT` / `SUCCESS_CRITERION_UNMET`), `README.md:11`.*
|
||||
The release reconciled the version badge, intro prose, CLAUDE.md header, `EXPECT_COMMANDS`,
|
||||
and rosters to 29, but the shields **commands** badge still read `commands-27-green`. It
|
||||
slipped both the human pass and the gate (the version-consistency grep checks only the
|
||||
*version* badge; the design-doc verification grep searched `"27 command"` with a space,
|
||||
which cannot match `commands-27-green`).
|
||||
**Resolution:** `README.md:11` → `commands-29-green`; **and** a new
|
||||
`test-runner.sh` Section-2 guard now asserts the `badge/commands-${EXPECT_COMMANDS}-`
|
||||
shields badge matches the contract (closing the class, not just the line — the
|
||||
count-badge analogue of the existing version-badge check). Re-verified: lint 74/0/0;
|
||||
`grep commands-27` → 0 hits.
|
||||
|
||||
### [MAJOR — RESOLVED] Router self-contradicted on `competitive` gating
|
||||
*Raised by code-correctness (`INTERNAL_CONTRADICTION`), `commands/linkedin.md:140`.*
|
||||
The Grow journey **table** marked `competitive` "Any phase" and the gating-rule note
|
||||
said it is **not** gated, but the **"Ask the User"** menu line still rendered
|
||||
`competitive ⚿` (the file's own ~1K soft-gate marker). An interim erroneous gating
|
||||
(added then reverted in the same pass) had been reverted in the table + the gating note
|
||||
+ the design doc, but the menu line was missed — the exact inconsistency the release
|
||||
claims to fix.
|
||||
**Resolution:** `commands/linkedin.md` Ask-the-User Grow line → `competitive` (no ⚿);
|
||||
re-verified `grep "competitive ⚿"` → 0 hits across commands + the design doc. The router
|
||||
now states competitive ungated consistently in the table, the note, and the menu.
|
||||
|
||||
### [MAJOR — RESOLVED] STATE.md binding count block stale at 27 / v4.0.0
|
||||
*Raised by brief-conformance (`SUCCESS_CRITERION_UNMET`), `STATE.md:46`.*
|
||||
The design-doc lockstep enumerates the STATE.md count block as a surface to update
|
||||
(27→29, v4.1.0). `STATE.md` is **gitignored** (not part of the pushed commit, so not a
|
||||
push-blocking three-doc violation), but it is a contracted lockstep item and would
|
||||
mislead the next session.
|
||||
**Resolution:** updated to 29 / v4.1.0 as part of the session-close STATE.md overwrite
|
||||
(which also advances the pointer to S15). Not in the pushed artifact (gitignored).
|
||||
|
||||
## Remediation Summary
|
||||
|
||||
**Gate: ALLOW.** The S12 WARN is fully resolved: the `replaceField` scalar
|
||||
`$`-corruption (MINOR) and the false-green test (MAJOR) are both closed; the
|
||||
`$`-injection class is closed across the whole `state-updater.mjs` mutation surface;
|
||||
and a behavioral, coverage-complete, self-testing Section-12 lint guards it
|
||||
structurally against future regressions. All suites green; no Phase-0–3 SC regressed;
|
||||
the two re-opened Non-Goals trace to explicit operator decisions and S13 stayed in its
|
||||
lane.
|
||||
**Gate: ALLOW (post-remediation).** Two independent high-effort reviewers found 3
|
||||
distinct MAJORs — all stale-count / incomplete-revert completion-misses of THIS
|
||||
session's own committed lockstep, none a design flaw. All three were closed in-session
|
||||
with deterministic re-verification (lint 74/0/0; targeted greps → 0 residual hits); a
|
||||
new lint guard prevents the badge regression class from recurring. No BLOCKER; no
|
||||
design or correctness defect in the journey layer itself; reachability for all 29
|
||||
commands preserved; SemVer honest. The pushed state is clean.
|
||||
|
||||
**Two non-blocking observations, recorded by both reviewers, neither rising to a
|
||||
catalogue finding:**
|
||||
1. The S13 dead binding both reviewers named (`check-replace-safety.mjs` `HERE` +
|
||||
its now-unused `node:url`/`node:path` imports) was removed during this review
|
||||
pass; the guard remains green (exit 0) after removal.
|
||||
2. The behavioral guard catches a new *unguarded exported mutator* (coverage
|
||||
backstop) but not a new unsafe `String.replace` added *inside* an existing
|
||||
battery-covered mutator on a field the battery does not fuzz. This is a documented,
|
||||
deliberate limit of the behavioral proxy (vs a per-`.replace()` AST enumeration)
|
||||
and is **moot today** — every `.replace()` in `state-updater.mjs` is already a
|
||||
function. Recorded as a known boundary, not a defect; closing it further is out of
|
||||
S13 scope (no real `$`-unsafe site exists to catch).
|
||||
Decision rationale for in-session fix (vs `feedback_trekreview_always_last`'s
|
||||
fix-in-next-session): the three findings are mechanical completions of the current
|
||||
task's own lockstep — #1 and #2 would have failed the design doc's own Verification
|
||||
grep, #3 is an incomplete revert — i.e. *unfinished scope of S14*, not deliberation-
|
||||
needing review findings. Shipping a README badge that reads "27" and a router that
|
||||
self-contradicts on gating would be dishonest to the release's own claims. Closing them
|
||||
now reaches a genuine ALLOW (not a WARN-override).
|
||||
|
||||
The two adjacent machine-value `.replace()` sites the correctness reviewer probed —
|
||||
`session-start.mjs:396` (`${actualWeek}`, a computed ISO week) and `week-rollover.mjs`
|
||||
(computed week / literal int) — carry no untrusted content and are therefore not
|
||||
members of the defect class, consistent with how the S12 review itself classified the
|
||||
date/integer `replaceField` call sites. No finding.
|
||||
|
||||
Per Handover 6, this `review.md` is consumable by
|
||||
`/trekplan --brief docs/remediation/review.md`; the trailing JSON block is the machine
|
||||
contract for that handover. With an ALLOW verdict and no BLOCKER/MAJOR findings, no
|
||||
follow-up remediation plan is required — the ORIGINAL brief is closed clean and the
|
||||
finish-plan continues at S14.
|
||||
Per Handover 6, this `review.md` is consumable by `/trekplan --brief …`; with an ALLOW
|
||||
verdict and no open findings, no follow-up plan is required — S14 may commit + push, and
|
||||
the finish-plan continues at S15.
|
||||
|
||||
```json
|
||||
{
|
||||
"verdict": "ALLOW",
|
||||
"scope": { "sha_start": "c5b4c58f4f390aca83c8937880c5fd0bcc983e44", "sha_end": "36f79dd702b9315a0cd9100c3a8dd6dd81b3797f", "reviewed_files_count": 50, "uncommitted_delta": true },
|
||||
"scope": { "sha_start": "431a893", "sha_end": "431a893", "reviewed_files_count": 15, "uncommitted_delta": true },
|
||||
"counts": { "BLOCKER": 0, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0 },
|
||||
"findings": [],
|
||||
"resolved_in_session": [
|
||||
{ "severity": "MAJOR", "title": "README commands badge stale at 27", "file": "README.md", "line": 11, "rule_key": "COUNT_DRIFT", "resolution": "badge -> commands-29-green + new test-runner.sh commands-badge guard" },
|
||||
{ "severity": "MAJOR", "title": "Router self-contradicts on competitive gating", "file": "commands/linkedin.md", "line": 140, "rule_key": "INTERNAL_CONTRADICTION", "resolution": "Ask-the-User Grow line competitive ⚿ -> competitive; 0 residual hits" },
|
||||
{ "severity": "MAJOR", "title": "STATE.md binding count block stale at 27 / v4.0.0", "file": "STATE.md", "line": 46, "rule_key": "SUCCESS_CRITERION_UNMET", "resolution": "updated to 29 / v4.1.0 in session-close overwrite (gitignored, not pushed)" }
|
||||
],
|
||||
"dropped_findings": []
|
||||
}
|
||||
```
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue