docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

[kyle-sexton]

Summary

youtube-digest's synthesis step assumed an implicit "the repo I'm in" as the applicability target, and two corpus-durability gaps (landing sub-path shape, binary retention) were unstated limitations rather than named, documented ones. This PR ships the doc/prose surface of issue #558's backlog: an explicit synthesis-target seam plus honest documentation of what remains unbuilt.

Fix

• Synthesis target resolution. Added an optional --target <repo> argument to any watch form, plus a documented resolution ladder in SKILL.md (explicit --target → the consuming project when invoked directly, no separate corpus session → ask). templates/recommendations/menu.md, templates/synthesis-item.md, and templates/readme-journey.md now frame against {target} instead of assuming the invoking repo; README.md's new **Target:** line records whichever rung resolved it, so a later resume never has to re-infer it.
• recommendations/** reframed as ephemeral/target-bound. Documented as this skill's own terminal deliverable for the resolved target, expected to be superseded by the designed-but-unbuilt /knowledge:apply report→diff→PR flow (docs/knowledge-integration-design.md) once that skill ships — a presence-gated forward pointer, not a functional dependency.
• Two remaining gaps named explicitly instead of left silent:
  • library_dir relocates the .work/<watch-epic>/<video-slug>/ work root, not that sub-path's shape — a corpus consumer whose own convention differs (e.g. sources/<type>/<slug>/) doesn't get that today.
  • Raw video, bulk frames, and contact sheets stay OS-temp-only by design — a consumer wanting them retained as a durable, re-runnable substrate doesn't get that today either.
• Bumped knowledge 0.8.3 → 0.8.4 with a matching CHANGELOG.md entry.

Doc-only — extraction/** is untouched; nothing parses --target or the renamed template fields yet (this is agent-followed prose, consistent with the rest of the skill's argument-routing style, same as every other action-router entry).

Related

Parked, not addressed here — each requires a real code change, not a doc change:

• Landing sub-path shape. Making .work/<watch-epic>/<video-slug>/ swappable for a differently-shaped consumer convention (e.g. sources/<type>/<slug>/) means centralizing that literal path construction, which is currently scattered across ~10 extraction scripts (run-watch.js, watch-state.js, queue-claim.js, transcript scripts, etc.), not just SKILL.md. Open design call on the seam's shape (new userConfig template string? a tracked-convention file, per the plugin philosophy's four config seams?) plus a real refactor. Tracked in knowledge/youtube-digest: centralize .work/<epic>/<slug> sub-path shape + binary-retention landing — deferred from #558 #856.
• Binary retention (raw video / bulk frames / contact sheets). Adding a durable, LFS-aware retention path is a genuine behavior change (what triggers retention, where it lands, how .gitattributes awareness threads through), not a prose fix. Open design call. Tracked in knowledge/youtube-digest: centralize .work/<epic>/<slug> sub-path shape + binary-retention landing — deferred from #558 #856.
• /knowledge:apply itself. Already fully speced in docs/knowledge-integration-design.md (wave-2 map, sub-issue implement(knowledge-integration)) — unbuilt, out of scope here by design; this PR only makes youtube-digest ready to hand its menu to that skill once it exists.
• Section D (corpus .gitattributes LFS gaps, pre-LFS epub blobs) belongs to melodic-software/knowledge-corpus, not this repo — tracked in the issue for visibility only.

Closes #558

Verification

• bash scripts/check-changelog-parity.sh --check — pass
• bash scripts/check-changelog-parity.sh --check-bump origin/main — pass
• node scripts/validate-plugin-contracts.mjs — pass (1900 files, 35 setup skills)
• bash scripts/check-skill-portability.sh origin/main — pass (no unexcused coupling tokens)
• Skill-quality gate (check-skill.sh youtube-digest) — PASS, 0 errors (SKILL.md 408/500 lines, within the hard cap)
• markdownlint-cli2 on all changed markdown — 0 errors
• Independent fresh-context review pass (code-reviewer subagent) against the diff — one IMPORTANT finding (target-recording instruction was ambiguous across resolution-ladder rungs, and no template field existed to hold it) fixed by broadening the ladder prose and adding the **Target:** line to readme-journey.md; remaining SUGGESTIONS (terminology consistency, action-router table shape) addressed inline

[claude]

Claude finished @kyle-sexton's task in 2m 51s —— View job

---

PR Review — docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

• Gather context and understand the PR
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Review SKILL.md changes (synthesis target + gap documentation)
• Review template changes (readme-journey.md, menu.md, synthesis-item.md)
• Assess overall alignment with CLAUDE.md / plugin design rules

---

Summary

This is a clean, well-scoped doc-only PR. The synthesis-target seam is logically sound, the resolution ladder is coherent, the template substitutions are internally consistent, and the two corpus-durability gaps are honestly documented rather than left silent. No code-behavior changes, no secrets, no security surface. Passes all REVIEW.md "always check" items (N/A for this scope).

Three 🟡 Nits below. None are blockers.

---

Findings

🟡 Nit — <repo> format never defined for agents invoking --target

SKILL.md line 4 (argument-hint) and line 288 (resolution ladder) reference --target <repo>, but neither specifies what form <repo> should take — a directory path, a repo name, a URL? An agent reading the skill for the first time would need to guess. Since the ladder's first rung is "explicit --target <repo> argument," a one-word clarification in the argument-hint (--target <repo-name-or-path>) or a parenthetical in the ladder prose would be sufficient. Fix this →

---

🟡 Nit — resume section doesn't reference reading **Target:** from README.md

SKILL.md line 288 states: "record the target name in README.md's **Target:** line … so a later resume or downstream consumer never has to re-infer it." But the Resume section (~line 320) only describes reading watch.json for phase state — it never tells the agent to recover the target from README.md. watch.json is documented as "phase-map + tempSession paths" with no target field, so README.md is the only documented recovery path. A single instruction in the Resume section ("read README.md's **Target:** line to recover the resolved target") would make the contract self-contained for the resume path. Fix this →

---

🟡 Nit — Resolution ladder → notation reads as sequence rather than fallthrough

SKILL.md line 288: "explicit --target <repo> argument → the consuming project (CLAUDE_PROJECT_DIR) when watch is invoked directly inside a repo, with no separate corpus session → ask" — the compound condition on the second rung ("when watch is invoked directly inside a repo, with no separate corpus session") makes this read as a sequence rather than an OR-else ladder. The PR says a prior review already broadened this prose, and the current form is readable with care. Adding "otherwise:" before the second rung or rephrasing as "if absent: the consuming project … ; if still ambiguous: ask" would make the fallthrough explicit without much added length.

---

Positive notes

• The gap documentation in the new library_dir scope paragraph (line 56) and the temp-only handling note (line 18) are precise and honest — tracking follow-ups rather than leaving consumers to discover limits by hand is the right pattern.
• The {resolved synthesis target — SKILL.md "Synthesis target resolution"} placeholder in readme-journey.md is a good self-referencing pattern — it tells the agent exactly where to look for the logic.
• The "ephemeral, target-bound deliverable" note (line 297) correctly frames recommendations/** and makes the /knowledge:apply forward pointer presence-gated rather than a hard dependency.
• CHANGELOG entry is well-structured — explains the why, names what didn't change, and matches the diff precisely.

[claude]

Claude finished @kyle-sexton's task in 1m 30s —— View job

---

Security Review — PR #854

• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

This PR is documentation-only — all six changed files are markdown prose (skill instructions, agent-fill templates, a changelog entry, and a semver bump in plugin.json). No code in extraction/** is touched; the PR explicitly states nothing parses the new --target argument or the renamed template fields yet.

Checked surfaces:

Surface	Finding
{target} placeholder in templates	Agent-filled prose at synthesis time; never shell-interpolated. No injection risk.
--target <repo> in argument-hint	Documented but unimplemented — no parsing code exists in this diff.
Pre-computed ! shell blocks in SKILL.md (lines 11–14)	Unchanged from prior version; use fixed binary invocations only.
${CLAUDE_PROJECT_DIR}/${user_config.library_dir} shell interpolation	Pre-existing; not introduced or widened by this PR.
Secrets / credentials	None introduced. The yt_dlp_cookies_file guidance ("Never commit cookie files") is pre-existing.
GitHub Actions / workflow files	Not touched.
Dependency pins / supply chain	Not touched.
Permission widening	None.

One thing to watch when the --target flag is eventually implemented in code: if the value is interpolated into a file-system path (e.g. to locate or write a target-repo's files), validate and sanitize it to prevent path traversal. That risk doesn't exist in the current PR since nothing parses it yet, but it's worth building in from the start when the implementation lands.

[kyle-sexton]

Post-green review classification (work lane):

Finding	Source	Classification	Disposition
Everything substantive	PR review + security review	No blockers, "clean, well-scoped" / "No security issues found"	No action needed
<repo> format undefined in --target <repo>	PR review (nit)	Non-blocking doc-polish suggestion	Left alone
Resume section doesn't reference README.md's **Target:** recovery	PR review (nit)	Non-blocking doc-polish suggestion	Left alone
Resolution-ladder → notation reads as sequence not fallthrough	PR review (nit)	Non-blocking, reviewer notes it's "readable with care"	Left alone
Future path-traversal risk when --target is eventually implemented in code	Security review (forward-looking note)	Not applicable to this PR (doc-only, nothing parses --target yet) — noted for whoever implements it later	No action needed now

No DEFER-FORBIDDEN class applies (doc-only change, no executable surface). No inline review threads to resolve (PR-level comments only).

Flipping to ready, handing off to the babysit lane for ongoing watch.

[claude]

Claude finished @kyle-sexton's task in 1m 55s —— View job

---

PR Review — docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

• Gather context and understand the PR
• Read changed files
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Review SKILL.md changes
• Review template changes
• Post final review

---

Summary

Doc-only, well-scoped, internally consistent. All CI gates pass; a prior Claude review found three nits, all classified non-blocking by the author and deliberately deferred — I've verified that assessment holds. Nothing here is a blocker.

---

Findings

Version bump — 0.8.3 → 0.8.4 in plugin.json line 4 is correct for a non-breaking doc change. Schema reference to json.schemastore.org/claude-code-plugin-manifest.json is unchanged and valid.

CHANGELOG.md — the 0.8.4 entry accurately names both changes (synthesis target ladder, two named gaps), calls out the doc-only constraint, and matches the diff precisely. Format is consistent with prior entries.

SKILL.md — five coherent additions:

1. argument-hint (line 4): --target <repo> added to all three watch forms, absent from queue, transcript, and resume — correct scope (synthesis is watch-time only).
2. Temp-only gap (line 18): named as "fixed, non-configurable" with a forward reference to a tracked follow-up rather than a silent limitation.
3. Scope-of-seam paragraph (line 56): correctly scopes library_dir to root relocation only, names the sub-path gap, and provides the workaround consumer path.
4. Action-router note (line 90): clarifies --target is a modifier not a dispatchable action — good disambiguation.
5. Synthesis target resolution block (line 288): the three-rung ladder is logically sound. The cross-reference to /knowledge:apply is correctly presence-gated ("when that skill is installed and built") rather than a hard dependency.

Templates — all three changes are internally consistent:

• readme-journey.md line 9: **Target:** {resolved synthesis target — SKILL.md "Synthesis target resolution"} — self-referencing pointer is the right pattern; agents know exactly where to look.
• menu.md line 3: for **{target}** with the resolution cross-reference matches the skill's documented ladder.
• synthesis-item.md line 5: Target touchpoints rename is correct; the field now points at the resolved target, not the implicit CWD.

Prior nits (confirmed non-blocking, no action needed): The three nits from the earlier automated review (<repo> format undefined, resume section not referencing README.md recovery, ladder → notation) were deliberately deferred by the author. Verified: none create a functional gap in the current doc-only surface. Each is a polish item appropriate for a follow-up pass when --target gets implementation.

---

Verdict

No blockers. Clean doc-only PR, well-scoped to the stated issue, passes all CI gates, and the prior review's nits are correctly classified as non-blocking polish. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 1m 23s —— View job

---

Security Review — PR #854

• Gather context and understand the PR
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

This PR is documentation-only. All six changed files are markdown prose — skill instructions, agent-fill templates, a changelog entry, and a semver bump in plugin.json. No executable code was added or modified; extraction/** is entirely untouched.

Surfaces reviewed

File	Change	Security surface
plugin.json	version 0.8.3 → 0.8.4	None
CHANGELOG.md	Prose description of 0.8.4 changes	None
SKILL.md	--target <repo> in argument-hint + synthesis-target-resolution ladder + two gap paragraphs	Agent-read prose only; --target is explicitly unimplemented (no parsing code in this diff)
templates/readme-journey.md	Added **Target:** field; TLDR re-phrased	Agent-fill template, never shell-interpolated
templates/recommendations/menu.md	{repo name} → {target}	Agent-fill template, never shell-interpolated
templates/synthesis-item.md	"Repo touchpoints" → "Target touchpoints"	Agent-fill template, never shell-interpolated

Forward-looking note (not a finding in this PR)

The new --target <repo> argument is deliberately unimplemented here — agent-followed prose only, consistent with every other argument-routing entry in this skill. When the implementation lands and the value is interpolated into a filesystem path (to locate or write files in a target repo), validate and sanitize the value to prevent path traversal — a crafted argument like ../../etc could escape the intended work root if naively joined to CLAUDE_PROJECT_DIR or library_dir. This was already flagged in the prior security review on this PR; echoing it here for the implementation author's record.

No GitHub Actions files, permissions, secrets, dependency pins, or MCP server configuration were touched.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9ea9d404d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 20s —— View job

---

PR Review — docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

• Gather context and understand the PR
• Read changed files on branch
• Review SKILL.md additions
• Review template changes
• Review plugin.json / CHANGELOG.md
• Assess Codex P2 finding validity
• Post final review

---

Summary

Doc-only, well-scoped, internally consistent. Prior automated reviews (two PR reviews + two security reviews) were thorough and I agree with their verdicts. The Codex P2 finding (target persistence before synthesis) was correctly classified by the babysitter as valid-deferred; I've verified it and concur. No blockers.

---

Findings

✅ Version bump — plugin.json line 4: 0.8.3 → 0.8.4 is correct for a non-breaking doc change. Schema $schema reference unchanged and valid.

✅ CHANGELOG.md — The 0.8.4 entry names both changes accurately, calls out the doc-only constraint, and matches the diff precisely.

✅ SKILL.md — five additions are coherent:

1. argument-hint (line 4): --target <repo> on all three watch forms only — correctly absent from queue, transcript, and resume (synthesis is watch-time only). ✓
2. Temp-only gap (line 19): named, honest, forward-referenced. ✓
3. Scope of seam (line 57): library_dir scoped to root relocation only, workaround path named. ✓
4. Action router note (line 91): --target correctly framed as a modifier, not a dispatchable action. ✓
5. Synthesis target resolution (line 289) + Ephemeral deliverable note (line 298): three-rung ladder is logically sound; /knowledge:apply forward pointer is presence-gated, not a hard dependency. ✓

✅ Templates — all three are internally consistent:

• readme-journey.md line 9: **Target:** field added before TLDR; TLDR updated to reference "the resolved synthesis target" — both halves agree.
• menu.md line 3: {repo name} → {target} with resolution cross-reference.
• synthesis-item.md line 5: "Repo touchpoints" → "Target touchpoints" with "in the resolved target" qualifier — correct.

---

On the Codex P2 finding

The babysitter's classification is correct: valid gap, appropriately deferred. I verified:

• Synthesis is the last resumable phase. The **Target:** field (readme-journey.md line 9) is written as part of synthesis's "Update README.md" step (SKILL.md line 295) — but that also means it's only written after synthesis begins, which is the end of the pipeline. An interrupted-before-synthesis watch loses the explicit --target across sessions.
• The Resume section (SKILL.md lines 318–330) reads watch.json for phase state but has no instruction to recover the target — and watch.json has no target field.

This is not a blocker for this PR because --target is explicitly unimplemented in this diff (no parsing code; agent-followed prose only). The gap only bites when --target gets code implementation. For the implementation author's record: write the resolved target to watch.json at bootstrap (phase acquire or earlier), and add a recovery step in the Resume section — storing it only in README.md after synthesis creates an unrecoverable window for phases 1–6.

---

Verdict

No blockers. This is a clean, honest doc-only change that names limitations instead of leaving consumers to discover them by hand, and sets up the synthesis-target seam correctly for when the code implementation follows. The three prior nits (deferred) and the Codex P2 (deferred, non-blocking for this doc surface) are all correctly classified. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 1m 48s —— View job

---

Security Review — PR #854

• Gather context and understand the PR
• Fetch PR diff and read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

All six changed files are documentation-only (markdown prose, agent-fill templates, a changelog entry, and a semver bump in plugin.json). No executable code was added or modified; extraction/** is entirely untouched, consistent with the PR description.

Surfaces reviewed

File	Change	Security surface
plugin.json	version 0.8.3 → 0.8.4	None
CHANGELOG.md	Prose description of 0.8.4 changes	None
SKILL.md	--target <repo> in argument-hint + synthesis-target-resolution ladder + two gap paragraphs + ephemeral-deliverable note	Agent-read prose only; --target is explicitly unimplemented (no parsing code in this diff). Pre-computed context blocks at lines 12–15 (!…`` `) are pre-existing, use fixed binary invocations with no user-supplied input, and are unchanged by this PR.
templates/readme-journey.md	Added **Target:** field; TLDR re-phrased	Agent-fill markdown template — never shell-interpolated or processed by a template engine; LLM-filled text at synthesis time
templates/recommendations/menu.md	{repo name} → {target} placeholder rename	Agent-fill template, same as above
templates/synthesis-item.md	"Repo touchpoints" → "Target touchpoints"	Pure text rename, no code

Forward-looking note (not a finding in this PR)

When --target <repo> is eventually implemented in extraction code and the value is joined into a filesystem path (e.g. against CLAUDE_PROJECT_DIR or library_dir), validate and sanitize the argument to prevent path traversal — a crafted value such as ../../etc could escape the intended work root if naively passed to path.join(). This has been noted in both prior security reviews on this PR; echoing it once more for the implementation author's record. It is not a risk in the current diff.

[claude]

Claude finished @kyle-sexton's task in 2m 1s —— View job

---

PR Review — docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

• Gather context and understand the PR
• Check what changed since last review (merge commit vs. content change)
• Read changed files on branch
• Review SKILL.md additions
• Review template changes
• Review plugin.json / CHANGELOG.md
• Assess prior findings and any new issues
• Post final review

---

Trigger context

This review fires on a synchronize event from commit a97d5aa — a merge-from-origin/main commit only. No content changes were introduced in this sync; the actual PR diff is unchanged from the prior reviews. All six changed files are identical to what was reviewed at e9ea9d4 / 7646ba3.

---

Summary

Doc-only, clean, and internally consistent. All REVIEW.md "always check" items are N/A or PASS for this diff. Prior reviews (two PR reviews + two security reviews) reached the same verdict; I've re-verified the diff independently and concur.

---

Findings

✅ Version bump — plugin.json line 4: 0.9.0 → 0.9.1 is correct. Note: the PR description says 0.8.3 → 0.8.4, but that language was written before subsequent merges from main advanced the knowledge plugin through additional versions. The diff and CHANGELOG entry (## [0.9.1]) are correct and consistent with what is on origin/main now (0.9.0).

✅ CHANGELOG.md — The [0.9.1] entry names both changes (synthesis target ladder, two named gaps), calls out the doc-only constraint, and matches the diff precisely. Entries are ordered newest-first.

✅ SKILL.md — five additions verified coherent:

1. argument-hint (line 4): --target <repo> on all three watch forms only — correctly absent from queue, transcript, and resume.
2. Temp-only gap (line 19): "fixed, non-configurable" with forward reference to a tracked follow-up. Honest framing.
3. Scope of seam (line 57): library_dir scoped to root relocation only; sub-path gap named; workaround path given.
4. Action-router note (line 91): --target correctly framed as a modifier, not a dispatchable action.
5. Synthesis target resolution (line 289) + ephemeral deliverable note (line 299): three-rung ladder is logically sound; /knowledge:apply cross-reference is correctly presence-gated ("when that skill is installed and built").

✅ Templates — all three changes internally consistent:

• readme-journey.md line 9: **Target:** field added before TLDR; TLDR updated to say "the resolved synthesis target."
• menu.md line 3: {repo name} → {target} with resolution ladder cross-reference.
• synthesis-item.md line 5: "Repo touchpoints" → "Target touchpoints" with "in the resolved target" qualifier.

✅ REVIEW.md "always check" criteria — all N/A or PASS: no handlers, no tenant data paths, no secrets, no string-concatenated inputs, no audit-log surface, no multi-record atomicity concern. This is prose-only.

---

Prior open findings — status unchanged

Finding	Source	Status
<repo> format undefined in --target <repo>	PR review (🟡 Nit)	Deferred by author — non-blocking polish for when --target gets implementation
Resume section doesn't reference README.md's **Target:** recovery	PR review (🟡 Nit)	Deferred — same reason
Resolution-ladder → notation reads as sequence not fallthrough	PR review (🟡 Nit)	Deferred — "readable with care"
Codex P2: persist explicit target before synthesis, not after	Codex + babysit	Valid gap; deferred as non-blocking for this doc-only surface — implementation owner should write resolved target to watch.json at bootstrap and add a recovery step to the Resume section

No new findings introduced by this PR sync.

---

Verdict

No blockers. Ready to merge. The only observable change since the prior reviews is a97d5aa, a merge-from-main commit that carries no content change to the six PR files.

[claude]

Claude finished @kyle-sexton's task in 1m 21s —— View job

---

Security Review — PR #854

• Gather context and understand the PR
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

All six changed files are documentation-only — markdown prose, agent-fill templates, a changelog entry, and a semver bump in plugin.json. No executable code in extraction/** is touched; the PR explicitly states nothing parses the new --target argument or the renamed template fields yet.

Surfaces reviewed

File	Change	Security surface
plugin.json	version 0.9.0 → 0.9.1	None — no new permissions, capabilities, MCP servers, or dependency changes
CHANGELOG.md	Prose description of 0.9.1 changes	None
SKILL.md	--target <repo> in argument-hint; synthesis-target-resolution ladder; two gap paragraphs; ephemeral-deliverable note	Agent-read prose only; --target is explicitly unimplemented (no parsing code in this diff). Pre-computed !-block shell commands at lines 12–15 are unchanged — fixed binary invocations with no user-supplied input (node -e with hardcoded env-var read, yt-dlp --version, ffmpeg -version, magick -version)
templates/readme-journey.md	Added **Target:** field; TLDR re-phrased	Agent-fill markdown template — {resolved synthesis target …} is LLM-filled text at synthesis time, never shell-interpolated or processed by a template engine
templates/recommendations/menu.md	{repo name} → {target} placeholder rename	Agent-fill template, same as above — rename only, no change in processing semantics
templates/synthesis-item.md	"Repo touchpoints" → "Target touchpoints"	Pure text rename, no code

Forward-looking note (not a finding in this PR)

When --target <repo> is eventually implemented in extraction code and the value is joined into a filesystem path (e.g. against CLAUDE_PROJECT_DIR or library_dir), validate and sanitize the argument to prevent path traversal — a crafted value such as ../../etc could escape the intended work root if naively passed to path.join(). This has been noted in prior security reviews on this PR (Jul 21 10:31, 16:23, 17:17); echoing it for the implementation author's record. It is not a risk in the current diff.

No GitHub Actions files, workflow permissions, secrets, dependency pins, or MCP server configuration were touched.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a97d5aa14a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

This was generated by AI (decide lane — decision-brief pass; parked PR-thread decision surfaced via periodic scan).

Decision brief — persist the synthesis Target durably (re: parked thread #discussion_r3624067559, which the babysit rescan verified and deferred to the author with no ruling)

Problem

youtube-digest's synthesis Target (the repo a menu is fitted to) is recorded only in README.md's **Target:** line, written at the synthesis phase — the last resumable phase. If a watch --target <repo> run is interrupted before synthesis, nothing durable holds the resolved Target, so resume <video-slug> silently re-infers it (falls back to the session repo or re-asks) and can fit the menu to the wrong repository — defeating the resumability guarantee this branch is about.

Today (verified at PR head a97d5aa)

• Where Target is set: SKILL.md synthesis-target-resolution block resolves the ladder (--target → consuming project → ask) and instructs the agent to record it in README.md's **Target:** line — at synthesis time, not at resolution/bootstrap.
• Durable bootstrap artifact: watch.json (extraction/watch/watch-state.js:60, path .work/<watch-epic>/<video-slug>/watch.json :5). Its WatchState typedef (:37-57) and createWatchState({videoId, videoSlug, sourceUrl, title}) (:73) carry no target field — only phase-map + tempSession paths.
• Resume: extraction/watch/run-resume.js takes only <video-slug> (:5, :30-33), reads watch.json (:38), and has zero target/README references — it has no channel to recover an explicit --target.

Net: README is a human-facing echo, not a machine-recovery channel (resume never parses it). Note --target parsing is itself unimplemented in this doc-only PR — so any fix here is a code change, sharing the extraction-code lane tracked in #856.

Options

A — New watch.json target field ✅ RECOMMENDED

Add one optional target string to WatchState, written when the ladder resolves (bootstrap/first durable write), read back by resume.

• Why: watch.json is already the single bootstrap-time durable artifact that resume opens (run-resume.js:38). Recovery becomes free — resume reads the field it already loads. Directly closes the flagged failure: resume can never silently lose or re-infer the Target.
• Tradeoffs: +1 optional field of schema growth — trivial against the correctness win. README **Target:** stays as the human echo.
• Reversibility: high — additive/optional; states lacking it fall back to today's ask behavior, so it's backward-compatible and cheap to remove.

B — Agent-step re-derive/re-prompt on resume

Keep schema fixed; write **Target:** at resolution time and have resume re-read README prose, or re-prompt.

• Tradeoffs: No schema change, but leans on the agent parsing a narrative template (README is display prose, not a parse target — brittle) or re-prompting, which is the exact resumability failure being flagged.
• Reversibility: high, but the durability guarantee is weaker — recovery depends on prose fidelity, not typed state.

C — Hybrid

watch.json target = machine source of truth for recovery; README **Target:** retained as the human-readable echo. This is A plus the existing README line — the honest end state, and what A already implies.

Recommendation

A (hybrid-flavored: A + keep the README echo). Durable typed state is the simplest thing that satisfies the one hard constraint — resume must not silently lose the Target — and it rides the seam resume already uses. Schema growth is one optional string; the alternative trades that away for prose-parsing brittleness. Author owns the call.

Cross-links

• knowledge/youtube-digest: centralize .work/<epic>/<slug> sub-path shape + binary-retention landing — deferred from #558 #856 — youtube-digest deferred code scope (same skill). The open seam-shape design call there (new userConfig template string vs. a tracked-convention file, per the plugin's config-seam philosophy) may govern where a watch.json target field lives and how --target is wired; this Target-persistence mechanism is smaller but lands in that same code lane. Decide the seam shape there, apply it here.
• #854 body — this PR is the doc/prose surface (--target ladder documented, nothing parses it yet); the persistence fix is explicitly a follow-on code change, not a doc edit.

[claude]

Claude finished @kyle-sexton's task in 1m 45s —— View job

---

PR Review — docs(knowledge): youtube-digest synthesis target + named agnosticism gaps

• Gather context and understand the PR
• Read the diff at HEAD (43b64b9) to see what's new since last reviews
• Review SKILL.md changes
• Assess the Codex P2 contact-sheet finding addressed by new commit
• Post final review

---

Trigger context

This review fires on a synchronize event from commit 43b64b9 — "docs(knowledge): distinguish contact-sheet DR snapshot from temp-only." This is the only content change since the prior review pass at a97d5aa.

---

Summary

The new commit correctly resolves the outstanding Codex P2 finding. The rest of the PR is unchanged from the prior reviews, which found no blockers. Ready to merge.

---

New commit analysis — 43b64b9

The Codex P2 finding (contact-sheet review thread): the previous temp-only caveat labeled all contact sheets as OS-temp-only, but snapshot-bootstrap.js copies them into key-frames/contact-sheets/*.jpg as a gitignored, slice-local DR snapshot — contradicting the Output contract table at lines 368–369 and misleading operators into thinking they must re-run acquisition when tempSession is gone.

What 43b64b9 fixes — two locations, both correct:

1. Preamble / overview (SKILL.md line 19):

   • "contact sheets" → "working contact sheets" (the OS-temp copies only)
   • Added the carve-out: snapshot-bootstrap.js copies sheets into key-frames/contact-sheets/*.jpg as a local DR snapshot — durable on disk, survives tempSession cleanup, but gitignored (not committed). "see the Output contract" cross-reference is valid — Output contract rows at lines 368–369 were already present and now match exactly.
   • Tightened the residual gap from "durable, re-runnable substrate" to "committed, re-runnable substrate (rather than regenerable temp state or a gitignored local snapshot)" — precisely names what is unbuilt (committed/LFS-aware retention) without misrepresenting what already works (DR snapshot).

2. Operational note (SKILL.md line 199):

   • Updated consistently: "working contact sheets stay in tempSession dirs (the sheets are additionally snapshotted to key-frames/contact-sheets/ for local disaster recovery — see the Output contract); re-run run-watch.js to regenerate bulk frames when temp expired."
   • Correctly scopes the re-run instruction to bulk frames only (not sheets), consistent with the new DR snapshot framing.

3. CHANGELOG.md (line 28):

   • Updated to match: "working contact sheets stay OS-temp-only by design (contact sheets do get a gitignored, slice-local disaster-recovery snapshot at key-frames/contact-sheets/, but that is not committed durable retention)."
   • CHANGELOG accurately reflects the refined framing without inflating scope.

Cross-ref validity check: Output contract table already had:

• key-frames/contact-sheets/snapshot-meta.json — committed, {tmp}-tokenized sourceDir
• key-frames/contact-sheets/*.jpg — never in git, local DR snapshot, gitignored

Both "see the Output contract" links in SKILL.md resolve correctly against these rows. No contradiction remains between preamble, operational note, and artifact inventory.

---

Prior open findings — status unchanged

Finding	Source	Status
<repo> format undefined in --target <repo>	PR review (🟡 Nit)	Deferred — non-blocking polish for --target implementation
Resume section doesn't reference README.md's **Target:** recovery	PR review (🟡 Nit)	Deferred — same
Resolution-ladder → notation reads as sequence	PR review (🟡 Nit)	Deferred — "readable with care"
Codex P2 — target persistence before synthesis	Codex + babysit	Deferred, non-blocking for doc-only surface — implement when --target gets code; write resolved target to watch.json at bootstrap and add a recovery step in the Resume section
Codex P2 — contact sheets mislabeled as temp-only	Codex	✅ Resolved by 43b64b9

---

Verdict

No blockers. The Codex P2 contact-sheet finding is correctly and precisely resolved. The three-way distinction (working sheets in temp / gitignored DR snapshot in slice / committed LFS-aware retention as the unbuilt thing) is now internally consistent across preamble, operational note, and the Output contract. All prior reviews found this PR sound; nothing in this commit changes that assessment.

[claude]

Claude finished @kyle-sexton's task in 1m 35s —— View job

---

Security Review — PR #854

• Gather context and understand the PR
• Fetch PR diff
• Read changed files
• Analyze for security vulnerabilities
• Post findings

---

No security issues found.

All six changed files are documentation-only — markdown prose, agent-fill templates, a changelog entry, and a semver bump in plugin.json. No executable code in extraction/** was added or modified. The latest commit (43b64b9) adds two sentences clarifying that contact-sheet JPGs at key-frames/contact-sheets/ are a gitignored local DR snapshot (not committed retention), directly responding to the Codex P2 finding from the prior review cycle.

Surfaces reviewed

File	Change	Security surface
plugin.json	version 0.9.0 → 0.9.1	None — no new permissions, capabilities, MCP servers, or dependency changes
CHANGELOG.md	Prose description of 0.9.1 changes	None
SKILL.md	Contact-sheet temp-vs-DR distinction clarified; --target <repo> argument-hint; synthesis-target-resolution ladder; scope-of-seam paragraph	Agent-read prose only. Pre-computed !-block shell commands at lines 12–15 are unchanged — fixed binary invocations with no user-supplied input (node -e reads only process.env.CLAUDE_PLUGIN_DATA, a trusted plugin-system variable; yt-dlp --version, ffmpeg -version, magick -version take no arguments). --target is explicitly unimplemented — no parsing code in this diff.
templates/readme-journey.md	Added **Target:** field; TLDR re-phrased	Agent-fill markdown template — {resolved synthesis target …} is LLM-filled at synthesis time, never shell-interpolated or processed by a template engine
templates/recommendations/menu.md	{repo name} → {target} placeholder rename	Agent-fill template, same as above — rename only, no change in processing semantics
templates/synthesis-item.md	"Repo touchpoints" → "Target touchpoints"	Pure text rename, no code

Forward-looking note (not a finding in this PR)

When --target <repo> is eventually implemented in extraction code and the value is joined into a filesystem path (e.g. against CLAUDE_PROJECT_DIR or library_dir), validate and sanitize the argument to prevent path traversal — a crafted value such as ../../etc could escape the intended work root if naively passed to path.join(). This has been noted in all prior security reviews on this PR; echoing it for the implementation author's record. It is not a risk in the current diff.

No GitHub Actions files, workflow permissions, secrets, dependency pins, or MCP server configuration were touched.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43b64b92a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".