You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Publishes a generic, repo-agnostic ai-briefing engine plugin, migrated from the medley in-repo skill. Ships the engine only — no employer/personal layers.
Multi-wave collection (Chrome/X, optional Grok, Perplexity, RSS, GitHub releases), dedup, categorize/rank, and retro / search / drift actions, plus the per-profile runner and the slides/HTML/PDF build pipeline.
Named-profile seam (profiled-folder convention): the curated following-list.json, an optional brand.js overlay, and an optional stack lens resolve from .claude/ai-briefing/[<profile>/], falling back to a bundled neutral seed/following-list.json of public vendor accounts.
State vs config split: machine-local run state (seen-items, per-run artifacts, generated decks) persists per profile under ${CLAUDE_PLUGIN_DATA}; curated config never does. In-repo runs fall back to the skill tree so the engine and its tests work outside a plugin install.
Neutral default brand (no employer logos); the pragmatic-use ranking lens and apolitical filter ship as documented, overridable defaults; the impact tag is profile-provided and optional.
/ai-briefing:setup — re-runnable action that scaffolds a profile and installs runtime dependencies under ${CLAUDE_PLUGIN_DATA} (the plugin cache is read-only).
Verification
claude plugin validate --strict passes on both the plugin and the catalog manifest.
Loaded via --plugin-dir in a clean non-source repo; both /ai-briefing:ai-briefing and /ai-briefing:setup are discovered.
Scripts test suite (6 suites) and build unit test pass; an end-to-end emit → HTML → PPTX smoke writes to the data root with the neutral no-logo brand.
PII/decoupling scrub verified by an independent fresh-context pass: no employer/personal content, no medley path coupling, no cache-isolation reach-outs, and no runtime-state files committed.
Notes
node_modules and runtime-state paths are gitignored; deps install at runtime under ${CLAUDE_PLUGIN_DATA}.
The SETWorks profile (curated follow-list + branding) is extracted separately by the medley-side cutover, not shipped here.
Refs melodic-software/medley#1442
Note
Medium Risk
Large new surface (browser scraping, external APIs, Playwright/PPTX build) and reliance on ${CLAUDE_PLUGIN_DATA} for deps/state; behavior is skill-orchestrated rather than a single hardened service boundary.
Overview
Adds the ai-briefing marketplace plugin — a repo-agnostic engine for aggregating AI-industry news into ranked briefings and optional presentation decks.
Catalog & packaging: Registers ai-briefing in marketplace.json and documents it in the root README. Ships plugin.json with an active_profile user option and .gitignore rules so runtime deps and per-run state stay out of git.
Skills:/ai-briefing:ai-briefing drives multi-wave collection (Chrome/X, Perplexity, RSS, GitHub releases, optional Grok), dedup, 13-bucket categorize/rank, meeting-window merge, and retro / search / drift actions. /ai-briefing:setup scaffolds a consumer profile under .claude/ai-briefing/ and installs runtime Node deps under ${CLAUDE_PLUGIN_DATA}.
Named-profile seam: Curated following-list.json, optional brand.js, and stack lens live in the project profile (with bundled neutral seed); machine state (seen-items, runs, decks) persists per profile in the plugin data directory.
Build pipeline: In-tree Node ESM flow — parse briefing markdown → slides-data.js → PPTX (pptxgenjs), single-file scroll-snap HTML, and PDF (Playwright), with Zod validation, provider logos, and overridable default brand/ranking defaults.
Reviewed by Cursor Bugbot for commit 729c19a. Bugbot is set up for automated code reviews on this repo. Configure here.
Author a repo-agnostic ai-briefing engine plugin from the medley in-repo skill:
multi-wave collection, dedup/rank, retro/search/drift actions, per-profile runner,
and the slides/html/pdf build pipeline — engine only, no employer/personal layers.
- Named-profile seam (profiled-folder convention): curated following-list, brand
overlay, and stack lens resolve from .claude/ai-briefing/[<profile>/], falling
back to a bundled neutral seed. Runtime state (seen-items, runs, generated decks)
persists per profile under ${CLAUDE_PLUGIN_DATA}; config never does.
- Neutral default brand (no employer logos); pragmatic-use ranking lens and
apolitical filter ship as documented, overridable defaults; the impact tag is
profile-provided and optional.
- Re-runnable /ai-briefing:setup action scaffolds a profile and persists runtime
deps under ${CLAUDE_PLUGIN_DATA} (plugin cache is read-only).
- Decoupled from medley paths; validated with claude plugin validate --strict and
loaded via --plugin-dir in a clean repo. Scripts + build unit tests green.
PR Review: feat(ai-briefing): publish generic engine plugin with named-profile seam
Tasks:
Gather context and understand the PR
Fetch fresh plugin/skill docs (CLAUDE.md mandate)
Review plugin manifest and marketplace registration
Review skill definitions (SKILL.md files)
Review JavaScript scripts and build pipeline
Review seed data and profile seam
Security and isolation review
Post comprehensive review feedback
Summary
This is a large but coherent plugin. The architecture (engine-only, no employer layer; state vs. config split; plugin-cache isolation; named-profile seam) is well-designed and correctly implements the CLAUDE.md plugin-form rules. The atomic-write pattern, Zod validation on all structured I/O, and the clean stateRoot() / configDir() env-var fallback ladder are all solid. Test coverage exists across all major components.
Several issues are worth addressing before merge, ranging from a data-quality bug in state management to a model-ID hard-coding that will cause future failures.
Findings
🔴 Correctness — source_type always "twitter" regardless of collection source
appendToSeenItems is called from the commit-and-checkoff subcommand after any profile completes, but the source_type is always hard-coded as "twitter" regardless of whether items came from Grok, RSS, Perplexity, or GitHub Releases. This means the seen-items registry can't distinguish collection source, corrupting the analytics used by drift, retro, and dedup logic.
Fix: Pass source type through the item shape (e.g. item.source_type ?? "twitter") and emit it from each collection path. Fix this →
🟡 Maintainability — Model ID hard-coded in two agents
"claude-sonnet-4-6" is hard-coded in both the synthesize and categorize claude -p invocations. When this model ID is deprecated or renamed, both S3 and S4 will break silently (the subprocess exits non-zero) with no model-configuration escape hatch. The plugin.json already has a userConfig section — adding an optional synthesis_model key would allow consumers to pin a different model without forking.
🟡 Security — Handle value injected into JS string literal without sanitization
The template embeds h directly into single-quoted JS string literals:
// In POSTS_EXTRACTOR_TEMPLATE:window.__capPosts={a: '@__HANDLE__', ...};// In REPLIES_EXTRACTOR_TEMPLATE:constauthored=art.querySelector(`a[href^="/${handle}/status/"]`);
bareHandle only strips the leading @; it doesn't sanitize characters that would break string context (', \n, \). A malicious entry in a following-list.json copied from an untrusted source (or a compromised profile repo) could inject arbitrary JavaScript that runs in the browser session. cutoffIso has the same issue (injected into new Date('__CUTOFF__')).
Fix: Escape both values for single-quoted JS string context before substitution, e.g. h.replace(/\\/g, "\\\\").replace(/'/g, "\\'"). Twitter handles are always alphanumeric+underscore in practice, but defensively sanitizing is cheap. Fix this →
🟡 Correctness — validate.js fails at import time with unhelpful error if emit hasn't run
This top-level await runs at module load time. When slides-data.js doesn't exist (e.g., a user runs node validate.js before node emit-slides-data.js), Node throws a dynamic-import error with a path, not a human-readable "run emit first" message. Wrapping in a try/catch that emits a clear diagnostic (and exits non-zero) would make this immediately actionable.
🟡 Security — --always-approve in headless Grok invocation
constargs=["--no-auto-update","--always-approve",// ← bypasses all Grok confirmations"--output-format","json",
...
--always-approve means any tool use Grok decides to invoke (file writes, web requests, API calls) runs without confirmation. The briefing capture prompt is read-only in intent, but the flag is a blanket override. Since the Grok CLI's tool set may evolve, this is a latent blast-radius concern. Consider documenting why it's safe here (or restricting via --allow-tools if Grok supports it).
🟢 Informational — runId = isoBasicTimestamp() not collision-safe
isoBasicTimestamp() strips milliseconds, so two init calls within the same second produce the same runId and would overwrite each other's master.json. The findLatestInProgress guard prevents a second init from proceeding, but not if the first master.json write and a concurrent process's init race at the same second. Appending a short random suffix (e.g. 4 hex bytes) would make this robust.
🟢 Informational — CHUNK_SIZE = 2 generates many MCP round-trips
The comment explains the constraint (MCP response truncation at ~1500 chars), but 2 tweets/chunk for a 50-tweet profile is 25 round-trips. The comment notes this needs a byte-budget recalc to raise. Leaving as-is for now is safe; just worth revisiting when the MCP truncation limit is clarified or lifted.
Positive Observations
Atomic writes throughout state.js via tmp+rename — no torn state on crash.
Zod validation on all structured outputs (extractor, synthesize, categorize, deck schema) — failures surface early and clearly.
stateRoot() / configDir() fallback ladder is clean: plugin env → in-repo → seed. Tests work unchanged outside a plugin install.
Plugin-cache isolation correctly implemented: plugin root is read-only, all mutable state under ${CLAUDE_PLUGIN_DATA}, curated config under ${CLAUDE_PROJECT_DIR}.
Neutral seed data — only public vendor accounts, no personal curation, no PII.
mapWithConcurrency correctly implements a bounded worker-pool without Promise.all blowup.
following-list.json seed cleanly separates accounts.* (semantic) from scan_priority.* (operational) and documents the parallel-structure requirement.
No employer/PII content — the scrub appears thorough; brand.js ships an empty org logo and a generic tagline.
Summary
The engine is well-engineered and the plugin-form compliance is solid. The two items I'd treat as blocking before merge are the source_type hard-coding (data quality) and the model ID hard-coding (future breakage). The JS injection sanitization is a good defensive hardening. Everything else is low-severity or informational.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot did not complete (check skipped due to usage limit), so automated review is unavailable and human review is needed before approval. No reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
- MD049: use asterisk emphasis in the SKILL.md data-file table.
- typos: invokable -> invocable; reword incidental "Canva"/"mis-parsed"/"Nd";
wrap the creator-handle list in providers.md with spellchecker off/on.
- exec-bit: mark scripts/per-profile-runner.js executable (it has a shebang).
Review plugin manifest and marketplace registration
Verify prior review findings and assess Codex-flagged issues
Post comprehensive review
Summary
The engine architecture (state vs. config split, plugin-cache isolation, named-profile seam, atomic writes, Zod validation throughout) is well-designed and the CLAUDE.md plugin-form rules are correctly followed. However, reading the code reveals several correctness gaps, two of which are blocking at runtime rather than merely latent.
scripts/package.json declares "type": "module", so all runner scripts are Node.js ESM. Node.js ESM intentionally does not support NODE_PATH: bare specifiers like import { z } from "zod" are resolved via the standard node_modules walk from the importing file's directory—not via NODE_PATH. Since the plugin root is read-only, node_modules can never land next to the scripts.
This means every runner command post-setup fails with ERR_MODULE_NOT_FOUND: zod.
Practical fixes (pick one):
Copy (or symlink) the scripts into ${CLAUDE_PLUGIN_DATA}/deps/scripts/ during setup so node_modules is a sibling and the standard resolution walk finds it.
Vendor zod directly into the plugin under scripts/vendor/ and adjust the imports (import { z } from "../vendor/zod.js").
Use --experimental-vm-modules + an import map (fragile; skip this).
exportfunctionbuildSubcommands(){return{init: cmdInit,"grok-check": cmdGrokCheck,"next-handle": cmdNextHandle,// ← no "grok-capture" entry
...
};}
grok-capture is documented in SKILL.md:142, execution-flow.md:68, and runner-architecture.md:80; exercised in grok-wave0-orchestration.test.js:79; and the library implementation exists in grok-capture-agent.js and grok-capture-spawn.js. But no dispatch entry exists in buildSubcommands(). Any call to per-profile-runner.js grok-capture --index=N returns "Unknown subcommand" and exits non-zero, making the documented Wave 0 Grok path entirely non-functional.
Relatedly, cmdInit never parses --grok-preload, so master.config.grok_preload is always undefined—meaning the test's branch at grok-wave0-orchestration.test.js:80 never takes the "preload enabled" path either.
When S3 or S4 exhaust retries the handlers correctly set profile.status = "partial" and push to profile.errors. But commit-and-checkoff then promotes every non-complete status (including partial) to complete. The end-of-run summary reads from per-profile JSON, so these dropped items appear as fully processed — the failed synthesis or categorization is invisible in the final report.
Fix: Preserve partial in by_status and the final profile JSON:
if(priorStatus!=="complete"&&priorStatus!=="partial"){profile.status="complete";}elseif(priorStatus!=="complete"){// stay "partial" — already recorded in profile.errors}
For a named profile that only overrides branding (no own following-list.json), resolution jumps directly from the named-profile dir to the bundled vendor seed, ignoring the project's default curated list at .claude/ai-briefing/following-list.json. Named-profile runs would scan the 30-handle neutral seed instead of the consumer's curated list — the opposite of the advertised overlay behavior.
Fix: Insert the default-profile path between named-profile and seed:
constcandidates=[path.join(configDir(),"following-list.json"),// named profilepath.join(configDir("default"),"following-list.json"),// default profile fallbackpath.join(SEED_DIR,"following-list.json"),// bundled seed];
appendToSeenItems is called with profile.categorized_items (the S4 output). Items that originated from Grok, Perplexity, RSS, or GitHub Releases will all be recorded as source_type: "twitter", poisoning the analytics used by retro, drift, and dedup.
Fix: Pass source_type through CandidateSchema / ItemSchema and emit it from each collection path:
import{brandasBRAND,themeasTHEME}from"./brand.js";// always the neutral bundle
This is a static top-level import, so the consumer's .claude/ai-briefing/[<profile>/]brand.js is never loaded. Any setup-created profile with custom org name, logo, or theme tokens produces decks with the neutral default brand. The named-profile branding seam is non-functional for deck generation.
Fix: Dynamically import the profile brand at runtime using configDir() and fall back to ./brand.js:
Both values are embedded into single-quoted JS string literals inside the template (e.g. new Date('__CUTOFF__') and window.__capPosts = {a: '@__HANDLE__', ...}). bareHandle only strips the leading @; a value with ' or \ breaks string context. cutoffIso is accepted from the --cutoff CLI flag without format-only validation.
"claude-sonnet-4-6" is hard-coded in both S3 and S4 invocations with no override path. When this model ID is deprecated both stages break silently (non-zero exit from claude -p). Adding an optional synthesis_model key to userConfig in plugin.json and threading it through as claudeModel parameter to synthesize() / categorize() would allow consumers to pin or upgrade the model without forking.
🟡 Security — --always-approve in grok-capture-agent
--always-approve grants blanket approval to all tool use the Grok subprocess decides to perform — file writes, web requests, API calls. The briefing capture prompt is read-only in intent, but there's no capability limit. As Grok's tool set evolves this is a latent blast-radius issue. Consider adding an explanatory comment for why this is safe here (or restricting via --allow-tools if Grok supports it).
🟢 Informational — validate.js top-level await gives an unhelpful error when emit hasn't run
const { meta, theme, slides, providerLogos } = await loadSlidesData(); runs at module load time. If slides-data.js doesn't exist, Node throws a module-resolution error rather than a human-readable "run emit-slides-data.js first." A try/catch wrapping this import with a clear diagnostic message would make this immediately actionable.
isoBasicTimestamp() strips milliseconds. Two concurrent init calls within the same second produce the same runId and overwrite each other's master.json. The findLatestInProgress guard prevents a second init only when the first has finished writing. Appending 4 random hex bytes (e.g. randomBytes(2).toString("hex")) makes this robust.
Positive observations
Atomic writes via tmp+rename throughout state.js — no torn state on crash.
Zod validation on all structured outputs (extractor, synthesize, categorize, deck schema) — failures surface at the boundary, not downstream.
stateRoot()/configDir() env-var fallback ladder is clean: plugin env → in-repo → seed. Tests work outside a plugin install.
Plugin-cache isolation correctly separates read-only plugin root, mutable state under ${CLAUDE_PLUGIN_DATA}, and curated config under ${CLAUDE_PROJECT_DIR}.
mapWithConcurrency implements a correct bounded worker-pool.
Neutral seed data — only public vendor accounts, no PII, no employer content.
Recommendation
The two ESM/NODE_PATH and missing grok-capture subcommand issues are blocking at runtime and should be resolved before merge. The other correctness items (partial→complete promotion, named-profile follow-list fallback, brand hardcoding) are functional gaps in advertised behavior. The security items are low-risk in practice but worth hardening given the plugin is public-marketplace-bound.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Did not approve: Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is unavailable. Human review is needed before merge; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…ture deferred
Address blocking review findings before publish:
- ESM ignores NODE_PATH, so a persisted node_modules beside the read-only plugin
cache is unreachable. setup now stages a runnable copy of the scripts/build
trees into ${CLAUDE_PLUGIN_DATA}/runtime with node_modules installed as a
sibling and runs from there, so the standard ESM walk resolves zod. Re-stages
on a plugin-version bump. Verified in a plugin-form simulation (zod resolves;
runner test suite passes from the staged copy).
- grok-capture is documented but not registered in the runner's buildSubcommands()
dispatch, so it would error. Marked NOT-YET-WIRED in the standing default and
the runner-architecture reference; --grok-preload is a planned flag until a
follow-up wires it. Chrome Wave 1 remains the full, canonical path.
Review changes introduced in the latest commit (0f2c826)
Assess which prior review findings were addressed and how well
Identify any remaining or new issues
Post comprehensive follow-up review
What the latest commit (0f2c826) addresses
The commit message is "fix(ai-briefing): stage runtime for ESM dep resolution; mark grok-capture deferred". Only documentation files changed (setup/SKILL.md, references/build-pipeline.md, references/runner-architecture.md, SKILL.md).
The fix is correct and well-designed. Rather than relying on NODE_PATH (which Node ESM ignores for bare specifiers), setup/SKILL.md now instructs setup to copy the full runner and build trees into ${CLAUDE_PLUGIN_DATA}/runtime/ and install node_modules as siblings there. Scripts are then invoked from the staged copy (node "${CLAUDE_PLUGIN_DATA}/runtime/scripts/per-profile-runner.js" …) so the standard ESM resolution walk finds zod and other deps.
The implementation details hold up under scrutiny:
The version guard (.version sentinel file) makes re-runs idempotent and triggers re-staging on plugin bumps — the && chain ensures .version is only written on successful install.
paths.js derives SKILL_ROOT from __dirname via path.resolve(__dirname, "..", ".."), which correctly resolves to ${CLAUDE_PLUGIN_DATA}/runtime when running from the staged scripts/lib/ copy — so SEED_DIR points at the co-staged runtime/seed/ correctly.
The build tree's paths.js uses CLAUDE_PLUGIN_DATA env to resolve stateRoot() when set, so the staged build scripts write artifacts to the right place.
✅ grok-capture subcommand — honestly deferred
SKILL.md rule 20 and runner-architecture.md now clearly state that grok-capture is not registered in buildSubcommands() in this release and that --grok-preload is a planned-but-not-yet-functional flag. This is a valid approach for a deferred feature — better than shipping a silent "Unknown subcommand" failure.
Issues carried forward — unchanged in this commit
The following findings from the prior reviews remain open in the current code:
🔴 Correctness — commit-and-checkoff still promotes partial → complete
constpriorStatus=profile.status;if(priorStatus!=="complete"){profile.status="complete";// ← still overwrites "partial"}
When S3 or S4 exhausts retries, handlers correctly set profile.status = "partial". commit-and-checkoff then unconditionally promotes every non-complete status — including partial — to complete. The master.by_status accounting does decrement partial and increment complete (lines 356–359), so the master counters are doubly wrong: partial count is decremented even though the profile was just forced to complete. The per-profile JSON file loses the partial distinction, making dropped synthesis/categorization results invisible in the final summary.
Fix:
if(priorStatus!=="complete"&&priorStatus!=="partial"){profile.status="complete";}// For partial, leave profile.status as-is; master.by_status should NOT// move partial→complete counts either.
A named profile that only overrides branding falls through directly from its own (empty) dir to the bundled 30-handle vendor seed — bypassing the consumer's curated default-profile list at .claude/ai-briefing/following-list.json. This is the opposite of the advertised overlay behavior and means named-profile runs silently scan the neutral seed. Note: setup/SKILL.md step 2 has the same gap — it seeds a named profile directly from the bundled seed rather than from the default profile dir.
Fix:
constcandidates=[path.join(configDir(),"following-list.json"),// named profilepath.join(configDir("default"),"following-list.json"),// default profile fallbackpath.join(SEED_DIR,"following-list.json"),// bundled seed];
import{brandasBRAND,themeasTHEME}from"./brand.js";// always the bundled neutral copy
After the ESM staging fix, deck generation runs from ${CLAUDE_PLUGIN_DATA}/runtime/build/emit-slides-data.js. The static ./brand.js import now resolves to the neutral brand that was copied during setup — the consumer's .claude/ai-briefing/[<name>/]brand.js is never read. Profile-specific org name, logo, and theme tokens are silently ignored in any deck output.
Fix: Replace the static import with a dynamic import inside main():
Items from Grok, RSS, Perplexity, and GitHub Releases are all recorded as "twitter" in seen-items.json, corrupting the analytics used by retro, drift, and dedup.
Fix:source_type: item.source_type ?? "twitter" — also requires adding source_type as an optional field to ItemSchema in categorize-agent.js and CandidateSchema in synthesize-agent.js so the field propagates from collection through categorization.
Both values are embedded unsanitized into single-quoted JS string literals inside the extractor templates. bareHandle only strips @; a value with ' or \ breaks string context. A malicious entry in an untrusted following-list.json copy could inject arbitrary JS into the browser session.
"claude-sonnet-4-6" is hard-coded in both S3 and S4 claude -p invocations with no override path. When this model ID is deprecated, both stages fail silently (non-zero exit). Adding an optional synthesis_model key to userConfig in plugin.json would allow consumers to pin a different model without forking.
🟢 Informational — validate.js gives an unhelpful error when emit hasn't run
The top-level await loadSlidesData() throws a module-not-found error (not a human-readable message) when slides-data.js hasn't been emitted yet. A try/catch with a clear "run node emit-slides-data.js first" diagnostic would make this immediately actionable.
🟢 Informational — runId second-precision timestamp is collision-prone under concurrent init
Two init calls within the same second produce the same runId and would overwrite each other's master.json. Appending a short random suffix (crypto.randomBytes(2).toString("hex")) makes this robust.
Summary
The ESM staging fix is solid and addresses the most critical runtime-blocking issue. The grok-capture deferral is handled honestly. Four functional issues from the prior reviews remain open in the current code:
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Did not approve: Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is unavailable. Human review is needed before merge; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Thanks — verified each finding at the source. Resolutions:
🔴 ESM ignores NODE_PATH (blocking) — FIXED (0f2c826). Correct: scripts/package.json is "type": "module", so a persisted node_modules beside the read-only cache is unreachable. setup now stages a runnable copy of the scripts/build trees into ${CLAUDE_PLUGIN_DATA}/runtime/ with node_modules installed as a sibling and runs from there, so the standard ESM walk resolves zod; it re-stages on a plugin-version bump. Verified in a plugin-form simulation (empty cache → stage → install → zod resolves and the full runner test suite passes from the staged copy).
🔴 grok-capture undispatched (blocking) — deferred, tracked in melodic-software/medley#1457. Confirmed absent from buildSubcommands(); it is pre-existing (the Wave 0 rebuild) and Grok is optional/degrading, not on the canonical Chrome-Wave-1 path. Rather than ship a dead subcommand, I marked it NOT-YET-WIRED in standing default #20 and the runner-architecture reference and flagged --grok-preload as a planned flag, so nothing presents it as callable. The wiring (thin adapter + adding its existing test to the suite + un-deferring the docs) is issue #1457.
🟡 source_type hard-coded "twitter", 🟡 model-ID hard-coding, 🟡 validate.js import-time error message, 🟡 handle/cutoff JS-string injection escaping — all pre-existing engine code carried over faithfully; none block plugin load/validate. Batched into #1457 with your suggested fixes.
Review current state of all open findings + new Codex findings
Post comprehensive follow-up review
What ff8fa99 (Merge branch 'main') brings
The merge is a no-op for ai-briefing: it pulls in only 166cfd0 (feat(code-tidying): add re-runnable setup skill for the tidy-lanes seam), which touches no files under plugins/ai-briefing/. Confirmed with git diff 0f2c826..ff8fa99 -- plugins/ai-briefing/ — zero diff.
Status of previously identified issues
✅ ESM / NODE_PATH blocking issue — fixed in 0f2c826
The runtime-staging approach is correct and unchanged. No regression.
✅ grok-capture missing dispatch — deferred and documented
Standing default #20 explicitly marks it NOT-YET-WIRED. SUBCOMMANDS in buildSubcommands() still has no grok-capture entry, which is the expected deferred state. Honest.
🔴 Still open — commit-and-checkoff promotes partial → complete
constpriorStatus=profile.status;if(priorStatus!=="complete"){profile.status="complete";// ← still overwrites "partial"}
And lines 356–359 still decrement master.by_status.partial and increment by_status.complete for any non-failed prior status, so the master counters misreport partial profiles as completed. Not addressed in Kyle's response; not yet in #1457. S3/S4 retry-exhaustion failures are invisible in the final summary.
Fix:
if(priorStatus!=="complete"&&priorStatus!=="partial"){profile.status="complete";}// master.by_status: only move pending/in_progress → complete, not partial → complete
A named profile that overrides only branding falls through directly to the 30-handle vendor seed, bypassing the consumer's curated list at .claude/ai-briefing/following-list.json. Unchanged from commit 0f2c826; not addressed in Kyle's response and not yet in #1457.
Fix: Add the default-profile dir as an intermediate fallback:
constcandidates=[path.join(configDir(),"following-list.json"),// named profilepath.join(configDir("default"),"following-list.json"),// default profile fallbackpath.join(SEED_DIR,"following-list.json"),// bundled seed];
cutoff_reached is therefore always false. The extractor does track a past boolean locally (lines 141–144) to decide when to stop scrolling, but stashes only {n, oldest, stable_iterations} in the returned summary — past is not exposed. This means profiles can be incorrectly flagged as needing retry when extraction was actually complete.
Fix: Propagate past through the returned summary:
// In POSTS_EXTRACTOR_TEMPLATE, add `past` to the stashed window object:window.__capPosts={a: '@__HANDLE__',n: results.length,oldest: ...,stable_iterations: stable,past: past,tw: results};returnJSON.stringify({n: results.length,oldest: ...,stable_iterations: stable,past: past});// In per-profile-validate.js:constcutoffReached=posts?.past||replies?.past;
plugin.json exposes userConfig.active_profile so consumers can pin a profile without a --profile argument. paths.js reads process.env.AI_BRIEFING_PROFILE. But SKILL.md never maps ${user_config.active_profile} to that env var:
# SKILL.md line 104 — only says:
Select the runtime profile by exporting AI_BRIEFING_PROFILE=<name>
# But never says: use ${user_config.active_profile} as the value
A consumer who sets active_profile: "acme" in their plugin config gets the substitution ${user_config.active_profile} = "acme" in the SKILL.md context, but the SKILL.md doesn't instruct Claude to propagate it. The runner scripts default to "default" and scan the wrong profile.
Fix: Add an explicit instruction to SKILL.md in the Profiles section:
When `${user_config.active_profile}` is set, treat it as the active profile name and
prepend `AI_BRIEFING_PROFILE=${user_config.active_profile}` to all runner and build
script invocations.
import{brandasBRAND,themeasTHEME}from"./brand.js";// always the staged neutral copy
After the ESM staging fix, emit-slides-data.js runs from ${CLAUDE_PLUGIN_DATA}/runtime/build/. The static ./brand.js resolves to the neutral copy staged during setup — the consumer's .claude/ai-briefing/[<name>/]brand.js is never loaded. Batched to #1457 per Kyle.
🟡 Still open — source_type always "twitter" in seen-items
Single-quoted JS string literals receive unescaped values. Batched to #1457.
🟡 Still open — model ID hard-coded in both agent callers
Files:synthesize-agent.js#L189, categorize-agent.js#L133. Batched to #1457.
Codex findings assessed — two ruled spurious
S4 bucket coverage (categorize-agent.js:22): VALID_BUCKETS correctly routes Microsoft/legal/compute/real-world through other per the CATEGORIZE_RULES prompt (line 55). The separate briefing sections for those areas are produced by the AI model reading the categorized JSON, not by the enum. Not a bug.
Deck parser bucket format (state.js:155): Codex described a format mismatch with briefing-deltas.md, but appendToSeenItems writes to seen-items.json (JSON), not to a markdown file. The markdown briefing is generated by the orchestration layer from structured state. Spurious finding.
Summary
The merge is neutral for ai-briefing. Two findings from prior reviews remain unaddressed and not yet tracked in #1457: the partial → complete status promotion and the named-profile follow-list fallback gap. These are the items I'd prioritize for a fix or explicit tracking before merge. Two new correctness issues are confirmed: cutoff_reached always false (a real functional gap in capture validation) and the active_profile userConfig not wired to the env var seam.
Priority
Finding
Status
🔴
partial → complete promotion hides failures
Open, not tracked
🔴
Named profile skips default curated follow-list
Open, not tracked
🟡
cutoff_reached always false (extractor past not propagated)
New finding
🟡
active_profile userConfig not wired to AI_BRIEFING_PROFILE
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Did not approve: Cursor Bugbot did not complete (check skipped), so the required automated review signal is unavailable. Human review is needed before merge; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Check what changed since last reviewed commit (ff8fa99)
Verify status of all open findings in current code
Assess new Codex findings
Post comprehensive follow-up review
What changed since the last review (ff8fa99)
Commits a2b94b9 and 23838f2 are both merge commits (merging main and the remote tracking branch respectively). Zero files changed under plugins/ai-briefing/ — confirmed with git diff ff8fa99..HEAD -- plugins/ai-briefing/. The trigger for this review cycle was the push of those merge commits.
Open issues — still present in current code
🔴 commit-and-checkoff still promotes partial → complete
constpriorStatus=profile.status;if(priorStatus!=="complete"){profile.status="complete";// still overwrites "partial"}
And lines 356–359 still decrement master.by_status.partial and increment by_status.complete for partial profiles — so both the per-profile JSON and the master counters misrepresent failed-stage runs as successful. Not addressed in Kyle's prior response; not tracked in #1457.
Fix:
if(priorStatus!=="complete"&&priorStatus!=="partial"){profile.status="complete";}// For partial: leave profile.status as-is; don't move partial→complete in by_status.
constcandidates=[path.join(configDir(),"following-list.json"),// .claude/ai-briefing/<name>/path.join(SEED_DIR,"following-list.json"),// bundled neutral seed — no intermediate stop];
A named profile with no own following-list.json falls straight through to the 30-handle vendor seed, skipping the consumer's curated default-profile list at .claude/ai-briefing/following-list.json. Not addressed; not tracked in #1457.
Fix:
constcandidates=[path.join(configDir(),"following-list.json"),// named profilepath.join(configDir("default"),"following-list.json"),// default profile fallbackpath.join(SEED_DIR,"following-list.json"),// bundled seed];
The extractor templates filter pre-cutoff tweets before adding them to results[], so posts.tw / replies.tw can never contain a tweet older than the cutoff. But S2 validation derives cutoff_reached by scanning those filtered arrays:
The extractor does maintain a local past boolean (it's what stops scrolling) but only stashes {n, oldest, stable_iterations} in the returned summary — past is dropped.
Fix: Add past to the extractor's returned JSON and stashed window object, then use it in validateCaptures:
// In extractor templates, add past to the return objectreturnJSON.stringify({n: results.length,oldest: ...,stable_iterations: stable,past: past});// In per-profile-validate.js:constcutoffReached=posts?.past||replies?.past;
plugin.json exposes userConfig.active_profile; paths.js reads process.env.AI_BRIEFING_PROFILE. SKILL.md line 104 says "the active_profileuserConfig scalar" is the selection mechanism, but never instructs Claude to propagate it as an env var. The Profiles section says to "export AI_BRIEFING_PROFILE=<name>" but that refers to the consumer doing it manually — ${user_config.active_profile} is never mentioned as the value. A consumer who sets active_profile: "acme" in their plugin config will find the runner always selects the "default" profile.
Fix: Add to SKILL.md Profiles section:
When ${user_config.active_profile} is set, prepend AI_BRIEFING_PROFILE=${user_config.active_profile} to all runner and build script invocations.
bucketKey() iterates BUCKET_ORDER and reaches the real-world entry (index 11) before extras (index 12). For a heading like ## EXTRAS (Robotics), h = "extras (robotics)". The robotics branch matches, returning "real-world" — the EXTRAS section is silently routed to the Real-world AI slides and the EXTRAS section never populates. This corrupts output whenever the briefing uses the advertised ## EXTRAS (Robotics) form from SKILL.md rule 11.
Fix: Guard the real-world alias against headings that start with extras:
setup/SKILL.md step 4 correctly establishes that all runner invocations must use the staged copy at ${CLAUDE_PLUGIN_DATA}/runtime/scripts/per-profile-runner.js. But execution-flow.md line 16 still shows:
node scripts/per-profile-runner.js detect-resume
And runner-architecture.md references node per-profile-runner.js <subcommand> throughout. Claude reads both reference files during an active briefing run — a bare scripts/ relative path will fail to resolve in an installed plugin (the CWD is the consumer project, not the plugin root) and would miss the staged node_modules even if it did.
Fix: Update all node scripts/per-profile-runner.js and node per-profile-runner.js references in both files to node "${CLAUDE_PLUGIN_DATA}/runtime/scripts/per-profile-runner.js".
No plugin code changed since the last review cycle. Six issues remain open across correctness, security, and doc consistency:
Priority
Finding
File
🔴
partial → complete promotion hides stage failures
per-profile-commands.js:346
🔴
Named profile skips default curated follow-list
per-profile-following.js:15
🟡
cutoff_reached always false (extractor past not propagated)
per-profile-validate.js:8, chrome-extract.js
🟡
active_profile userConfig not wired to AI_BRIEFING_PROFILE
SKILL.md:104
🟡
EXTRAS (Robotics) heading misroutes to real-world bucket
emit-slides.js:56
🟡
Reference docs use bare runner path (contradicts staged setup)
execution-flow.md:16, runner-architecture.md
The two 🔴 items (status promotion and follow-list fallback) are functional correctness gaps that affect every named-profile run. The EXTRAS bucket misrouting and cutoff_reached always false are newly confirmed. Recommending these six be addressed or explicitly deferred before merge.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Did not approve: Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is unavailable. Human review is needed before merge; no reviewers were assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Did not approve because Cursor Bugbot completed with skip status, so the required automated review did not complete successfully. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Check what changed since last reviewed commit (23838f2)
Verify status of all open findings in current code
Post comprehensive follow-up review
What 729c19a (Merge branch 'main') brings
The merge pulls in only 8e594f0 (chore(evals): author skill evals for claude-ops + prototype), which touches files in plugins/claude-ops/ and plugins/prototype/ — zero changes under plugins/ai-briefing/. All previously identified issues carry forward unchanged.
Issues still open in current code
🔴 commit-and-checkoff still promotes partial → complete
constpriorStatus=profile.status;if(priorStatus!=="complete"){profile.status="complete";// ← still overwrites "partial"}// …}elseif(priorStatus==="partial"){master.by_status.partial=Math.max(0,(master.by_status.partial||0)-1);}master.by_status.complete=(master.by_status.complete||0)+1;
Two bugs in one block: (1) profile.status is unconditionally promoted from partial → complete, and (2) the master counter decrements partial and increments complete for the same case — so both the per-profile JSON and the aggregate counters misreport partial profiles as fully completed. S3/S4 retry-exhaustion failures become invisible in the final summary.
Fix:
if(priorStatus!=="complete"&&priorStatus!=="partial"){profile.status="complete";}// In master.by_status block: don't increment complete for partial profiles.
constcandidates=[path.join(configDir(),"following-list.json"),// .claude/ai-briefing/<name>/path.join(SEED_DIR,"following-list.json"),// bundled neutral seed — no intermediate stop];
A named profile that overrides only branding falls directly from its own (empty) dir to the 30-handle vendor seed, bypassing the consumer's curated list at .claude/ai-briefing/following-list.json. The advertised overlay behavior is non-functional.
Fix:
constcandidates=[path.join(configDir(),"following-list.json"),// named profilepath.join(configDir("default"),"following-list.json"),// default profile fallbackpath.join(SEED_DIR,"following-list.json"),// bundled seed];
The extractor templates filter pre-cutoff tweets before adding them to results[], so posts.tw / replies.tw never contain a tweet older than the cutoff. The extractor's internal past boolean (which stops scrolling) is not included in the returned JSON summary. cutoff_reached therefore reads as false for every completed capture, potentially sending runs down unnecessary retry/partial paths.
Fix: Propagate past through the extractor's returned summary and use it in validateCaptures:
// extractor template: add past to return JSONreturnJSON.stringify({n: results.length,oldest: ...,stable_iterations: stable,past: past});// per-profile-validate.js:constcutoffReached=posts?.past||replies?.past;
plugin.json exposes userConfig.active_profile; paths.js reads process.env.AI_BRIEFING_PROFILE. SKILL.md describes both — but never instructs Claude to prepend AI_BRIEFING_PROFILE=${user_config.active_profile} to runner invocations when the userConfig is set. Consumers who configure active_profile: "acme" get the default profile instead.
Fix: Add to SKILL.md Profiles section (after the existing resolution ladder description):
When ${user_config.active_profile} is set, prepend AI_BRIEFING_PROFILE=${user_config.active_profile} to all runner and build script invocations.
BUCKET_ORDER places real-world (index 11) before extras (index 12). A heading like ## EXTRAS (Robotics) normalizes to h = "extras (robotics)". The robotics branch matches, returning "real-world" — EXTRAS content populates the Real-world AI section and the EXTRAS section is never filled. This corrupts output for the advertised ## EXTRAS (Robotics) form.
Fix: Guard against headings that start with extras:
execution-flow.md:16 still shows node scripts/per-profile-runner.js detect-resume. runner-architecture.md uses node per-profile-runner.js <subcommand> at lines 41, 167, 194, and 226. After the ESM staging fix, all invocations must use the staged copy at ${CLAUDE_PLUGIN_DATA}/runtime/scripts/per-profile-runner.js — a bare relative path fails in an installed plugin and misses node_modules. Claude reads both files during an active briefing run.
Fix: Replace all bare node scripts/per-profile-runner.js and node per-profile-runner.js occurrences in both files with node "${CLAUDE_PLUGIN_DATA}/runtime/scripts/per-profile-runner.js".
No plugin code changed in 729c19a. Six issues from prior review cycles remain open. The two 🔴 items are correctness gaps that affect every run (status accounting, follow-list fallback). The remaining four are functional or doc gaps worth resolving before this lands in the public marketplace.
Priority
Finding
File
🔴
partial → complete promotion hides stage failures
per-profile-commands.js:346
🔴
Named profile skips default curated follow-list
per-profile-following.js:15
🟡
cutoff_reached always false
per-profile-validate.js:8
🟡
active_profile userConfig not wired to AI_BRIEFING_PROFILE
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Did not approve: Cursor Bugbot did not complete (check skipped due to usage limit), so the required automated review signal is unavailable. Human review is needed before merge; no reviewers were assigned because the PR author is the sole human repository contributor.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…findings (#156)
Post-publish behavior fixes for the `ai-briefing` plugin, surfaced by
bot review on the publish PR (#122). All were pre-existing engine
characteristics carried over in the migration; none block plugin
load/validate.
## Ships
- **Wire `grok-capture` (Wave 0 S0).** Registered `cmdGrokCapture` in
the runner dispatch (thin adapter over `grok-capture-agent.js`). `init
--grok-preload` now probes the Grok CLI and sets `config.grok_preload` /
`grok_preload_requested` — degrades to `grok_degraded` on a
missing/unsigned CLI, or hard-fails with `--require-grok`. `next-handle`
surfaces `grok_preload`; S3 `synthesize` merges Grok captures with
Chrome (`mergeTweetLists`, Chrome URLs win on dedup). Non-blocking:
preload-disabled or a capture error records `S0_grok_capture: skipped`
and exits 0 so Chrome Wave 1 always proceeds. Added
`grok-wave0-orchestration.test.js` to the `scripts` test script and
un-deferred the docs (SKILL.md standing default #20,
`references/runner-architecture.md`).
- **`source_type` no longer hard-coded `"twitter"`** in `state.js`
`appendToSeenItems` — honors `item.source_type ?? "twitter"`. This
runner is the Wave-1 X (twitter) loop, so the default stays correct for
its own items; the change generalizes the writer so the
main-session-driven RSS/Perplexity/GitHub waves (which append to the
same store per the documented `twitter|blog|changelog|github` schema)
can record their own source. No other in-plugin caller sets a
non-twitter source today — the threading is the forward-compatible fix,
deliberately, not a missed one.
- **Single model constant.** `lib/models.js` `BRIEFING_AGENT_MODEL`;
`synthesize-agent.js` + `categorize-agent.js` route through it (one edit
to bump).
- **`output/build/validate.js`** wraps `loadSlidesData()` in try/catch —
prints "run `emit-slides-data.js` first" and exits non-zero instead of a
raw dynamic-import stack.
- **JS-string injection hardening** in `chrome-extract.js`
`buildExtractorJs`: `handle` + `cutoffIso` are escaped for single-quoted
JS-string context (`\` then `'`) before substitution, via function
replacers so a `$` in the value can't form a replacement pattern.
Bumps plugin `version` 0.2.0 → 0.3.0 (behavior change → consumers
receive it on `/plugin marketplace update`).
## Verification
- `scripts` suite: `npm test` — all green incl. new
`grok-wave0-orchestration.test.js` (env-isolated from leaked
`CLAUDE_PLUGIN_DATA`).
- `output/build` suite: `node --test` — 2 pass.
- S3 Chrome-wins merge verified directly (dry-run skips the merge path).
- Escaping verified against a malicious `evil'];window.__pwned=1;//`
handle; backslash-doubling confirmed.
- `validate.js` diagnostic path exercised (no `slides-data.js` → clean
message, exit 1).
- `claude plugin validate plugins/ai-briefing --strict` — passed.
Refs melodic-software/medley#1457
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches browser-injected extractor JS and optional external Grok CLI
orchestration; failures are designed to degrade without blocking Chrome
Wave 1, but S3 merge semantics and seen-items `source_type` affect dedup
and downstream briefing content.
>
> **Overview**
> **ai-briefing 0.3.0** wires optional **Grok Wave 0** into the
per-profile runner and folds in several publish-review fixes.
>
> **Grok Wave 0** registers `grok-capture` in the runner. `init
--grok-preload` probes the Grok CLI, sets `config.grok_preload` /
`grok_degraded`, and supports `--require-grok` for hard failure.
`cmdGrokCapture` is non-blocking (skip + exit 0 on disabled preload or
errors). S3 **synthesize** merges Grok and Chrome posts via
`mergeTweetLists` (Chrome wins on URL dedup). Docs and CLI usage drop
the “not yet wired” deferral; tests add
`grok-wave0-orchestration.test.js`.
>
> **Other behavior changes:** `appendToSeenItems` uses `item.source_type
?? "twitter"` for forward-compatible RSS/GitHub waves.
**`BRIEFING_AGENT_MODEL`** in `models.js` centralizes the S3/S4 `claude
-p` model. **`buildExtractorJs`** escapes handle/cutoff for
single-quoted JS literals. **`validate.js`** fails clearly when
`slides-data.js` is missing.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c602645. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Summary
Publishes a generic, repo-agnostic
ai-briefingengine plugin, migrated from the medley in-repo skill. Ships the engine only — no employer/personal layers.retro/search/driftactions, plus the per-profile runner and the slides/HTML/PDF build pipeline.following-list.json, an optionalbrand.jsoverlay, and an optional stack lens resolve from.claude/ai-briefing/[<profile>/], falling back to a bundled neutralseed/following-list.jsonof public vendor accounts.${CLAUDE_PLUGIN_DATA}; curated config never does. In-repo runs fall back to the skill tree so the engine and its tests work outside a plugin install./ai-briefing:setup— re-runnable action that scaffolds a profile and installs runtime dependencies under${CLAUDE_PLUGIN_DATA}(the plugin cache is read-only).Verification
claude plugin validate --strictpasses on both the plugin and the catalog manifest.--plugin-dirin a clean non-source repo; both/ai-briefing:ai-briefingand/ai-briefing:setupare discovered.Notes
node_modulesand runtime-state paths are gitignored; deps install at runtime under${CLAUDE_PLUGIN_DATA}.Refs melodic-software/medley#1442
Note
Medium Risk
Large new surface (browser scraping, external APIs, Playwright/PPTX build) and reliance on
${CLAUDE_PLUGIN_DATA}for deps/state; behavior is skill-orchestrated rather than a single hardened service boundary.Overview
Adds the
ai-briefingmarketplace plugin — a repo-agnostic engine for aggregating AI-industry news into ranked briefings and optional presentation decks.Catalog & packaging: Registers
ai-briefinginmarketplace.jsonand documents it in the rootREADME. Shipsplugin.jsonwith anactive_profileuser option and.gitignorerules so runtime deps and per-run state stay out of git.Skills:
/ai-briefing:ai-briefingdrives multi-wave collection (Chrome/X, Perplexity, RSS, GitHub releases, optional Grok), dedup, 13-bucket categorize/rank, meeting-window merge, andretro/search/driftactions./ai-briefing:setupscaffolds a consumer profile under.claude/ai-briefing/and installs runtime Node deps under${CLAUDE_PLUGIN_DATA}.Named-profile seam: Curated
following-list.json, optionalbrand.js, and stack lens live in the project profile (with bundled neutral seed); machine state (seen-items, runs, decks) persists per profile in the plugin data directory.Build pipeline: In-tree Node ESM flow — parse briefing markdown →
slides-data.js→ PPTX (pptxgenjs), single-file scroll-snap HTML, and PDF (Playwright), with Zod validation, provider logos, and overridable default brand/ranking defaults.Reviewed by Cursor Bugbot for commit 729c19a. Bugbot is set up for automated code reviews on this repo. Configure here.