From a0d110dc09c490946ad232e2d05105fce8b3032b Mon Sep 17 00:00:00 2001 From: Allora Bisk <259535990+allora2026@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:30:19 +0000 Subject: [PATCH] fix: retry incomplete assessments and encrypt attempt diagnostics --- README.md | 10 +++ action.yml | 36 +++++++++- scripts/capture-provider-attempt.py | 66 +++++++++++++++++++ scripts/encrypt-validation-report.py | 16 +++-- scripts/extract-provider-report.py | 51 ++++++++++++-- scripts/parse-validation-report.py | 12 +++- scripts/test-action-dependencies.sh | 2 +- scripts/validate.sh | 60 +++++++++++++++-- system-prompt.md | 2 +- tests/test_assessment_completion.py | 90 +++++++++++++++++++++++++ tests/test_attempt_capture.py | 99 ++++++++++++++++++++++++++++ 11 files changed, 424 insertions(+), 20 deletions(-) create mode 100644 scripts/capture-provider-attempt.py create mode 100644 tests/test_assessment_completion.py create mode 100644 tests/test_attempt_capture.py diff --git a/README.md b/README.md index 850e71f..ba9fe93 100644 --- a/README.md +++ b/README.md @@ -1125,3 +1125,13 @@ artifact's workflow/run/head provenance and investigate the cited code. Encrypti protects confidentiality and ciphertext integrity; it does not authenticate a reviewer's identity or make a finding correct. Do not paste decrypted content into public comments, logs, or a public validator PR. + +### Opt-in assessment completion and encrypted attempt diagnostics + +`require-complete-assessment: 'true'` requires exactly one `- **Assessment**: COMPLETE` line in the final report's Validation Outcome and an OpenCode terminal finish reason of `stop`. Missing or INCOMPLETE assessments consume the existing whole-review retry budget, then the configured fallback, under the same shared deadline. A completed FAIL, including zero critical issues, remains a completed blocking verdict. Defaults preserve existing consumers. This status verifies the execution contract, not the truth of claimed review coverage. + +Separate `encrypted-attempt-recipient` and `encrypted-attempt-recipient-sha256` inputs explicitly authorize exporting each attempt's assembled prompt, provider JSON stdout and stderr. These can contain confidential source and tool results. Supply a trusted RSA public certificate and its SHA256 DER fingerprint, independently of PR-controlled content; keep its private key off-runner. No environment dump or provider configuration is collected. The action captures immediately after every provider invocation, before extraction and before retry files are overwritten. The bundle includes exact input hashes, attempt/model/exit status and allowlisted run/ref provenance. It encrypts using authenticated CMS AES-256-GCM and RSA-OAEP/SHA256, uploads ciphertext only with one-day retention, then removes its local ciphertext during unconditional cleanup. Capture failure blocks execution, with no plaintext fallback. Missing private final reports no longer prevent diagnostics when this separate option is enabled. + +Public extraction metadata includes allowlisted finish reasons, bounded numeric token/cache/cost fields, and tool-state counts. Unknown text and malformed numeric fields become fixed unknown/null values. No commands, tool arguments, arbitrary errors or raw transcript fields enter these diagnostics. The diff and web-fetch policy now permits read-only Git scope verification and GitHub API GET requests for exact-head CI evidence without permitting general external downloads or GitHub mutations. + +This candidate does not establish why a provider stopped early. Use captured evidence before changing context limits or models. Required grounding, existing model configuration, strict terminal extraction, publication policy and failed-verdict gates remain in force. Consumer adoption, stable release and shared alias promotion are separate gates. diff --git a/action.yml b/action.yml index 5b71f27..0fa0315 100644 --- a/action.yml +++ b/action.yml @@ -78,6 +78,18 @@ inputs: description: 'Trusted SHA256 of the recipient certificate DER (64 hex characters). Required with encrypted-report-recipient; never derive from PR-controlled input.' required: false default: '' + encrypted-attempt-recipient: + description: 'Explicit opt-in RSA PEM recipient for encrypted per-attempt prompt/stdout/stderr capture. These may contain confidential source/tool data. Keep the private key off-runner; authorize this separate export.' + required: false + default: '' + encrypted-attempt-recipient-sha256: + description: 'Trusted SHA256 DER fingerprint for the attempt recipient; required together. Never derive from untrusted PR input.' + required: false + default: '' + require-complete-assessment: + description: 'Opt in to explicit COMPLETE assessment and normal OpenCode stop reason. Missing/incomplete assessment retries within the shared deadline; completed FAIL remains blocking.' + required: false + default: 'false' max-retries: description: 'Maximum retry attempts for retryable provider failures or incomplete final reports' required: false @@ -174,6 +186,7 @@ runs: VALIDATOR_COMMENT_MODE: ${{ inputs.comment-mode }} VALIDATOR_MERGE_PROMPT: ${{ inputs.merge-custom-prompt }} VALIDATOR_ALLOW_WEB_FETCH: ${{ inputs.allow-web-fetch }} + VALIDATOR_COMPLETE_ASSESSMENT: ${{ inputs.require-complete-assessment }} VALIDATOR_MAX_RETRIES: ${{ inputs.max-retries }} VALIDATOR_TIMEOUT: ${{ inputs.timeout-minutes }} VALIDATOR_PROVIDER: ${{ inputs.provider }} @@ -245,6 +258,11 @@ runs: exit 1 fi + if [[ ! "$VALIDATOR_COMPLETE_ASSESSMENT" =~ ^(true|false)$ ]]; then + echo "::error::Invalid require-complete-assessment. Must be true or false" + exit 1 + fi + # Validate provider if [[ ! "$VALIDATOR_PROVIDER" =~ ^(opencode|gemini)$ ]]; then echo "::error::Invalid provider. Must be: opencode or gemini" @@ -491,6 +509,9 @@ runs: OPENCODE_MODEL: ${{ inputs.opencode-model }} FALLBACK_OPENCODE_PROVIDER: ${{ inputs.fallback-opencode-provider }} FALLBACK_OPENCODE_MODEL: ${{ inputs.fallback-opencode-model }} + ENCRYPTED_ATTEMPT_RECIPIENT: ${{ inputs.encrypted-attempt-recipient }} + ENCRYPTED_ATTEMPT_RECIPIENT_SHA256: ${{ inputs.encrypted-attempt-recipient-sha256 }} + REQUIRE_COMPLETE_ASSESSMENT: ${{ inputs.require-complete-assessment }} MAX_RETRIES: ${{ inputs.max-retries }} VALIDATION_TIMEOUT_MINUTES: ${{ inputs.timeout-minutes }} PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} @@ -514,6 +535,15 @@ runs: run: | ${{ github.action_path }}/scripts/validate.sh + - name: Upload Encrypted Attempt Diagnostics + if: always() && steps.validate.outputs.attempt_diagnostics_ready == 'true' && steps.publication.outputs.report_path != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ${{ steps.set-artifact-name.outputs.artifact_name }}-attempts-encrypted + path: ${{ steps.publication.outputs.report_path }}.attempts/*.cms + retention-days: 1 + if-no-files-found: error + - name: Upload Encrypted Final Report if: always() && steps.validate.outputs.encrypted_report_ready == 'true' && steps.publication.outputs.report_path != '' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -614,14 +644,16 @@ runs: # Only this invocation's known file/directory, never a broad tmp cleanup. if [ -n "$REPORT_PATH" ]; then rm -f -- "$REPORT_PATH" "${REPORT_PATH}.cms" + rm -f -- "${REPORT_PATH}.attempts/"*.cms + rmdir -- "${REPORT_PATH}.attempts" 2>/dev/null || true rmdir -- "$(dirname "$REPORT_PATH")" 2>/dev/null || true fi echo "✅ Temporary files cleaned up" - - name: Fail on Critical Violations + - name: Fail on Unsuccessful Validation if: inputs.fail-on-critical == 'true' && steps.validate.outputs.validation_passed == 'false' shell: bash run: | - echo "::error::❌ Critical violations found in PR validation" + echo "::error::Validation failed or assessment did not complete" echo "Critical issues: ${{ steps.validate.outputs.critical_issues }}" exit 1 diff --git a/scripts/capture-provider-attempt.py b/scripts/capture-provider-attempt.py new file mode 100644 index 0000000..143b65c --- /dev/null +++ b/scripts/capture-provider-attempt.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Opt-in action-owned encrypted attempt capture. No raw diagnostics on stdout.""" +import argparse +import base64 +import hashlib +import importlib.util +import json +import os +from pathlib import Path +import stat +import subprocess +import sys + +SPEC = importlib.util.spec_from_file_location( + "encryption", Path(__file__).with_name("encrypt-validation-report.py")) +ENCRYPTION = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(ENCRYPTION) +MAX_BYTES = 16 * 1024 * 1024 + + +def read_input(path): + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK) + with os.fdopen(fd, "rb") as handle: + if not stat.S_ISREG(os.fstat(handle.fileno()).st_mode): + raise ValueError("not_regular") + data = handle.read(MAX_BYTES + 1) + if len(data) > MAX_BYTES: + raise ValueError("too_large") + return {"sha256": hashlib.sha256(data).hexdigest(), "bytes": len(data), + "base64": base64.b64encode(data).decode("ascii")} + + +def capture(args): + if not 1 <= args.attempt <= 100 or not 0 <= args.exit_code <= 255: + raise ValueError("invalid_attempt") + bundle = { + "schema": 1, "attempt": args.attempt, "provider": args.provider, + "model": args.model, "exit_code": args.exit_code, + "provenance": {key: os.environ.get(key, "") for key in ( + "GITHUB_RUN_ID", "GITHUB_RUN_ATTEMPT", "GITHUB_JOB", + "GITHUB_SHA", "BASE_BRANCH", "HEAD_BRANCH")}, + "inputs": {name: read_input(getattr(args, name)) for name in ("prompt", "stdout", "stderr")}, + } + # Deliberately never collect environment dumps or provider credential configs. + ENCRYPTION.encrypt_payload(json.dumps(bundle, separators=(",", ":")).encode(), args.output, + os.environ.get("ENCRYPTED_ATTEMPT_RECIPIENT", ""), + os.environ.get("ENCRYPTED_ATTEMPT_RECIPIENT_SHA256", "")) + + +def main(): + parser = ENCRYPTION.SafeParser() + for name in ("prompt", "stdout", "stderr", "output", "model"): + parser.add_argument("--" + name, required=True) + parser.add_argument("--provider", choices=("opencode", "gemini"), required=True) + parser.add_argument("--attempt", type=int, required=True) + parser.add_argument("--exit-code", type=int, required=True) + try: + capture(parser.parse_args()) + except (OSError, ValueError, OverflowError, UnicodeError, subprocess.SubprocessError): + print("Encrypted attempt capture failed; no plaintext fallback.", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/encrypt-validation-report.py b/scripts/encrypt-validation-report.py index 1190baf..a571f88 100644 --- a/scripts/encrypt-validation-report.py +++ b/scripts/encrypt-validation-report.py @@ -26,9 +26,7 @@ def openssl(*args, data=None): ).stdout -def encrypt(report, output): - pem = os.environ.get("ENCRYPTED_REPORT_RECIPIENT", "") - fingerprint = os.environ.get("ENCRYPTED_REPORT_RECIPIENT_SHA256", "") +def encrypt_payload(data, output, pem, fingerprint): if not re.fullmatch(r"[a-fA-F0-9]{64}", fingerprint): raise ValueError("invalid_fingerprint") if len(pem) > 16384 or not re.fullmatch( @@ -36,8 +34,6 @@ def encrypt(report, output): r"-----END CERTIFICATE-----\s*", pem ): raise ValueError("invalid_certificate") - text = PUBLICATION.read_private(report) - PUBLICATION.PARSER.parse_report(text) if not openssl("version").startswith(b"OpenSSL 3."): raise ValueError("openssl_3_required") with tempfile.TemporaryDirectory(prefix="validator-recipient-") as directory: @@ -66,7 +62,7 @@ def encrypt(report, output): "cms", "-encrypt", "-binary", "-aes-256-gcm", "-outform", "DER", "-recip", str(certificate), "-keyopt", "rsa_padding_mode:oaep", "-keyopt", "rsa_oaep_md:sha256", "-keyopt", "rsa_mgf1_md:sha256", - data=text.encode("utf-8"), + data=data, ) if not ciphertext: raise ValueError("empty_ciphertext") @@ -84,6 +80,14 @@ def encrypt(report, output): temporary.unlink(missing_ok=True) +def encrypt(report, output): + text = PUBLICATION.read_private(report) + PUBLICATION.PARSER.parse_report(text) + encrypt_payload(text.encode("utf-8"), output, + os.environ.get("ENCRYPTED_REPORT_RECIPIENT", ""), + os.environ.get("ENCRYPTED_REPORT_RECIPIENT_SHA256", "")) + + class SafeParser(argparse.ArgumentParser): def error(self, message): self.exit(2, "Encrypted report rejected: invalid arguments.\n") diff --git a/scripts/extract-provider-report.py b/scripts/extract-provider-report.py index 2c88d2f..7258e05 100755 --- a/scripts/extract-provider-report.py +++ b/scripts/extract-provider-report.py @@ -15,6 +15,7 @@ import argparse from collections import Counter import json +import math import re import sys @@ -45,7 +46,44 @@ def _response_metadata(provider, response): } -def _opencode(text): +FINISH_REASONS = {"stop", "tool-calls", "length", "content-filter", "error", "other", "unknown"} + + +def _number(value, maximum): + if type(value) is int and 0 <= value <= maximum: + return value + if type(value) is float and math.isfinite(value) and 0 <= value <= maximum: + return value + return None + + +def _step_metadata(events): + steps = [] + tools = Counter() + for event in events: + part = event.get("part") + if not isinstance(part, dict): + continue + if event.get("type") == "tool_use": + state = part.get("state") + status = state.get("status") if isinstance(state, dict) else None + tools[status if status in ("completed", "error", "running", "pending") else "unknown"] += 1 + if event.get("type") != "step_finish": + continue + reason = part.get("reason") + item = {"reason": reason if isinstance(reason, str) and reason in FINISH_REASONS else "unknown"} + tokens = part.get("tokens") + if isinstance(tokens, dict): + item["tokens"] = {key: _number(tokens.get(key), 10**12) for key in ("total", "input", "output", "reasoning")} + cache = tokens.get("cache") + if isinstance(cache, dict): + item["cache"] = {key: _number(cache.get(key), 10**12) for key in ("read", "write")} + item["cost"] = _number(part.get("cost"), 10**6) + steps.append(item) + return {"steps": steps[-128:], "steps_omitted": max(0, len(steps) - 128), "tool_states": dict(tools)} + + +def _opencode(text, require_stop_reason=False): events = [] counts = Counter() message_ids = set() @@ -95,6 +133,8 @@ def _opencode(text): "terminal_event": _safe_event_type(events[-1].get("type")) if events else None, }) + diagnostics.update(_step_metadata(events)) + if not events: diagnostics["error_code"] = "empty_output" raise ExtractionError("empty_output", "OpenCode output has no events", diagnostics) @@ -124,6 +164,8 @@ def _opencode(text): "OpenCode terminal step has an invalid part", diagnostics, ) + if require_stop_reason and terminal_part.get("reason") != "stop": + raise ExtractionError("terminal_reason_not_stop", "OpenCode did not finish normally", diagnostics) message_id = terminal_part.get("messageID") if not isinstance(message_id, str) or not message_id: diagnostics["error_code"] = "missing_terminal_message_id" @@ -232,9 +274,9 @@ def _gemini(text): return response, diagnostics -def extract_with_diagnostics(provider, text): +def extract_with_diagnostics(provider, text, require_stop_reason=False): if provider == "opencode": - return _opencode(text) + return _opencode(text, require_stop_reason=require_stop_reason) if provider == "gemini": return _gemini(text) raise ExtractionError("unsupported_provider", "unsupported provider", {"provider": provider}) @@ -256,11 +298,12 @@ def _log_diagnostics(diagnostics): def main(argv=None): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--provider", choices=("opencode", "gemini"), required=True) + parser.add_argument("--require-stop-reason", action="store_true") parser.add_argument("input") args = parser.parse_args(argv) try: with open(args.input, encoding="utf-8") as handle: - result, diagnostics = extract_with_diagnostics(args.provider, handle.read()) + result, diagnostics = extract_with_diagnostics(args.provider, handle.read(), require_stop_reason=args.require_stop_reason) except OSError as exc: _log_diagnostics({"provider": args.provider, "error_code": "input_read_failed"}) print(f"Could not extract provider response: {exc}", file=sys.stderr) diff --git a/scripts/parse-validation-report.py b/scripts/parse-validation-report.py index 3090b50..5fe2b3d 100755 --- a/scripts/parse-validation-report.py +++ b/scripts/parse-validation-report.py @@ -60,7 +60,7 @@ def normalize_report(text): return normalized -def parse_report(text): +def parse_report(text, require_complete=False): text = normalize_report(text) if text.count(OUTCOME) != 1: raise ValueError("report must contain exactly one validation outcome") @@ -72,6 +72,13 @@ def parse_report(text): critical_counts = CRITICAL_RE.findall(outcome) if len(statuses) != 1 or len(critical_counts) != 1: raise ValueError("validation outcome must contain one status and one critical count") + if require_complete: + # Count every field, including annotated or malformed values, before + # validating its value. A second contradictory field must not vanish. + assessment = re.findall(r"^[ \t]*[-*][ \t]+(?:\*\*)?Assessment(?:\*\*)?[ \t]*:([^\r\n]*)$", outcome, re.MULTILINE) + canonical = re.findall(r"^- \*\*Assessment\*\*:[ \t]*COMPLETE[ \t]*$", outcome, re.MULTILINE) + if len(assessment) != 1 or len(canonical) != 1: + raise ValueError("assessment_missing_or_incomplete") status = statuses[0] critical_issues = int(critical_counts[0]) if critical_issues > 1000: @@ -90,6 +97,7 @@ def parse_report(text): def main(argv=None): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--normalize", action="store_true", help="print normalized report Markdown") + parser.add_argument("--require-complete-assessment", action="store_true") parser.add_argument("report") args = parser.parse_args(argv) try: @@ -108,7 +116,7 @@ def main(argv=None): return 0 try: - result = parse_report(text) + result = parse_report(text, require_complete=args.require_complete_assessment) except ValueError as exc: print(f"Invalid validation report: {exc}", file=sys.stderr) return 1 diff --git a/scripts/test-action-dependencies.sh b/scripts/test-action-dependencies.sh index c10d9ba..bd4490a 100755 --- a/scripts/test-action-dependencies.sh +++ b/scripts/test-action-dependencies.sh @@ -30,7 +30,7 @@ assert_count 1 \ assert_count 1 \ '^[[:space:]]*uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9\.0\.0$' \ 'github-script v9.0.0 pin' -assert_count 2 \ +assert_count 3 \ '^[[:space:]]*uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7\.0\.1$' \ 'upload-artifact v7.0.1 pins' assert_count 1 \ diff --git a/scripts/validate.sh b/scripts/validate.sh index 9b58f38..efdcfd7 100755 --- a/scripts/validate.sh +++ b/scripts/validate.sh @@ -391,7 +391,7 @@ ${OVERRIDE_COMMENT} if [ "${ALLOW_WEB_FETCH:-false}" = "true" ]; then web_fetch_policy="**Web fetch is ENABLED** for this validation. You may use the \`web_fetch\` tool to retrieve external resources if needed for validation (e.g., checking external documentation, standards, or references). Use this capability responsibly and only when necessary." else - web_fetch_policy="**Web fetch is DISABLED** for this validation. DO NOT use the \`web_fetch\` tool or attempt to download content from URLs. All validation must be performed using only the git repository contents, PR context, and Usable MCP knowledge base." + web_fetch_policy="**Web fetch is DISABLED** for this validation. DO NOT use the \`web_fetch\` tool for general external browsing. Read-only GitHub API GET requests for exact-head repository CI evidence are allowed; no external source downloads or GitHub mutations are authorized. Use repository contents, PR context and Usable grounding for standards." fi # Read prompt template @@ -410,6 +410,14 @@ ${OVERRIDE_COMMENT} PROMPT_CONTENT="${PROMPT_CONTENT//\{\{PR_AUTHOR\}\}/${PR_AUTHOR}}" PROMPT_CONTENT="${PROMPT_CONTENT//\{\{PR_LABELS\}\}/${PR_LABELS:-none}}" + if [ "${REQUIRE_COMPLETE_ASSESSMENT:-false}" = true ]; then + PROMPT_CONTENT="${PROMPT_CONTENT} + +## Assessment completion contract +Add exactly one line in Validation Outcome: - **Assessment**: COMPLETE or - **Assessment**: INCOMPLETE. +COMPLETE means the assigned whole-PR review and required evidence are assessed, independently of PASS/FAIL. A completed review with legitimate findings must still say COMPLETE and FAIL. Unassessed material areas require INCOMPLETE and FAIL. Missing or INCOMPLETE status retries the review within the existing shared deadline and fallback policy, never becomes PASS." + fi + if [ -f "$REQUIRED_GROUNDING_FILE" ]; then PROMPT_CONTENT="${PROMPT_CONTENT} @@ -445,6 +453,29 @@ $(cat "$REQUIRED_GROUNDING_FILE")" echo "$output_file" } +# Capture before extraction or the next retry truncates the private files. +PROVIDER_ATTEMPT_COUNT=0 +capture_provider_attempt() { + local prompt_file="$1" provider="$2" model="$3" exit_code="$4" + if [ -z "${ENCRYPTED_ATTEMPT_RECIPIENT:-}${ENCRYPTED_ATTEMPT_RECIPIENT_SHA256:-}" ]; then + return 0 + fi + PROVIDER_ATTEMPT_COUNT=$((PROVIDER_ATTEMPT_COUNT + 1)) + local directory="${REPORT_PATH}.attempts" + mkdir -p "$directory" + chmod 700 "$directory" + if ! python3 "$SCRIPT_DIR/scripts/capture-provider-attempt.py" \ + --prompt "$prompt_file" --stdout /tmp/validation-full-output.md \ + --stderr /tmp/validation-provider-stderr.log --provider "$provider" \ + --model "$model" --attempt "$PROVIDER_ATTEMPT_COUNT" --exit-code "$exit_code" \ + --output "$directory/attempt-${PROVIDER_ATTEMPT_COUNT}.cms"; then + return 1 + fi + if [ -n "${GITHUB_OUTPUT:-}" ]; then + echo "attempt_diagnostics_ready=true" >> "$GITHUB_OUTPUT" + fi +} + # Run Gemini validation with retry logic run_gemini() { local prompt_file="$1" @@ -500,6 +531,7 @@ run_gemini() { local exit_code=$? set -e # Re-enable exit on error + capture_provider_attempt "$prompt_file" gemini "$GEMINI_MODEL" "$exit_code" || return 1 echo "" echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" @@ -507,6 +539,21 @@ run_gemini() { if [ $exit_code -eq 0 ]; then echo "✅ Gemini CLI completed successfully (exit code: 0)" echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" + if [ "${REQUIRE_COMPLETE_ASSESSMENT:-false}" = true ]; then + local probe_file + probe_file=$(mktemp /tmp/validation-report.candidate.XXXXXX) + chmod 600 "$probe_file" + if ! extract_report /tmp/validation-full-output.md gemini "$probe_file"; then + rm -f "$probe_file" + retry_count=$((retry_count + 1)) + if [ "$retry_count" -gt "$max_retries" ]; then + return 1 + fi + sleep_before_validation_retry "$((2 ** retry_count))" || return 1 + continue + fi + rm -f "$probe_file" + fi return 0 else echo "❌ Gemini CLI failed (exit code: $exit_code)" @@ -610,6 +657,7 @@ run_opencode() { local exit_code=$? set -e + capture_provider_attempt "$prompt_file" opencode "$full_model" "$exit_code" || return 1 echo "" echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" @@ -689,8 +737,12 @@ extract_report() { return 1 fi - if ! "$SCRIPT_DIR/scripts/extract-provider-report.py" \ - --provider "$provider" "$full_output" > "$candidate_file"; then + local extractor_options=(--provider "$provider" "$full_output") parser_options=("$candidate_file") + if [ "${REQUIRE_COMPLETE_ASSESSMENT:-false}" = true ]; then + extractor_options+=(--require-stop-reason) + parser_options+=(--require-complete-assessment) + fi + if ! "$SCRIPT_DIR/scripts/extract-provider-report.py" "${extractor_options[@]}" > "$candidate_file"; then echo "::error::Could not isolate the final assistant response from provider output" return 1 fi @@ -705,7 +757,7 @@ extract_report() { fi mv -f "$normalized_file" "$candidate_file" - if ! "$SCRIPT_DIR/scripts/parse-validation-report.py" "$candidate_file" >/dev/null; then + if ! "$SCRIPT_DIR/scripts/parse-validation-report.py" "${parser_options[@]}" >/dev/null; then echo "::error::Normalized AI output did not contain a valid structured final verdict" return 1 fi diff --git a/system-prompt.md b/system-prompt.md index 95b494f..8366333 100644 --- a/system-prompt.md +++ b/system-prompt.md @@ -270,7 +270,7 @@ After documenting the deviation: - ✅ **Read the current state** of files that changed (respecting Status) - ✅ **Focus on the modified line ranges** mentioned in the summary - ✅ **Check related files** when you need context - - ❌ **Do NOT try to run `git diff`** - you already have the change list + - ✅ Use the supplied change list, and verify the full base-to-head PR diff with read-only Git commands when scope or repository rules require it. Never substitute the latest commit for the whole PR. - ❌ **Do NOT try to read DELETED files** - respect the Status annotation **Example workflow:** diff --git a/tests/test_assessment_completion.py b/tests/test_assessment_completion.py new file mode 100644 index 0000000..8a4c4dd --- /dev/null +++ b/tests/test_assessment_completion.py @@ -0,0 +1,90 @@ +import json +import os +from pathlib import Path +import subprocess +import tempfile +import unittest +from test_validation_contract import ROOT, load + +REPORT = "# PR Validation Report\n## Validation Outcome\n- **Status**: FAIL\n- **Critical Issues**: 0\n" + + +def events(report, reason="stop"): + return [ + {"type": "step_start", "part": {"type": "step-start", "messageID": "final"}}, + {"type": "text", "part": {"type": "text", "messageID": "final", "text": report, "time": {"end": 1}}}, + {"type": "step_finish", "part": {"type": "step-finish", "messageID": "final", "reason": reason}}, + ] + + +class AssessmentTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.parser = load("completion_parser", "parse-validation-report.py") + cls.extractor = load("completion_extractor", "extract-provider-report.py") + + def test_opt_in_requires_complete_but_preserves_legitimate_fail(self): + self.assertEqual(self.parser.parse_report(REPORT)["status"], "failed") + for status in ("", "- **Assessment**: INCOMPLETE\n", "- **Assessment**: COMPLETE\n" * 2): + with self.subTest(status=status), self.assertRaises(ValueError): + self.parser.parse_report(REPORT + status, require_complete=True) + result = self.parser.parse_report(REPORT + "- **Assessment**: COMPLETE\n", require_complete=True) + self.assertFalse(result["passed"]) + self.assertEqual(result["critical_issues"], 0) + + def test_annotated_or_malformed_duplicate_assessment_cannot_bypass_completion(self): + for duplicate in ("- **Assessment**: INCOMPLETE (evidence missing)", + "- **Assessment**: unknown", "- **Assessment**:", + "- Assessment: INCOMPLETE", "* **Assessment**: INCOMPLETE"): + with self.subTest(duplicate=duplicate), self.assertRaises(ValueError): + self.parser.parse_report(REPORT + "- **Assessment**: COMPLETE\n" + duplicate + "\n", require_complete=True) + + def test_terminal_reason_and_numeric_diagnostics_never_echo_arbitrary_data(self): + for reason in (None, "length", "content-filter", "unknown", "PRIVATE_CANARY", ["PRIVATE_CANARY"]): + with self.subTest(reason=reason), self.assertRaises(self.extractor.ExtractionError): + self.extractor.extract_with_diagnostics("opencode", "\n".join(map(json.dumps, events(REPORT, reason))), require_stop_reason=True) + records = events(REPORT) + records[-1]["part"].update(tokens={"input": 42, "output": "PRIVATE_CANARY", "reasoning": True, "total": -1, "cache": {"read": 3, "write": 10**100}}, cost=float("inf")) + _, metadata = self.extractor.extract_with_diagnostics("opencode", "\n".join(map(json.dumps, records)), require_stop_reason=True) + self.assertNotIn("PRIVATE_CANARY", json.dumps(metadata)) + self.assertEqual(metadata["steps"][0]["tokens"], {"input": 42, "output": None, "reasoning": None, "total": None}) + self.assertEqual(metadata["steps"][0]["cache"], {"read": 3, "write": None}) + self.assertIsNone(metadata["steps"][0]["cost"]) + + def run_review(self, mode): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "prompt").write_text("synthetic review") + for state in ("INCOMPLETE", "COMPLETE"): + (root / state).write_text("\n".join(map(json.dumps, events(REPORT + f"- **Assessment**: {state}\n")))) + fake = root / "opencode" + fake.write_text('''#!/usr/bin/env python3 +import os, pathlib, sys +root = pathlib.Path(os.environ["PROBE_ROOT"]) +model = sys.argv[sys.argv.index("-m") + 1] +with (root / "calls").open("a") as f: f.write(model + "\\n") +count = len((root / "calls").read_text().splitlines()) +complete = os.environ["PROBE_MODE"] == "complete" or "fallback" in model or (os.environ["PROBE_MODE"] == "retry" and count > 1) +sys.stdout.write((root / ("COMPLETE" if complete else "INCOMPLETE")).read_text()) +''') + fake.chmod(0o700) + script = f'''set -euo pipefail +export ACTION_PATH={ROOT} +export VALIDATE_SH_LIBRARY_ONLY=true MAX_RETRIES=1 VALIDATION_TIMEOUT_MINUTES=1 REQUIRE_COMPLETE_ASSESSMENT=true +source {ROOT}/scripts/validate.sh +rc=0 +run_opencode {root}/prompt openrouter openai/primary || rc=$? +if [ "$rc" = 2 ]; then run_opencode {root}/prompt openrouter openai/fallback; else exit "$rc"; fi +''' + result = subprocess.run(["bash", "-c", script], env={**os.environ, "PATH": f"{root}:{os.environ['PATH']}", "PROBE_ROOT": str(root), "PROBE_MODE": mode}, capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr) + return (root / "calls").read_text().splitlines() + + def test_incomplete_assessment_uses_retry(self): + self.assertEqual(self.run_review("retry"), ["openrouter/openai/primary"] * 2) + + def test_exhausted_incomplete_assessment_uses_existing_fallback(self): + self.assertEqual(self.run_review("fallback"), ["openrouter/openai/primary"] * 2 + ["openrouter/openai/fallback"]) + + def test_completed_failed_verdict_is_not_retried_or_fallbacked(self): + self.assertEqual(self.run_review("complete"), ["openrouter/openai/primary"]) diff --git a/tests/test_attempt_capture.py b/tests/test_attempt_capture.py new file mode 100644 index 0000000..b1101df --- /dev/null +++ b/tests/test_attempt_capture.py @@ -0,0 +1,99 @@ +import base64 +import hashlib +import json +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).parents[1] +CANARY = "SYNTHETIC_PRIVATE_ATTEMPT_5c3a" + + +class AttemptCaptureTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.directory = tempfile.TemporaryDirectory() + root = Path(cls.directory.name) + cls.key, cls.cert = root / "private.pem", root / "cert.pem" + subprocess.run(["openssl", "req", "-x509", "-newkey", "rsa:3072", "-nodes", "-keyout", str(cls.key), "-out", str(cls.cert), "-days", "1", "-subj", "/CN=Synthetic attempt test/"], check=True, capture_output=True) + der = subprocess.check_output(["openssl", "x509", "-in", str(cls.cert), "-outform", "DER"]) + cls.env = {**os.environ, "ENCRYPTED_ATTEMPT_RECIPIENT": cls.cert.read_text(), "ENCRYPTED_ATTEMPT_RECIPIENT_SHA256": hashlib.sha256(der).hexdigest(), "UNRELATED_SECRET_MUST_NOT_COLLECT": CANARY + "_ENV"} + + @classmethod + def tearDownClass(cls): + cls.directory.cleanup() + + def run_capture(self, root, attempt, changes=None): + return subprocess.run(["python3", str(ROOT / "scripts/capture-provider-attempt.py"), "--prompt", str(root / "prompt"), "--stdout", str(root / "stdout"), "--stderr", str(root / "stderr"), "--output", str(root / f"attempt-{attempt}.cms"), "--provider", "opencode", "--model", "synthetic", "--attempt", str(attempt), "--exit-code", "0"], env={**self.env, **(changes or {})}, text=True, capture_output=True) + + def test_attempts_survive_source_overwrite_and_contain_only_explicit_inputs(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + payloads = {"prompt": CANARY.encode(), "stdout": b'{"type":"text"}', "stderr": b"synthetic stderr"} + for name, data in payloads.items(): + (root / name).write_bytes(data) + for attempt in (1, 2): + result = self.run_capture(root, attempt) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn(CANARY, result.stdout + result.stderr) + self.assertEqual((root / f"attempt-{attempt}.cms").stat().st_mode & 0o777, 0o600) + (root / "stdout").write_bytes(b"replacement") + for attempt in (1, 2): + plain = subprocess.check_output(["openssl", "cms", "-decrypt", "-binary", "-inform", "DER", "-in", str(root / f"attempt-{attempt}.cms"), "-recip", str(self.cert), "-inkey", str(self.key)]) + data = json.loads(plain) + self.assertEqual(data["attempt"], attempt) + self.assertNotIn((CANARY + "_ENV").encode(), plain) + self.assertEqual(base64.b64decode(data["inputs"]["prompt"]["base64"]), payloads["prompt"]) + self.assertEqual(base64.b64decode(data["inputs"]["stdout"]["base64"]), payloads["stdout"] if attempt == 1 else b"replacement") + self.assertEqual(data["inputs"]["prompt"]["sha256"], hashlib.sha256(payloads["prompt"]).hexdigest()) + + def test_missing_invalid_or_unreadable_capture_fails_without_plaintext(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + for name in ("prompt", "stdout", "stderr"): + (root / name).write_text(CANARY) + for changes in ({"ENCRYPTED_ATTEMPT_RECIPIENT_SHA256": "0" * 64}, {"ENCRYPTED_ATTEMPT_RECIPIENT": ""}): + result = self.run_capture(root, 1, changes) + self.assertNotEqual(result.returncode, 0) + self.assertNotIn(CANARY, result.stdout + result.stderr) + self.assertFalse((root / "attempt-1.cms").exists()) + (root / "stderr").unlink() + (root / "stderr").symlink_to(root / "prompt") + result = self.run_capture(root, 1) + self.assertNotEqual(result.returncode, 0) + self.assertNotIn(CANARY, result.stdout + result.stderr) + self.assertFalse((root / "attempt-1.cms").exists()) + + def test_orchestrator_captures_each_attempt_before_retry_and_failure_blocks(self): + from test_assessment_completion import events, REPORT + for valid in (True, False): + with self.subTest(valid=valid), tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "prompt").write_text(CANARY) + for status in ("INCOMPLETE", "COMPLETE"): + (root / status).write_text("\n".join(map(json.dumps, events(REPORT + f"- **Assessment**: {status}\n")))) + fake = root / "opencode" + fake.write_text("#!/usr/bin/env python3\nimport pathlib,os,sys\nr=pathlib.Path(os.environ['PROBE_ROOT'])\np=r/'count'\nn=int(p.read_text())+1 if p.exists() else 1\np.write_text(str(n))\nsys.stdout.write((r/('COMPLETE' if n>1 else 'INCOMPLETE')).read_text())\n") + fake.chmod(0o700) + script = f"""set -euo pipefail +export ACTION_PATH={ROOT} VALIDATE_SH_LIBRARY_ONLY=true +export REQUIRE_COMPLETE_ASSESSMENT=true MAX_RETRIES=1 VALIDATION_TIMEOUT_MINUTES=1 +export VALIDATION_REPORT_PATH={root}/report.md GITHUB_OUTPUT={root}/outputs +source {ROOT}/scripts/validate.sh +run_opencode {root}/prompt openrouter synthetic +""" + env = {**self.env, "PATH": f"{root}:{os.environ['PATH']}", "PROBE_ROOT": str(root)} + if not valid: + env["ENCRYPTED_ATTEMPT_RECIPIENT_SHA256"] = "0" * 64 + result = subprocess.run(["bash", "-c", script], env=env, capture_output=True, text=True) + self.assertNotIn(CANARY, result.stdout + result.stderr) + self.assertEqual(result.returncode == 0, valid, result.stderr) + self.assertEqual((root / "count").read_text(), "2" if valid else "1") + archives = sorted((root / "report.md.attempts").glob("*.cms")) + self.assertEqual(len(archives), 2 if valid else 0) + for archive, status in zip(archives, ("INCOMPLETE", "COMPLETE")): + plain = subprocess.check_output(["openssl", "cms", "-decrypt", "-binary", "-inform", "DER", "-in", str(archive), "-recip", str(self.cert), "-inkey", str(self.key)]) + bundle = json.loads(plain) + self.assertIn(f"- **Assessment**: {status}", base64.b64decode(bundle["inputs"]["stdout"]["base64"]).decode())