Skip to content

runner: run package — executor, leg.json sentinels, and resume - #23

Merged
marwen-abid merged 2 commits into
mainfrom
runner-04-execute
Aug 2, 2026
Merged

marwen-abid merged 2 commits into
mainfrom
runner-04-execute

Conversation

@marwen-abid

@marwen-abid marwen-abid commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

This PR adds the run package: the executor that walks a plan. Execute runs the steps in order and always sequentially, because two benchmarks that share a machine measure the sharing. It returns one result per step — ok, failed, skipped, or resumed — and it writes a completion sentinel into every timed leg.

What is in this PR

  • run.go — the walk, the step dispatch, and leg.json. The runner writes the sentinel into each timed --out dir, on success and on failure. invocation.json cannot do this job: the measured process writes it, so a killed process leaves none.
  • resume.goclassifyLegDir reads a leg's --out dir and returns absent, partial, failed-earlier, or complete. Only complete is skipped. Every other state is wiped and re-run. A bundle with no leg.json falls back to invocation.json plus driver.csv.
  • Keep-going semantics: a failed step marks its dependents skipped, transitively. FailFast stops the walk instead.
  • lock.go — an exclusive non-blocking flock on <BENCH_ROOT>/.campaign.lock refuses a second campaign at once. log.goNotef prints == [HH:MM:SS] msg, and OpenCampaignLog appends, so every session lands in one campaign.log.
  • dataset.goprepareDataset materializes into <root>.partial and renames onto <root> once whole. It handles packs-local, packs-gs, bsb-s3, and fixture.
  • source.goEnsureSrc keeps the build clone at $BENCH_ROOT/src and treats stellar-rpc as a black box: clone once, repoint origin, fetch, hard reset, then clean -qfd. No -x, so build caches survive.

Review focus

  • The sentinel trust rules in resume.go: schema version, leg id, and a present exit_code. ExitCode is a pointer, so an absent field does not read as exit 0.
  • Execute skips the tarball and publish steps. The wiring runs them after the final provenance writes.
  • runLeg creates the --out dir itself, so the sentinel lands even when the binary dies at once. A leg whose sentinel cannot be written is failed.

Verification

cd runner && go vet ./... && go test ./...

CI runs the same gate as .github/workflows/runner-go.yml.


Stack: part 4 of 8 · base runner-03-plan · replaces a slice of #16

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-02 00:46 UTC

@marwen-abid
marwen-abid marked this pull request as ready for review August 1, 2026 22:02
Copilot AI review requested due to automatic review settings August 1, 2026 22:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

There are correctness issues in command environment overriding and error reporting paths (flock failure handling and EnsureSrc clone decision) that should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds a new runner/internal/run package that executes a plan.Plan sequentially, supports resume behavior via per-leg leg.json sentinels, and includes supporting utilities for dataset preparation, logging, and exclusive campaign locking.

Changes:

  • Implemented a sequential plan executor (Execute) with keep-going / fail-fast semantics, per-leg sentinel writing, and a RunStep helper for epilogue steps.
  • Added resume classification (classifyLegDir) that trusts leg.json first and falls back to bash-era invocation.json + driver.csv.
  • Added operational helpers: dataset materialization with atomic .partial rename, unified campaign logging, BENCH_ROOT flock locking, and build-clone convergence + ref resolution.
File summaries
File Description
runner/internal/run/doc.go Package documentation for the new executor/resume module.
runner/internal/run/run.go Core executor, leg sentinel writing, command runner, and helper utilities.
runner/internal/run/run_test.go Comprehensive tests for executor semantics, sentinel contract, resume, lock, and logging.
runner/internal/run/resume.go Resume decision logic for classifying leg output directories.
runner/internal/run/log.go Campaign log utilities and note formatting.
runner/internal/run/lock.go Exclusive non-blocking BENCH_ROOT lock via flock.
runner/internal/run/dataset.go Dataset preparation with .partial staging and atomic rename.
runner/internal/run/dataset_test.go Tests for dataset preparation choreography and wipe behavior.
runner/internal/run/source.go Persistent build-clone convergence and ref resolution helpers.
runner/internal/run/source_test.go Tests for clone convergence behavior, cache retention, and ref resolution.
Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 3
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread runner/internal/run/lock.go
Comment thread runner/internal/run/source.go
Comment thread runner/internal/run/run.go
@marwen-abid
marwen-abid force-pushed the runner-04-execute branch 2 times, most recently from 60be19a to 85516f4 Compare August 2, 2026 00:29
Base automatically changed from runner-03-plan to main August 2, 2026 00:43
@marwen-abid

Copy link
Copy Markdown
Collaborator Author

@claude[agent] review

Sequential executor over the plan: leg.json completion sentinels in
every timed --out dir, --resume that trusts them, keep-going semantics,
dataset preparation, and the persistent stellar-rpc source/build clone.
@Claude

Claude AI commented Aug 2, 2026

Copy link
Copy Markdown

@claude[agent] review

Reviewed the current state at 85516f4. The three prior Copilot concerns are all resolved in the code:

  • lock.go:30-36AcquireLock only reports contention for EWOULDBLOCK; every other flock errno surfaces as lock <path>: <err>.
  • source.go:19-25EnsureSrc clones only on os.IsNotExist; any other stat error surfaces as inspect build clone at <src>: <err>.
  • run.go — step Env is appended after os.Environ(), and os/exec's dedupEnv keeps the last value, giving last-wins override.

go vet ./... is clean and go test ./... passes across config, plan, and run. No changes needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants