Skip to content

feat(knowledge): harden docpage-digest pipeline (0.13.5) - #3099

Merged
kyle-sexton merged 4 commits into
mainfrom
cursor/3015-docpage-hardening-cfcf
Aug 21, 2026
Merged

feat(knowledge): harden docpage-digest pipeline (0.13.5)#3099
kyle-sexton merged 4 commits into
mainfrom
cursor/3015-docpage-hardening-cfcf

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #3015

Summary

Lands the interview-ratified /knowledge:docpage-digest pipeline-hardening bundle from the 9-slice cloud-fleet corpus run. Knowledge plugin 0.13.40.13.5. Keeps the 0.13.4 / #3014 heading.

Fix

  1. Fence mandate. Every verbatim quote is a column-0 fenced container under a bold **CN.** label. Blockquotes and inline code spans are forbidden quote carriers.
  2. Standing gates. Ships scripts/check-fences-exact.py and scripts/check-snippets.py alongside the quote gate. Both fail loud on zero-parse and compare payloads without .strip(). Negative-control suites (test_check_*.py) are the evidence a PASS needs.
  3. Freeze/pin. Pin on agent-REPORTED completion, never file presence. verification/pin-manifest.json freezes the tree; each arm restates the hashes it audited. A verdict file on disk is an intermediate write, never a report.
  4. Subagent-death ladder. Retry window → inline-with-disclosure → degraded marker + re-run trigger. Distinct from the existing degraded-verifier rule (missing cross-vendor arm).
  5. Gate-as-claim + blind spots. SKILL.md enumerates each gate's parse surface. Format and pin-manifest shape live in context/pipeline-hardening.md.

Verification

  • bash plugins/knowledge/skills/docpage-digest/scripts/check-fences-exact.test.sh — 14/14 PASS (zero-parse, indented fence, lost trailing space, blockquote/inline substitutes, fabricated quote).
  • bash plugins/knowledge/skills/docpage-digest/scripts/check-snippets.test.sh — 8/8 PASS (missing section, unparsed prose, fabricated snippet, indented fence, recognised none-marker).
  • scripts/check-changelog-parity.sh --check, --check-order, --check-bump origin/main PASS. Newest heading is ## [0.13.5]; ## [0.13.4] (knowledge plugin: publisher-profile amendment bundle from the 9-slice cloud-fleet corpus run #3014) is retained.
  • CHECK_SKILL_SKILLS_ROOT=plugins/knowledge/skills check-skill.sh --require-evals docpage-digest PASS (0 errors). Trigger phrases preserved. Script tests invoked by the checker.
  • npx markdownlint-cli2 on the four markdown files: 0 issues.
  • check-evals-quality.sh PASS (only the pre-existing Q4 narration warn on eval id=2).

Related

Refs the 9-slice interview + dual-validator pass recorded in the issue body. Profile amendments already landed as #3014 / 0.13.4. Graduation-gate residual-verification is out of scope (operator decision record; binds graduation, not this skill PR).

Interview-ratified #3015 bundle: fence mandate, standing
check-fences-exact.py / check-snippets.py gates, pin-on-report plus
hash manifest, subagent-death fallback ladder, and gate-as-claim
blind-spot enumeration. Bumps knowledge 0.13.4 → 0.13.5 and keeps
the 0.13.4 / #3014 heading.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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


Security review — /review:security-review

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

Skip gate hit: no security-relevant surface in this diff.

All 13 changed files are markdown (SKILL.md, CHANGELOG, context doc, checklist template, evals.json) plus four new stdlib-only Python scripts and their bash test wrappers under plugins/knowledge/skills/docpage-digest/scripts/:

  • check-fences-exact.py and check-snippets.py only open() (read-only) the --source/--digest paths passed via argparse, no shell=True, no eval/exec/subprocess in the gate scripts themselves.
  • The test_check_*.py suites spawn subprocess.run([sys.executable, GATE, ...]) with a fixed argument list (no shell=True, nothing untrusted interpolated) — no command-injection surface.
  • The .test.sh wrappers resolve python3/python via command -v and parse a version floor out of a fixed, repo-local digest_fences.py (not user/network input) before invoking it — no injection surface there either.
  • No changes touch .github/workflows/**, permissions, auth, secrets handling, or network I/O. This is documentation/process-hardening for an existing skill (verbatim-quote fencing rules, a freeze/pin manifest, a subagent-retry ladder) plus the deterministic gate scripts and their negative-control tests described in the PR body.

Per the skip gate, no findings to report — no security review comments posted.

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 4m 56s —— View job


Claude code review — /review:code-review lane

  • Load /review:code-review lane instructions
  • Gather PR diff and context (gh pr diff 3099, origin/main...HEAD unavailable in this shallow checkout — used gh pr diff instead)
  • Read changed files (digest_fences.py, check-fences-exact.py, check-snippets.py, their test suites, SKILL.md, context/pipeline-hardening.md, evals.json, templates/checklist.md, CHANGELOG.md, plugin.json)
  • Identify high-signal correctness/maintainability findings
  • Adversarially validate candidate findings (traced execution by hand against the negative-control tests; sandbox blocked local python3 execution in this session, so the suite runs below are author-claimed, not independently re-run)
  • Post inline comment for the anchorable finding
  • Post final summary

Finding

1 correctness issue posted inline on scripts/digest_fences.py:182-186:

is_none_section() treats a completely blank ## Prompt snippets body as a valid "no snippets" pass, silently returning True when body.strip() is empty — before it even checks against NONE_MARKERS. That contradicts this PR's own contract in context/pipeline-hardening.md:36-37: "A recognised none-marker (none, n/a, (none), no prompt snippets) is the only legal empty form." A digest agent that leaves the heading in place but never writes anything under it (never affirmatively asserting "none") gets a silent PASS from check-snippets.py instead of a failure — the exact "unparsed section content is the attack surface" class this gate exists to catch. test_check_snippets.py doesn't cover a truly blank body either; only the explicit none marker and non-empty unparsed prose are tested.

Other areas checked, no issues found

  • payload_in_source's "lost trailing space" match logic (digest_fences.py) — traced through the exact-match, mid-line-match, and trailing-space-loss branches; behavior matches its docstring and the test_trailing_space_stripped_fails / test_trailing_space_preserved cases.
  • Fence/claim parsing (extract_fences, parse_claims, _is_fence_opener/_is_fence_closer) — column-0 vs indented detection, closer-line strictness (rejects a language-tagged line as a closer), unlabelled/duplicate-label detection all check out against their test coverage.
  • check-fences-exact.py / check-snippets.py exit-code contract (0/1/2/3) and check-skill.sh's scripts/*.test.sh wiring (plugins/skill-quality/scripts/check-skill.sh:565-587) — consistent with the PR's claims.
  • Cross-platform *.test.sh wrapper pattern (parses MIN_PYTHON out of the engine module, probes python3/python) — matches the existing convention used by scripts/check-manifest-duplicate-keys.py, plugins/disk-hygiene/.../hygiene.py, and others in this repo.
  • CHANGELOG.md heading order/version bump (0.13.40.13.5) and plugin.json version are consistent.

Not independently verified

The PR body's test-run counts (14/14, 8/8), markdownlint-cli2, check-changelog-parity.sh, and check-skill.sh results are author-claimed — this session's sandbox declined to run python3/gh pr diff-adjacent shell commands beyond the read-only ones above, so those are not re-confirmed here.

@github-actions

Copy link
Copy Markdown

Last security-reviewed head: 757faca97c5f7e76dbbf4abf7fa84555fddad456. 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: 757faca97c

ℹ️ 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/knowledge/skills/docpage-digest/scripts/digest_fences.py Outdated
Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py
Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py Outdated
Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py
editorconfig-checker flags physical trailing whitespace in *.py.
The load-bearing trailing space now lives in a KEEP constant so
the negative-control payload stays exact without dirtying the file.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py Outdated
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

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

Blank Prompt-snippets is not a none-marker. Empty fence payloads
do not match the source. A leftover blockquote or inline span is
still a forbidden carrier when a later fence is valid. H2 lines
inside a fence are payload, not section boundaries.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 6m 45s —— View job


Code review — /review:code-review

  • Gather context (PR diff via gh pr diff 3099, changed files)
  • Invoke /review:code-review skill criteria (correctness + maintainability, security out of scope)
  • Review new gate scripts (digest_fences.py, check-fences-exact.py, check-snippets.py) and their tests
  • Review SKILL.md / context/pipeline-hardening.md / templates/checklist.md / evals.json changes
  • Cross-check the 4 existing Codex findings against the code
  • Post findings (inline + summary)

Note on head SHA: the task header names 5dc8980 as HEAD, but this checkout's actual current commit is a55fc60 ("fix(knowledge): close the four standing-gate review holes") — one commit ahead. I reviewed the PR as it stands now, since that's what's actually on the branch.

The four chatgpt-codex-connector findings from the earlier revision (757faca) are fixed in the current code (digest_fences.py): forbidden carriers reject even when a fence follows later (blockquote/inline-code check runs on the prose before the first fence, independent of claim.fence), empty fence payloads fail before source-matching (payload_in_source short-circuits on not payload, plus explicit empty-payload checks in both gate scripts), a blank Prompt-snippets section now fails instead of passing as an implicit none-marker, and H2-looking lines inside a fence no longer truncate a section (iter_h2_sections tracks fence state while scanning for ## boundaries). The negative-control tests (test_check_fences_exact.py, test_check_snippets.py) cover all four cases by name.

New finding posted inline on digest_fences.py L117–154: _is_fence_closer matches on backtick-count alone, without comparing against the specific opener's run length. A legitimately CommonMark-nested fence (e.g. a 4-backtick outer fence wrapping source text that itself contains a 3-backtick example — a real, plausible shape for documentation about Markdown/code-fence syntax) gets its payload truncated at the first inner backtick-only line. That can either false-fail a correct quote as "empty," or worse, false-PASS a truncated prefix that still happens to be an exact substring of the source — silently validating an incomplete quote, which is the exact defect class this PR is hardening against. I traced this statically (couldn't execute Python in this sandboxed session to confirm with a run — noted in the inline comment).

Everything else reviewed (SKILL.md, context/pipeline-hardening.md, templates/checklist.md, evals/evals.json, CHANGELOG.md, plugin.json version bump, the two .test.sh wrappers) is consistent, low-risk documentation/process content with no correctness issues.

Verification note: I was not able to execute the negative-control test suites or the gate scripts in this sandboxed environment (subprocess/tempfile execution outside the working directory required approval that wasn't available here) — the 14/14 and 8/8 PASS counts in the PR description are author-claimed, not independently re-run by me. I did read the test bodies in full and they exercise the right fixtures for the four already-fixed issues.

Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py Outdated
Comment thread plugins/knowledge/skills/docpage-digest/scripts/digest_fences.py Outdated
CommonMark closers must be at least as long as the opener. A 3-tick
line inside a 4-tick wrapper is payload, not a close — the old any-3+
rule truncated nested source examples to "" or a prose prefix.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 2m 41s —— View job


Code review — /review:code-review

  • Gather context (gh pr diff 3099; shallow checkout has no merge-base with origin/main, used gh pr diff as fallback)
  • Invoke /review:code-review skill criteria
  • Review new/changed content at current HEAD (f681e1f) — the fence-closer backtick-run-length fix
  • Cross-check prior findings against current code
  • Post findings

Only 3 files changed since the last review round (a55fc60f681e1f): digest_fences.py, test_check_fences_exact.py, test_check_snippets.py. SKILL.md, context/pipeline-hardening.md, templates/checklist.md, evals/evals.json, CHANGELOG.md, and plugin.json are unchanged from the already-reviewed a55fc60 state.

This commit fixes the previously-reported fence-closer bug. digest_fences.py:119-143:

  • _is_fence_opener now returns the matched backtick-run length alongside (is_opener, indented).
  • _is_fence_closer(line, min_ticks) counts the closer's own backtick run and requires rest == "" and ticks >= min_ticks — a shorter nested run (e.g. a 3-tick line inside a 4-tick wrapper) is correctly treated as payload, not a close.
  • Both call sites that matter were updated: extract_fences (digest_fences.py:146-170) now tracks the opener's ticks and passes it through to the interior closer check; iter_h2_sections (digest_fences.py:81-107) does the same via open_ticks, so a nested fence inside a claim's payload no longer causes a false H2-section-boundary truncation either.

I traced this by hand against the two new negative-control tests (test_longer_outer_fence_keeps_inner_backtick_run, test_four_tick_wrapper_around_immediate_inner_fence in both test_check_fences_exact.py and test_check_snippets.py) and the logic checks out: a 4-backtick outer fence wrapping a 3-backtick inner example now yields the full, untruncated payload rather than "" (false FAIL) or a truncated-but-still-substring-matching prefix (false PASS).

No new findings. The four earlier Codex/Claude findings (forbidden-carrier-after-fence, empty-payload, blank-none-section, H2-inside-fence) remain fixed as of a55fc60, and this commit's own fence-closer fix resolves the fifth (backtick-run-length) finding from the prior round.

Not independently executed: this lane's allowed-tools scope Bash to gh pr diff/view/comment/review only — I could not run python3 -m unittest or the .test.sh wrappers in this session, so the 14/14 / 8/8 PASS counts and the two new nested-fence tests are traced statically, not independently re-run. Everything else (SKILL.md, context doc, checklist, evals, CHANGELOG, plugin.json) was previously reviewed and found consistent with no changes since.

@kyle-sexton
kyle-sexton merged commit 10849be into main Aug 21, 2026
50 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/3015-docpage-hardening-cfcf branch August 21, 2026 14:40
kyle-sexton added a commit that referenced this pull request Aug 21, 2026
…3100)

## Summary

Compress the session-flow handoff instruction walk (`save-point.md`,
`structure.md`, `skills/handoff/SKILL.md`) under docs-hygiene
flavor-only discipline. Re-review the Stop-hook escalation from #3018;
no defect found, hook not shipped.

## Fix

Author-time compression of the three named files: drop filler and
verbose verbs that do not carry contract meaning. Preserve find-handoff
detection contract (rails, copy instruction, `Read @…-handoff-…`
directive, `Re-arm <i> of <n> — <L> lines:`), redaction rules,
rooted-path / `Handoff origin:` rationale, and STOP / output-order
contract. Bump session-flow 0.32.2 → 0.32.3. Stop-hook re-review: 0.26.1
already shipped the STOP/output-order contract; one observed occurrence;
find-handoff rung 1 still recovers file-mode; the 0.22% cut does not
change occupancy. No hook change.

## Test plan

- [x] `markdownlint-cli2` on the three compressed files + CHANGELOG: 0
issues
- [x] `scripts/changelog-parity.sh --check --check-bump origin/main
--check-order --check-preserved origin/main`
- [x] Contract tokens remain: rails, copy instruction, Read
@…-handoff-…, Re-arm header, redaction markers, STOP / output-order
- [x] Byte delta vs author-time snapshots: 188 B / 0.22% (save-point 152
B, structure 27 B, SKILL 9 B)

## Verification

- `markdownlint-cli2` on the three files + CHANGELOG: 0 issues.
- Changelog-parity `--check`, `--check-bump origin/main`,
`--check-order`, `--check-preserved origin/main`: all pass.
- Contract-token grep: rails, copy instruction, `Read @…-handoff-…`,
`Re-arm <i> of <n> — <L> lines:`, redaction markers, STOP / output-order
all still present.
- Byte savings vs `/tmp/wt-3018-snap`: save-point 40831→40679 (152 B),
structure 23296→23269 (27 B), SKILL 19641→19632 (9 B); total 188 B /
0.22%.

## Related

- Closes #3018
- Follows #3097 / #3098 / #3099
- Stop-hook re-review; no defect — hook not shipped

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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.

knowledge plugin: docpage-digest pipeline hardening (fence mandate, standing gates, freeze/pin rules, fallback ladder)

2 participants