SecPal

secpal-pr-review

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.

SecPal 0 Updated 1mo ago

Resources

2
GitHub

Install

npx skillscat add secpal/github/secpal-pr-review

Install via the SkillsCat registry.

SKILL.md

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's
scripts/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 protected
CURRENT 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 \
  --apply

Repeat --thread-id REVIEW_THREAD_NODE_ID for each additional fixed thread.

When and only when the final attestation carries an
exceptional_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.json

Omit 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. Use
scripts/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 derives
NO_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/repository and 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: valid metadata 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-batch for 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-validation for one deterministic
    complete-validation attestation bound to the finished local head; and
  • scripts/secpal-resolve-fixed-threads.py for default post-push resolution of
    eligible fixed threads and for the resolution-only path defined above; and
  • scripts/secpal-pr-review.py plus 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 reports
REVIEW_NOT_TERMINAL; the same workspace may resume after external state
changes.

Finite lifecycle continuity

When a completed-feedback workflow consumes persistent lifecycle state, use
scripts/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 a
Ready -> Draft mutation only when the current user instruction explicitly
authorizes that exact transition and gives its separate reason. Any later
Draft -> 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. Only
issue_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-only
resolve-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.

  1. Create a mode-0700 temporary session directory. Capture stable feedback
    once with resolve-batch --capture-reviewed-state; do not create a Package
    2.1 or Package 2.2 snapshot in normal mode.
  2. 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.
  3. 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.
  4. 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.
  5. 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 through attest-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.
  6. When remediation changed the staged tree, create one signed commit containing
    exactly that tree, use the receipt digest as its single
    SecPal-Validation-Receipt trailer, and use attest-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.
  7. 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.
  8. 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.
  9. On ordinary success, report only RESULT, HEAD, MUTATIONS, EVIDENCE,
    relevant lifecycle counters, BLOCKER, and NEXT 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 against
references/fast-path-batch.schema.json. Such a batch may contain only
THREAD_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.