Skip to content

docs(plugin-audit-port): correct the context: fork rejection rationale - #1274

Merged
kyle-sexton merged 7 commits into
mainfrom
docs/1268-plugin-audit-port-fork-rationale
Jul 25, 2026
Merged

docs(plugin-audit-port): correct the context: fork rejection rationale#1274
kyle-sexton merged 7 commits into
mainfrom
docs/1268-plugin-audit-port-fork-rationale

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #1268

Summary

docs/topics/plugin-audit-port/ rejected context: fork for the deep-audit phase on the stated grounds that a fork "inherits the degraded history the gate exists to escape" (PLAN.md:78-79, and again in the alternatives table at :457).

That is false for context: fork. Per the skills reference (fetched 2026-07-24) a forked skill "won't have access to your conversation history" — it would satisfy the fresh-eyes requirement it was being rejected for.

The decision is unchanged. The real basis was already recorded in the same PLAN's [EXEC-SHAPE] agent decision at :550: plugin agents start with fresh context, and the Brief requires a named subagent, which a forked skill is not. Both prose sites now carry that basis, and design/design-resolution.md — which stated the rule with no rationale at all — gains it.

A wrong rationale in a design record propagates into later decisions that cite it, which is why the reason is corrected even though the outcome stands.

Test plan

  • grep -rn -i "degraded history" docs/topics/plugin-audit-port/ — no matches
  • The [EXEC-SHAPE] decision at :550 is unmodified; the corrected text points at it rather than restating it
  • Docs-only change; no plugin version or CHANGELOG bump applies

Related

One claim is deliberately attributed rather than asserted. History inheritance is documented for the Agent tool's separate fork subagent type. #1258 reports empirically that Agent-tool forks did not inherit the conversation, contradicting that doc. The corrected text therefore says "documented for", not "does". The correction here does not depend on how #1258 resolves — the named-subagent requirement is the load-bearing basis, and the skills reference settles the context: fork half independently.

Prior art, both closed: #1053 fixed the identical inversion in docs-hygiene's audit-derivability rubric and evals; #1062 clarified its Hard Rules wording. This is the same class of error in a different file, not a duplicate.

Adjacent, non-overlapping: #1252 also edits plugin-audit-port/PLAN.md, at :551 — the table row directly below the [EXEC-SHAPE] row this PR points at. It does not touch that row, and this PR does not touch :551.

Closes #1268

The deep-audit phase rejects `context: fork` on the stated grounds that a fork
"inherits the degraded history the gate exists to escape". That is false for
`context: fork`. Per the skills reference (fetched 2026-07-24) a forked skill
"won't have access to your conversation history" — it would satisfy the
fresh-eyes requirement it was being rejected for.

The decision is unchanged. The real basis was already recorded in the same
PLAN's [EXEC-SHAPE] agent decision: plugin agents start with fresh context and
the Brief requires a NAMED subagent, which a forked skill is not. Both prose
sites now carry that basis, and `design-resolution.md` — which stated the rule
with no rationale — gains it.

History inheritance is documented for the Agent tool's separate `fork` subagent
type. That attribution is stated as documented rather than as observed, because
#1258 reports Agent-tool forks not inheriting the conversation in practice
either. The correction does not depend on which way #1258 resolves: the
named-subagent requirement is the load-bearing basis, and the skills reference
settles the `context: fork` half independently.

A wrong rationale in a design record propagates into later decisions that cite
it, which is why the reason is corrected even though the outcome stands.
The same inversion was already fixed once in docs-hygiene under #1053.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read changed files and gather context
  • Verify corrected rationale against skills reference
  • Check internal consistency between the two changed files
  • Confirm no remaining instances of the old false claim
  • Post final review

Verdict: Approve with one advisory note.

The correction is accurate and the two changed files are internally consistent. Both carry the right load-bearing reasons now, and the [EXEC-SHAPE] decision row at PLAN.md:551 — which this PR defers to — already stated both grounds correctly before this change.

What the diff does well

Constraints section (PLAN.md:78-80)
The two-part replacement is tight: plugin agents already start fresh (so context: fork adds nothing), and the Brief requires a named subagent (which a forked skill cannot provide). Pointing at [EXEC-SHAPE] rather than restating the full decision is the right call — the authority lives there, and a pointer rots less than a copy.

Alternatives table (PLAN.md:458)
The cell is long for a table, but the length is earned. It explicitly separates what the actual basis is from what it is not, flags the skills-vs-Agent-tool fork distinction, and cites #1258 without depending on its resolution. The "deliberately attributed rather than asserted" discipline from the PR description is visible in the text: "documented for" vs "does."

Design resolution (design-resolution.md:25)
The rationale that was absent before is now present, and "both start with fresh context" correctly explains why context: fork is also redundant, not just disqualified by the naming requirement.

One advisory note

PLAN.md:368 still reads:

Never a fork (fresh-eyes doctrine).

This is a third site in the same file that cites the rejection basis — but here it is without the corrected rationale. The "fresh-eyes doctrine" as defined in docs/PLUGIN-PHILOSOPHY.md:392–394 explicitly grounds the fork exclusion in history inheritance ("A fork does not: it inherits the parent session's full conversation history"), which is the claim this PR corrects. A future reader of this line could trace the cite back to that claim and arrive at the same wrong conclusion the PR is fixing.

This site is in the agents/auditor.md creation step (phase B3), so contextually "never a fork" refers to the Agent-tool fork type in the auditing-agent dispatch scenario — distinct from the skill context: fork option in the alternatives table. That distinction is real, but it is not visible at line 368. The line was pre-existing before this PR, so it is out of scope here; recording it in case a follow-up addresses this file's remaining ambiguity is sufficient.

Test plan verification

  • grep -rn "degraded history" docs/topics/plugin-audit-port/ — no matches confirmed by my search
  • [EXEC-SHAPE] row at :551 is unmodified
  • No version or CHANGELOG bump needed (docs-only)
  • The #1258 forward-reference is hedged correctly; this PR's correctness does not depend on that issue's resolution

No functional changes; no security surface; no plugin manifest edits. Ready to merge.

@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: e179e2f8c8

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

Comment thread docs/topics/plugin-audit-port/PLAN.md Outdated
…tating it

Review finding on #1274. The "why not context: fork" rationale had grown a
third and fourth independent wording — the alternatives-table cell and
design-resolution.md's design-thread map each stated their own version, and
design-resolution.md's had already drifted, asserting "both start with fresh
context" which the EXEC-SHAPE basis does not say.

Both now point at the `[EXEC-SHAPE]` plugin-agent decision, which is the single
home for that basis. Per the topic-docs single-home rule and point-dont-copy, a
reworded paraphrase drifts exactly as a verbatim copy does — this PR was
creating the drift it exists to correct.

The mechanism disambiguation stays in the alternatives cell, because that cell is
where the false claim lived and a reader scanning rejected options needs to see
which "fork" was meant.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): point at the EXEC-SHAPE basis instead of restating it

  • Read changed files and gather context
  • Evaluate Codex reviewer's challenge to the named-subagent rationale
  • Check docs/PLUGIN-PHILOSOPHY.md:125 for context: fork + agent pairing claim
  • Check new commit (df3d064) delta against prior reviewed commit (e179e2f)
  • Post final review

Verdict: Approve. The Codex challenge does not land; the second commit addresses my prior advisory.

Second commit (df3d064)

This commit was made in response to my prior review's observation that the alternatives-table cell and design-resolution.md were restating the rationale rather than pointing at [EXEC-SHAPE]. Both sites now point at the decision row. The mechanism disambiguation (which "fork" was meant) correctly stays in the alternatives table because that is where the false claim lived and where a reader scanning rejected options would look.

design-resolution.md's prior restatement "both start with fresh context" was already drifted from the actual [EXEC-SHAPE] basis (which does not say that); the pointer is tighter and rot-resistant.

This is the right approach per the topic-docs single-home rule: point, don't copy.

Codex reviewer challenge

When context: fork is combined with agent: auditor, the skill runs in isolated context using that named custom subagent; the repository itself records the context: fork + agent pairing in docs/PLUGIN-PHILOSOPHY.md:125.

This does not land. PLUGIN-PHILOSOPHY.md:125 lists newer skill frontmatter fields that the repo has adopted case-by-case:

paths, context: fork (+ agent), arguments, skill-scoped hooks with once

The (+ agent) parenthetical notes that agent is a companion frontmatter field that can appear alongside context: fork. It is not a claim that the combination produces a named subagent. A skill running with context: fork + agent frontmatter is still the skill dispatching with a specified agent type — it is not the same as an agents/auditor.md plugin agent defined in the agents directory and dispatched by name.

The [EXEC-SHAPE] decision at PLAN.md:551 is explicit: "Brief requires a NAMED subagent, never a fork" — and "SKILL.md step 2 dispatches by agent name." A context: fork skill, with or without an agent frontmatter field, cannot be dispatched as a named agent from agents/. That is what the requirement means.

The Codex challenge conflates two different mechanisms that share vocabulary (agent as a frontmatter field in a skill vs. a named agent definition in agents/). The PR's named-subagent basis is not undermined.

Unchanged advisory (pre-existing, out of scope)

PLAN.md:368 — "Never a fork (fresh-eyes doctrine)" — is unmodified by this PR. As noted in my prior review, this line is in the agents/auditor.md creation step where "fork" means the Agent-tool fork subagent type, not the skills context: fork. PLUGIN-PHILOSOPHY.md:392–394's claim about history inheritance is correct for Agent-tool forks; the ambiguity is which mechanism a future reader understands "fork" to mean. Still pre-existing and out of scope.

Test plan verification

  • grep -rn "degraded history" docs/topics/plugin-audit-port/ — no matches
  • [EXEC-SHAPE] row at :551 unmodified
  • Docs-only; no version or CHANGELOG bump needed
  • #1258 forward-reference hedged correctly; this PR's correctness does not depend on that issue's resolution

No functional changes, no security surface, no plugin manifest edits. Ready to merge.

@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: df3d0648e8

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

Comment thread docs/topics/plugin-audit-port/PLAN.md Outdated
kyle-sexton and others added 2 commits July 24, 2026 22:06
… fork

The audit skill told every invocation that a fork "would inherit this
session's degraded history". A skill's `context: fork` frontmatter does the
opposite: the forked subagent starts with no conversation history at all.
Conversation inheritance belongs to the Agent tool's separate `fork` subagent
type. Step 2 now names that type explicitly and also forbids running inline,
which is the failure mode the sentence was reaching for. The skill-lens
composition question likewise stops treating a forked sub-skill's lack of
history as a defect and asks whether the choice fits the step.

The plan's rejection rationale was also unsupported. It claimed `context: fork`
is anonymous, but the skills frontmatter `agent` field selects the subagent type
for a forked skill, and discovery's explore-deep ships that pairing. Neither
"fresh context" nor "named" discriminates, so neither can be the basis.

The real basis is topological: `context: fork` forks the whole skill, and only
steps 2-3 want a fresh context. Step 1 reads this session's own conversation to
capture evidence, and steps 4 and 6 need user-interactive surfaces a forked
skill does not get. A skill cannot fork one of its own steps. That basis now has
one home under the [EXEC-SHAPE] agent decision, self-standing rather than
circularly deferring to the Brief, with the Brief, the alternatives row, phase
B3, and design-resolution pointing at it.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…context note

Fourth site carrying the ambiguity. `agents/auditor.md` said it has no
conversation history "by design", leaving the reader to infer why. It now states
the mechanism — a named subagent, not a conversation fork — so the sentence
cannot be read as a claim about `context: fork`.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read changed files and gather context
  • Review new commits (d70ee7a, 73abd75) addressing Codex second review
  • Check SKILL.md for removal of false claim about history inheritance
  • Check agents/auditor.md for the fork disambiguation
  • Verify PLUGIN-PHILOSOPHY.md consistency
  • Post final review

Verdict: Approve. All previous review concerns resolved; the PR is complete.

Third commit (d70ee7a) — stop attributing history inheritance to context: fork

This addresses the Codex second review's finding that SKILL.md:127-128 still carried the false claim.

Before: "A fork would inherit this session's degraded history — the fresh-eyes doctrine is the point."

After: "Never run this step inline in the main thread, and never dispatch it as the Agent tool's fork subagent type — that fork inherits this session's conversation and carries its degraded evidence forward."

This is the correct distinction. The mechanism that inherits history is the Agent tool's fork subagent type (the sub-agents page's subject). The skills context: fork starts the subagent blank. The new text names the right mechanism and gives the correct reason for excluding it.

The composition lens in references/component-types/skill.md follows the same correction — it no longer asserts that forking "loses history" as a defect, instead asking whether the inline-vs-context: fork choice matches what the step needs. That is the right question for an audit lens.

Fourth commit (73abd75) — name the mechanism in the auditor agent

agents/auditor.md now reads: "you are a named subagent, not a conversation fork". This resolves my prior advisory from the second review: the ambiguous "by design" is gone and the mechanism is explicit. A reader can now correctly infer that the fresh-context property comes from being dispatched as a plugin agent, not from anything fork-related.

New [EXEC-SHAPE] basis subsection in PLAN.md

The most significant addition. The prior two commits pointed every rejection site at [EXEC-SHAPE] without expanding that row; this commit delivers the full basis those pointers promised. The three-reason topological argument is sound:

  1. context: fork forks the whole skill body — steps 1, 4, and 6 would go to the subagent with it, which is the opposite of what the design requires.
  2. Step 1 can't run in a forked skill — evidence capture is conversation history (hook failures, permission denials, anomaly that triggered the audit); a fork that starts blank destroys the audit's only input.
  3. Steps 4 and 6 need user-interactive surfaces — the AskUserQuestion claim here is the most specific and is attributed to skills + sub-agents docs verified 2026-07-24. This point is not load-bearing: points 1 and 2 alone establish the topological incompatibility.

The section also correctly handles the Codex first review's context: fork + agent pairing challenge — it acknowledges the pairing is real and would be fresh-context + named-agent-type, then explains why that still doesn't work (topology, not naming or freshness). The discovery plugin's explore-deep skill is cited as evidence the pairing is legitimate, which makes the rejection rest on topology rather than mechanism unfamiliarity.

PLUGIN-PHILOSOPHY.md consistency

Unchanged and already accurate. The fresh-eyes doctrine at line 392–394 specifically cites [subagents] (the Agent tool's fork mechanism) and says "A fork does not: it inherits the parent session's full conversation history." That claim is correct for the Agent tool's fork subagent type. The PLAN.md changes now correctly attribute the doctrine's scope to that mechanism rather than misapplying it to skills context: fork.

Plugin manifest and CHANGELOG

  • Version 0.1.0 → 0.1.1: correct patch bump for bug fixes in shipped behavior.
  • CHANGELOG correctly categorizes all three fixes under Fixed and describes each accurately.

Test plan verification

  • No matches for "degraded history" anywhere in plugins/plugin-quality/ — confirmed by diff inspection.
  • [EXEC-SHAPE] row is modified (expanded) consistently with the surrounding pointer text; no pointer is now dangling.
  • Docs-only design record + plugin-source fix; no external dependencies changed; no version bump required on the docs/ side.
  • #1258 forward-reference present and correctly hedged; this PR's correctness does not depend on that issue's resolution.

No functional changes, no security surface, no manifest schema violations. Ready to merge.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


Security Review — docs(plugin-audit-port): correct the context: fork rejection rationale

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

No security issues found.

This PR is a documentation correction with a corresponding patch-version bump. All seven changed files were reviewed against the diff:

File Change Security surface
docs/topics/plugin-audit-port/PLAN.md Corrects fork-rationale text; adds [EXEC-SHAPE] basis section None — prose only; URLs cite code.claude.com/docs/en/ (trusted first-party domain)
docs/topics/plugin-audit-port/design/design-resolution.md Updates one table cell None — prose only
plugins/plugin-quality/.claude-plugin/plugin.json "version": "0.1.0""0.1.1" None — patch bump, no new dependencies or permission fields
plugins/plugin-quality/CHANGELOG.md Adds 0.1.1 entry None
plugins/plugin-quality/agents/auditor.md Rewrites one sentence explaining why the agent starts with no history None — existing tool grants (Read, Grep, Glob, WebFetch, Bash, Write), Bash scope restrictions, Write path restriction (evidence-packet directory only), and prompt-injection defense are all unchanged
plugins/plugin-quality/skills/audit/SKILL.md Replaces false fork-inheritance claim with accurate mechanism description None — no new dispatch paths or execution patterns
plugins/plugin-quality/skills/audit/references/component-types/skill.md Updates composition-check rubric None — prose only

No code execution paths, file-access patterns, network-access surfaces, permission grants, GitHub Actions workflows, dependency pins, or secrets were added or modified. The auditor.md agent's untrusted-content posture (prompt-injection defense) is unaffected by the wording change.

@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: 73abd759b7

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

Comment thread docs/topics/plugin-audit-port/PLAN.md Outdated
Comment thread plugins/plugin-quality/skills/audit/SKILL.md Outdated
…, not as fork behavior

Review findings on #1274. Two more rationales fell.

Three successive bases for rejecting `context: fork` have now each been refuted:
it inherits degraded history (false — a forked skill has no conversation access);
a forked skill is anonymous (false — the `agent` frontmatter field names the
subagent type); a forked sibling cannot receive per-run inputs (false —
`$ARGUMENTS` is that channel, and `plugins/discovery/skills/explore-deep/SKILL.md`
ships the pattern).

Every one of them argued from what fork does. The requirement does not depend on
that, so it is stated positively instead: the deep phase needs a context that
provably excludes this session's evidence, and a named dispatch target that makes
the dispatch site auditable. `agents/auditor.md` supplies both and already ships.

`context: fork` is therefore no longer rejected as impossible — it is rejected on
cost, because it needs a second skill existing only to be forked, spending shared
skill-listing budget (#1271 measures that budget and the silent description drops
it causes) for zero user-facing capability.

The shipped step-2 instruction no longer asserts that the Agent-tool fork inherits
the conversation. That claim is contested by #1258, and a shipped prompt should
not state a contested empirical claim as settled — especially not one it uses to
forbid an execution mode. The invariant holds either way #1258 resolves.

Out of scope, recorded: `plugins/planning/skills/audit-answers/SKILL.md:49,96`
carry the same fork-inheritance assertion. Different plugin, and fixing it here
would widen this diff and pull in a second version bump.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read changed files and gather context
  • Review latest commit (dac51f3) against prior approved commits
  • Check the Codex third review's $ARGUMENTS challenge
  • Verify internal consistency of the new changes
  • Post final review

Verdict: Approve. The fifth commit answers all three Codex challenges; the PR is complete.

What dac51f3 changes

The commit concedes the Codex third review's $ARGUMENTS challenge and the remaining SKILL.md concern, then restates the design requirement as a positive invariant so no further rationale can be defeated.

PLAN.md — alternatives table (:460)
"Cannot express the audit's step topology" → "Not impossible — rejected on cost." This is the correct move. The Codex reviewer was right that $ARGUMENTS closes the per-run input channel and explore-deep ships the pattern; continuing to deny it would have been wrong. The cost basis (second skill existing only to be forked, skill-listing budget, two-file split requiring sync) is a valid and independently verifiable reason that does not depend on what any fork inherits.

PLAN.md — [EXEC-SHAPE] invariant subsection (:591–613)
The three defeated rationales are now recorded explicitly and correctly labeled as defeated. The positive restatement — fresh context + named dispatch target — is then applied to the sibling-skill proposal, where the agent: + context: fork combination satisfies both "arguably" (hedged appropriately per the second Codex review's agent-type-vs-plugin-agent distinction) but still loses on cost. The "arguably (2)" hedge keeps the text from re-asserting the disputed equivalence while still advancing the argument.

The three topological points that precede the invariant section (lines 576–589) are unmodified. They apply specifically to forking the main audit skill — not the sibling-skill approach — and remain accurate. A reader who asks "but what about a sibling?" finds the answer immediately below.

SKILL.md — step 2 dispatch instruction (:127–132)
The previous text ("that fork inherits this session's conversation and carries its degraded evidence forward") was the Codex second review's concern — a contested empirical claim stated as settled in a shipped instruction. The new text removes the claim entirely and states the requirement positively: two properties the named agent supplies, with "any other mechanism must be justified against those two properties — not against what a fork does or does not inherit, which is contested." This formulation is invariant to how #1258 resolves and gives future readers the correct framing without asserting contested behavior.

CHANGELOG and version (0.1.10.1.2)
Changed category is correct — this commit reformulates a framing, not just a bug fix, so it is distinct from the Fixed entries in 0.1.1. Patch bump is appropriate (shipped instruction corrected; no API surface change). The CHANGELOG entry accurately describes what changed and correctly notes the #1258 caveat.

Codex review scorecard

Codex challenge Commit it landed on Status after dac51f3
Named-subagent basis unsupported (context: fork + agent: pairing) e179e2f Prior Claude review established this was already addressed in df3d064; still correct
SKILL.md:127-128 still carried the stale fork-inheritance claim df3d064 d70ee7a removed it; dac51f3 further replaces the contested unconditional wording
$ARGUMENTS closes the sibling-input channel; explore-deep ships it 73abd75 Conceded in dac51f3; rejection now rests on cost, not impossibility

Out-of-scope, noted

The commit message records plugins/planning/skills/audit-answers/SKILL.md:49,96 carrying the same fork-inheritance assertion in a different plugin. Deferring is correct — fixing it here would cross a plugin boundary and pull in a second version bump. Worth a follow-up issue if the pattern needs systematic correction.

Test plan verification

  • grep -rn "degraded history" docs/topics/plugin-audit-port/ — no matches
  • grep -rn "degraded history" plugins/plugin-quality/ — no matches (SKILL.md no longer carries this)
  • [EXEC-SHAPE] row at :553 is unmodified; the basis subsection it points to is expanded but consistent
  • No functional changes, no security surface, no manifest schema violations, no new permissions
  • #1258 forward-reference hedged correctly; this PR's correctness does not depend on that issue's resolution

Ready to merge.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


Security Review — docs(plugin-audit-port): correct the context: fork rejection rationale

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

No security issues found.

All seven changed files were reviewed against the full diff. The PR is a documentation correction with a patch-version bump and carries no security surface.

File Change Security surface
docs/topics/plugin-audit-port/PLAN.md Corrects fork-rationale prose; adds [EXEC-SHAPE] basis subsection None — prose only; URLs cite code.claude.com/docs/en/ (trusted first-party domain, not fetched at runtime)
docs/topics/plugin-audit-port/design/design-resolution.md Updates one table cell None — prose only
plugins/plugin-quality/.claude-plugin/plugin.json "version": "0.1.0""0.1.2" None — patch bump; no new permissions, dependencies, tool grants, MCP servers, or remote URLs
plugins/plugin-quality/CHANGELOG.md Adds 0.1.1 and 0.1.2 entries None
plugins/plugin-quality/agents/auditor.md Rewrites one sentence in the agent's system prompt None — no new tool grants, no permission changes; the untrusted-content posture (audited plugin source is data, not instructions) is unchanged
plugins/plugin-quality/skills/audit/SKILL.md Replaces fork-inheritance claim with mechanism-accurate description of dispatch requirements None — no new dispatch paths, no new execution patterns
plugins/plugin-quality/skills/audit/references/component-types/skill.md Updates composition-check rubric None — prose only

Checked specifically:

  • No GitHub Actions workflows added or modified — no pull_request_target / workflow_run / github context injection risks.
  • No permissions: widening — no workflow or settings files touched.
  • No dependency pins loosened — no package files changed.
  • No secrets, tokens, or credentials introduced or referenced.
  • No shell scripts or run: blocks — no command injection surface.
  • No SQL, path traversal, or template injection surfaces.
  • No network egress paths — URLs in PLAN.md are documentation-reference prose, not runtime fetches.

@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: dac51f3e69

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

Comment thread plugins/plugin-quality/skills/audit/SKILL.md Outdated
…evidence packet

Review finding on #1274, and the invariant I wrote was self-contradicting.

It said the auditor's context "provably excludes this session's evidence". The
opposite is required: `agents/auditor.md`'s procedure opens by reading the evidence
packet as its ground truth, and step 1 exists solely to write that packet. The
chosen implementation violated the property the text used to reject alternatives.

What must not cross is the main thread's conversation history and prior reasoning
— the reasoning that produced the work under review, which a same-context
self-check cannot escape. The packet is the deliberate channel across the
boundary, which is also what makes the design compaction-proof and resumable.

Corrected at both sites: the shipped step-2 instruction and the plan's
`[EXEC-SHAPE]` basis.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


Security Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read PR diff and gather context
  • Analyze changed files for security vulnerabilities
  • Post findings

No security issues found.

All seven changed files were reviewed against the full diff. The PR is a documentation correction with patch-version bumps and carries no security surface.

File Change Security surface
docs/topics/plugin-audit-port/PLAN.md Corrects fork-rationale prose; adds [EXEC-SHAPE] basis subsection None — prose only; URLs cite code.claude.com/docs/en/ (trusted first-party domain, not fetched at runtime)
docs/topics/plugin-audit-port/design/design-resolution.md Updates one table cell None — prose only
plugins/plugin-quality/.claude-plugin/plugin.json "version": "0.1.0""0.1.2" None — patch bump; no new permissions, dependencies, tool grants, MCP servers, or remote URLs
plugins/plugin-quality/CHANGELOG.md Adds 0.1.1 and 0.1.2 entries None
plugins/plugin-quality/agents/auditor.md Rewrites one sentence in the agent's system prompt None — tool grants (Read, Grep, Glob, WebFetch, Bash, Write), Bash scope restrictions, Write path restriction (evidence-packet directory only), and untrusted-content posture (prompt-injection defense) are all unchanged
plugins/plugin-quality/skills/audit/SKILL.md Replaces fork-inheritance claim with invariant-based dispatch requirement None — no new dispatch paths, no new permissions, no new execution surfaces
plugins/plugin-quality/skills/audit/references/component-types/skill.md Updates composition-check rubric None — prose only

Checked specifically:

  • No GitHub Actions workflows added or modified — no pull_request_target / workflow_run / github context injection risks.
  • No permissions: widening — no workflow or settings files touched.
  • No dependency pins loosened — no package files changed.
  • No secrets, tokens, or credentials introduced or referenced.
  • No shell scripts or run: blocks — no command injection surface.
  • No SQL, path traversal, or template injection surfaces.
  • No network egress paths added — URLs in PLAN.md are documentation-reference prose, not runtime fetches.
  • auditor.md security posture unchangedBash and Write are pre-existing tool grants; Bash is scoped to validation/probes, Write is restricted to the evidence-packet directory. The standing untrusted-content instruction (prompt-injection defense) is unaffected by the wording change.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read changed files and gather context
  • Review latest commit (9101dc9) addressing Codex fourth review
  • Check the invariant correction in SKILL.md
  • Verify internal consistency across all changed files
  • Post final review

Verdict: Approve with one remaining fix needed — CHANGELOG [0.1.2] still carries the corrected-away wording.

What 9101dc9 changes

The Codex fourth review found the invariant stated in dac51f3 was self-contradicting: it said the auditor's context "provably excludes this session's evidence," but agents/auditor.md:31-33 opens by reading the evidence packet as its ground truth, and step 1 exists solely to write that packet. The chosen implementation violated the property the text used to reject alternatives.

9101dc9 corrects this at both sites:

PLAN.md:601-604
Property 1 was: "A context that provably excludes this session's evidence"
Property 1 is now: "A context carrying the evidence packet but NOT this session's conversation history or prior reasoning."

SKILL.md:128-134
The step-2 dispatch instruction follows the same correction. The distinction is now explicit: the packet is the deliberate channel across the boundary; what must not cross is the reasoning that produced the work under review.

Both corrections are accurate, internally consistent, and align with agents/auditor.md's actual procedure.

One remaining issue

CHANGELOG.md:14-15 was written in dac51f3 and not touched by 9101dc9. It still reads:

stated as an invariant the step must satisfy — a context that provably excludes the session's
evidence, and a named dispatch target that makes the dispatch site auditable

This is the self-contradicting phrasing that 9101dc9 corrects everywhere else. As shipped, the [0.1.2] CHANGELOG entry describes an invariant the implementation does not hold — anyone reading it to understand what 0.1.2 changed will find the old (incorrect) framing. The fix is to update that entry to match the corrected invariant: the context carries the evidence packet but excludes the session's conversation history and prior reasoning. A version bump is not needed — 0.1.2 already exists; only the entry body needs updating.

Fix this →

Test plan verification

  • grep -rn "excludes.*session.*evidence" plugins/plugin-quality/ docs/topics/plugin-audit-port/ — one hit: CHANGELOG.md:14 (the stale entry)
  • grep -rn "degraded history" docs/topics/plugin-audit-port/ — matches only in the [EXEC-SHAPE] historical record (defeated-rationale marker) and CHANGELOG [0.1.1] description, both correct
  • [EXEC-SHAPE] row at :553 unmodified; the basis subsection is expanded consistently
  • No functional changes, no security surface, no permission additions, no manifest schema violations
  • #1258 forward-reference hedged correctly in both PLAN.md and SKILL.md; this PR's correctness does not depend on its resolution

No further version bump needed — 0.1.2 is the ship version for this PR; only the CHANGELOG entry body requires the one-line correction above.

@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: 9101dc98e1

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

Comment thread docs/topics/plugin-audit-port/PLAN.md Outdated
Comment thread plugins/plugin-quality/CHANGELOG.md Outdated
…raming

Review findings on #1274. Both were sites the previous two commits missed.

The constraints bullet still said `context: fork` "cannot express this topology"
while the [EXEC-SHAPE] basis in the same revision concedes a forked sibling can
run steps 2-3 with the parent retaining 1, 4 and 6. The bullet now matches the
alternatives table: not chosen, on listing-budget and synchronization cost.

The 0.1.2 changelog entry still described the invariant as excluding the session's
evidence — the same contradiction the shipped wording had just been corrected for,
reintroduced in the release note a reader consults first. It now states the
invariant as excluding conversation history and prior reasoning, and says the
packet crossing the boundary is deliberate.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


PR Review — docs(plugin-audit-port): clear two internal contradictions in the reframing

  • Read changed files and gather context
  • Review latest commit (875f1cc) addressing Codex fifth review
  • Check PLAN.md topology claim fix
  • Check CHANGELOG.md invariant wording fix
  • Verify internal consistency across all changed files
  • Post final review

Verdict: Approve. Both Codex fifth review challenges resolved; the PR is complete.

What 875f1cc changes

Two sites were out of sync with the revision that preceded them:

PLAN.md:79–82 — constraint bullet

Before: `context: fork` cannot express this topology
After: `context: fork` is not chosen — rejected on listing-budget and synchronization cost, not as inexpressible

The [EXEC-SHAPE] basis section (:609–613) already conceded a forked sibling can run steps 2–3 with the parent retaining 1, 4, and 6. The constraint bullet was still asserting impossibility the same revision had abandoned. It now matches the alternatives table.

CHANGELOG.md:14–15 — 0.1.2 entry

Before: a context that provably excludes the session's evidence
After: a context that carries the evidence packet but **not** this session's conversation history or prior reasoning

This was the issue the prior Claude review flagged and 9101dc9 corrected in PLAN.md and SKILL.md but missed in the release note. The CHANGELOG is now consistent with the invariant wording at both shipped sites.

Three-site consistency check

All three locations that describe the context: fork rejection now align:

Site Language
Constraint bullet (:80–81) "not chosen — rejected on listing-budget and synchronization cost, not as inexpressible"
Alternatives table (:461) "Not impossible — rejected on cost"
[EXEC-SHAPE] basis (:609–613) "not rejected as impossible. It is rejected on cost"

Test plan verification

  • grep -rn "cannot express" docs/topics/plugin-audit-port/ — no matches ✓
  • grep -rn "excludes.*session.*evidence" plugins/plugin-quality/ docs/topics/plugin-audit-port/ — no matches ✓
  • grep -rn "degraded history" docs/topics/plugin-audit-port/ plugins/plugin-quality/ — only in historical/defeated-rationale context (PLAN.md:593, CHANGELOG.md:25) ✓
  • [EXEC-SHAPE] row at :553 unmodified ✓
  • No functional changes, no security surface, no permission additions, no manifest schema violations ✓
  • #1258 forward-reference hedged correctly in PLAN.md and CHANGELOG; this PR's correctness does not depend on its resolution ✓

No outstanding review concerns. Ready to merge.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

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


Security Review — docs(plugin-audit-port): correct the context: fork rejection rationale

  • Read PR diff and gather context
  • Analyze changed files for security vulnerabilities
  • Post findings

No security issues found.

All seven changed files were reviewed against the full diff. The PR is a documentation correction with patch-version bumps and carries no security surface.

File Change Security surface
docs/topics/plugin-audit-port/PLAN.md Corrects fork-rationale prose; adds [EXEC-SHAPE] basis subsection; fixes internal contradiction in the "excludes evidence" wording None — prose only; URLs cite code.claude.com/docs/en/ (trusted first-party domain, not fetched at runtime)
docs/topics/plugin-audit-port/design/design-resolution.md Updates one table cell None — prose only
plugins/plugin-quality/.claude-plugin/plugin.json "version": "0.1.0""0.1.2" None — patch bump; full file confirms no new permissions, dependencies, tool grants, MCP servers, or remote URLs
plugins/plugin-quality/CHANGELOG.md Adds 0.1.1 and 0.1.2 entries None
plugins/plugin-quality/agents/auditor.md Rewrites one sentence in the agent's system prompt None — tool grants (Read, Grep, Glob, WebFetch, Bash, Write), Bash scope restrictions (validate/probe/safe reproductions only), Write path restriction (evidence-packet directory only), and untrusted-content posture (prompt-injection defense) are all unchanged
plugins/plugin-quality/skills/audit/SKILL.md Replaces fork-inheritance claim with invariant-based dispatch requirement None — no new dispatch paths, no new permissions, no new execution surfaces
plugins/plugin-quality/skills/audit/references/component-types/skill.md Updates composition-check rubric None — prose only

Checked specifically:

  • No GitHub Actions workflows added or modified — no pull_request_target / workflow_run / github context injection risks.
  • No permissions: widening — no workflow or settings files touched.
  • No dependency pins loosened — no package files changed.
  • No secrets, tokens, or credentials introduced or referenced.
  • No shell scripts or run: blocks — no command injection surface.
  • No SQL, path traversal, or template injection surfaces.
  • No network egress paths added — URLs in PLAN.md are documentation-reference prose, not runtime fetches.
  • auditor.md security posture unchangedBash and Write are pre-existing tool grants; Bash is scoped to validation/probes, Write is restricted to the evidence-packet directory. The standing untrusted-content instruction (prompt-injection defense at line 23–27) is unaffected by the one-sentence wording change.

@kyle-sexton
kyle-sexton merged commit 329b3d0 into main Jul 25, 2026
26 checks passed
@kyle-sexton
kyle-sexton deleted the docs/1268-plugin-audit-port-fork-rationale branch July 25, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant