fix(harness): package provenance and replay corrections - #616
fix(harness): package provenance and replay corrections#616KooshaPari wants to merge 18 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61ff247e96
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| repo=repo, | ||
| profile=profile, | ||
| plan_hash=command_hash, | ||
| subject_commit=discovery.manifest.commit or "", |
There was a problem hiding this comment.
When execute-phase-2-harness.sh iterates clones/*, those directories are not guaranteed to be Git repositories; the existing smoke test creates clones/toyrepo that way. With this new argument, discovery.manifest.commit is empty, add_envelope raises before writing *-run.json, the phase-2 script swallows the non-zero exit, and the run is reported as MISSING instead of PASS; if a resolved Git identity is mandatory, the phase-2 path needs to initialize, skip, or handle non-Git targets before invoking the envelope.
Useful? React with 👍 / 👎.
| "plan": commands, | ||
| "command_count": len(commands), | ||
| "reproducibility": _reproducibility_metadata(profile, args), | ||
| "fixture": { |
There was a problem hiding this comment.
This added fixture/envelope assembly pushes harness/scripts/run-harness.py from exactly 500 lines to 528 lines, exceeding the repository's hard 500-line source-file limit; moving this assembly into a helper/module would keep the runner reviewable and within the documented constraint.
AGENTS.md reference: AGENTS.md:L66-L72
Useful? React with 👍 / 👎.
| "provenance": {"collector": "helios-harness", "collected_at": now, "source_hashes": {"plan": plan_hash}}, | ||
| "provenance": {"collector": "helios-harness", "collected_at": now, "source_ref": subject_ref, "source_sha": subject_commit, "source_hashes": {"plan": plan_hash}}, | ||
| "signature": {"algorithm": "placeholder", "key_id": "unconfigured", "signature_b64": ""}, | ||
| "result_code": "PASS" if passed else "FAIL", |
There was a problem hiding this comment.
When discovery finds no runnable commands, run_runner sets result_code to WARN, but this assignment rewrites every non-PASS payload to FAIL; for example, a valid Git repo with only a README now emits command_count: 0 and result_code: FAIL. That loses the intended warning/no-evidence state for downstream consumers, so keep the original WARN/FAIL code and derive the canonical result.status separately.
Useful? React with 👍 / 👎.
| "lease": {"lease_id": f"lease_{attempt_id[4:20]}", "owner": "helios-harness", "ttl_seconds": 120, "heartbeat_interval_seconds": 20}, | ||
| "task_manifest": {"task_id": f"plan:{plan_hash}", "input_sha256": plan_hash, "timeout_seconds": 1, "assertions": [{"id": "plan_discovered", "kind": "command_plan", "expected": True}], "judge": {"name": "helios-harness", "version": "0.1.0"}}, | ||
| "tasks": tasks, | ||
| "runs": runs, |
There was a problem hiding this comment.
For real runs, evidence_payload already supplies runs entries with stdout_file, stderr_file, artifact_dir, timestamps, attempts, errors, and skipped state, but this override replaces them with the smaller benchmark run objects. The written evidence therefore loses the command log paths and other legacy fields that later normalization/debugging relies on; store the new task-run projection separately or merge it without discarding the original run metadata.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| def _require_resolved_git_sha(subject_commit: str) -> str: | ||
| if not _GIT_SHA_PATTERN.fullmatch(subject_commit) or len(subject_commit) != 40: |
There was a problem hiding this comment.
Git supports repositories initialized with an alternate object format (git init -h exposes --object-format <hash>), and git rev-parse --verify HEAD returns a 64-character object ID for SHA-256 repos. This check rejects those full IDs even though the regex allows 64 hex characters, so run-harness.py run raises before writing evidence for such repositories; accept the full supported hash lengths rather than hard-coding 40.
Useful? React with 👍 / 👎.
| assert {event["type"] for event in payload["events"]} >= {"checkpoint", "compaction"} | ||
|
|
||
|
|
||
| def test_real_runs_promote_populated_tasks_and_runs(): |
There was a problem hiding this comment.
The newly added tests starting here do not include the required Functional Requirement traceability marker, so the new benchmark-envelope behavior and error-path coverage cannot be mapped back to an FR as required for all tests; add a # Traces to: FR-HELIOS-... comment to each new test.
AGENTS.md reference: AGENTS.md:L123-L129
Useful? React with 👍 / 👎.
KooshaPari
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
…chain-refresh (#628) Shards the unique work from 3 helios-cli branches onto current main: - feat/benchmark-provenance (#616): benchmark_envelope + harness scripts + CI - wip/20260722-helios-harness-preservation (#614): direct envelope test - feat/toolchain-refresh (#610): package.json + pnpm-lock.yaml Co-authored-by: ForgeCode <forgecode@local>
|
Closing: shard merged in #628 (benchmark-provenance + harness-preservation + toolchain-refresh). No unique work remains. |
Summary
This follow-up keeps benchmark evidence tied to resolved source provenance and makes the
installable Helios harness usable from the repository root. It also packages the schema
validation dependencies and makes replay hashes stable across collection timestamps.
Cause and effect
The harness root package shadowed the
src/harnesspackage, so thehelios-harnessentrypoint and imports were not reliable without
PYTHONPATH. Non-Git inputs were alsopromoted into evidence with fabricated fallback provenance. Replay hashes included
collection-time timestamps, making identical runs appear different. The Rust workflow
used an unavailable Trunk action revision and the dual harness had formatting drift.
Changes
src/harnessthrough the repository-root package and move secondary CLI commandsinto a dedicated module;
jsonschemaandfastjsonschemadependencies;WARNevidence for unresolvednon-Git inputs, and retain populated task/run metadata;
full-SHA subjects, warning metadata, and deterministic replay hashes.
Validation
python3 -B -m pytest harness/tests/test_entrypoint_import.py harness/tests/test_runner_unit.py harness/tests/test_cli_integration.py harness/tests/test_benchmark_envelope_direct.py -q(20 passed)python3 -B -m pytest harness/tests/test_run_harness.py::test_harness_replay_and_validate harness/tests/test_schema.py harness/tests/test_schema_roundtrip.py -q(7 passed)harness/.venv/bin/helios-harness --helpand package/import smoke (passed)cargo fmt --check --manifest-path crates/harness_runner/Cargo.toml(passed)cargo test -p harness_runner(7 passed)The local Ruff configuration currently fails to parse its existing
line-lengthkey;that baseline tooling issue is unchanged by this PR.