scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-09 03:17:54 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

refactor(ci): address the evidence path directly instead of threading it

Browse source 
The upload step needs a path that does not depend on the sweep step
surviving. It did not need shared state to get one: `runner.temp` is
available in a step, only not in a job-level `env:`, so each of the three
consumers can name `${RUNNER_TEMP}/wfevolve` itself. That deletes the
env var and the step that published it — the previous fix swapped one
threading channel for a sturdier one where no channel was required.

Also from the same review pass:

- `announce`/`keep` drop their default-argument capture of `task["id"]`
  and `per_arm`. Late binding only bites a closure invoked after the loop
  moves on; these are called synchronously inside `sweep_task_cells`,
  which blocks until every wave completes. The trick was guarding against
  a race that cannot happen here, while implying to the next reader that
  it can.
- `_stub_cell_dependencies` returns the list its teardown appends to
  rather than taking it as an out-parameter, dropping the boilerplate
  from every call site.
- The workflow's `WORKERS` comment points at the `--workers` help text
  instead of restating it, so the rationale has one home.
This commit is contained in:
Gergo Magyar 2026-08-01 19:34:57 +00:00
parent d18dbd4143
commit 8076b98fcc
4 changed files with 27 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
.github/workflows/gitnexus-skill-evolution.yml vendored
View file

@ -134,25 +134,13 @@ jobs:
env:
GENERATIONS: ${{ inputs.generations || '1' }}
RUNS: ${{ inputs.runs || '3' }}
# Serial by default. A cell that loses CPU to its siblings takes longer,
# and a session that reaches its timeout is an excluded run the promotion
# gate refuses to work with — so this only goes up when the runner has the
# vCPUs to back it (the box is sized for one cell at a time today).
# Serial by default — see workflow_bench.runner --workers for why, and
# raise it only to match the runner's vCPUs.
WORKERS: ${{ inputs.workers || '1' }}
MODEL: ${{ inputs.model || 'claude-sonnet-5' }}
PROPOSER_MODEL: ${{ inputs.proposer_model || 'claude-opus-4-8' }}
INCLUDE_EXPENSIVE: ${{ inputs.include_expensive && '1' || '' }}
steps:
- name: Pin the evidence path before anything can run
# The upload step must not take its path from the sweep step's outputs
# — that is the step whose death is the reason the upload matters. A
# job-level `env:` cannot hold it either (the `runner` context does not
# exist there), so publish it to GITHUB_ENV first: every later step
# sees it, including the `if: always()` upload after a killed sweep.
run: |
set -euo pipefail
echo "OUT_ROOT=${RUNNER_TEMP}/wfevolve" >> "${GITHUB_ENV}"
- name: Require the benchmark auth secret
env:
HAS_TOKEN: ${{ secrets.GITNEXUS_BENCH_AUTH_TOKEN != '' }}
@ -322,7 +310,7 @@ jobs:
--runs "${RUNS}" \
--workers "${WORKERS}" \
--claude-bin "${RUNNER_TEMP}/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude" \
--out-root "${OUT_ROOT}" \
--out-root "${RUNNER_TEMP}/wfevolve" \
--apply \
"${extra[@]}"
working-directory: eval
@ -335,7 +323,10 @@ jobs:
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: gitnexus-evolution-${{ github.run_id }}-${{ github.run_attempt }}
path: ${{ env.OUT_ROOT }}
# Addressed directly rather than carried from the sweep step: that is
# the step whose death is the reason this upload matters, and a value
# threaded from it would not be there when it counts.
path: ${{ runner.temp }}/wfevolve
retention-days: 14
if-no-files-found: warn
@ -369,7 +360,7 @@ jobs:
# generation's decisions could surface in the PR body. The heredoc
# uses a per-run random delimiter so a summary value that ever
# contains the marker cannot close the block early and inject keys.
promotion_file="$(find "${OUT_ROOT}" -name promotion.json | sort -V | tail -1)"
promotion_file="$(find "${RUNNER_TEMP}/wfevolve" -name promotion.json | sort -V | tail -1)"
delim="PROMOTION_EOF_$(openssl rand -hex 16)"
{
echo "summary<<${delim}"

22
eval/tests/test_runner_hardening.py
View file

@ -3,6 +3,7 @@
import hashlib
import json
from contextlib import nullcontext
from pathlib import Path
from types import SimpleNamespace
import pytest
@ -433,8 +434,12 @@ def _cell_context(tmp_path, **overrides):
return runner.TaskCellContext(**fields)
def _stub_cell_dependencies(monkeypatch, tmp_path, removed):
"""Replace everything a cell shells out to, so only its own logic runs."""
def _stub_cell_dependencies(monkeypatch, tmp_path):
"""Replace everything a cell shells out to, so only its own logic runs.
Returns the clone it will hand out and the list its teardown appends to.
"""
removed: list[Path] = []
worktree = tmp_path / "clone"
worktree.mkdir()
monkeypatch.setattr(runner, "make_worktree", lambda *_a, **_k: worktree)
@ -454,12 +459,11 @@ def _stub_cell_dependencies(monkeypatch, tmp_path, removed):
monkeypatch.setattr(runner, "capture_patch", lambda *_a, **_k: b"diff")
monkeypatch.setattr(runner, "run_arm", lambda *_a, **_k: {"resolved": True, "ok": True, "error_kind": None})
monkeypatch.setattr(runner, "remove_clone", lambda path: removed.append(path))
return worktree
return worktree, removed
def test_run_cell_returns_a_row_bound_to_its_task_and_snapshots(monkeypatch, tmp_path):
removed: list = []
_stub_cell_dependencies(monkeypatch, tmp_path, removed)
_, removed = _stub_cell_dependencies(monkeypatch, tmp_path)
record = runner.run_cell(_cell_context(tmp_path), 2, "workflow")
@ -497,8 +501,7 @@ def test_run_cell_returns_a_row_bound_to_its_task_and_snapshots(monkeypatch, tmp
ids=["managed-process", "sandbox", "os", "runtime", "value"],
)
def test_run_cell_records_an_expected_failure_and_still_removes_its_clone(monkeypatch, tmp_path, failure):
removed: list = []
_stub_cell_dependencies(monkeypatch, tmp_path, removed)
_, removed = _stub_cell_dependencies(monkeypatch, tmp_path)
def explode(*_args, **_kwargs):
raise failure
@ -514,8 +517,7 @@ def test_run_cell_records_an_expected_failure_and_still_removes_its_clone(monkey
def test_run_cell_lets_an_unexpected_failure_escape_rather_than_scoring_it(monkeypatch, tmp_path):
removed: list = []
_stub_cell_dependencies(monkeypatch, tmp_path, removed)
_, removed = _stub_cell_dependencies(monkeypatch, tmp_path)
def explode(*_args, **_kwargs):
raise KeyError("harness bug")
@ -529,7 +531,7 @@ def test_run_cell_lets_an_unexpected_failure_escape_rather_than_scoring_it(monke
def test_run_cell_reports_a_cleanup_failure_over_its_primary_outcome(monkeypatch, tmp_path):
_stub_cell_dependencies(monkeypatch, tmp_path, [])
_stub_cell_dependencies(monkeypatch, tmp_path)
def refuse(_path):
raise OSError("clone is busy")

16
eval/workflow_bench/runner.py
View file

@ -1425,22 +1425,16 @@ def main() -> None:
per_arm: dict[str, list[dict[str, Any]]] = {a: [] for a in args.arms}
cells = [(run_idx, arm) for run_idx in range(args.runs) for arm in args.arms]
def announce(run_idx: int, arm: str, task_id: str = task["id"]) -> None:
def announce(run_idx: int, arm: str) -> None:
nonlocal started_cells
started_cells += 1
print(
f"[{task_id}][{arm}][run {run_idx}] starting "
f"[{task['id']}][{arm}][run {run_idx}] starting "
f"({started_cells}/{total_cells}, {(time.monotonic() - sweep_started) / 60:.0f}m elapsed)"
)
def keep(
run_idx: int,
arm: str,
record: dict[str, Any],
task_id: str = task["id"],
rows: dict[str, list[dict[str, Any]]] = per_arm,
) -> None:
rows[arm].append(record)
def keep(run_idx: int, arm: str, record: dict[str, Any]) -> None:
per_arm[arm].append(record)
with results_path.open("a") as fh:
# Redact any API token a session-error stderr_tail echoed
# into error_detail before it enters the uploaded
@ -1448,7 +1442,7 @@ def main() -> None:
# sink was not).
fh.write(redact_text(json.dumps(record), [args.auth_token or ""]) + "\n")
print(
f"[{task_id}][{arm}][run {run_idx}] resolved={record['resolved']} "
f"[{task['id']}][{arm}][run {run_idx}] resolved={record['resolved']} "
f"in={record['input_tokens']} out={record['output_tokens']} "
f"cost=${_na(record['cost_usd'])} "
f"took={_na(record.get('duration_s'))}s "

10
gitnexus/test/unit/skill-evolution-workflow.test.ts
View file

@ -151,19 +151,13 @@ describe('gitnexus skill-evolution workflow contract', () => {
expect(jobBudget as number).toBeLessThanOrEqual(21 * 60);
});
it('uploads benchmark evidence unconditionally, on a path fixed before the sweep runs', () => {
// OUT_ROOT is published to GITHUB_ENV by the first step, not held in a
// job-level `env:` — the `runner` context does not exist there, so
// `${{ runner.temp }}/wfevolve` would silently resolve to `/wfevolve`.
const first = evolveJob?.steps?.[0];
expect(first?.name).toBe('Pin the evidence path before anything can run');
expect(first?.run).toContain('echo "OUT_ROOT=${RUNNER_TEMP}/wfevolve" >> "${GITHUB_ENV}"');
it('uploads benchmark evidence unconditionally, on a path it addresses itself', () => {
const upload = evolveJob?.steps?.find(({ name }) => name === 'Upload benchmark evidence');
// The sweep appends results.jsonl and transcripts as it goes, so a killed
// generation still holds the evidence explaining why — and a path taken
// from the killed step's outputs is exactly what would not be there.
expect(upload?.if).toBe('always()');
expect(upload?.with?.path).toBe('${{ env.OUT_ROOT }}');
expect(upload?.with?.path).toBe('${{ runner.temp }}/wfevolve');
});
it('documents the App secrets and protected Environment on the activation checklist', () => {