fix(discovery): researcher must not assume skills preload succeeded

[kyle-sexton]

Closes #2338

Summary

The researcher agent no longer assumes agent skills: preload injected the research discipline.

Fix

Requires confirming the preload-liveness sentinel or Reading the skill before work, and mandates preload_token in the return payload.

Verification

• discovery 0.15.2 version bump and CHANGELOG entry

Related

N/A

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

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

---

Security Review

• Gather context (PR diff, changed files)
• Read plugins/discovery/agents/researcher.md changes
• Review plugins/discovery/.claude-plugin/plugin.json and CHANGELOG.md
• Analyze for security issues (injection, trust boundaries, token/secret handling, logic flaws)
• Post findings

This PR only touches plugin documentation/instruction files (an agent-definition prompt, a manifest version bump, and a changelog) — no executable code, workflows, or dependencies. Findings below are limited to the reasoning content of researcher.md, per the review scope.

---

IMPORTANT — Preload-liveness check is self-defeating (tautological): the sentinel is embedded in the always-loaded base prompt it's supposed to gate

Confidence: Medium

plugins/discovery/agents/researcher.md#L16-L29

The whole point of this PR is to stop the researcher agent from silently assuming skills: preload succeeded, by having it check for a "preload-liveness sentinel" string that only arrives if the skill body (SKILL.md) was actually injected:

Before any research work, confirm the preload-liveness sentinel is already in your context:

    discovery-research-preload-4c1f9a

If you do not see it verbatim, Read ${CLAUDE_PLUGIN_ROOT}/skills/research/SKILL.md ...

The problem: this exact sentinel string is now printed inside researcher.md itself, which is the agent's base system prompt — always loaded regardless of whether the skills: preload succeeded. That makes "confirm the sentinel is already in your context" trivially true on every run, preload success or not, because the very instruction performing the check quotes the value it's checking for. The check can no longer distinguish "the skill body preloaded" from "I'm just reading the sentence that names the sentinel."

This is exactly the failure mode the file itself warns about a few paragraphs later (the existing "Preload liveness" section): "An undisciplined run that still writes an artifact and still reports coverage: complete is indistinguishable from a good one at every other seam, which is exactly the failure the preload exists to prevent." By hard-coding the literal sentinel into the instruction text rather than only into the preloaded skill body, this fix reintroduces the same silent-bypass risk it's meant to close: a model that pattern-matches on "I see that string" (present in the base prompt regardless) rather than reasoning about where it came from can satisfy the gate without ever reading SKILL.md, and would go on to run undisciplined research (weaker source-tier/corroboration standards) while still emitting a valid-looking preload_token.

This is a prompt-engineering/behavioral-reliability issue rather than a classic injection or auth bug, but it's security-relevant because it's the exact bypass this PR sets out to close (issue #2338), and the failure is silent by design — the agent has no way to self-detect it got this wrong. Consider only stating the sentinel format/purpose in the base prompt without printing the literal token value there (keep the literal value solely in SKILL.md), or phrase the check to require corroborating preloaded discipline content (phase structure, tier tables) rather than the bare string.

Fix this →

---

No other security issues found. Specifically checked and clear:

• No secrets/credentials introduced — the sentinel is a liveness marker, not an authentication token, and its plaintext presence in the repo is by design.
• No new injection surface — ${CLAUDE_PLUGIN_ROOT} path interpolation is unchanged in pattern from existing usages elsewhere in the file and is not attacker-influenced.
• No changes to the file's "Untrusted-content posture" or write-boundary sections — fetched content is still treated as data, not instructions.
• No permission/trust-boundary widening in plugin.json (version bump only) or CHANGELOG.md (docs only).
• No GitHub Actions/workflow files touched by this PR.

Branch: cursor/discovery-researcher-preload-63e1

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 131a3dc91d

ℹ️ 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 3m 9s —— View job

---

Review: fix(discovery): researcher must not assume skills preload succeeded

• Gather context (diff, CLAUDE.md, REVIEW.md)
• Review changed files for correctness and alignment
• Post findings

Scope: plugins/discovery/agents/researcher.md, plugins/discovery/CHANGELOG.md, plugins/discovery/.claude-plugin/plugin.json (this repo has a claude-security-review workflow, so this lane omits security findings per REVIEW.md).

Verified independently: the sentinel string discovery-research-preload-4c1f9a is consistent across agents/researcher.md, skills/research/SKILL.md, and skills/research/evals/evals.json — no drift introduced. Version bump (0.15.1 → 0.15.2) and CHANGELOG entry follow the existing format in this file.

Findings

1. Ambiguous deferral scope in the new fallback instruction (agents/researcher.md lines 26–30) — inline comment posted on the line. The new sentence groups "Read SKILL.md" together with "the discipline file it names" under one "at the phase that needs it rather than up front" deferral. Only the sibling discipline file was meant to be deferred in the original text; SKILL.md itself carries the phase structure/outcome gate the agent needs before starting any phase. As phrased, an agent could defer reading SKILL.md itself — reintroducing an undisciplined run through a different door than the one this PR closes.

2. New preamble duplicates an existing, unedited section without cross-referencing it (🟡 maintainability/documentation). This PR adds preload-liveness handling at the top of the file (lines 16–30), but the file already had a section doing overlapping work, left untouched by this diff:
## Preload liveness — the first thing you do

That section pre-dates this PR (present in the parent commit 4a1184c) and already instructed the agent to echo preload_token or set it to MISSING and stop with status: truncated when no skill content arrived. Two effects worth a second pass:

• The heading "the first thing you do" is no longer accurate — the new top-of-file block (before the "dispatch prompt must carry these" section) is now the actual first check.
• The two sections describe recovery paths that don't obviously reconcile: the new section says to self-heal via Read and continue; the old section says to set MISSING and stop when "no skill content reached you." It's inferable that the old section covers the case where the Read fallback itself fails, but nothing states that relationship — and this file elsewhere is explicit that restatement without a single source of truth is how drift happens (line 118: "Read that table rather than a restatement of it; three restatements is how it drifted."). Worth folding these into one section, or having the new preamble point at the existing one instead of restating it.

Not reviewed (out of lane)

Security-relevant aspects (if any) are left to the claude-security-review lane per REVIEW.md.
· branch cursor/discovery-researcher-preload-63e1

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.