Process completed review feedback for one named SecPal repository and pull request after an explicit remediation request, or handle explicitly named fixed-thread resolution-only requests after the findings were validated, committed, and pushed. Do not use for generic review, PR creation, review requests, CI-only debugging, ordinary implementation, or merge-only work.
Resources
2Install
npx skillscat add secpal/github/secpal-pr-review Install via the SkillsCat registry.
SecPal PR Review
Use this skill only when the user explicitly asks to process already-completed
pull-request feedback for a specific repository and PR number, or asks to
resolve explicitly named threads whose findings have already been handled as
defined below. This skill is not a reviewer: it independently verifies reviewer
leads and remediates only findings that source, tests, and repository context
prove valid.
Simple fixed-thread resolution
When the user asks only to resolve explicitly named review threads whose
findings have already been evaluated, fixed where necessary, validated,
committed, and pushed, use the central source repository'sscripts/secpal-resolve-fixed-threads.py. Require the exact repository, pull
request number, expected current head OID, reviewed-state file, and thread IDs.
Also require the captured reviewed-state digest and successful validation
evidence in the final attestation for a new verified fix commit. A raw
validation receipt for an unchanged head is not authenticated by that existing
commit and cannot authorize resolution. Require the target repository root and
an eligibility manifest that covers every requested thread exactly with an
allowed classification/disposition, finding IDs, evidence digest, and the exact
canonical follow-up identity for every TRACKED_AS_FOLLOW_UP finding. Finalize
the manifest before complete validation and bind its canonical digest into the
signed validation receipt and final attestation for the fix commit. The command
rejects a swapped state file, a non-matching local commit,
and stale, missing, incomplete, unauthenticated, or differently bound evidence
before any GitHub read.
The reviewed state may contain canonical provider_review_requests; the
resolver admits that optional category only through the shared closed Stable
Feedback verifier. Recovered Ready capture must reauthenticate protectedCURRENT and derive the provider head through the maintained Ready-source
binding; it never accepts a caller-selected provider head.
For an eligibility-bound typed Ready integration, also supply the canonical
integration evidence. The resolver accepts only the closed version-1.2
integration-resolution attestation and verifies its ordered two-parent
topology, tree, receipt and integration trailers, signer, reviewed state, and
eligibility digest through the integration-specific verifier. Never treat a
historical integration attestation as an ordinary sole-parent attestation.
When prior Ready evidence genuinely predates persistence, do not invent the
missing reviewed-state, receipt, eligibility, or attestation bytes. First
authenticate and publish the explicit one-use pre-persistence recovery through
the maintained lifecycle-publication authority.
Recovery safety facts carry no process-local authority. The maintained issuer
selects the immutable protected-main registry and commands, captures the
complete current feedback universe including resolved threads, executes that
validation against the exact candidate, and signs the complete capture,
source-complete technical decisions, policy/command identity, and receipt into
the existing one-use recovery authorization. The accepted authorization signer
and protected publication are the first trusted recovery boundaries. Raw caller
policy, feedback snapshots, decisions, or claimed-success receipts are never
recovery authority.
Ready integration may then use the recovered version-1.2 prior-authority manifest with no historical companion
files. Supplying any historical companion file selects no fallback: malformed,
partial, or invalid ordinary evidence remains blocking.
The command first reads every target completely and requires its current
comment identities, body digests, and resolution state to match that reviewed
state. It then performs two complete stable target rechecks of the open PR,
expected head, membership, resolved/outdated state, and canonical
target-comment state immediately before each write or successful
already-resolved report.
Immediately before resolving OUTSIDE_PR_SCOPE + TRACKED_AS_FOLLOW_UP, require
the canonical work-graph reader to prove that the authenticated exact follow-up
is accessible, open, and structurally complete. It may be blocked; resolution
means tracked disposition, not implementation or completion.
Apply the same live follow-up verification before an already-resolved tracked
conversation is reported as safely dispositioned/tracked; external resolution
alone is not authenticated tracking evidence.
Invoke the resolver in write mode; omitting --apply is only a dry run:
python3 scripts/secpal-resolve-fixed-threads.py \
--repo OWNER/REPOSITORY \
--pr PULL_REQUEST_NUMBER \
--repo-root /path/to/OWNER/REPOSITORY \
--expected-head FULL_40_CHARACTER_HEAD_OID \
--reviewed-state REVIEWED_STATE.json \
--expected-reviewed-state-digest REVIEWED_STATE_SHA256 \
--validation-evidence VALIDATION_EVIDENCE.json \
--eligibility-evidence ELIGIBILITY_EVIDENCE.json \
--thread-id REVIEW_THREAD_NODE_ID \
--applyRepeat --thread-id REVIEW_THREAD_NODE_ID for each additional fixed thread.
When and only when the final attestation carries anexceptional_recovery_evidence_digest, retain the existing accepted Recovery
document and its exact signed orchestration authorization and add this closed
tuple to the same resolver invocation:
--delivery-issue DELIVERY_ISSUE_NUMBER \
--exceptional-recovery-evidence READY_EXCEPTIONAL_RECOVERY.json \
--exceptional-recovery-authorization SIGNED_RECOVERY_AUTHORIZATION.jsonOmit all three inputs for ordinary non-Recovery evidence. Never combine them
with Ready-integration evidence. The shared Exceptional Recovery verifier owns
the lifecycle, publication, and signer authentication; the resolver does not
reconstruct that authority.
For an exact technically non-blocking target absent from authenticated final
eligibility and observed on the unchanged final delivery head, do not create an
empty delivery commit. Require the complete final reviewed state and the typed
final-eligibility boundary: either its canonical manifest or the maintained
exact authenticated-absence record. A supplied invalid manifest never falls
back to absence. Derive REVIEWED_BUT_INELIGIBLE when the target is present in final
reviewed state, or ABSENT_FROM_BOTH when it is absent; never accept a
caller-selected origin or amend original eligibility. Require
the canonical corrected/actionable, invalid/disproven, or
informational/non-actionable pair for REVIEWED_BUT_INELIGIBLE;ABSENT_FROM_BOTH retains only invalid/disproven or
informational/non-actionable. Require technically_blocking=false for every
pair. Reject every other decision and every technical blocker. Usescripts/secpal-create-late-classification.py to capture and authenticate the
exact decision, then use scripts/secpal-create-late-disposition.py to verify it
and create the canonical detached disposition artifact and signature.
Both creators must verify the existing final reviewed state, typed final
eligibility boundary, receipt/attestation, final tree, receipt trailer, origin,
head, and commit signature before deriving the delivery signer and reading the
explicitly named thread. They accept ordinary final-delivery evidence or, when--integration-evidence is supplied, canonical eligibility-bound Ready-
integration evidence through the same integration-specific verifier used by
the resolver. The authenticated attestation shape selects the evidence family;
there is no caller-selected compatibility mode. A final eligibility manifest must match its
authenticated digest, every eligible thread must exist in the reviewed state,
and the proposed target must be absent from final eligibility. Each creator and
the resolver must independently derive reviewed-state membership or absence and
enforce the matching closed decision policy. Resolve it
only through scripts/secpal-resolve-fixed-threads.py with--delivery-issue, --late-disposition-evidence, and--late-disposition-signature, together with the matching--late-classification-evidence and --late-classification-signature, and the
same --integration-evidence for a Ready-integration source. The
resolver independently verifies the same
final evidence, requires the detached SSH/OpenPGP signer to equal the verified
delivery signer, and fails closed on any artifact, classification, action,
head, thread, top-level comment, body, reply, resolved, or outdated-state drift.
The disposition creator computes the classification-evidence digest from the
verified canonical classification artifact; caller-provided digests and
blocking facts have no authority. Read
references/late-classification.schema.json
for that input shape and read
references/late-disposition.schema.json
for the exact artifact shape. This exception consumes no review/remediation
counter and has no commit, push, CI, Ready, or merge authority.
For a historical schema-1.1 Ready integration, supply its exact original
validation receipt as --final-validation-receipt to both producers and as--integration-validation-receipt to the resolver, and omit final eligibility.
The integration-specific verifier must authenticate the exact source while both
receipt and attestation omit eligibility_evidence_digest. That derivesNO_COMMIT_BOUND_READY_INTEGRATION_ELIGIBILITY only; schema 1.1 never directly
authorizes a thread. The signed classification/disposition chain remains
mandatory. Never substitute the special final-eligibility-absence recovery,
invent an eligibility artifact, or downgrade schema 1.2.
“Post-push” is lifecycle shorthand for this authenticated disposition
boundary. The evidence proves reviewed-state membership or absence and
commit-bound eligibility absence; it does not claim cryptographic proof of
GitHub wall-clock ordering relative to a push.
This resolution-only path does not capture or reclassify PR-wide feedback, run
validation, inspect CI or readiness, create commits, push, or make a merge
decision. Report the exact resolved and already-resolved targets, then stop.
Use the full workflow below only when the user asks to process feedback,
perform remediation, audit readiness, or evaluate merge authorization.
Required inputs and boundaries
For the full feedback-remediation workflow, require all of the following before
starting:
- an explicit PR-feedback remediation request;
- the exact
owner/repositoryand pull request number; - a clean worktree on the current topic branch with its upstream configured;
- matching local, remote, and PR head OIDs;
- an open pull request, an understood base, and an exact explained commit set;
- locally verified SSH or OpenPGP signatures for user-authored commits, as
permitted by the repository registry, plus valid GitHub verification metadata
when the registry requires it; - GitHub
verified: true,reason: validmetadata for GitHub-generated
commits; and - one canonical stable-feedback read containing no Required Check results.
Do not use this skill for generic code review, creating a PR, requesting any
review, debugging CI without completed feedback, ordinary implementation,
Draft-to-Ready transitions, or merge-only requests. Never request another
review. The skill helper never marks a PR Ready, merges, or enables auto-merge;
an explicitly authorized larger delivery may use the separate maintained
lifecycle and merge mechanisms before or after this feedback path. Never amend a
reviewed commit, force-push, bypass hooks, or use administrator privileges.
Read references/contract.md completely before acting.
Load the matching entry from
references/repositories.json, validated against
references/repositories.schema.json.
Repository-local AGENTS.md and focused instruction files remain authoritative
for development rules.
Locate the trust surfaces
Resolve this skill directory canonically. Its central source repository is three
directories above it. Use that source repository's:
scripts/secpal-pr-review-actions.py resolve-batchfor normal stable-feedback
capture only; its guarded readiness batch is available only when the current
user instruction explicitly requests readiness or merge evaluation;scripts/secpal-pr-review-actions.py attest-validationfor one deterministic
complete-validation attestation bound to the finished local head; andscripts/secpal-resolve-fixed-threads.pyfor default post-push resolution of
eligible fixed threads and for the resolution-only path defined above; andscripts/secpal-pr-review.pyplus evidence-only legacy action commands only
when the user explicitly selects forensic/audit snapshot mode. Any legacy
command that reads hosted-CI readiness additionally requires the current user
instruction to explicitly request readiness or merge evaluation.
Never import or call the action helper from the evidence helper. Never add a
mutation command to the evidence helper. Execute configured validation commands
as argument arrays in the target repository, without a shell.
Delivery continuity and proof reuse
Apply SIMPLIFY_BEFORE_EXTEND to the workflow. A prompt boundary is not an
evidence invalidator, and a mechanical checkpoint is not a user decision
boundary. On first delivery entry, perform the relevant full preflight. During
the same delivery, refresh only operation-relevant facts whose defined
invalidators may have occurred. A head change invalidates head-bound proof; a
tree change invalidates validation proof; a new CURRENT publication invalidates
prior CURRENT proof; relevant feedback or reviewed-head change invalidates
stable feedback; native graph mutation invalidates work-graph proof; volatile
readiness is freshly read at its actual boundary. Create no freshness store.
Before mutation, derive the canonical operation and its exact preconditions
from authenticated current maintained repository authority. Prompt expectations
bind scope and authority but remain assertions to verify. If they conflict with
the maintained contract, fail closed before mutation and report the discrepancy.
Commit creation, push, receipt/attestation binding, lifecycle publication,
eligible thread resolution, and bounded read-back require no new prompt while
they remain inside current authority. The same applies to an eligible Ready
transition or merge when the current instruction explicitly authorizes that
later mutation conditionally on its maintained gate. New user authority is
required only for a genuine new operation, material scope, recovery, or
unresolved decision boundary. The helper's own mutation allowlist remains
unchanged.
Stable feedback is captured once per reviewed head only after every triggered
review provider has successful terminal evidence. QUEUED, PENDING,RUNNING, FAILED, and INDETERMINATE block stable capture and merge; empty
comments or threads do not prove completion. Classify the complete bounded
snapshot before one coherent remediation batch. The existing capture query
rejects a visible non-terminal Codex summary, a missing Codex summary on a
Ready PR, and a pending Copilot review request before admitting stable feedback.
A currently authorized full delivery may observe provider state for about 30
minutes at 60-to-90-second intervals without requesting another review,
consuming another review cycle, or polling unrelated hosted CI. Expiry reportsREVIEW_NOT_TERMINAL; the same workspace may resume after external state
changes.
Finite lifecycle continuity
When a completed-feedback workflow consumes persistent lifecycle state, usescripts/secpal_pr_review/lifecycle_orchestration.py over independently
authenticated CURRENT publication. Never create a new lifecycle because a PR
was replaced, recovered, reopened, pushed, reviewed again, or observed by CI.
Never reset review/remediation counters or represent Cycle 3.
After the first authorized Ready transition, keep that PR Ready during ordinary
or exceptional source correction. A changed head requires fresh validation and
a later bounded readiness assessment, not Ready -> Draft -> Ready. Perform aReady -> Draft mutation only when the current user instruction explicitly
authorizes that exact transition and gives its separate reason. Any laterDraft -> Ready requires another explicit authorization and retains the same
lifecycle and counters.
Treat every review object and configured automated review as feedback evidence,
not authority for another review, remediation cycle, or Draft/Ready transition.
One explicitly authorized additional review permits one bounded current-head
assessment and stops. Its canonical signed authorization must be bound to
CURRENT and consumed through append-only lifecycle publication before the
assessment; a caller-created mapping is not authority and a published
authorization cannot be replayed. A material post-Ready blocker requires a
user decision for any remaining bounded recovery authority. A tracked-follow-up
resolution
must be reported as safely dispositioned/tracked, never fixed, implemented, or
completed, with technical and mechanical blocking stated separately.
When provider-owned material threads from the already-consumed assessment
appear after its canonical Stable Feedback capture, do not request another
review or rewrite that capture. The maintained ordinary Ready provider-growth
verifier derives the complete additive delta from the first remediation's
sealed reviewed state and the candidate remediation's sealed reviewed state.
It requires protected CURRENT at Review 1/1 and Remediation 1/2, exact H0 to H1
ordinary lineage, Ready true, Cycle 3 absent, the unique Copilot request and
review chronology, comment-to-review identity, the first remediation's bound
eligibility for predecessor resolutions, and complete existing ordinary
eligibility for every added provider thread. Onlyissue_ready_remediation_provider_growth_authorization may turn that sealed
authority into the existing signed REMEDIATION_COMPLETED scope. The path
consumes the remaining normal remediation slot and grants no review,
classification, resolution, recovery, continuation, or recursive-growth
authority.
Capture its H1 state only through the read-onlyresolve-batch --ready-remediation-provider-binding FILE --capture-reviewed-state FILE form. The command derives the reviewed provider
head from the verifier-created ephemeral projection of protected CURRENT, and
the authority verifier independently repeats the same capture before issuance.
Run the finite invocation
The following state machine applies only to the full feedback-remediation path.
- Create a mode-
0700temporary session directory. Capture stable feedback
once withresolve-batch --capture-reviewed-state; do not create a Package
2.1 or Package 2.2 snapshot in normal mode. - Split compound comments into stable logical findings and classify every item
from source, tests, and repository context. Preserve each finding's source
identity/digest, optional unresolved thread, classification, disposition,
and evidence. Cover top-level reviews, conversation comments, and unresolved
thread comments. Reactions and unrelated feedback are not thread-resolution
preconditions. Never infer truth from reviewer identity, keywords, or CI. - Reproduce every valid finding, add a failing test first, make the smallest
coherent corrections, and use focused validation while editing.
Never use a complete, repository-wide, or aggregate suite as focused
validation by default. If the relevant regression is available only through
such a suite, isolate the smallest direct test, filter, or fixture before
running it. A registered focused-only command explicitly authorized by its
matching manual gate is the bounded exception. It remains excluded from
unconditional complete validation. - Perform the one holistic audit across correctness, security, privacy, data
integrity, lifecycle, rollout, and avoidable complexity. Complete all source,
provenance, edge-case, and diff inspection here, and fix material in-scope
defects before the complete validation. - Finalize the eligibility manifest for every thread that may be resolved,
binding its classifications, dispositions, finding IDs, evidence digests,
any exact tracked-follow-up identities, reviewed head, and reviewed-state
digest. Stage the finished tree and run
the registered unconditional focused commands plus every required local
validation exactly once throughattest-validation, supplying that manifest
and explicit satisfied evidence for every registered manual gate. Preserve
its deterministic staged-tree,
parent-head, registry, command-set, manual-gate, result, and reviewed-feedback
receipt. Do not continue discovery or change that candidate tree after this
step begins.
A failed command produces no receipt and rejects that candidate tree. Do not
retry the unchanged failed candidate or represent it as successful evidence.
If the failure is diagnosed, its focused correction remains inside current
authority, no external mutation occurred, and the changed tree receives
focused validation plus a fresh holistic audit, perform one complete
validation of the changed candidate in the same invocation. If that
corrected candidate also fails, stop with a validation blocker; do not create
a third candidate or validation attempt. Require a new
invocation only when correction needs new scope, recovery, authority, or a
user decision. Never repeat a successful complete validation on an unchanged
tree. - When remediation changed the staged tree, create one signed commit containing
exactly that tree, use the receipt digest as its singleSecPal-Validation-Receipttrailer, and useattest-validation --bind-commit
to verify that signed binding and its local signature without rerunning
validation. When remediation changes no tracked source file and every finding
is safely disposed, prove the local, remote, and PR heads still equal the
reviewed head and skip the commit and push states; never create an artificial
empty commit. A receipt created after that existing commit is not
authenticated by it and cannot authorize a thread-resolution mutation; stop
without commit-bound resolution when no authenticated fix-commit attestation
exists. Only an exact post-final-push thread satisfying the separately signed
late-disposition contract above may use its resolution-only exception. - For the changed-tree path only, recheck the remote predecessor, push once
without bypassing local hooks, and verify that local, remote, and PR heads
equal the signed commit. Do not inspect hosted CI as a consequence of the
push. - Resolve every eligible fixed thread with
scripts/secpal-resolve-fixed-threads.py --apply, binding the exact
repository, PR, repository root, current head OID, reviewed-state file and
digest, successful validation evidence for that fix commit, exact
per-thread eligibility evidence whose canonical digest is authenticated by
the signed receipt, and thread IDs. A Recovery-bound ordinary attestation
also requires the retained delivery issue, accepted Recovery document, and
exact signed orchestration authorization described above. This
reads only the named targets, requires their comments to equal the reviewed
feedback, and does not reclassify or gate on unrelated PR state. - On ordinary success, report only
RESULT,HEAD,MUTATIONS,EVIDENCE,
relevant lifecycle counters,BLOCKER, andNEXT DECISION. Add detailed
audit evidence for blockers, exceptional paths, or complex security
decisions. The helper then stops; a separately maintained merge mechanism
may continue only under explicit current authority.
Short-circuit immediately to the applicable terminal outcome when a blocker is
detected. Green CI alone never establishes technical truth or merge readiness.
Fast-path and forensic-plan discipline
Only when the current user instruction explicitly requests readiness or merge
evaluation, validate fast-path batch inputs againstreferences/fast-path-batch.schema.json. Such a batch may contain onlyTHREAD_RESOLUTION operations and must bind repository, PR, expected head,
reviewed base branch/SHA, authenticated actor, reviewed-state and feedback digests, and eligible
classified findings. The reviewed state must originate from an open PR. Every
top-level review/comment and its reactions, stable pull-request reaction, unresolved
reviewed thread, and comment/reaction in it must be covered; PR-level EYES
activity markers are excluded before digesting. Each resolution names exactly its threaded
findings, and classification/disposition pairs must follow policy. A
partial failure stops later writes, reports every applied/failed/blocked target,
and never retries a write. Applied-target report entries are audit output, not
reusable authorization. A manual rerun with an already-resolved or otherwise changed thread fails closed.
The normal stable-feedback projection contains review identities, body digests,
thread/comment identities and state, stable reactions, actors, repository, PR,
head, and the reviewed base branch/SHA. It excludes PR-level EYES activity
markers and contains no Required Check results.
Explicitly requested volatile readiness separately contains heads, the registered default/base
repository boundary, Required Checks, mergeability and merge-state policy,
worktree, signatures, the signed validation-receipt trailer, and the
validation-attestation identity. A readiness request performs one bounded
current-state read, reports it, and stops without polling, waiting, sleeping, or
automatic repetition. Merge remains separately user-authorized.
The following immutable mutation-plan rules remain available only for explicit
forensic/audit mode.
Validate plans against
references/mutation-plan.schema.json.
The plan must preserve every source ID, contain no secrets, match the selected
production registry entry, bind each operation to the same immutable
repository/PR/snapshot/finding/target/head state, and serialize deterministically.
It may contain only REACTION, EVIDENCE_REPLY, and THREAD_RESOLUTION
operations.
inspect-actor is an exact read-only identity query used to bind the intended
writer. Each operation separately binds its immutable source actor. Without--apply, an action command may make one bounded idempotency read but
must make zero writes. With --apply, it reads the current target, confirms the
expected actor, target, and head, applies at most once, and reports the returned
mutation identity. Record that identity in later-state plans so authorized
writes can be distinguished from late feedback. A failure ends the invocation
without retry.
Reporting
Report every finding's sources, classification, proof, disposition, and any
authorized operation identity. For ordinary remediation, report only the
commit, branch, remote synchronization, local validation, worktree state, PR
identity, and resolution results. Report hosted-CI state only when the current
user instruction explicitly requests one status or readiness read; report the
observed state once and stop without suggesting monitoring or another run. Do
not post redundant “fixed,” “addressed,” SHA-status, or progress comments on the
PR.