Skip to content

fix(discovery): explorer reads the skill body from disk on a preload miss and reports preload provenance - #4075

Open
kyle-sexton wants to merge 1 commit into
mainfrom
discovery-explorer-preload-miss-and-arti
Open

fix(discovery): explorer reads the skill body from disk on a preload miss and reports preload provenance#4075
kyle-sexton wants to merge 1 commit into
mainfrom
discovery-explorer-preload-miss-and-arti

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #4074

Summary

A dispatched discovery:explorer whose skills: preload silently failed had one instruction: set preload_token: MISSING and stop. Nothing told it where the token lives or to read the skill body from disk, and its payload had no way to say how the body reached it. The research and trace-intent lanes already carry that contract; the explore lane did not, so a preload miss on the explorer produced an undisciplined run that wrote an artifact shaped like the dispatch prompt's description of the deliverable instead of EXPLORE.md plus sidecars.

Fix

  • agents/explorer.md: before any exploration work the agent confirms the skill body is in context and otherwise Reads ${CLAUDE_PLUGIN_ROOT}/skills/explore/SKILL.md; the scope, slice path, and reason still come only from the dispatch prompt. The payload gains preload: fired | fallback, a matching token is file-identity rather than preload proof, and a token found by Reading is never reported as fired. MISSING plus status: truncated remains the outcome when neither route delivered the body.
  • skills/explore/SKILL.md: the sentinel paragraph says the disk fallback Reads the same file; the hard-failure paragraph names preload: fired | fallback as provenance with fallback the accepted recovery; gate step 1 requires the field, treats a missing or unrecognized value as an out-of-date agent definition, and does not read a matching token as proof that preload fired.
  • skills/explore/reference/dispatch.md: new "Discipline liveness — why a token at all" section, ported from the research lane's dispatch contract with explore nouns; the parent MUST NOT infer fired from the token.
  • skills/explore/evals/evals.json: eval 13, matching-token-is-file-identity-not-preload-proof, a preload: fallback run the parent accepts.
  • scripts/contract.test.sh section 11: nine explorer-lane assertions beside the research and trace-intent blocks (token absent from the agent definition, disk fallback named, preload: fired in the payload, the never-report-a-Read-token-as-fired rule, the skill's file-identity and structured-field statements, the gate's out-of-date classification, the dispatch reference's MUST NOT infer, and the eval).
  • discovery 0.19.9, CHANGELOG Fixed entry.

No script behavior changes; check-dispatch-artifact.sh grades the disk and is untouched. The parent envelope in reference/parent-contract.md is unchanged.

Verification

  • Red then green: the nine new contract.test.sh assertions failed against the unchanged files (eight FAIL lines, the token-absence check passing as expected) and pass after the edits; every pre-existing assertion passed throughout.
  • bash plugins/discovery/scripts/contract.test.sh: all contract assertions passed.
  • bash plugins/discovery/agents/tool-honesty.test.sh: all tests passed.
  • scripts/affected-tests.sh --explain selects six suites for the seven files (the other 228 in its listing are the documented hub over-selection through dispatch.md, evals.json, and changed-files.sh); --run is a Linux gate per the script header, so the six ran by hand on this Windows host: plugins/discovery/scripts/contract.test.sh, plugins/discovery/agents/tool-honesty.test.sh, plugins/discovery/scripts/check-dispatch-artifact.test.sh, and scripts/check-changed-skills.test.sh (PASS=19) all pass; scripts/check-stale-base-overlap.test.sh (3) and scripts/lib/changed-files.test.sh (1) fail on this host only, the first because the machine's git config forces GPG signing inside the suite's throwaway repo and the second on the CRLF pathname case the header lists, neither in a file this PR touches. CI's Linux lanes are the gate of record.
  • bash scripts/check-changed-skills.sh origin/main: explore PASS, 0 errors; the warnings are the pre-existing line-count and judgment-language advisories.
  • jq parses the evals file; 13 cases, the new one last.
  • Fresh-context review:code-reviewer pass over all seven files against the research and trace-intent lanes: no contradictions, no scope-leak reading of the fallback, every new assertion confirmed to fail on the pre-change files, no rule violations in the bodies, changelog factual. Two advisory notes, both left as is: the intro paragraph carries no dedicated test pin (scene-setting; the operative sentences are pinned), and eval 13's open-questions expectation is the same extrapolation research eval 19 makes.
  • Against the item's symptom: in agents/explorer.md the Read instruction sits in "Preload liveness — the first thing you do" (line 84), above every exploration section, and the intro paragraph points there first.

Related

  • Handoff-inbox item 20260909-124500-discovery-explorer-preload-miss-and-artifact-shape (local queue, not in this repository).
  • Parity sources: agents/researcher.md, agents/intent-tracer.md, skills/research/SKILL.md, skills/research/context/dispatch.md.
  • Deferred, Kyle's call: whether the parent should surface preload: fallback runs to the operator as a signal that preload is failing on the machine. No lane does today.

🤖 Generated with Claude Code

…miss and reports preload provenance

A dispatched discovery:explorer whose skills: preload failed silently
had one instruction: set preload_token MISSING and stop. Nothing told
it where the token lives or to Read the skill body from disk, and its
payload had no field to say how the body reached it. The research and
trace-intent lanes already carry that contract; the explore lane did
not, so a preload miss on the explorer ran undisciplined and wrote an
artifact shaped by the dispatch prompt instead of EXPLORE.md plus
sidecars.

agents/explorer.md now confirms the body is in context before any
exploration work, Reads skills/explore/SKILL.md otherwise, and reports
preload: fired | fallback; a token found by Reading is never reported
as fired, and MISSING plus status: truncated remains the outcome when
neither route delivered the body. skills/explore/SKILL.md demotes a
matching token to file-identity, grades preload: in gate step 1, and
treats a payload without the field as an out-of-date agent definition.
reference/dispatch.md carries the rationale. contract.test.sh section
11 pins the explore lane beside the other two, and the explore evals
gain the fallback-recovery case. discovery 0.19.9.

Closes #4074

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 11, 2026 05:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T05:14:06.200945Z 5279a9f Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

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


Claude finished the code review

  • Gather context (PR diff, files, custom instructions)
  • Invoke /review:code-review and follow its instructions
  • Check parity against agents/researcher.md, skills/research/context/dispatch.md, skills/research/evals/evals.json
  • Verify contract.test.sh section 11 assertions against the actual file contents
  • Post final summary

Review

Scope: this is the CODE-REVIEW lane; security review is scoped out (claude-security-review.yml exists in this repo). PR is open and not a draft, so the skip gate does not apply.

I read all seven changed files (agents/explorer.md, skills/explore/SKILL.md, skills/explore/reference/dispatch.md, skills/explore/evals/evals.json, scripts/contract.test.sh, CHANGELOG.md, plugin.json) and cross-checked them against the two parity sources named in the PR body (agents/researcher.md, skills/research/context/dispatch.md, skills/research/evals/evals.json).

No blocking findings. Specifically verified:

  • The preload: fired | fallback field, the file-identity/not-proof language, and the "MUST NOT infer fired" rule in explore/reference/dispatch.md are faithful ports of the research lane's existing wording (nouns changed, no verification: pending/discipline logic dropped or altered) — dispatch.md#L15-L39 vs. research/context/dispatch.md lines 74-97.
  • agents/explorer.md does not embed the literal token (discovery-explore-preload-8e2b7d appears only in SKILL.md, the eval, and contract.test.sh), matching the same pattern used for researcher.md/intent-tracer.md.
  • All nine new contract.test.sh section-11 assertions (contract.test.sh#L271-L291) grep for strings that are actually present verbatim in the target files — I checked each pattern against the current file contents rather than trusting the PR's "red then green" claim.
  • evals/evals.json is valid JSON with 13 cases (jq empty + jq '.evals | length'), new eval 13 is well-formed and consistent with the new gate rule.
  • CHANGELOG.md entry and the 0.19.80.19.9 bump in plugin.json accurately describe the diff; no other version pins reference this plugin (marketplace.json's discovery entry points at ./plugins/discovery, unpinned).
  • No stale references to the old "preload_token: MISSING and stop" behavior remain elsewhere (reference/parent-contract.md doesn't mention preload_token at all; the only files touching preload_token/preload: are the ones in this diff plus the pre-existing research/trace-intent lanes).

One pre-existing wrinkle I looked into but am not flagging: existing (unchanged) evals 8/9/10 in explore/evals/evals.json describe a "well-formed payload" without including the new preload: field. That reads odd against the new gate rule at first glance, but the identical gap exists verbatim in the research lane's own eval 13 (research/evals/evals.json line 188, the exact eval this PR's new eval 13 is modeled on), so it's a pre-existing convention in the parity source this PR intentionally matches, not a regression this PR introduces. Not a blocking finding, and it doesn't sit on a changed line.

I was not able to execute bash scripts/contract.test.sh or gh pr view directly in this sandbox (only gh pr diff was permitted), so the PR's own "red then green" and bash .../contract.test.sh: all pass claims are author-reported and unverified by me directly — however I independently confirmed every new assertion's target string is present in the current files, which is the same thing the script checks.

@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 49s —— View job


Security review — PR #4075

  • Gather context (PR diff, changed files)
  • Invoke /review:security-review skill criteria
  • Post findings

Result: no security issues found.

All seven changed files are prompt/contract/documentation surface for the discovery plugin's
explorer agent-dispatch lane, plus a test script and a version/changelog bump:

  • agents/explorer.md, skills/explore/SKILL.md, skills/explore/reference/dispatch.md — agent
    instructions and provenance-field rules for a preload-miss fallback (the agent Reads
    ${CLAUDE_PLUGIN_ROOT}/skills/explore/SKILL.md, a fixed path built only from the harness-supplied
    plugin root, no external or user-controlled input).
  • scripts/contract.test.sh — new grep/assert_present checks against hardcoded literal
    strings, no dynamic input.
  • skills/explore/evals/evals.json, .claude-plugin/plugin.json, CHANGELOG.md — eval fixture and
    metadata/version changes.

There is no new code-execution path, no injection surface (command/SQL/path/template), no
credential or secret handling, and no authorization/access-control logic in this diff. The
preload_token / preload: fired|fallback fields are a self-reported liveness sentinel between a
dispatching parent and a sandboxed subagent for artifact-discipline purposes, not an authentication
or trust boundary — the underlying "the subagent's self-report can be wrong" property is inherent to
the existing dispatch design and unchanged in kind by this PR (if anything, requiring the structured
preload: field and explicitly forbidding inferring fired from a matching token tightens the
existing gate rather than weakening it).

GitHub Actions hardening is out of scope for this lane (zizmor's advisory lane); this PR does not
touch any workflow files.

@github-actions

Copy link
Copy Markdown

Last security-reviewed head: 5279a9f3121b05f3612d07c5861c37d348aa3209. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5279a9f312

ℹ️ 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".

```yaml
preload_token: <echoed verbatim from the preloaded skill, or MISSING>
preload_token: <echoed verbatim from the skill file, or MISSING>
preload: fired # fired | fallback — how the skill body reached you; never inferred from the token

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Set preload provenance in early payloads

When a fallback run is interrupted after following the early-emission instructions at lines 237–242, that checklist tells it to set preload_token but omits preload:; copying this template can therefore leave the default fired value in place, while omitting it makes the newly strengthened gate reject the payload as out-of-date. This defeats the early payload's purpose and can report false provenance precisely on the preload-miss path, so require preload: to be set in the early-emission checklist as the researcher and intent-tracer definitions do.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

discovery/explorer: a dispatched explorer has no disk fallback on a preload miss and its payload carries no preload provenance

1 participant