Files
gitea-agent/harness/agents/review/skills/gitea-code-review/references/native-review.md
T

15 KiB

Native code-review publication adapter

Use the canonical harness source at <HARNESS_ROOT>/harness/agents/review/skills/gitea-code-review/scripts/native_review.py. Do not copy the helper into a standalone profile: it reuses public JSON, complete-pagination, cache-path and canonical spec-review codecs from the adjacent harness spec skill. The spec publisher/renderer and spec source files are not changed.

This adapter is a publication guard, not a substitute for the main reviewer's complete diff, surrounding-code, test and runtime review. Keep the existing approval/blocker rubric. A blocked review is local only and has no Native write payload.

Supported interface

  • Python API: build_payload(plan), validate_snapshot(plan, snapshot), verify_publication(plan, before, after, exact), and publish(plan, baseline, api, lease, journal_directory).
  • REST bridge: ApiAdapter(authenticated_request, origin=..., credential_source=...).
  • Primary lease bridge: BrokerIssueLease(broker_handle, absolute_cache_policy, deadline_monotonic) backed by managed_lock_broker.py; HeldIssueLease(issue_fd, absolute_cache_policy, deadline_monotonic) remains compatibility-only for already-admitted legacy callers. Broker requests are limited to status, renew, acquire_repository, release_repository and release; renew before expiry and use canonical release/wait/reap/probe cleanup.
  • Read-only CLI: preflight --plan PLAN --baseline SNAPSHOT and verify --plan PLAN --before BEFORE --after AFTER --exact EXACT.
  • There is intentionally no live publish CLI. Live publication uses the Python API with the workflow's existing authenticated transport and still-held Issue lock. Never invent a capture, build, publish, or submit command or guess a legacy helper schema.

Use Python 3 stdlib; no package installation is needed. Discover an absolute canonical HARNESS_ROOT using the repository-cache contract first. Invoke the canonical helper, not a relocated skill copy.

Plan schema: gitea-agent.code-review-publication/v1

All fields are explicit; extra/missing fields fail closed.

  • schema: the literal schema above.
  • target: owner, repository, positive integer issue, positive integer pull, lowercase full 40-hex head and base.
  • contract_sha256: lowercase 64-hex SHA-256 of the exact, unmodified UTF-8 Discovery Issue body.
  • reviewer: exact {id, login} from authenticated /user; login must be gitea-review-agent.
  • spec_reviewer: exact {id, login} from authoritative /users/gitea-spec-agent; never infer an account ID from historical comments.
  • spec_review_id: the single nonstale, nondismissed, canonical Native APPROVED spec review on this head and contract.
  • spec_paths: unique, safe repository-relative paths in the approved spec's evidence scope. Supply source-head-verified surrounding paths as well as changed paths; an approved spec may legitimately reference unchanged tests or callers. Do not restrict this set to the PR diff or read another role profile's artifacts to reconstruct it. The main review/admission must establish this scope.
  • verdict: exactly approve or request_changes. Do not normalize malformed identifiers, hashes, states or verdicts.
  • body: finalized UTF-8 review body, beginning with the exact code-review marker and containing exactly one - Verdict: \`line.marker(plan)creates the binding line:`.
  • provider_contract: three explicit booleans supersede_same_author, retouch_dismissed, unordered_commit_files, and evidence, a list of redacted, verifiable provider-contract/observation references. Disable exceptions lacking evidence; never set every flag simply to make a failing check pass.

Owner/repository currently support ASCII alphanumeric, underscore, dot and hyphen identifiers, with an alphanumeric/underscore first character. Unsupported names are rejected, not rewritten. Inline comments in a new review are not supported: the Native payload always explicitly sets comments: [].

Trusted transport and lease prerequisites

Keep gitea-access.md and repository-cache.md authoritative. Before calling publish:

  1. Admit the canonical, existing role-owned repository cache and impl-created, registered detached/locked worktree. Complete the independent code review and collect evidence on the exact source head.
  2. Authenticate one transport from the current role's $HERMES_HOME/.env GITEA_TOKEN, or an explicitly authorized role-scoped external secret source. No ambient-token fallback, cross-profile credentials, credential-bearing Git URLs or anonymous downgrade is allowed.
  3. Validate the exact fixed origin https://gitea.jsfamily.quest/api/v1 and GET /user account ID/login. The transport must verify TLS, reject redirects, bound network timeouts and not retry mutation requests. The bridge's provenance labels are assertions supplied by this already-validated caller, not a mechanism to load or certify an arbitrary client.
  4. Use the common broker Issue lease throughout publication/readback. Pass a live monotonic deadline, not an old PID/ready.json flag. BrokerIssueLease checks broker status and canonical worktree invariants; renew before expiry. HeldIssueLease is compatibility-only and may be used only by an already-admitted legacy caller, never to create a second lock protocol. The caller must run canonical release/wait/reap/probe cleanup. A custom supervisor lease must implement the same invariants: check(plan) returns None only on success and otherwise raises; boolean/dict success flags are unsupported. Finalization follows the review execution procedure: complete required final source verification under the held lease and preserve its evidence before releasing. verified.json proves publication verification, not workflow cleanup. Retain the same publication journal and existing result artifacts when cleanup is incomplete; report workflow partial and reconcile only unfinished cleanup checks, without replaying a full review, suite or POST. Keep the common Issue-resource free=true completion requirement; do not add a released-only terminal-handle gate or a new automatic retry/heartbeat loop.
  5. Put the journal outside every source checkout in a trusted artifact parent. Use the same journal for every retry/resume of this plan, not a new directory after a timeout. The adapter creates mode 0700 directories and 0600 atomic/fsynced artifacts, rejects symlinks/hardlinks/foreign or unsafe modes, and holds a separate nonblocking journal mutex. Redact secrets before constructing plans/artifacts; do not log request headers, raw transport exceptions or token values. Journals contain complete source snapshots and must remain private.

The current helper is strict about worktree child directories: group/world-writable checkout paths are rejected. Do not change a real worktree's permissions to force publication; stop and reconcile an admission/publisher compatibility conflict. The legacy compatibility bridge is Linux-only: it validates an already-admitted broker-held whole-file lease represented by an exclusive FLOCK owned by this process in /proc/self/fdinfo, and never acquires or reacquires that resource. New callers use BrokerIssueLease and the broker public request API. It uses a root-owned, non-group/world-writable /usr/bin/git with a fixed minimal environment, not the caller's PATH.

These guards assume a cooperative, trusted role-local filesystem and transport. They do not isolate a malicious process with the same UID or pin every filesystem ancestor across subprocess path resolution. At-most-once creation is scoped to the retained journal and held canonical Issue lease, not arbitrary alternate journal paths, unrelated publishers, or multiple hosts. Do not share one lease across simultaneous publication callers or change the journal after an uncertain attempt.

The request callback is authenticated_request(relative_api_path, payload=None) -> (decoded_json, headers, integer_status). None means GET; a dict means exactly one POST. It always uses the fixed origin and original role credential, with no authorization fallback. It is provided by the existing role-local transport; do not regenerate the full gate/publisher per review run.

ApiAdapter.snapshot(target) collects repository/Issue/PR metadata, identity/spec identity, all changed files, commits, reviews, historical review comments, Issue/PR comments and head statuses. Use this same collector for the baseline and subsequent snapshots. Every collection records contiguous 50-item pages, declared totals and Link headers; duplicates, inconsistent totals and truncated enumeration fail closed. A saved older collector snapshot may be replayed if it provides these complete facts, but differently shaped collectors must not be mixed for live source-drift comparison.

Live integration

After absolute root discovery, role-authenticated transport capability validation, complete review, and Issue lease acquisition, load the module once using importlib.util.spec_from_file_location and retain it. These are existing workflow objects, not placeholders for an alternative credential source:

api = native_review.ApiAdapter(
    authenticated_request,
    origin="https://gitea.jsfamily.quest/api/v1",
    credential_source="$HERMES_HOME/.env:GITEA_TOKEN",
)
lease = native_review.BrokerIssueLease(broker_handle, absolute_cache_policy, deadline_monotonic)
baseline = api.snapshot(plan["target"])
native_review.validate_snapshot(plan, baseline)
result = native_review.publish(plan, baseline, api, lease, journal_directory)

Preserve the original baseline until publish returns verified or the uncertainty is manually reconciled. The helper rechecks authoritative role/head/base/contract/default branch/source Issue linkage/spec approval and complete source snapshots immediately before creating. It explicitly sends commit_id, body, event, comments and performs at most one create_review call per durable attempt.

A matching canonical review is exact-read and verified as a write-free Native no-op; conflicting/multiple/current-head pending reviews stop rather than overwrite or duplicate. Reposting a different body/verdict on the same binding is not an automatic update path.

A durable intent.json is fsynced before the POST. After timeout, partial success, readback error, process crash or lease expiry, resume the same journal and perform only authoritative reads. If there is no unique match, retain the attempt and report uncertainty; never delete the journal, switch directories or silently POST again. A definitive failure may require an explicitly authorized manual recovery decision, outside this helper.

Verification semantics

  • Confirm the exact new review ID, role ID/login, reviewed head, body bytes, state, target PR URL, empty inline comments, typed flags and timezone-bearing timestamps against the complete review collection.
  • Confirm precisely one new review and unchanged historical author/body/head/state/comments. No unrelated native review, comment, repository, head, branch, Issue contract or source change is silently accepted.
  • Validate a source Issue repository binding when the provider includes it. A PR head may legitimately belong to a fork: the fixed base repository, exact head SHA and unchanged complete PR metadata bind it, rather than incorrectly requiring every head repository to equal the base.
  • official is a provider merge-policy flag, not an additional verdict or role-identity requirement. Current accepted entry/publication rules require canonical role/head/contract/state and nondismissed/nonstale status; they do not require official=True. Changing that policy requires an explicit acceptance-criteria change.
  • When explicitly supported by provider evidence, same-author official-review supersession or retouching an already-dismissed same-author formal review may change only official, dismissed, updated_at. Require the specific expected false/true flag transitions and a timestamp equal to this exact new review's submitted/updated timestamp. This is not a global mutable-field mask. Timestamp/pattern agreement is provider-contract consistency evidence, not proof of a unique server-side causal transaction; a coincident manual mutation matching the full permitted pattern cannot be distinguished without additional authoritative event evidence.
  • Only commit files arrays are conditionally sorted by unique filename. File record contents, commit order, parent order and other arrays are still compared without blanket normalization. Changed-file records are compared by unique filename.
  • Source Issue/PR timestamp, PR review-count and requested-reviewer changes are narrowly tied to the exact added review; unrelated derived-state mutation fails.
  • The CLI is saved-artifact replay, not fresh network verification or approval authority. Its read_only: true output must not be presented as a newly published review. In live results wrote refers to a Native POST by that invocation; local journal writes still occur for no-ops.

Read-only commands and regression tests

python3 "$HARNESS_ROOT/harness/agents/review/skills/gitea-code-review/scripts/native_review.py" preflight \
  --plan "$RUN/plan.json" --baseline "$RUN/before.json"
python3 "$HARNESS_ROOT/harness/agents/review/skills/gitea-code-review/scripts/native_review.py" verify \
  --plan "$RUN/plan.json" --before "$RUN/before.json" --after "$RUN/after.json" --exact "$RUN/exact.json"
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s harness/tests/review -p 'test_native_review.py' -v
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s harness/tests/spec -v
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s harness/tests/impl -v

Run the test commands from the canonical harness repository root. The existing spec suite requires /opt/data/cache/scratch: prepare that directory only if absent after confirming a nonsymlink, role-owned, non-group/world-writable parent, and require mode 0700 for the scratch child. Preserve existing shared parent permissions; do not chmod/chown them to satisfy a test. Use isolated/local runner variables and properly quoted command arguments rather than reusing mutable interpreter globals across review steps. Keep local stub/loopback transport checks, real temporary Git/worktree/lease checks, and saved production-wire replay clearly distinguished from a live Gitea mutation test. Introducing this helper does not authorize changing any existing product PR verdict, source, Issue state, merge or deployment.