From 7e3b2e4ca3c0a518d1e22c005e96aa85c1e1903d Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:28:58 +0530 Subject: [PATCH 1/8] ci: enforce evidence byte budgets and external asset metadata (Refs #273, R6) --- .github/workflows/ci-ready.yml | 18 ++- docs/agent-collaboration.md | 6 +- tests/test_evidence_budget.py | 116 +++++++++++++++++ tools/check_evidence_budget.py | 221 +++++++++++++++++++++++++++++++++ 4 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 tests/test_evidence_budget.py create mode 100644 tools/check_evidence_budget.py diff --git a/.github/workflows/ci-ready.yml b/.github/workflows/ci-ready.yml index 1d888328b..36941037d 100644 --- a/.github/workflows/ci-ready.yml +++ b/.github/workflows/ci-ready.yml @@ -68,6 +68,22 @@ jobs: PR_BODY: ${{ github.event.pull_request.body }} run: python3 tools/check_pr_references.py + evidence-budget: + # Enforces per-file (512 KiB) and per-PR (2.0 MiB) byte budgets on evidence/ (Refs #273, R6) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + - name: Verify evidence byte budgets + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: python3 tools/check_evidence_budget.py --base "$BASE_SHA" + kv-task-qualification: # Bounded software-control qualification; not a branch-protection or efficacy gate. if: github.event.pull_request.draft == false @@ -118,4 +134,4 @@ jobs: ${{ runner.temp }}/kv-actor-qualification/qualification.json ${{ runner.temp }}/kv-actor-qualification/raw-evidence.zip if-no-files-found: error - retention-days: 14 + retention-days: 14 \ No newline at end of file diff --git a/docs/agent-collaboration.md b/docs/agent-collaboration.md index b0f9864cf..6fe269c8c 100644 --- a/docs/agent-collaboration.md +++ b/docs/agent-collaboration.md @@ -96,7 +96,7 @@ sequenceDiagram 1. **Scope:** identify requirements, exclusions, owner and relevant dependencies. The accountable agent tracks Issue-level criteria and delegates bounded work; PR readiness and Issue completion are reported separately. Keep each PR a coherent delivery slice; small follow-ups may reuse an Issue. 2. **Isolation:** concurrent writers use separate working directories/indexes and private scratch files. Branch names alone are insufficient. Inspect staged/committed content and coordinate shared IDs, schemas and interfaces. -3. **Evidence:** link checks to actual checked content and relevant environment/test definitions. Distinguish passed, failed, cancelled, skipped, timed-out and not-run. H/B/T—reviewed head, assessed base, tested content—may come from existing records, not mandatory worksheets. +3. **Evidence:** link checks to actual checked content and relevant environment/test definitions. Distinguish passed, failed, cancelled, skipped, timed-out and not-run. H/B/T—reviewed head, assessed base, tested content—may come from existing records, not mandatory worksheets. Tracked evidence files are bounded by a 512 KiB per-file budget and a 2.0 MiB per-PR net addition budget. Larger raw artifacts (traces, weights) must be hosted as GitHub Release assets and registered in `tools/evidence_inventory.json` with an accessible HTTPS URL, SHA-256 digest, byte count, and retention owner. The ready lane's `evidence-budget` job reports violations. 4. **Refresh:** reassess affected evidence after relevant head/base/content/environment/check changes. Refresh checks or explain continued applicability. Neither blanket reuse nor full hardware reruns is the default; changed hashes alone prove neither content change nor equivalence. 5. **Handoff:** only for an actual transfer or accepted decision change, record the delta, recipient and next action. Keep acceptance status/link, communication status and application status separate. "Applied" needs artifact/evidence; otherwise retain pending or unknown. No full conversation traces or new ledger. 6. **Completion:** merged PR ≠ accepted goal ≠ campaign `PASS` ≠ release. Use non-closing `Refs #N` until closure is authorized. GitHub treats a closing keyword followed by an Issue reference as closing syntax even inside a negated sentence, so never write one in PR text; for an authorized closure, put `Closes #N` alone on its own line. The ready lane's `pr-references` job reports any other use. Subtasks cannot implicitly close a parent goal. Knowledge proposals to [#17](https://github.com/InferMatrixLab/InferRSI/issues/17) are optional; no automatic promotion. Before an authorized closure, publish a closure summary card ([closure summaries](#closure-summaries)). @@ -175,7 +175,7 @@ flowchart LR ## Records -- **Issue:** goal/criteria; scope; accountable account and parent agent if assigned; acting contributor if known; acceptance/delegation link; completion/closure owner. Unknown fields may remain TBD while proposed. Ordinary work starts from the [scoped-task template](../.github/ISSUE_TEMPLATE/scoped-task.md); baseline-vs-candidate campaigns use the [campaign acceptance form](../.github/ISSUE_TEMPLATE/campaign-acceptance.yml). +- **Issue:** goal/criteria; scope; accountable account and parent agent if assigned; acting contributor if known; acceptance/delegation link; completion/closure owner. Unknown fields may remain TBD while proposed. Ordinary work starts from the [scoped-task template](../.github/ISSUE_TEMPLATE/scoped-task.md); baseline-vs-campaign campaigns use the [campaign acceptance form](../.github/ISSUE_TEMPLATE/campaign-acceptance.yml). - **PR:** governing Issue; delivered artifacts; checks/results/limitations; dependencies and gaps. Add requirement-to-artifact mapping only when useful, and handoff fields only when applicable. No author-prefilled independent approval. Start from the [PR template](../.github/PULL_REQUEST_TEMPLATE.md). - **Commit:** short descriptive subject; why/what when useful. Keep governing Issue references out of commit subjects, bodies and trailers — the PR body carries them. Preserve author identity and the required DCO sign-off (`git commit -s`); never invent a signing identity. Agent-assisted commits identify material agent contributors with `Agent-assisted-by: ()` trailers; repeat for multiple contributors and retain attribution when squashing. Protected knowledge contributions require the [current-head knowledge declaration](knowledge-model-policy.md) and use `Agent-assisted-by: / [effort=] ()` when known. Unknown values stay truthful and do not satisfy admission; model/version remains optional for other work. The trailer is self-reported provenance, not cryptographic signing, independent review or approval; review attribution belongs on the revision-scoped review record. - **Squash merge:** the required integration method. Before an authorized merge, prepare a concise squash message: subject plus a short summary, no governing Issue references, no redundant intermediate-commit messages; preserve the required DCO sign-off and material `Agent-assisted-by` attribution. Never invent identities or sign-offs. @@ -257,4 +257,4 @@ They are proposals, not demonstrated Magpie enforcement or a causal speedup. Described practice, inspected implementation, observed records and proposed adaptations remain distinct evidence classes; see #21 for the per-case limits, including the unresolved C1 root cause and the unestablished final -tested/merged equivalence. +tested/merged equivalence. \ No newline at end of file diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py new file mode 100644 index 000000000..e48184b81 --- /dev/null +++ b/tests/test_evidence_budget.py @@ -0,0 +1,116 @@ +from __future__ import annotations + +import json +from pathlib import Path + +from tools import check_evidence_budget as check + +LIMIT_FILE = 512 * 1024 +LIMIT_PR = 2 * 1024 * 1024 + + +def test_file_under_budget_passes(): + files = [("evidence/campaign_x/summary.json", 100 * 1024)] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert violations == [] + assert total == 100 * 1024 + + +def test_file_exceeding_budget_fails(): + files = [("evidence/campaign_x/large_trace.parquet", 600 * 1024)] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert len(violations) == 1 + assert "exceeds limit" in violations[0] + assert total == 600 * 1024 + + +def test_aggregate_pr_evidence_budget_fails(): + # 5 files of 450 KiB each = 2,250 KiB (> 2,048 KiB limit), though each file <= 512 KiB + files = [ + (f"evidence/campaign_x/trace_{i}.json", 450 * 1024) + for i in range(5) + ] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert len(violations) == 1 + assert "exceeds per-PR limit" in violations[0] + assert total == 5 * 450 * 1024 + + +def test_non_evidence_files_not_counted_in_evidence_budget(): + files = [ + ("src/inferrsi/engine.py", 100 * 1024), + ("tests/test_large.py", 200 * 1024), + ] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert violations == [] + assert total == 0 + + +def test_allowlist_bypasses_budget(): + path = "evidence/historical/grandfathered_raw.tar.gz" + files = [(path, 10 * 1024 * 1024)] + violations, total = check.check_file_budgets( + files, + per_file_limit=LIMIT_FILE, + per_pr_limit=LIMIT_PR, + allowlist={path}, + ) + assert violations == [] + assert total == 0 + + +def test_validate_external_asset_manifest_valid(tmp_path: Path): + manifest = tmp_path / "inventory.json" + dummy_sha = "e" * 64 + data = { + "version": 1, + "bundles": { + "qwen3-omni-two-gpu": { + "external_assets": [ + { + "url": "https://github.com/InferMatrixLab/InferRSI/releases/tag/v0.3.0", + "sha256": dummy_sha, + "bytes": 48920000, + "retention_owner": "Dong1017", + } + ] + } + }, + } + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert errors == [] + + +def test_validate_external_asset_manifest_missing_and_invalid(tmp_path: Path): + manifest = tmp_path / "inventory_invalid.json" + data = { + "version": 1, + "bundles": { + "bad_bundle": { + "external_assets": [ + { + "url": "http://insecure.site/trace.bin", + "sha256": "bad_sha", + "bytes": -5, + "retention_owner": "", + } + ] + } + }, + } + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert len(errors) == 4 + assert any("https URL" in e for e in errors) + assert any("invalid sha256" in e for e in errors) + assert any("positive integer" in e for e in errors) + assert any("retention_owner must be non-empty" in e for e in errors) diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py new file mode 100644 index 000000000..641e14906 --- /dev/null +++ b/tools/check_evidence_budget.py @@ -0,0 +1,221 @@ +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + +# R6 Byte Budgets (Refs #273, F5/R6) +# Per-file budget: 512 KiB for files tracked in evidence/ +PER_FILE_MAX_BYTES = 512 * 1024 +# Per-PR net evidence addition budget: 2.0 MiB +PER_PR_EVIDENCE_MAX_BYTES = 2 * 1024 * 1024 + +ADVICE = ( + "Evidence byte budget exceeded. Large raw artifacts must be stored outside " + "Git as GitHub Release assets, recording their URL, SHA-256, byte count, " + "and retention owner in tools/evidence_inventory.json. See working rule 3 " + "in docs/agent-collaboration.md and Issue #273." +) + + +def get_changed_files( + base_ref: str | None = None, + repo_root: Path | None = None, +) -> list[tuple[str, int]]: + """Return list of (relative_path, file_size_bytes) for added or modified files.""" + root = repo_root or ROOT + files: list[tuple[str, int]] = [] + + if base_ref: + try: + # Get added (A), copied (C), or modified (M) files relative to base_ref + res = subprocess.run( + ["git", "diff", "--name-only", "--diff-filter=ACM", f"{base_ref}...HEAD"], + cwd=root, + capture_output=True, + text=True, + check=True, + ) + rel_paths = [p.strip() for p in res.stdout.splitlines() if p.strip()] + except (subprocess.SubprocessError, FileNotFoundError): + rel_paths = get_local_git_changed_files(root) + else: + rel_paths = get_local_git_changed_files(root) + + for rel_path in rel_paths: + full_path = root / rel_path + if full_path.is_file(): + files.append((rel_path.replace("\\", "/"), full_path.stat().st_size)) + + return files + + +def get_local_git_changed_files(root: Path) -> list[str]: + """Inspect local git status for untracked, added, or modified files.""" + try: + res = subprocess.run( + ["git", "status", "--porcelain"], + cwd=root, + capture_output=True, + text=True, + check=True, + ) + paths = [] + for line in res.stdout.splitlines(): + line = line.strip() + if not line: + continue + parts = line.split(maxsplit=1) + if len(parts) == 2: + status, p = parts + if "D" not in status: + paths.append(p.strip('"')) + return paths + except (subprocess.SubprocessError, FileNotFoundError): + return [] + + +def check_file_budgets( + changed_files: list[tuple[str, int]], + per_file_limit: int = PER_FILE_MAX_BYTES, + per_pr_limit: int = PER_PR_EVIDENCE_MAX_BYTES, + allowlist: set[str] | None = None, +) -> tuple[list[str], int]: + """Check changed files against per-file and per-PR evidence budgets.""" + violations: list[str] = [] + total_evidence_bytes = 0 + allowed = allowlist or set() + + for rel_path, size in changed_files: + norm_path = rel_path.replace("\\", "/") + if norm_path in allowed: + continue + + if norm_path.startswith("evidence/"): + total_evidence_bytes += size + if size > per_file_limit: + violations.append( + f"file {norm_path}: {size:,} bytes exceeds limit of {per_file_limit:,} bytes" + ) + + if total_evidence_bytes > per_pr_limit: + violations.append( + f"evidence net addition: {total_evidence_bytes:,} bytes " + f"exceeds per-PR limit of {per_pr_limit:,} bytes" + ) + + return violations, total_evidence_bytes + + +def validate_external_asset_manifest(manifest_path: Path) -> list[str]: + """Verify that external asset entries record url, sha256, bytes, and retention_owner.""" + errors: list[str] = [] + if not manifest_path.exists(): + return errors + + try: + data = json.loads(manifest_path.read_text(encoding="utf-8")) + except Exception as exc: + return [f"manifest error: failed to parse {manifest_path.name}: {exc}"] + + bundles = data.get("bundles", {}) + if not isinstance(bundles, dict): + return errors + + sha_re = re.compile(r"^[0-9a-f]{64}$") + + for bundle_name, bundle in bundles.items(): + if not isinstance(bundle, dict): + continue + external_assets = bundle.get("external_assets", []) + if not isinstance(external_assets, list): + continue + + for idx, asset in enumerate(external_assets): + prefix = f"bundle {bundle_name} external_assets[{idx}]" + if not isinstance(asset, dict): + errors.append(f"{prefix}: expected JSON object") + continue + + for field in ("url", "sha256", "bytes", "retention_owner"): + if field not in asset: + errors.append(f"{prefix}: missing required field '{field}'") + + url = asset.get("url", "") + if not isinstance(url, str) or not url.startswith("https://"): + errors.append(f"{prefix}: url must be a valid https URL") + + sha = asset.get("sha256", "") + if not isinstance(sha, str) or not sha_re.match(sha): + errors.append(f"{prefix}: invalid sha256 hex digest") + + byte_count = asset.get("bytes", -1) + if not isinstance(byte_count, int) or byte_count <= 0: + errors.append(f"{prefix}: bytes must be a positive integer") + + owner = asset.get("retention_owner", "") + if not isinstance(owner, str) or not owner.strip(): + errors.append(f"{prefix}: retention_owner must be non-empty") + + return errors + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Check evidence byte budgets and external asset metadata." + ) + parser.add_argument("--base", help="Base git reference/SHA (e.g. origin/main or $BASE_SHA)") + parser.add_argument("--files", nargs="*", help="Explicit list of files to check") + parser.add_argument( + "--inventory", default="tools/evidence_inventory.json", help="Path to evidence inventory" + ) + parser.add_argument( + "--max-file-bytes", type=int, default=PER_FILE_MAX_BYTES, help="Per-file max byte budget" + ) + parser.add_argument( + "--max-pr-bytes", + type=int, + default=PER_PR_EVIDENCE_MAX_BYTES, + help="Per-PR max byte budget", + ) + args = parser.parse_args() + + violations: list[str] = [] + + if args.files is not None: + file_list: list[tuple[str, int]] = [] + for p_str in args.files: + p = Path(p_str) + if p.is_file(): + rel = str(p.relative_to(ROOT) if p.is_relative_to(ROOT) else p).replace("\\", "/") + file_list.append((rel, p.stat().st_size)) + diff_violations, _ = check_file_budgets(file_list, args.max_file_bytes, args.max_pr_bytes) + violations.extend(diff_violations) + else: + changed_files = get_changed_files(args.base) + diff_violations, _ = check_file_budgets( + changed_files, args.max_file_bytes, args.max_pr_bytes + ) + violations.extend(diff_violations) + + inv_path = ROOT / args.inventory + if inv_path.exists(): + violations.extend(validate_external_asset_manifest(inv_path)) + + for item in violations: + print(item) + + if violations: + print(ADVICE) + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 1afe7515f44e85142bdb079c1464e1251b17b5b4 Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:41:44 +0530 Subject: [PATCH 2/8] fix: address review findings on base diff, net addition and asset URLs (Refs #273) --- docs/agent-collaboration.md | 2 +- tests/test_evidence_budget.py | 62 +++++++---- tools/check_evidence_budget.py | 187 +++++++++++++++++++++++---------- 3 files changed, 172 insertions(+), 79 deletions(-) diff --git a/docs/agent-collaboration.md b/docs/agent-collaboration.md index 6fe269c8c..bd754ca0c 100644 --- a/docs/agent-collaboration.md +++ b/docs/agent-collaboration.md @@ -96,7 +96,7 @@ sequenceDiagram 1. **Scope:** identify requirements, exclusions, owner and relevant dependencies. The accountable agent tracks Issue-level criteria and delegates bounded work; PR readiness and Issue completion are reported separately. Keep each PR a coherent delivery slice; small follow-ups may reuse an Issue. 2. **Isolation:** concurrent writers use separate working directories/indexes and private scratch files. Branch names alone are insufficient. Inspect staged/committed content and coordinate shared IDs, schemas and interfaces. -3. **Evidence:** link checks to actual checked content and relevant environment/test definitions. Distinguish passed, failed, cancelled, skipped, timed-out and not-run. H/B/T—reviewed head, assessed base, tested content—may come from existing records, not mandatory worksheets. Tracked evidence files are bounded by a 512 KiB per-file budget and a 2.0 MiB per-PR net addition budget. Larger raw artifacts (traces, weights) must be hosted as GitHub Release assets and registered in `tools/evidence_inventory.json` with an accessible HTTPS URL, SHA-256 digest, byte count, and retention owner. The ready lane's `evidence-budget` job reports violations. +3. **Evidence:** link checks to actual checked content and relevant environment/test definitions. Distinguish passed, failed, cancelled, skipped, timed-out and not-run. H/B/T—reviewed head, assessed base, tested content—may come from existing records, not mandatory worksheets. 4. **Refresh:** reassess affected evidence after relevant head/base/content/environment/check changes. Refresh checks or explain continued applicability. Neither blanket reuse nor full hardware reruns is the default; changed hashes alone prove neither content change nor equivalence. 5. **Handoff:** only for an actual transfer or accepted decision change, record the delta, recipient and next action. Keep acceptance status/link, communication status and application status separate. "Applied" needs artifact/evidence; otherwise retain pending or unknown. No full conversation traces or new ledger. 6. **Completion:** merged PR ≠ accepted goal ≠ campaign `PASS` ≠ release. Use non-closing `Refs #N` until closure is authorized. GitHub treats a closing keyword followed by an Issue reference as closing syntax even inside a negated sentence, so never write one in PR text; for an authorized closure, put `Closes #N` alone on its own line. The ready lane's `pr-references` job reports any other use. Subtasks cannot implicitly close a parent goal. Knowledge proposals to [#17](https://github.com/InferMatrixLab/InferRSI/issues/17) are optional; no automatic promotion. Before an authorized closure, publish a closure summary card ([closure summaries](#closure-summaries)). diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index e48184b81..8ebb1abd4 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -3,6 +3,8 @@ import json from pathlib import Path +import pytest + from tools import check_evidence_budget as check LIMIT_FILE = 512 * 1024 @@ -10,7 +12,7 @@ def test_file_under_budget_passes(): - files = [("evidence/campaign_x/summary.json", 100 * 1024)] + files = [("evidence/campaign_x/summary.json", 100 * 1024, 100 * 1024)] violations, total = check.check_file_budgets( files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR ) @@ -19,33 +21,44 @@ def test_file_under_budget_passes(): def test_file_exceeding_budget_fails(): - files = [("evidence/campaign_x/large_trace.parquet", 600 * 1024)] + files = [("evidence/campaign_x/large_trace.parquet", 600 * 1024, 600 * 1024)] violations, total = check.check_file_budgets( files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR ) assert len(violations) == 1 - assert "exceeds limit" in violations[0] + assert "exceeds per-file limit" in violations[0] assert total == 600 * 1024 def test_aggregate_pr_evidence_budget_fails(): - # 5 files of 450 KiB each = 2,250 KiB (> 2,048 KiB limit), though each file <= 512 KiB + # 5 files added with 450 KiB each = 2,250 KiB (> 2,048 KiB limit), though each file <= 512 KiB files = [ - (f"evidence/campaign_x/trace_{i}.json", 450 * 1024) + (f"evidence/campaign_x/trace_{i}.json", 450 * 1024, 450 * 1024) for i in range(5) ] violations, total = check.check_file_budgets( files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR ) assert len(violations) == 1 - assert "exceeds per-PR limit" in violations[0] + assert "exceeds per-PR net limit" in violations[0] assert total == 5 * 450 * 1024 +def test_modified_file_counts_true_net_addition_not_full_size(): + # A 500 KiB existing file modified with only +10 KiB net growth + files = [("evidence/campaign_x/existing.json", 500 * 1024, 10 * 1024)] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert violations == [] + # Total net evidence addition is 10 KiB, not 500 KiB + assert total == 10 * 1024 + + def test_non_evidence_files_not_counted_in_evidence_budget(): files = [ - ("src/inferrsi/engine.py", 100 * 1024), - ("tests/test_large.py", 200 * 1024), + ("src/inferrsi/engine.py", 100 * 1024, 100 * 1024), + ("tests/test_large.py", 200 * 1024, 200 * 1024), ] violations, total = check.check_file_budgets( files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR @@ -56,7 +69,7 @@ def test_non_evidence_files_not_counted_in_evidence_budget(): def test_allowlist_bypasses_budget(): path = "evidence/historical/grandfathered_raw.tar.gz" - files = [(path, 10 * 1024 * 1024)] + files = [(path, 10 * 1024 * 1024, 10 * 1024 * 1024)] violations, total = check.check_file_budgets( files, per_file_limit=LIMIT_FILE, @@ -70,13 +83,16 @@ def test_allowlist_bypasses_budget(): def test_validate_external_asset_manifest_valid(tmp_path: Path): manifest = tmp_path / "inventory.json" dummy_sha = "e" * 64 + valid_url = ( + "https://github.com/InferMatrixLab/InferRSI/releases/download/v0.3.0/traces.tar.gz" + ) data = { "version": 1, "bundles": { "qwen3-omni-two-gpu": { "external_assets": [ { - "url": "https://github.com/InferMatrixLab/InferRSI/releases/tag/v0.3.0", + "url": valid_url, "sha256": dummy_sha, "bytes": 48920000, "retention_owner": "Dong1017", @@ -90,18 +106,18 @@ def test_validate_external_asset_manifest_valid(tmp_path: Path): assert errors == [] -def test_validate_external_asset_manifest_missing_and_invalid(tmp_path: Path): - manifest = tmp_path / "inventory_invalid.json" +def test_validate_external_asset_manifest_rejects_tag_view_url(tmp_path: Path): + manifest = tmp_path / "inventory_tag.json" data = { "version": 1, "bundles": { - "bad_bundle": { + "tag_bundle": { "external_assets": [ { - "url": "http://insecure.site/trace.bin", - "sha256": "bad_sha", - "bytes": -5, - "retention_owner": "", + "url": "https://github.com/InferMatrixLab/InferRSI/releases/tag/v0.3.0", + "sha256": "a" * 64, + "bytes": 1000, + "retention_owner": "Dong1017", } ] } @@ -109,8 +125,10 @@ def test_validate_external_asset_manifest_missing_and_invalid(tmp_path: Path): } manifest.write_text(json.dumps(data), encoding="utf-8") errors = check.validate_external_asset_manifest(manifest) - assert len(errors) == 4 - assert any("https URL" in e for e in errors) - assert any("invalid sha256" in e for e in errors) - assert any("positive integer" in e for e in errors) - assert any("retention_owner must be non-empty" in e for e in errors) + assert len(errors) == 1 + assert "releases/download" in errors[0] + + +def test_invalid_base_ref_raises_error(tmp_path: Path): + with pytest.raises(ValueError, match="cannot be resolved"): + check.get_changed_files_with_net_sizes("non_existent_ref_12345", repo_root=tmp_path) diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index 641e14906..9e00d2c66 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -9,50 +9,112 @@ ROOT = Path(__file__).resolve().parents[1] -# R6 Byte Budgets (Refs #273, F5/R6) +# Proposed R6 Byte Budgets (Refs #273, F5/R6 - under discussion) # Per-file budget: 512 KiB for files tracked in evidence/ PER_FILE_MAX_BYTES = 512 * 1024 # Per-PR net evidence addition budget: 2.0 MiB PER_PR_EVIDENCE_MAX_BYTES = 2 * 1024 * 1024 +RELEASE_ASSET_URL_PATTERN = re.compile( + r"^https://github\.com/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+/releases/download/[^/]+/[^/]+$" +) + ADVICE = ( - "Evidence byte budget exceeded. Large raw artifacts must be stored outside " - "Git as GitHub Release assets, recording their URL, SHA-256, byte count, " - "and retention owner in tools/evidence_inventory.json. See working rule 3 " - "in docs/agent-collaboration.md and Issue #273." + "Evidence byte budget exceeded. Large raw artifacts should be stored outside " + "Git as GitHub Release assets, recording their downloadable asset URL, " + "SHA-256 digest, byte count, and retention owner in tools/evidence_inventory.json. " + "See discussion in Issue #273 (Remedy R6)." ) -def get_changed_files( +def get_git_object_size(root: Path, obj_ref: str) -> int: + """Return byte size of a git blob object using git cat-file -s.""" + try: + res = subprocess.run( + ["git", "cat-file", "-s", obj_ref], + cwd=root, + capture_output=True, + text=True, + check=True, + ) + return int(res.stdout.strip()) + except (subprocess.SubprocessError, ValueError): + return 0 + + +def get_changed_files_with_net_sizes( base_ref: str | None = None, repo_root: Path | None = None, -) -> list[tuple[str, int]]: - """Return list of (relative_path, file_size_bytes) for added or modified files.""" +) -> list[tuple[str, int, int]]: + """Return list of (rel_path, current_size_bytes, net_addition_bytes). + + If base_ref is supplied, uses git diff --raw against base_ref. + If base_ref fails or is missing, raises ValueError (no silent fallback in CI). + If base_ref is None, falls back to inspecting local git status. + """ root = repo_root or ROOT - files: list[tuple[str, int]] = [] + results: list[tuple[str, int, int]] = [] if base_ref: - try: - # Get added (A), copied (C), or modified (M) files relative to base_ref - res = subprocess.run( - ["git", "diff", "--name-only", "--diff-filter=ACM", f"{base_ref}...HEAD"], - cwd=root, - capture_output=True, - text=True, - check=True, + # Validate base_ref exists before diffing + rev_check = subprocess.run( + ["git", "rev-parse", "--verify", base_ref], + cwd=root, + capture_output=True, + text=True, + ) + if rev_check.returncode != 0: + raise ValueError( + f"Base ref '{base_ref}' cannot be resolved. " + "Ensure target base ref is fetched before checking diff." ) - rel_paths = [p.strip() for p in res.stdout.splitlines() if p.strip()] - except (subprocess.SubprocessError, FileNotFoundError): - rel_paths = get_local_git_changed_files(root) - else: - rel_paths = get_local_git_changed_files(root) - for rel_path in rel_paths: - full_path = root / rel_path - if full_path.is_file(): - files.append((rel_path.replace("\\", "/"), full_path.stat().st_size)) + res = subprocess.run( + ["git", "diff", "--raw", f"{base_ref}...HEAD"], + cwd=root, + capture_output=True, + text=True, + check=True, + ) + + for line in res.stdout.splitlines(): + line = line.strip() + if not line.startswith(":"): + continue + # Format: :mode_src mode_dst sha_src sha_dst status\tpath (or path1\tpath2 for rename) + header, _, path_part = line.partition("\t") + parts = header.split() + if len(parts) < 5: + continue + sha_src, status = parts[2], parts[4] + rel_path = path_part.split("\t")[0].strip('"').replace("\\", "/") + + full_path = root / rel_path + current_size = full_path.stat().st_size if full_path.is_file() else 0 + + # Calculate true net delta + if status.startswith("A"): + net_addition = current_size + elif status.startswith("M"): + src_size = get_git_object_size(root, sha_src) + net_addition = max(0, current_size - src_size) + elif status.startswith("D"): + net_addition = 0 + else: + net_addition = current_size + + results.append((rel_path, current_size, net_addition)) - return files + else: + # Local development fallback without base ref + local_paths = get_local_git_changed_files(root) + for rel_path in local_paths: + full_path = root / rel_path + if full_path.is_file(): + sz = full_path.stat().st_size + results.append((rel_path.replace("\\", "/"), sz, sz)) + + return results def get_local_git_changed_files(root: Path) -> list[str]: @@ -81,39 +143,40 @@ def get_local_git_changed_files(root: Path) -> list[str]: def check_file_budgets( - changed_files: list[tuple[str, int]], + file_records: list[tuple[str, int, int]], per_file_limit: int = PER_FILE_MAX_BYTES, per_pr_limit: int = PER_PR_EVIDENCE_MAX_BYTES, allowlist: set[str] | None = None, ) -> tuple[list[str], int]: - """Check changed files against per-file and per-PR evidence budgets.""" + """Check file records (rel_path, current_size, net_addition) against budgets.""" violations: list[str] = [] - total_evidence_bytes = 0 + total_net_evidence_bytes = 0 allowed = allowlist or set() - for rel_path, size in changed_files: + for rel_path, current_size, net_addition in file_records: norm_path = rel_path.replace("\\", "/") if norm_path in allowed: continue if norm_path.startswith("evidence/"): - total_evidence_bytes += size - if size > per_file_limit: + total_net_evidence_bytes += net_addition + if current_size > per_file_limit: violations.append( - f"file {norm_path}: {size:,} bytes exceeds limit of {per_file_limit:,} bytes" + f"file {norm_path}: {current_size:,} bytes exceeds per-file limit " + f"of {per_file_limit:,} bytes" ) - if total_evidence_bytes > per_pr_limit: + if total_net_evidence_bytes > per_pr_limit: violations.append( - f"evidence net addition: {total_evidence_bytes:,} bytes " - f"exceeds per-PR limit of {per_pr_limit:,} bytes" + f"evidence net addition: +{total_net_evidence_bytes:,} bytes " + f"exceeds per-PR net limit of {per_pr_limit:,} bytes" ) - return violations, total_evidence_bytes + return violations, total_net_evidence_bytes def validate_external_asset_manifest(manifest_path: Path) -> list[str]: - """Verify that external asset entries record url, sha256, bytes, and retention_owner.""" + """Verify external asset entries record downloadable URL, sha256, bytes, and owner.""" errors: list[str] = [] if not manifest_path.exists(): return errors @@ -147,8 +210,11 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: errors.append(f"{prefix}: missing required field '{field}'") url = asset.get("url", "") - if not isinstance(url, str) or not url.startswith("https://"): - errors.append(f"{prefix}: url must be a valid https URL") + if not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): + errors.append( + f"{prefix}: url must point to a downloadable GitHub Release asset " + f"(https://github.com///releases/download//)" + ) sha = asset.get("sha256", "") if not isinstance(sha, str) or not sha_re.match(sha): @@ -187,21 +253,30 @@ def main() -> int: violations: list[str] = [] - if args.files is not None: - file_list: list[tuple[str, int]] = [] - for p_str in args.files: - p = Path(p_str) - if p.is_file(): - rel = str(p.relative_to(ROOT) if p.is_relative_to(ROOT) else p).replace("\\", "/") - file_list.append((rel, p.stat().st_size)) - diff_violations, _ = check_file_budgets(file_list, args.max_file_bytes, args.max_pr_bytes) - violations.extend(diff_violations) - else: - changed_files = get_changed_files(args.base) - diff_violations, _ = check_file_budgets( - changed_files, args.max_file_bytes, args.max_pr_bytes - ) - violations.extend(diff_violations) + try: + if args.files is not None: + file_list: list[tuple[str, int, int]] = [] + for p_str in args.files: + p = Path(p_str) + if p.is_file(): + rel = str( + p.relative_to(ROOT) if p.is_relative_to(ROOT) else p + ).replace("\\", "/") + sz = p.stat().st_size + file_list.append((rel, sz, sz)) + diff_violations, _ = check_file_budgets( + file_list, args.max_file_bytes, args.max_pr_bytes + ) + violations.extend(diff_violations) + else: + changed_records = get_changed_files_with_net_sizes(args.base) + diff_violations, _ = check_file_budgets( + changed_records, args.max_file_bytes, args.max_pr_bytes + ) + violations.extend(diff_violations) + except Exception as exc: + print(f"evidence-budget error: {exc}", file=sys.stderr) + return 1 inv_path = ROOT / args.inventory if inv_path.exists(): From afad886d7e354264178c077dae63fa59635a52b8 Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:56:54 +0530 Subject: [PATCH 3/8] fix: address review feedback on -z parsing, signed net delta, and manifest validation (Refs #273) --- .github/workflows/ci-ready.yml | 8 +- docs/agent-collaboration.md | 4 +- tests/test_evidence_budget.py | 104 ++++++++++++++- tools/check_evidence_budget.py | 227 ++++++++++++++++++++++++--------- 4 files changed, 273 insertions(+), 70 deletions(-) diff --git a/.github/workflows/ci-ready.yml b/.github/workflows/ci-ready.yml index 36941037d..ec967583e 100644 --- a/.github/workflows/ci-ready.yml +++ b/.github/workflows/ci-ready.yml @@ -69,7 +69,7 @@ jobs: run: python3 tools/check_pr_references.py evidence-budget: - # Enforces per-file (512 KiB) and per-PR (2.0 MiB) byte budgets on evidence/ (Refs #273, R6) + # Candidate evidence byte budget check (Refs #273, R6 under discussion) runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -127,11 +127,11 @@ jobs: --revision "$GITHUB_SHA" --output "$RUNNER_TEMP/kv-actor-qualification" - name: Retain separate scripted actor receipt including failures if: always() - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: + shell: bash + run: | name: kv-actor-qualification-${{ github.run_id }}-${{ github.run_attempt }} path: | ${{ runner.temp }}/kv-actor-qualification/qualification.json ${{ runner.temp }}/kv-actor-qualification/raw-evidence.zip if-no-files-found: error - retention-days: 14 \ No newline at end of file + retention-days: 14 diff --git a/docs/agent-collaboration.md b/docs/agent-collaboration.md index bd754ca0c..b0f9864cf 100644 --- a/docs/agent-collaboration.md +++ b/docs/agent-collaboration.md @@ -175,7 +175,7 @@ flowchart LR ## Records -- **Issue:** goal/criteria; scope; accountable account and parent agent if assigned; acting contributor if known; acceptance/delegation link; completion/closure owner. Unknown fields may remain TBD while proposed. Ordinary work starts from the [scoped-task template](../.github/ISSUE_TEMPLATE/scoped-task.md); baseline-vs-campaign campaigns use the [campaign acceptance form](../.github/ISSUE_TEMPLATE/campaign-acceptance.yml). +- **Issue:** goal/criteria; scope; accountable account and parent agent if assigned; acting contributor if known; acceptance/delegation link; completion/closure owner. Unknown fields may remain TBD while proposed. Ordinary work starts from the [scoped-task template](../.github/ISSUE_TEMPLATE/scoped-task.md); baseline-vs-candidate campaigns use the [campaign acceptance form](../.github/ISSUE_TEMPLATE/campaign-acceptance.yml). - **PR:** governing Issue; delivered artifacts; checks/results/limitations; dependencies and gaps. Add requirement-to-artifact mapping only when useful, and handoff fields only when applicable. No author-prefilled independent approval. Start from the [PR template](../.github/PULL_REQUEST_TEMPLATE.md). - **Commit:** short descriptive subject; why/what when useful. Keep governing Issue references out of commit subjects, bodies and trailers — the PR body carries them. Preserve author identity and the required DCO sign-off (`git commit -s`); never invent a signing identity. Agent-assisted commits identify material agent contributors with `Agent-assisted-by: ()` trailers; repeat for multiple contributors and retain attribution when squashing. Protected knowledge contributions require the [current-head knowledge declaration](knowledge-model-policy.md) and use `Agent-assisted-by: / [effort=] ()` when known. Unknown values stay truthful and do not satisfy admission; model/version remains optional for other work. The trailer is self-reported provenance, not cryptographic signing, independent review or approval; review attribution belongs on the revision-scoped review record. - **Squash merge:** the required integration method. Before an authorized merge, prepare a concise squash message: subject plus a short summary, no governing Issue references, no redundant intermediate-commit messages; preserve the required DCO sign-off and material `Agent-assisted-by` attribution. Never invent identities or sign-offs. @@ -257,4 +257,4 @@ They are proposals, not demonstrated Magpie enforcement or a causal speedup. Described practice, inspected implementation, observed records and proposed adaptations remain distinct evidence classes; see #21 for the per-case limits, including the unresolved C1 root cause and the unestablished final -tested/merged equivalence. \ No newline at end of file +tested/merged equivalence. diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index 8ebb1abd4..68924aa4a 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import subprocess from pathlib import Path import pytest @@ -51,10 +52,25 @@ def test_modified_file_counts_true_net_addition_not_full_size(): files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR ) assert violations == [] - # Total net evidence addition is 10 KiB, not 500 KiB assert total == 10 * 1024 +def test_net_addition_with_deletion_and_shrinkage(): + # Real maintainer scenario: delete 2 MiB, add 5 files of 450 KiB (+2,250 KiB) + # Net addition is +202 KiB, which is well below the 2.0 MiB limit + files = [ + ("evidence/old_large.bin", 0, -2 * 1024 * 1024), + ] + [ + (f"evidence/new_{i}.bin", 450 * 1024, 450 * 1024) + for i in range(5) + ] + violations, total = check.check_file_budgets( + files, per_file_limit=LIMIT_FILE, per_pr_limit=LIMIT_PR + ) + assert violations == [] + assert total == (5 * 450 * 1024) - (2 * 1024 * 1024) + + def test_non_evidence_files_not_counted_in_evidence_budget(): files = [ ("src/inferrsi/engine.py", 100 * 1024, 100 * 1024), @@ -129,6 +145,92 @@ def test_validate_external_asset_manifest_rejects_tag_view_url(tmp_path: Path): assert "releases/download" in errors[0] +def test_validate_external_asset_manifest_rejects_non_list_external_assets(tmp_path: Path): + manifest = tmp_path / "inventory_dict_assets.json" + data = { + "version": 1, + "bundles": { + "bad_bundle": { + "external_assets": { + "url": "https://github.com/InferMatrixLab/InferRSI/releases/download/v1/a.bin" + } + } + }, + } + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert len(errors) == 1 + assert "must be a list of asset objects" in errors[0] + + +def test_validate_external_asset_manifest_rejects_non_dict_bundles(tmp_path: Path): + manifest = tmp_path / "inventory_bad_bundles.json" + data = {"version": 1, "bundles": "not_a_dictionary"} + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert len(errors) == 1 + assert "'bundles' must be a JSON object" in errors[0] + + def test_invalid_base_ref_raises_error(tmp_path: Path): with pytest.raises(ValueError, match="cannot be resolved"): check.get_changed_files_with_net_sizes("non_existent_ref_12345", repo_root=tmp_path) + + +def test_git_rename_and_deletion_regression(tmp_path: Path): + """End-to-end Git test verifying rename, move, delete net delta and non-ASCII paths.""" + repo = tmp_path / "test_repo" + repo.mkdir() + + def git(*args: str): + subprocess.run(["git", *args], cwd=repo, capture_output=True, check=True) + + git("init", "-b", "main") + git("config", "user.name", "Tester") + git("config", "user.email", "tester@example.com") + + # Initial base commit + ev_dir = repo / "evidence" + ev_dir.mkdir() + (ev_dir / "old_600k.bin").write_bytes(b"A" * (600 * 1024)) + (ev_dir / "to_delete_2m.bin").write_bytes(b"B" * (2 * 1024 * 1024)) + outside_dir = repo / "outside" + outside_dir.mkdir() + (outside_dir / "external.bin").write_bytes(b"C" * (100 * 1024)) + git("add", ".") + git("commit", "-m", "base commit") + + # Make changes in HEAD: + # 1. Rename 600 KiB file inside evidence/ -> should trigger per-file limit on dst! + git("mv", "evidence/old_600k.bin", "evidence/new_600k.bin") + + # 2. Delete 2 MiB file in evidence/ -> should yield -2 MiB delta + (ev_dir / "to_delete_2m.bin").unlink() + git("rm", "evidence/to_delete_2m.bin") + + # 3. Add non-ASCII file in evidence/ + (ev_dir / "é_trace.json").write_text('{"status": "ok"}', encoding="utf-8") + + # 4. Move outside file into evidence/ -> +100 KiB net addition + git("mv", "outside/external.bin", "evidence/external.bin") + + git("add", ".") + git("commit", "-m", "head commit") + + records = check.get_changed_files_with_net_sizes("main~1", repo_root=repo) + paths = {r[0] for r in records} + assert "evidence/new_600k.bin" in paths + assert "evidence/é_trace.json" in paths + assert "evidence/external.bin" in paths + + violations, net_bytes = check.check_file_budgets(records) + # Per-file violation must catch evidence/new_600k.bin + assert any("evidence/new_600k.bin" in v and "exceeds per-file limit" in v for v in violations) + # Net evidence addition: + # new_600k: 600K - 600K = 0 + # to_delete_2m: -2M + # external.bin (moved in): +100K + # é_trace.json: +17 bytes + # Total net evidence is negative (~ -1.9 MB), so PR net limit is not violated + assert net_bytes < 0 + assert not any("per-PR net limit" in v for v in violations) diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index 9e00d2c66..6e6130b10 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -9,7 +9,7 @@ ROOT = Path(__file__).resolve().parents[1] -# Proposed R6 Byte Budgets (Refs #273, F5/R6 - under discussion) +# Candidate R6 Byte Budgets (Refs #273, F5/R6 - proposed defaults under owner discussion) # Per-file budget: 512 KiB for files tracked in evidence/ PER_FILE_MAX_BYTES = 512 * 1024 # Per-PR net evidence addition budget: 2.0 MiB @@ -20,7 +20,7 @@ ) ADVICE = ( - "Evidence byte budget exceeded. Large raw artifacts should be stored outside " + "Candidate evidence byte budget exceeded. Large raw artifacts should be stored outside " "Git as GitHub Release assets, recording their downloadable asset URL, " "SHA-256 digest, byte count, and retention owner in tools/evidence_inventory.json. " "See discussion in Issue #273 (Remedy R6)." @@ -29,6 +29,8 @@ def get_git_object_size(root: Path, obj_ref: str) -> int: """Return byte size of a git blob object using git cat-file -s.""" + if not obj_ref or set(obj_ref) == {"0"}: + return 0 try: res = subprocess.run( ["git", "cat-file", "-s", obj_ref], @@ -48,15 +50,14 @@ def get_changed_files_with_net_sizes( ) -> list[tuple[str, int, int]]: """Return list of (rel_path, current_size_bytes, net_addition_bytes). - If base_ref is supplied, uses git diff --raw against base_ref. + If base_ref is supplied, uses git diff --raw -z against base_ref. If base_ref fails or is missing, raises ValueError (no silent fallback in CI). - If base_ref is None, falls back to inspecting local git status. + If base_ref is None, inspects local git status with -z. """ root = repo_root or ROOT results: list[tuple[str, int, int]] = [] if base_ref: - # Validate base_ref exists before diffing rev_check = subprocess.run( ["git", "rev-parse", "--verify", base_ref], cwd=root, @@ -70,43 +71,109 @@ def get_changed_files_with_net_sizes( ) res = subprocess.run( - ["git", "diff", "--raw", f"{base_ref}...HEAD"], + ["git", "diff", "--raw", "-z", f"{base_ref}...HEAD"], cwd=root, capture_output=True, - text=True, check=True, ) - for line in res.stdout.splitlines(): - line = line.strip() - if not line.startswith(":"): + tokens = res.stdout.split(b"\0") + i = 0 + while i < len(tokens): + tok = tokens[i] + if not tok: + i += 1 continue - # Format: :mode_src mode_dst sha_src sha_dst status\tpath (or path1\tpath2 for rename) - header, _, path_part = line.partition("\t") - parts = header.split() + if not tok.startswith(b":"): + i += 1 + continue + + header_str = tok.decode("utf-8", errors="replace") + parts = header_str.split() if len(parts) < 5: + i += 1 continue - sha_src, status = parts[2], parts[4] - rel_path = path_part.split("\t")[0].strip('"').replace("\\", "/") - full_path = root / rel_path - current_size = full_path.stat().st_size if full_path.is_file() else 0 + sha_src, sha_dst, status = parts[2], parts[3], parts[4] + + is_rename_or_copy = status.startswith(("R", "C")) + if is_rename_or_copy: + if i + 2 >= len(tokens): + break + src_path = tokens[i + 1].decode("utf-8", errors="replace").replace("\\", "/") + dst_path = tokens[i + 2].decode("utf-8", errors="replace").replace("\\", "/") + i += 3 + + src_is_ev = src_path.startswith("evidence/") + dst_is_ev = dst_path.startswith("evidence/") - # Calculate true net delta - if status.startswith("A"): - net_addition = current_size - elif status.startswith("M"): src_size = get_git_object_size(root, sha_src) - net_addition = max(0, current_size - src_size) - elif status.startswith("D"): - net_addition = 0 - else: - net_addition = current_size + full_dst = root / dst_path + dst_size = ( + full_dst.stat().st_size + if full_dst.is_file() + else get_git_object_size(root, sha_dst) + ) + + if status.startswith("R"): + # Rename: src replaced by dst + if src_is_ev and dst_is_ev: + delta = dst_size - src_size + results.append((dst_path, dst_size, delta)) + elif not src_is_ev and dst_is_ev: + # Moved into evidence + results.append((dst_path, dst_size, dst_size)) + elif src_is_ev and not dst_is_ev: + # Moved out of evidence + results.append((src_path, 0, -src_size)) + else: + results.append((dst_path, dst_size, 0)) + else: + # Copy: src still exists, dst is added + delta = dst_size if dst_is_ev else 0 + results.append((dst_path, dst_size, delta)) - results.append((rel_path, current_size, net_addition)) + else: + if i + 1 >= len(tokens): + break + path = tokens[i + 1].decode("utf-8", errors="replace").replace("\\", "/") + i += 2 + + is_ev = path.startswith("evidence/") + full_path = root / path + + if status.startswith("D"): + src_size = get_git_object_size(root, sha_src) + delta = -src_size if is_ev else 0 + results.append((path, 0, delta)) + elif status.startswith("A"): + cur_size = ( + full_path.stat().st_size + if full_path.is_file() + else get_git_object_size(root, sha_dst) + ) + delta = cur_size if is_ev else 0 + results.append((path, cur_size, delta)) + elif status.startswith("M") or status.startswith("T"): + src_size = get_git_object_size(root, sha_src) + cur_size = ( + full_path.stat().st_size + if full_path.is_file() + else get_git_object_size(root, sha_dst) + ) + delta = (cur_size - src_size) if is_ev else 0 + results.append((path, cur_size, delta)) + else: + cur_size = ( + full_path.stat().st_size + if full_path.is_file() + else get_git_object_size(root, sha_dst) + ) + delta = cur_size if is_ev else 0 + results.append((path, cur_size, delta)) else: - # Local development fallback without base ref + # Local development fallback inspecting uncommitted changes with -z local_paths = get_local_git_changed_files(root) for rel_path in local_paths: full_path = root / rel_path @@ -118,25 +185,32 @@ def get_changed_files_with_net_sizes( def get_local_git_changed_files(root: Path) -> list[str]: - """Inspect local git status for untracked, added, or modified files.""" + """Inspect local git status with -z for untracked, added, or modified files.""" try: res = subprocess.run( - ["git", "status", "--porcelain"], + ["git", "status", "--porcelain", "-z"], cwd=root, capture_output=True, - text=True, check=True, ) - paths = [] - for line in res.stdout.splitlines(): - line = line.strip() - if not line: + tokens = res.stdout.split(b"\0") + paths: list[str] = [] + i = 0 + while i < len(tokens): + tok = tokens[i] + if not tok: + i += 1 continue - parts = line.split(maxsplit=1) - if len(parts) == 2: - status, p = parts - if "D" not in status: - paths.append(p.strip('"')) + status_code = tok[:2].decode("latin-1", errors="replace") + path = tok[3:].decode("utf-8", errors="replace").replace("\\", "/") + if "R" in status_code: + # In git status -z for rename: new path is first, old path is second token + paths.append(path) + i += 2 + else: + if "D" not in status_code: + paths.append(path) + i += 1 return paths except (subprocess.SubprocessError, FileNotFoundError): return [] @@ -167,8 +241,9 @@ def check_file_budgets( ) if total_net_evidence_bytes > per_pr_limit: + sign = "+" if total_net_evidence_bytes >= 0 else "-" violations.append( - f"evidence net addition: +{total_net_evidence_bytes:,} bytes " + f"evidence net addition: {sign}{abs(total_net_evidence_bytes):,} bytes " f"exceeds per-PR net limit of {per_pr_limit:,} bytes" ) @@ -176,7 +251,11 @@ def check_file_budgets( def validate_external_asset_manifest(manifest_path: Path) -> list[str]: - """Verify external asset entries record downloadable URL, sha256, bytes, and owner.""" + """Verify external asset manifest syntax and fields conform to the schema. + + Validates schema structure, types, and GitHub Release download URL syntax. + Does not probe remote HTTP endpoints or verify remote payload bytes over the network. + """ errors: list[str] = [] if not manifest_path.exists(): return errors @@ -186,17 +265,31 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: except Exception as exc: return [f"manifest error: failed to parse {manifest_path.name}: {exc}"] - bundles = data.get("bundles", {}) + if not isinstance(data, dict): + return ["manifest error: root of manifest must be a JSON object"] + + if "bundles" not in data: + return ["manifest error: missing required key 'bundles'"] + + bundles = data.get("bundles") if not isinstance(bundles, dict): - return errors + return ["manifest error: 'bundles' must be a JSON object"] sha_re = re.compile(r"^[0-9a-f]{64}$") for bundle_name, bundle in bundles.items(): if not isinstance(bundle, dict): + errors.append(f"bundle {bundle_name}: expected JSON object") + continue + + if "external_assets" not in bundle: continue - external_assets = bundle.get("external_assets", []) + + external_assets = bundle["external_assets"] if not isinstance(external_assets, list): + errors.append( + f"bundle {bundle_name}: 'external_assets' must be a list of asset objects" + ) continue for idx, asset in enumerate(external_assets): @@ -209,24 +302,32 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: if field not in asset: errors.append(f"{prefix}: missing required field '{field}'") - url = asset.get("url", "") - if not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): - errors.append( - f"{prefix}: url must point to a downloadable GitHub Release asset " - f"(https://github.com///releases/download//)" - ) - - sha = asset.get("sha256", "") - if not isinstance(sha, str) or not sha_re.match(sha): - errors.append(f"{prefix}: invalid sha256 hex digest") - - byte_count = asset.get("bytes", -1) - if not isinstance(byte_count, int) or byte_count <= 0: - errors.append(f"{prefix}: bytes must be a positive integer") - - owner = asset.get("retention_owner", "") - if not isinstance(owner, str) or not owner.strip(): - errors.append(f"{prefix}: retention_owner must be non-empty") + url = asset.get("url") + if url is not None: + if not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): + errors.append( + f"{prefix}: url must point to a downloadable GitHub Release asset " + f"(https://github.com///releases/download//)" + ) + + sha = asset.get("sha256") + if sha is not None: + if not isinstance(sha, str) or not sha_re.match(sha): + errors.append(f"{prefix}: invalid sha256 hex digest") + + byte_count = asset.get("bytes") + if byte_count is not None: + if ( + not isinstance(byte_count, int) + or isinstance(byte_count, bool) + or byte_count <= 0 + ): + errors.append(f"{prefix}: bytes must be a positive integer") + + owner = asset.get("retention_owner") + if owner is not None: + if not isinstance(owner, str) or not owner.strip(): + errors.append(f"{prefix}: retention_owner must be non-empty") return errors From 2594a5aeec872e8e8e20657395bc3c147d1a3878 Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:58:08 +0530 Subject: [PATCH 4/8] fix: preserve upload-artifact steps in ci-ready.yml (Refs #273) --- .github/workflows/ci-ready.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-ready.yml b/.github/workflows/ci-ready.yml index ec967583e..0996ef2ee 100644 --- a/.github/workflows/ci-ready.yml +++ b/.github/workflows/ci-ready.yml @@ -127,8 +127,8 @@ jobs: --revision "$GITHUB_SHA" --output "$RUNNER_TEMP/kv-actor-qualification" - name: Retain separate scripted actor receipt including failures if: always() - shell: bash - run: | + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: name: kv-actor-qualification-${{ github.run_id }}-${{ github.run_attempt }} path: | ${{ runner.temp }}/kv-actor-qualification/qualification.json From e3b00f0e89bb32c8ef92b6468a2f9e9942bd461a Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:23:39 +0530 Subject: [PATCH 5/8] fix: resolve blob object sizing, local untracked expansion, and null manifest checks (Refs #273) --- tests/test_evidence_budget.py | 117 ++++++++++++++++++++++++++++--- tools/check_evidence_budget.py | 122 ++++++++++++++++----------------- 2 files changed, 170 insertions(+), 69 deletions(-) diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index 68924aa4a..432456763 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -57,7 +57,7 @@ def test_modified_file_counts_true_net_addition_not_full_size(): def test_net_addition_with_deletion_and_shrinkage(): # Real maintainer scenario: delete 2 MiB, add 5 files of 450 KiB (+2,250 KiB) - # Net addition is +202 KiB, which is well below the 2.0 MiB limit + # Net addition is +206,848 bytes, which is well below the 2.0 MiB limit files = [ ("evidence/old_large.bin", 0, -2 * 1024 * 1024), ] + [ @@ -69,6 +69,7 @@ def test_net_addition_with_deletion_and_shrinkage(): ) assert violations == [] assert total == (5 * 450 * 1024) - (2 * 1024 * 1024) + assert total == 206848 def test_non_evidence_files_not_counted_in_evidence_budget(): @@ -172,13 +173,39 @@ def test_validate_external_asset_manifest_rejects_non_dict_bundles(tmp_path: Pat assert "'bundles' must be a JSON object" in errors[0] +def test_validate_external_asset_manifest_rejects_null_fields(tmp_path: Path): + manifest = tmp_path / "inventory_null_fields.json" + data = { + "version": 1, + "bundles": { + "null_bundle": { + "external_assets": [ + { + "url": None, + "sha256": None, + "bytes": None, + "retention_owner": None, + } + ] + } + }, + } + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert len(errors) == 4 + assert any("null required field 'url'" in e for e in errors) + assert any("null required field 'sha256'" in e for e in errors) + assert any("null required field 'bytes'" in e for e in errors) + assert any("null required field 'retention_owner'" in e for e in errors) + + def test_invalid_base_ref_raises_error(tmp_path: Path): with pytest.raises(ValueError, match="cannot be resolved"): check.get_changed_files_with_net_sizes("non_existent_ref_12345", repo_root=tmp_path) def test_git_rename_and_deletion_regression(tmp_path: Path): - """End-to-end Git test verifying rename, move, delete net delta and non-ASCII paths.""" + """End-to-end Git test verifying rename, move in/out, delete net delta and non-ASCII paths.""" repo = tmp_path / "test_repo" repo.mkdir() @@ -194,6 +221,7 @@ def git(*args: str): ev_dir.mkdir() (ev_dir / "old_600k.bin").write_bytes(b"A" * (600 * 1024)) (ev_dir / "to_delete_2m.bin").write_bytes(b"B" * (2 * 1024 * 1024)) + (ev_dir / "to_move_out.bin").write_bytes(b"D" * (50 * 1024)) outside_dir = repo / "outside" outside_dir.mkdir() (outside_dir / "external.bin").write_bytes(b"C" * (100 * 1024)) @@ -214,6 +242,9 @@ def git(*args: str): # 4. Move outside file into evidence/ -> +100 KiB net addition git("mv", "outside/external.bin", "evidence/external.bin") + # 5. Move file OUT of evidence/ -> -50 KiB net addition + git("mv", "evidence/to_move_out.bin", "outside/moved_out.bin") + git("add", ".") git("commit", "-m", "head commit") @@ -222,15 +253,85 @@ def git(*args: str): assert "evidence/new_600k.bin" in paths assert "evidence/é_trace.json" in paths assert "evidence/external.bin" in paths + assert "evidence/to_move_out.bin" in paths + + # Move out of evidence must record negative delta + move_out_record = next(r for r in records if r[0] == "evidence/to_move_out.bin") + assert move_out_record[1] == 0 + assert move_out_record[2] == -(50 * 1024) violations, net_bytes = check.check_file_budgets(records) # Per-file violation must catch evidence/new_600k.bin assert any("evidence/new_600k.bin" in v and "exceeds per-file limit" in v for v in violations) - # Net evidence addition: - # new_600k: 600K - 600K = 0 - # to_delete_2m: -2M - # external.bin (moved in): +100K - # é_trace.json: +17 bytes - # Total net evidence is negative (~ -1.9 MB), so PR net limit is not violated + # Total net evidence is negative (~ -1.95 MB), so PR net limit is not violated assert net_bytes < 0 assert not any("per-PR net limit" in v for v in violations) + + +def test_git_symlink_not_measured_as_target_size(tmp_path: Path): + """Verify symlink blob is measured by its link target text length, not followed.""" + repo = tmp_path / "symlink_repo" + repo.mkdir() + + def git(*args: str, input_bytes: bytes | None = None): + return subprocess.run( + ["git", *args], + cwd=repo, + input=input_bytes, + capture_output=True, + check=True, + ) + + git("init", "-b", "main") + git("config", "user.name", "Tester") + git("config", "user.email", "tester@example.com") + + # Create 600 KiB file outside evidence + outside_dir = repo / "outside" + outside_dir.mkdir() + (outside_dir / "target.bin").write_bytes(b"X" * (600 * 1024)) + git("add", ".") + git("commit", "-m", "base commit") + + # Create git symlink object pointing to target.bin + blob_sha = git( + "hash-object", "-w", "--stdin", input_bytes=b"outside/target.bin" + ).stdout.decode().strip() + git("update-index", "--add", "--cacheinfo", "120000", blob_sha, "evidence/symlink.bin") + git("commit", "-m", "add symlink") + + records = check.get_changed_files_with_net_sizes("main~1", repo_root=repo) + symlink_record = next(r for r in records if r[0] == "evidence/symlink.bin") + # Blob size of 'outside/target.bin' is 18 bytes, NOT 600 KiB + assert symlink_record[1] == len("outside/target.bin") + violations, _ = check.check_file_budgets(records) + assert violations == [] + + +def test_local_mode_untracked_files_expanded(tmp_path: Path): + """Verify local mode without base ref expands untracked nested directories.""" + repo = tmp_path / "local_repo" + repo.mkdir() + + def git(*args: str): + subprocess.run(["git", *args], cwd=repo, capture_output=True, check=True) + + git("init", "-b", "main") + git("config", "user.name", "Tester") + git("config", "user.email", "tester@example.com") + (repo / "README.md").write_text("# Repo") + git("add", ".") + git("commit", "-m", "init") + + # Add untracked deep nested file exceeding per-file limit + deep_dir = repo / "evidence" / "nested" / "run_01" + deep_dir.mkdir(parents=True) + (deep_dir / "large_raw.bin").write_bytes(b"Z" * (600 * 1024)) + + records = check.get_changed_files_with_net_sizes(base_ref=None, repo_root=repo) + matching = [r for r in records if "evidence/nested/run_01/large_raw.bin" in r[0]] + assert len(matching) == 1 + assert matching[0][1] == 600 * 1024 + + violations, _ = check.check_file_budgets(records) + assert any("large_raw.bin" in v and "exceeds per-file limit" in v for v in violations) \ No newline at end of file diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index 6e6130b10..be479d83a 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -51,8 +51,10 @@ def get_changed_files_with_net_sizes( """Return list of (rel_path, current_size_bytes, net_addition_bytes). If base_ref is supplied, uses git diff --raw -z against base_ref. + Sizes are obtained from git blob objects directly (preventing symlink + following and worktree path confusion). If base_ref fails or is missing, raises ValueError (no silent fallback in CI). - If base_ref is None, inspects local git status with -z. + If base_ref is None, inspects local git status with -z and --untracked-files=all. """ root = repo_root or ROOT results: list[tuple[str, int, int]] = [] @@ -100,20 +102,19 @@ def get_changed_files_with_net_sizes( if is_rename_or_copy: if i + 2 >= len(tokens): break - src_path = tokens[i + 1].decode("utf-8", errors="replace").replace("\\", "/") - dst_path = tokens[i + 2].decode("utf-8", errors="replace").replace("\\", "/") + src_path = tokens[i + 1].decode("utf-8", errors="replace") + dst_path = tokens[i + 2].decode("utf-8", errors="replace") i += 3 src_is_ev = src_path.startswith("evidence/") dst_is_ev = dst_path.startswith("evidence/") src_size = get_git_object_size(root, sha_src) - full_dst = root / dst_path - dst_size = ( - full_dst.stat().st_size - if full_dst.is_file() - else get_git_object_size(root, sha_dst) - ) + dst_size = get_git_object_size(root, sha_dst) + if dst_size == 0 and sha_dst and set(sha_dst) != {"0"}: + full_dst = root / dst_path + if full_dst.exists() or full_dst.is_symlink(): + dst_size = full_dst.lstat().st_size if status.startswith("R"): # Rename: src replaced by dst @@ -136,59 +137,54 @@ def get_changed_files_with_net_sizes( else: if i + 1 >= len(tokens): break - path = tokens[i + 1].decode("utf-8", errors="replace").replace("\\", "/") + path = tokens[i + 1].decode("utf-8", errors="replace") i += 2 is_ev = path.startswith("evidence/") - full_path = root / path if status.startswith("D"): src_size = get_git_object_size(root, sha_src) delta = -src_size if is_ev else 0 results.append((path, 0, delta)) elif status.startswith("A"): - cur_size = ( - full_path.stat().st_size - if full_path.is_file() - else get_git_object_size(root, sha_dst) - ) + cur_size = get_git_object_size(root, sha_dst) + if cur_size == 0 and sha_dst and set(sha_dst) != {"0"}: + full_path = root / path + if full_path.exists() or full_path.is_symlink(): + cur_size = full_path.lstat().st_size delta = cur_size if is_ev else 0 results.append((path, cur_size, delta)) elif status.startswith("M") or status.startswith("T"): src_size = get_git_object_size(root, sha_src) - cur_size = ( - full_path.stat().st_size - if full_path.is_file() - else get_git_object_size(root, sha_dst) - ) + cur_size = get_git_object_size(root, sha_dst) + if cur_size == 0 and sha_dst and set(sha_dst) != {"0"}: + full_path = root / path + if full_path.exists() or full_path.is_symlink(): + cur_size = full_path.lstat().st_size delta = (cur_size - src_size) if is_ev else 0 results.append((path, cur_size, delta)) else: - cur_size = ( - full_path.stat().st_size - if full_path.is_file() - else get_git_object_size(root, sha_dst) - ) + cur_size = get_git_object_size(root, sha_dst) delta = cur_size if is_ev else 0 results.append((path, cur_size, delta)) else: - # Local development fallback inspecting uncommitted changes with -z + # Local development fallback inspecting uncommitted changes with -z -uall local_paths = get_local_git_changed_files(root) for rel_path in local_paths: full_path = root / rel_path - if full_path.is_file(): - sz = full_path.stat().st_size - results.append((rel_path.replace("\\", "/"), sz, sz)) + if full_path.is_file() or full_path.is_symlink(): + sz = full_path.lstat().st_size + results.append((rel_path, sz, sz)) return results def get_local_git_changed_files(root: Path) -> list[str]: - """Inspect local git status with -z for untracked, added, or modified files.""" + """Inspect local git status with -z and --untracked-files=all.""" try: res = subprocess.run( - ["git", "status", "--porcelain", "-z"], + ["git", "status", "--porcelain", "-z", "--untracked-files=all"], cwd=root, capture_output=True, check=True, @@ -202,7 +198,7 @@ def get_local_git_changed_files(root: Path) -> list[str]: i += 1 continue status_code = tok[:2].decode("latin-1", errors="replace") - path = tok[3:].decode("utf-8", errors="replace").replace("\\", "/") + path = tok[3:].decode("utf-8", errors="replace") if "R" in status_code: # In git status -z for rename: new path is first, old path is second token paths.append(path) @@ -228,7 +224,7 @@ def check_file_budgets( allowed = allowlist or set() for rel_path, current_size, net_addition in file_records: - norm_path = rel_path.replace("\\", "/") + norm_path = rel_path if norm_path in allowed: continue @@ -298,36 +294,40 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: errors.append(f"{prefix}: expected JSON object") continue - for field in ("url", "sha256", "bytes", "retention_owner"): - if field not in asset: - errors.append(f"{prefix}: missing required field '{field}'") - + # url check url = asset.get("url") - if url is not None: - if not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): - errors.append( - f"{prefix}: url must point to a downloadable GitHub Release asset " - f"(https://github.com///releases/download//)" - ) + if url is None: + errors.append(f"{prefix}: missing or null required field 'url'") + elif not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): + errors.append( + f"{prefix}: url must point to a downloadable GitHub Release asset " + f"(https://github.com///releases/download//)" + ) + # sha256 check sha = asset.get("sha256") - if sha is not None: - if not isinstance(sha, str) or not sha_re.match(sha): - errors.append(f"{prefix}: invalid sha256 hex digest") + if sha is None: + errors.append(f"{prefix}: missing or null required field 'sha256'") + elif not isinstance(sha, str) or not sha_re.match(sha): + errors.append(f"{prefix}: invalid sha256 hex digest") + # bytes check byte_count = asset.get("bytes") - if byte_count is not None: - if ( - not isinstance(byte_count, int) - or isinstance(byte_count, bool) - or byte_count <= 0 - ): - errors.append(f"{prefix}: bytes must be a positive integer") - + if byte_count is None: + errors.append(f"{prefix}: missing or null required field 'bytes'") + elif ( + not isinstance(byte_count, int) + or isinstance(byte_count, bool) + or byte_count <= 0 + ): + errors.append(f"{prefix}: bytes must be a positive integer") + + # retention_owner check owner = asset.get("retention_owner") - if owner is not None: - if not isinstance(owner, str) or not owner.strip(): - errors.append(f"{prefix}: retention_owner must be non-empty") + if owner is None: + errors.append(f"{prefix}: missing or null required field 'retention_owner'") + elif not isinstance(owner, str) or not owner.strip(): + errors.append(f"{prefix}: retention_owner must be non-empty string") return errors @@ -359,11 +359,11 @@ def main() -> int: file_list: list[tuple[str, int, int]] = [] for p_str in args.files: p = Path(p_str) - if p.is_file(): + if p.is_file() or p.is_symlink(): rel = str( p.relative_to(ROOT) if p.is_relative_to(ROOT) else p - ).replace("\\", "/") - sz = p.stat().st_size + ) + sz = p.lstat().st_size file_list.append((rel, sz, sz)) diff_violations, _ = check_file_budgets( file_list, args.max_file_bytes, args.max_pr_bytes From 1382ac5a1a253c2b4869ca9e63cd62b68e30cff6 Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:32:00 +0530 Subject: [PATCH 6/8] fix: eliminate worktree fallback on zero-byte blobs in base diff mode (Refs #273) --- tests/test_evidence_budget.py | 37 ++++++++++++++++++++++++++++++ tools/check_evidence_budget.py | 42 +++++++++++++++------------------- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index 432456763..df3747a73 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -308,6 +308,43 @@ def git(*args: str, input_bytes: bytes | None = None): assert violations == [] +def test_git_empty_blob_with_dirty_worktree_not_misreported(tmp_path: Path): + """Verify empty blob committed in HEAD is not inflated by dirty uncommitted worktree.""" + repo = tmp_path / "empty_blob_repo" + repo.mkdir() + + def git(*args: str): + subprocess.run(["git", *args], cwd=repo, capture_output=True, check=True) + + git("init", "-b", "main") + git("config", "user.name", "Tester") + git("config", "user.email", "tester@example.com") + (repo / "README.md").write_text("# Base") + git("add", ".") + git("commit", "-m", "base") + + # In HEAD commit, commit an empty file in evidence/ + ev_dir = repo / "evidence" + ev_dir.mkdir() + empty_file = ev_dir / "empty.bin" + empty_file.write_bytes(b"") + git("add", ".") + git("commit", "-m", "add empty file") + + # Now write 600 KiB dirty worktree content without committing + empty_file.write_bytes(b"Z" * (600 * 1024)) + + records = check.get_changed_files_with_net_sizes("main~1", repo_root=repo) + empty_record = next(r for r in records if r[0] == "evidence/empty.bin") + # Committed blob in HEAD is 0 bytes, not 600 KiB + assert empty_record[1] == 0 + assert empty_record[2] == 0 + + violations, net_bytes = check.check_file_budgets(records) + assert violations == [] + assert net_bytes == 0 + + def test_local_mode_untracked_files_expanded(tmp_path: Path): """Verify local mode without base ref expands untracked nested directories.""" repo = tmp_path / "local_repo" diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index be479d83a..a72c0fb45 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -28,20 +28,28 @@ def get_git_object_size(root: Path, obj_ref: str) -> int: - """Return byte size of a git blob object using git cat-file -s.""" + """Return byte size of a git blob object using git cat-file -s. + + Raises ValueError if git cat-file fails. + """ if not obj_ref or set(obj_ref) == {"0"}: return 0 - try: - res = subprocess.run( - ["git", "cat-file", "-s", obj_ref], - cwd=root, - capture_output=True, - text=True, - check=True, + res = subprocess.run( + ["git", "cat-file", "-s", obj_ref], + cwd=root, + capture_output=True, + text=True, + ) + if res.returncode != 0: + raise ValueError( + f"Failed to read size for git object '{obj_ref}': {res.stderr.strip()}" ) + try: return int(res.stdout.strip()) - except (subprocess.SubprocessError, ValueError): - return 0 + except ValueError as exc: + raise ValueError( + f"Invalid size output for git object '{obj_ref}': {res.stdout.strip()}" + ) from exc def get_changed_files_with_net_sizes( @@ -52,7 +60,7 @@ def get_changed_files_with_net_sizes( If base_ref is supplied, uses git diff --raw -z against base_ref. Sizes are obtained from git blob objects directly (preventing symlink - following and worktree path confusion). + following, empty blob confusion, and worktree path confusion). If base_ref fails or is missing, raises ValueError (no silent fallback in CI). If base_ref is None, inspects local git status with -z and --untracked-files=all. """ @@ -111,10 +119,6 @@ def get_changed_files_with_net_sizes( src_size = get_git_object_size(root, sha_src) dst_size = get_git_object_size(root, sha_dst) - if dst_size == 0 and sha_dst and set(sha_dst) != {"0"}: - full_dst = root / dst_path - if full_dst.exists() or full_dst.is_symlink(): - dst_size = full_dst.lstat().st_size if status.startswith("R"): # Rename: src replaced by dst @@ -148,19 +152,11 @@ def get_changed_files_with_net_sizes( results.append((path, 0, delta)) elif status.startswith("A"): cur_size = get_git_object_size(root, sha_dst) - if cur_size == 0 and sha_dst and set(sha_dst) != {"0"}: - full_path = root / path - if full_path.exists() or full_path.is_symlink(): - cur_size = full_path.lstat().st_size delta = cur_size if is_ev else 0 results.append((path, cur_size, delta)) elif status.startswith("M") or status.startswith("T"): src_size = get_git_object_size(root, sha_src) cur_size = get_git_object_size(root, sha_dst) - if cur_size == 0 and sha_dst and set(sha_dst) != {"0"}: - full_path = root / path - if full_path.exists() or full_path.is_symlink(): - cur_size = full_path.lstat().st_size delta = (cur_size - src_size) if is_ev else 0 results.append((path, cur_size, delta)) else: From add0aefb2b9389d2a36c69547fdfa639c4fe97ba Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:26:30 +0530 Subject: [PATCH 7/8] fix: add report-only mode for candidate budgets and clarify asset URL format check (Refs #273) --- .github/workflows/ci-ready.yml | 6 +-- tests/test_evidence_budget.py | 96 +++++++++++++++++++++++++++++++++- tools/check_evidence_budget.py | 42 +++++++++++---- 3 files changed, 129 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci-ready.yml b/.github/workflows/ci-ready.yml index 0996ef2ee..c24215123 100644 --- a/.github/workflows/ci-ready.yml +++ b/.github/workflows/ci-ready.yml @@ -69,7 +69,7 @@ jobs: run: python3 tools/check_pr_references.py evidence-budget: - # Candidate evidence byte budget check (Refs #273, R6 under discussion) + # Candidate evidence byte budget check (Refs #273, R6 under discussion - report-only pending owner acceptance) runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -79,10 +79,10 @@ jobs: with: fetch-depth: 0 persist-credentials: false - - name: Verify evidence byte budgets + - name: Verify evidence byte budgets (report-only pending #273 owner acceptance) env: BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: python3 tools/check_evidence_budget.py --base "$BASE_SHA" + run: python3 tools/check_evidence_budget.py --base "$BASE_SHA" --report-only kv-task-qualification: # Bounded software-control qualification; not a branch-protection or efficacy gate. diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index df3747a73..6fd3efc34 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -144,6 +144,7 @@ def test_validate_external_asset_manifest_rejects_tag_view_url(tmp_path: Path): errors = check.validate_external_asset_manifest(manifest) assert len(errors) == 1 assert "releases/download" in errors[0] + assert "format check" in errors[0] def test_validate_external_asset_manifest_rejects_non_list_external_assets(tmp_path: Path): @@ -371,4 +372,97 @@ def git(*args: str): assert matching[0][1] == 600 * 1024 violations, _ = check.check_file_budgets(records) - assert any("large_raw.bin" in v and "exceeds per-file limit" in v for v in violations) \ No newline at end of file + assert any("large_raw.bin" in v and "exceeds per-file limit" in v for v in violations) + + +def test_main_negative_case_fails_without_report_only(tmp_path: Path, monkeypatch, capsys): + """Negative case: candidate budget violation without --report-only exits with code 1.""" + monkeypatch.setattr(check, "ROOT", tmp_path) + ev_dir = tmp_path / "evidence" + ev_dir.mkdir() + ev_file = ev_dir / "large_raw.bin" + ev_file.write_bytes(b"A" * (600 * 1024)) + + monkeypatch.setattr( + "sys.argv", + [ + "check_evidence_budget.py", + "--files", + str(ev_file), + "--max-file-bytes", + str(512 * 1024), + "--inventory", + "nonexistent_inv.json", + ], + ) + rc = check.main() + captured = capsys.readouterr() + assert rc == 1 + assert "exceeds per-file limit" in captured.out + assert "Candidate evidence byte budget exceeded" in captured.out + assert "Running in report-only mode" not in captured.out + + +def test_main_report_only_mode_warns_and_exits_zero(tmp_path: Path, monkeypatch, capsys): + """Report-only mode: candidate budget violation warns but exits 0 pending #273 acceptance.""" + monkeypatch.setattr(check, "ROOT", tmp_path) + ev_dir = tmp_path / "evidence" + ev_dir.mkdir() + ev_file = ev_dir / "candidate_large.bin" + ev_file.write_bytes(b"B" * (600 * 1024)) + + monkeypatch.setattr( + "sys.argv", + [ + "check_evidence_budget.py", + "--files", + str(ev_file), + "--max-file-bytes", + str(512 * 1024), + "--report-only", + "--inventory", + "nonexistent_inv.json", + ], + ) + rc = check.main() + captured = capsys.readouterr() + assert rc == 0 + assert "exceeds per-file limit" in captured.out + assert "Running in report-only mode" in captured.out + assert "Refs #273" in captured.out + + +def test_validate_external_asset_manifest_explicit_format_check_diagnostic(tmp_path: Path): + """External asset manifest URL diagnostic explicitly identifies validation as a format check.""" + manifest = tmp_path / "invalid_manifest.json" + data = { + "bundles": { + "test_bundle": { + "external_assets": [ + { + "url": "https://example.com/asset.zip", + "sha256": "0" * 64, + "bytes": 1024, + "retention_owner": "maintainer", + } + ] + } + } + } + manifest.write_text(json.dumps(data), encoding="utf-8") + errors = check.validate_external_asset_manifest(manifest) + assert len(errors) == 1 + assert "url format check failed" in errors[0] + assert "format check only; does not verify downloadability or remote hash" in errors[0] + + +def test_main_cli_help_mentions_format_check_and_report_only(monkeypatch, capsys): + """CLI help output explicitly mentions format check and --report-only mode.""" + monkeypatch.setattr("sys.argv", ["check_evidence_budget.py", "--help"]) + with pytest.raises(SystemExit) as exc: + check.main() + assert exc.value.code == 0 + captured = capsys.readouterr() + assert "format check" in captured.out + assert "--report-only" in captured.out + diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index a72c0fb45..123149ae3 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -220,7 +220,7 @@ def check_file_budgets( allowed = allowlist or set() for rel_path, current_size, net_addition in file_records: - norm_path = rel_path + norm_path = rel_path.replace("\\", "/") if norm_path in allowed: continue @@ -245,8 +245,8 @@ def check_file_budgets( def validate_external_asset_manifest(manifest_path: Path) -> list[str]: """Verify external asset manifest syntax and fields conform to the schema. - Validates schema structure, types, and GitHub Release download URL syntax. - Does not probe remote HTTP endpoints or verify remote payload bytes over the network. + Validates schema structure, types, and performs a format check on GitHub Release download URLs. + This is a format check only and does not verify asset downloadability or remote hashes. """ errors: list[str] = [] if not manifest_path.exists(): @@ -290,14 +290,15 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: errors.append(f"{prefix}: expected JSON object") continue - # url check + # url format check url = asset.get("url") if url is None: errors.append(f"{prefix}: missing or null required field 'url'") elif not isinstance(url, str) or not RELEASE_ASSET_URL_PATTERN.match(url): errors.append( - f"{prefix}: url must point to a downloadable GitHub Release asset " - f"(https://github.com///releases/download//)" + f"{prefix}: url format check failed: url must match pattern " + f"https://github.com///releases/download// " + f"(format check only; does not verify downloadability or remote hash)" ) # sha256 check @@ -330,12 +331,14 @@ def validate_external_asset_manifest(manifest_path: Path) -> list[str]: def main() -> int: parser = argparse.ArgumentParser( - description="Check evidence byte budgets and external asset metadata." + description="Check evidence byte budgets and external asset metadata (format check)." ) parser.add_argument("--base", help="Base git reference/SHA (e.g. origin/main or $BASE_SHA)") parser.add_argument("--files", nargs="*", help="Explicit list of files to check") parser.add_argument( - "--inventory", default="tools/evidence_inventory.json", help="Path to evidence inventory" + "--inventory", + default="tools/evidence_inventory.json", + help="Path to evidence inventory (performs format check only on asset metadata)", ) parser.add_argument( "--max-file-bytes", type=int, default=PER_FILE_MAX_BYTES, help="Per-file max byte budget" @@ -346,6 +349,14 @@ def main() -> int: default=PER_PR_EVIDENCE_MAX_BYTES, help="Per-PR max byte budget", ) + parser.add_argument( + "--report-only", + action="store_true", + help=( + "Report candidate budget violations as warnings without exiting with error code " + "(for evaluation while candidate thresholds are under discussion in #273)" + ), + ) args = parser.parse_args() violations: list[str] = [] @@ -355,10 +366,13 @@ def main() -> int: file_list: list[tuple[str, int, int]] = [] for p_str in args.files: p = Path(p_str) + if not p.is_absolute(): + p = (ROOT / p) if p.is_file() or p.is_symlink(): - rel = str( - p.relative_to(ROOT) if p.is_relative_to(ROOT) else p - ) + try: + rel = p.relative_to(ROOT).as_posix() + except ValueError: + rel = p_str.replace("\\", "/") sz = p.lstat().st_size file_list.append((rel, sz, sz)) diff_violations, _ = check_file_budgets( @@ -384,6 +398,12 @@ def main() -> int: if violations: print(ADVICE) + if args.report_only: + print( + "[NOTICE] Running in report-only mode: candidate byte budget violations " + "reported above as warnings; check will not fail CI (Refs #273)." + ) + return 0 return 1 return 0 From 64432e2981d47ebec882e0274607db71d843377c Mon Sep 17 00:00:00 2001 From: Rohan Kakade <88588904+Rohank143@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:54:16 +0530 Subject: [PATCH 8/8] fix: enforce manifest validation in report-only mode and preserve raw POSIX backslash paths (Refs #273) --- tests/test_evidence_budget.py | 104 +++++++++++++++++++++++++++++++++ tools/check_evidence_budget.py | 24 +++++--- 2 files changed, 120 insertions(+), 8 deletions(-) diff --git a/tests/test_evidence_budget.py b/tests/test_evidence_budget.py index 6fd3efc34..890b9f715 100644 --- a/tests/test_evidence_budget.py +++ b/tests/test_evidence_budget.py @@ -466,3 +466,107 @@ def test_main_cli_help_mentions_format_check_and_report_only(monkeypatch, capsys assert "format check" in captured.out assert "--report-only" in captured.out + +def test_git_raw_posix_backslash_top_level_path_not_treated_as_evidence(tmp_path: Path): + """A top-level file literally named 'evidence\\big.bin' must not count as evidence/.""" + repo = tmp_path / "backslash_repo" + repo.mkdir() + + def git(*args: str, input_bytes: bytes | None = None): + return subprocess.run( + ["git", *args], + cwd=repo, + input=input_bytes, + capture_output=True, + check=True, + ) + + git("init", "-b", "main") + git("config", "user.name", "Tester") + git("config", "user.email", "tester@example.com") + (repo / "README.md").write_text("# Repo") + git("add", ".") + git("commit", "-m", "init") + + # Base commit SHA + base_commit = git("rev-parse", "HEAD").stdout.decode().strip() + + # Create a 600 KiB blob in the git object database + blob_sha = git( + "hash-object", "-w", "--stdin", input_bytes=b"Z" * (600 * 1024) + ).stdout.decode().strip() + + # Use git mktree to construct a POSIX tree with top-level 'evidence\\raw_top_level.bin' + tree_input = f"100644 blob {blob_sha}\tevidence\\raw_top_level.bin\n".encode("utf-8") + tree_sha = git("mktree", input_bytes=tree_input).stdout.decode().strip() + + # Commit tree with base_commit as parent + head_commit = git( + "commit-tree", tree_sha, "-p", base_commit, "-m", "add top-level backslash file" + ).stdout.decode().strip() + + # Point HEAD / branch to head_commit + git("update-ref", "refs/heads/main", head_commit) + + records = check.get_changed_files_with_net_sizes(base_commit, repo_root=repo) + matching = [r for r in records if "evidence\\raw_top_level.bin" in r[0]] + assert len(matching) == 1 + rel_path, cur_size, _ = matching[0] + assert rel_path == "evidence\\raw_top_level.bin" + assert cur_size == 600 * 1024 + + # Top-level file with literal backslash must NOT count as evidence/ directory + violations, net_bytes = check.check_file_budgets(records) + assert violations == [] + assert net_bytes == 0 + + +def test_main_report_only_mode_does_not_hide_manifest_errors(tmp_path: Path, monkeypatch, capsys): + """Report-only mode applies to candidate budgets; manifest schema/format errors still fail.""" + monkeypatch.setattr(check, "ROOT", tmp_path) + ev_dir = tmp_path / "evidence" + ev_dir.mkdir() + ev_file = ev_dir / "valid.bin" + ev_file.write_bytes(b"A" * 1024) + + # Invalid inventory manifest with invalid URL pattern and invalid sha256 + inv_file = tmp_path / "tools" / "evidence_inventory.json" + inv_file.parent.mkdir(parents=True) + inv_file.write_text( + json.dumps({ + "version": 1, + "bundles": { + "bad_bundle": { + "external_assets": [ + { + "url": "https://invalid.com/not_a_github_release", + "sha256": "bad_sha", + "bytes": 1024, + "retention_owner": "owner", + } + ] + } + }, + }), + encoding="utf-8", + ) + + monkeypatch.setattr( + "sys.argv", + [ + "check_evidence_budget.py", + "--files", + str(ev_file), + "--report-only", + "--inventory", + "tools/evidence_inventory.json", + ], + ) + rc = check.main() + captured = capsys.readouterr() + # Must exit 1 despite --report-only because manifest errors are enforced + assert rc == 1 + assert "url format check failed" in captured.err + assert "invalid sha256 hex digest" in captured.err + + diff --git a/tools/check_evidence_budget.py b/tools/check_evidence_budget.py index 123149ae3..933a1b404 100644 --- a/tools/check_evidence_budget.py +++ b/tools/check_evidence_budget.py @@ -220,7 +220,7 @@ def check_file_budgets( allowed = allowlist or set() for rel_path, current_size, net_addition in file_records: - norm_path = rel_path.replace("\\", "/") + norm_path = rel_path if norm_path in allowed: continue @@ -359,7 +359,7 @@ def main() -> int: ) args = parser.parse_args() - violations: list[str] = [] + budget_violations: list[str] = [] try: if args.files is not None: @@ -378,32 +378,40 @@ def main() -> int: diff_violations, _ = check_file_budgets( file_list, args.max_file_bytes, args.max_pr_bytes ) - violations.extend(diff_violations) + budget_violations.extend(diff_violations) else: changed_records = get_changed_files_with_net_sizes(args.base) diff_violations, _ = check_file_budgets( changed_records, args.max_file_bytes, args.max_pr_bytes ) - violations.extend(diff_violations) + budget_violations.extend(diff_violations) except Exception as exc: print(f"evidence-budget error: {exc}", file=sys.stderr) return 1 + manifest_errors: list[str] = [] inv_path = ROOT / args.inventory if inv_path.exists(): - violations.extend(validate_external_asset_manifest(inv_path)) + manifest_errors.extend(validate_external_asset_manifest(inv_path)) - for item in violations: + for item in budget_violations: print(item) - if violations: + if budget_violations: print(ADVICE) if args.report_only: print( "[NOTICE] Running in report-only mode: candidate byte budget violations " "reported above as warnings; check will not fail CI (Refs #273)." ) - return 0 + + for err in manifest_errors: + print(err, file=sys.stderr) + + if manifest_errors: + return 1 + + if budget_violations and not args.report_only: return 1 return 0