Skip to content

feat(xtest): read benchmark arms and provenance from the installation manifest - #670

Draft
dmihalcik-virtru wants to merge 1 commit into
DSPX-4372-s3-multi-armfrom
DSPX-4372-s3-read-manifest
Draft

dmihalcik-virtru wants to merge 1 commit into
DSPX-4372-s3-multi-armfrom
DSPX-4372-s3-read-manifest

Conversation

@dmihalcik-virtru

@dmihalcik-virtru dmihalcik-virtru commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Part 3c of the DSPX-4372-t1 plan, split out of #658. Stacked on #658. It completes stage 2's item 7: nobody assembles arms or provenance by hand for a prepared run.

What it does

When BENCH_INSTALLATION_MANIFEST names a manifest (otdf-local env exports it after otdf-sdk-mgr install benchmark: #642, #626), pytest --bench measures exactly the arms preparation installed, in request order. Explicit --bench-refs or --bench-baseline/--bench-candidate still win.

  • perf/installation.py reads the manifest contract (feat(xtest): otdf-sdk-mgr benchmark installation manifest contract #668) without importing otdf-sdk-mgr. It checks the schema version and refuses a manifest it cannot fully read. An unknown commit stays None, not "None".
  • Set but unreadable is a usage error at configure time, not a quiet fallback to whatever is in dist/.
  • Neutral preparation: every requested ref was one commit, so that SDK's cells skip with the reason, the summary says NEUTRAL rather than NOTHING MEASURED, and the run passes. A failed or partial preparation also skips, and the run fails as having measured nothing.
  • Provenance: the JSON artifact carries metadata.installation (each arm's alias, commit and dist tag) and metadata.preparation.
  • No platform for a neutral run. test_sdk_performance requests the payload, ciphertext, attribute and scratch fixtures only once a cell is known to be measurable.

The README's local-run sequence, which #642 adds, still names the arms explicitly. Making it rely on the manifest needs both #642 and this PR, so that edit happens where the tracks meet in stage 5.

Validation

  • uv run pytest test_bench_*.py: 326 passed, rebased onto main and chore(xtest): measure benchmark cells over a tuple of arms #669/feat(xtest): measure two to four benchmark arms against a reference #658's review fixes (conflicts in tests only, no change in intent). ruff, ruff format and pyright are clean.
  • Tests cover manifest parsing and refusal, a null commit, manifest-driven arms and budget, explicit options winning, neutral and failed preparations, another SDK's manifest being ignored, and the NEUTRAL summary.
  • Real pytest --bench --sdks go sessions with no platform: a neutral manifest exits 0 with the NEUTRAL headline and 7 cells skipped with the reason; a failed one exits 1 as NOTHING MEASURED.

Stack (DSPX-4372-t1 plan)

Two parallel tracks from main; they share no imports and meet in stage 5.

Part PR Base
2a: bare-repo and js install fixes #667 main
2b: manifest contract #668 2a
2c: install benchmark #626 2b
2d: otdf-local env exports #642 2c
3a: arms as a tuple #669 main
3b: two to four arms #658 3a
3c: pytest reads the manifest #670 (this PR) 3b
4: candidate-to-candidate conclusions #659 3c
5, 6: CI and report #660, #661 not yet moved onto these stacks

@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

dmihalcik-virtru added a commit that referenced this pull request Oct 23, 2026
…pted js installs (#667)

Part 2a of the [DSPX-4372-t1
plan](#657), split out of #626:
fixes the benchmark needs, which every `otdf-sdk-mgr` install benefits
from. **Based on `main`.**

### What it does
- **Bare repos refresh their branches.** `git clone --bare` records no
fetch refspec, so the `fetch --all --tags` before every checkout
refreshed tags only. Every branch stayed where the first clone found it,
and `install tip main` or `--platform main` built that stale commit.
`checkout.BARE_REFRESH` now fetches origin's branches into
`refs/remotes/origin/*` with `--prune`, for the SDK and platform bare
repos alike, and `checkout.upstream_ref` maps a branch name onto them.
Fetching into the bare repo's own `refs/heads/*` instead would move
branches that other worktrees have checked out, leaving their HEAD ahead
of their files, and would fail for good once upstream replaced a branch
`foo` with `foo/bar`.
- **Worktrees are detached** and reset to the refreshed ref. A branch
origin no longer has is refused rather than built from the clone's stale
copy, and `HEAD` means origin's default branch as of the clone; a
platform `HEAD` worktree used to reset to itself.
- **Go source rebuilds work a second time.** `checkout_go_from_platform`
reset an existing worktree to a `FETCH_HEAD` fetched into the bare repo,
which no worktree can see, so every rebuild after the first failed with
`pathspec 'FETCH_HEAD' did not match`. It now resets to the refreshed
ref, like the SDK and platform paths.
- **`install tip` reports checkout failures as errors.** A refused
branch, or a failed git command with git's stderr, now prints `Error: …`
and exits 1 for go, js and java, as the platform path already did,
instead of a traceback.
- **An interrupted js install leaves nothing behind.** #663 already
removes a partial `dist/` when npm fails. A Ctrl-C mid-install still
left a directory holding only `cli.sh`, which `install_release` then
skipped as installed from then on. Both js install paths now remove it
unless the install finished.
- `AGENTS.md` claimed the bare clone mapped `refs/heads/*`, which is
what hid the first bug. It and the code comments also said `git pull`
fails in a worktree; it merges origin's default branch into whatever is
checked out.

### Validation
- `uv run pytest` in `otdf-sdk-mgr`: 207 passed, and 321 at the top of
this track (#642). ruff, ruff format and pyright are clean.
- `test_checkout.py` runs real git against a local origin. Every shared
behaviour is parametrized over the SDK, go and platform entry points:
new and existing worktrees follow their branch, `HEAD` (including beside
a tag named `main`), a deleted branch is refused, tags and SHAs are
checked out and then reused, and a force-pushed `refs/pull/N/head` is
followed. This replaces the stubbed command-string test for platform PR
refs. SDK-only tests guard other worktrees, force pushes and `x` → `x/y`
renames; platform-only tests cover the helper-scripts worktree. A
`CliRunner` test covers the `install tip` error path.
- Mutation-checked: `symbolic-ref --short`, treating every non-origin
name as a deleted branch, dropping go's explicit PR-ref fetch or its
`+`, swallowing go's refusal, and dropping the `install tip` catch each
fail a test.
- Two new tests interrupt each js install path.
- `isolated_git` keeps those tests off the developer's git config, any
inherited `GIT_DIR`, and the network (`GIT_ALLOW_PROTOCOL=file`).

### Follow-ups
Older problems found while reviewing this PR are fixed in a stack of
drafts based on it: #673 (don't reset a directory that isn't a worktree)
→ #674 (branch names vs versions) → #675 (tags moved upstream) → #676
(`checkout go` / `--all`).

### Stack ([DSPX-4372-t1
plan](#657))

Two parallel tracks from `main`; they share no imports and meet in stage
5.

| Part | PR | Base |
|---|---|---|
| 2a: bare-repo and js install fixes | **#667 (this PR)** | `main` |
| 2b: manifest contract | #668 | 2a |
| 2c: `install benchmark` | #626 | 2b |
| 2d: `otdf-local env` exports | #642 | 2c |
| 3a: arms as a tuple | #669 | `main` |
| 3b: two to four arms | #658 | 3a |
| 3c: pytest reads the manifest | #670 | 3b |
| 4: candidate-to-candidate conclusions | #659 | 3c |
| 5, 6: CI and report | #660, #661 | not yet moved onto these stacks |



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
* SDK and platform worktrees now follow updated branches and pull
request refs, including after force-pushes. Missing branches produce a
clear error instead of a traceback.
* Interrupted JavaScript installations no longer leave partial
distribution files behind.

* **Documentation**
* Updated platform installation guidance for branch refreshes, detached
worktrees, and pull request refs.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@dmihalcik-virtru
dmihalcik-virtru force-pushed the DSPX-4372-s3-read-manifest branch from 402772a to 5e3653a Compare October 26, 2026 17:52
@github-actions

Copy link
Copy Markdown

X-Test Failure Report

dmihalcik-virtru added a commit that referenced this pull request Oct 28, 2026
)

Part 2b of the [DSPX-4372-t1
plan](#657), split out of #626.
**Based on `main`** (#667 has merged).

Adds a benchmark manifest: a JSON representation of the SDK(s) under
test, including how it was created or installed (built from what source,
what version, etc). This record that connects benchmark preparation to
measurement, reviewable on its own before anything writes or reads it.

- otdf-sdk-mgr install benchmark` (#626) writes it
- pytest reads it (#670) through a reader that imports nothing from this
package
- this JSON shape is the contract between them.

### What it does
- **`manifest.py`, schema `v1alpha1`.** `InstallationManifest` holds the
measured arms in request order, the platform and provisioning `otdfctl`
pins separately, an explicit status (`success`, `neutral`, `partial` or
`failed`) with a message, and `metadata.sdk`. Each `ArtifactIdentity`
records the requested alias, repository, commit, `dist/` directory,
install method and build settings.
- **Strict reads.** `from_dict` and `read()` validate rather than
coerce:
- The manifest and each arm must have exactly the keys `to_dict` writes;
missing and unknown keys are reported together.
- Only `platform`/`otdfctl` (no pin), `commit_sha` and `installed_path`
may be `null`. `metadata.sdk` must be non-empty.
- The status must match the contents: NEUTRAL has two or more arms and
nothing installed, SUCCESS has one or more arms that are all installed,
and any other status carries a `status_message`.
- Every failure raises `ManifestError`, a `ValueError`, and names the
JSON path (`sdk_arms[2].commit_sha`) and, for an enum, the allowed
values. `read()` adds the file path and wraps JSON decode errors, so one
`except ValueError` covers everything.
- **Invariants at construction.** `ArtifactIdentity` and
`ResolvedArtifact` refuse an unknown install method, an empty `dist/`
name, or a `commit_sha` that isn't a full lowercase 40- or 64-character
hex SHA. An unknown commit (a js release found on npm but not in git) is
`null`, never `"None"` or `""`. `write()` creates the parent directory
and replaces the file atomically, and removes its temp file if it fails.
- **Checks that run before anything builds.** `dist_slug_for` predicts
each arm's `dist/` directory, so `check_collision` can reject two refs
that would build into one directory at different commits (`sdk/feature`
and `feature`, or `Feature` and `feature` on a case-insensitive disk),
and `is_neutral_preparation` can tell when every ref names one build.
- **One slug function.** `refs.dist_slug` is shared by `install tip` and
`dist_slug_for`, so the predicted directory is the one make creates. It
rejects a ref that names no directory.

### Compatibility
- `install tip` output paths are unchanged, except that a ref naming no
directory (`--ref sdk/`) now exits 1 with an error instead of resetting
`src/` itself. That checkout and `make` agree on the `src/` name for
`sdk/` and `otdfctl/` refs is fixed separately, on `main`, in #681.
- Nothing reads the manifest yet. The xtest reader in #670 needs no
change: field names and locations are as before.

### Size
About 540 changed lines of non-test code, over the plan's ~400. About
235 of them are code. The rest are the docstrings that the JSON contract
needs, because its readers don't import this package. Review asked for
the strict reader and those docs in this PR, rather than shipping a
lenient reader and tightening it later.

### Validation
- `uv run pytest`: 342 passed. ruff, ruff format and pyright are clean
on ruff 0.17, with no `noqa`.
- `test_manifest.py` covers:
- a literal golden of the JSON shape, asserted in both directions, plus
round trips of every status by dataclass equality;
- every rejection: missing, unknown and mistyped keys, falsy `metadata`,
a non-object top level, and the status/contents rules;
- null commits, and collisions and neutrality with three arms of which
only some share a commit;
- aliases at one commit, case-only collisions, release slugs taken from
the resolved tag, and the `write()` cleanup.
- 20 hand-made mutations of the new checks each fail a test.
- `TestCmdTip` checks that `install tip` builds into the directory
`dist_slug_for` predicts, across SDKs and ref shapes.

### Stack ([DSPX-4372-t1
plan](#657))

Two parallel tracks from `main`; they share no imports and meet in stage
5.

| Part | PR | Base |
|---|---|---|
| 2a: bare-repo and js install fixes | #667 (merged) | `main` |
| 2b: manifest contract | **#668 (this PR)** | `main` |
| 2c: `install benchmark` | #626 | 2b |
| 2d: `otdf-local env` exports | #642 | 2c |
| 3a: arms as a tuple | #669 | `main` |
| 3b: two to four arms | #658 | 3a |
| 3c: pytest reads the manifest | #670 | 3b |
| 4: candidate-to-candidate conclusions | #659 | 3c |
| 5, 6: CI and report | #660, #661 | not yet moved onto these stacks |

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
… manifest

When $BENCH_INSTALLATION_MANIFEST names a manifest (`otdf-local env`
exports it after `otdf-sdk-mgr install benchmark`), pytest --bench
measures exactly the arms preparation installed, in request order, without
anyone restating them. Explicit --bench-refs or --bench-baseline/
--bench-candidate still win.

- perf/installation.py reads the manifest contract (schema v1alpha1)
  without importing otdf-sdk-mgr: the arms, the pins, the status and
  metadata.sdk. It checks the schema version and refuses a manifest it
  cannot fully read. An unknown commit stays None rather than "None".
- Set but unreadable is a usage error at configure time, not a quiet
  fallback to whatever is in dist/.
- A neutral preparation skips that SDK's cells with its reason; the run
  reports NEUTRAL instead of NOTHING MEASURED and does not fail. A failed
  or partial preparation also skips, and the run fails as having measured
  nothing.
- The JSON artifact carries metadata.installation (each arm's alias,
  commit and dist tag) and metadata.preparation.
- test_sdk_performance requests the payload, ciphertext, attribute and
  scratch fixtures only once a cell is known to be measurable, so a
  neutral preparation needs no running platform.

Part 3c of the DSPX-4372-t1 plan; completes stage 2's item 7.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@github-actions

Copy link
Copy Markdown

X-Test Failure Report

@sonarqubecloud

Copy link
Copy Markdown

dmihalcik-virtru added a commit that referenced this pull request Oct 30, 2026
… record benchmark arms (#626)

## TL;DR

Adds **`otdf-sdk-mgr install benchmark <sdk> <ref>...`**: one command
that resolves any number of SDK refs (releases, branches, SHAs, `pr:N`)
to commits, installs them side by side under `xtest/sdk/<sdk>/dist/`,
and writes a manifest recording exactly which commit is in each
directory. It makes comparing SDK variants a local, one-command job
instead of a CI dispatch or a hand-built set of `dist/` directories. It
is part 2c of the [DSPX-4372-t1
plan](#657), based on `main` now
that #667 and #668 (the manifest contract) have merged, and it is the
preparation step CI will share (#660).

## Why: quicker local testing of multiple SDK variants

Today, there are two ways to compare SDK builds:

- **Dispatch the xtest workflow with `run-benchmarks`.** That is about
45 minutes per SDK per attempt, and a mis-set input (`main` rather than
`main latest`) spends the whole run before it says so.
- **Reproduce locally**, per `xtest/perf/README.md`: "whatever two
builds you want, side by side under `sdk/<name>/dist/`". You install
each build yourself and need to know:
- whether each ref is a release or a source build, and the right `--ref`
for it;
- which directory it will land in: `pr:123` builds into
`refs--pull--123--head`, and `otdfctl/v0.24.0` and `v0.24.0` share
`dist/v0.24.0`;
  - how to provision `otdfctl` and the platform separately.

Nothing records which commit a directory holds. A `dist/main` left from
last week is measured, and labelled, as today's `main`.

With this PR:

```bash
otdf-sdk-mgr install benchmark go v0.38.0 main pr:123 --platform main --otdfctl v0.38.1
```

- **Any number of variants in one command.** Refs are kept in order, and
the first one is the reference. Stage 3 (#658) measures up to four.
- **Fails fast.** Every ref, and the `--otdfctl` pin, is resolved to a
full commit before anything builds, so a mistyped branch or tag costs a
lookup rather than minutes of compilation. Two refs that would share one
`dist/` directory at different commits are rejected before either
builds. (A full SHA is not checked to exist; see Follow-ups.)
- **Doesn't measure a commit against itself.** If every ref resolves to
one commit, preparation stops with `neutral` instead of benchmarking A
against A.
- **Cheap re-runs.** A source build records its commit in
`dist/<slug>/.version` and is reused only when that record matches.
Re-running after pushing to a branch rebuilds the branch, not
everything. The record is all it checks, so after a hand `make` of local
edits, delete `dist/<slug>` (the README says so).
- **A manifest instead of hand-built env vars.**
`xtest/sdk/benchmark.installed.json` records, for each arm, the
requested alias, repository, commit, `dist/` directory and install
method, plus the platform and `otdfctl` pins. `otdf-local env` exports
its path and the `otdfctl` pin (#642), and from #670 `pytest --bench`
reads the arms from it. Until then, name the arms with
`--bench-baseline`/`--bench-candidate`, as the README now says. CI runs
this same command (#660), so a local result and a CI result come from
the same preparation.

## What's in this PR

- **`install benchmark`** (`cli_install.py`, `bench_prepare.py`)
resolves, checks for collisions, installs releases via `install_release`
and source refs via `cmd_tip`, then installs the platform and the
`otdfctl` pin.
- **Separate provisioning pin.** `--otdfctl` is recorded apart from the
measured arms. It joins the collision check only when it shares the
arms' `dist/` tree, which is only the case for Go.
- **Collision and neutral checks** run before anything builds. They,
`dist_slug`, and the manifest's data model are in #668; this PR is the
command that uses them.
- **Manifest** (schema `v1alpha1`, defined in #668). The status is
`success`, `neutral`, `partial` or `failed`, with a `status_message`:
- `failed` means nothing was installed; `partial` means some installs
completed but not all.
- The manifest is written on every exit path, an exception or Ctrl-C
included.
  - The exit code is 0 for `success` or `neutral`, 1 otherwise.

**Size.** About 640 non-test lines, over the plan's ~400-line budget.
About 315 of them are code; the rest are docstrings, comments and the
README section. Resolution, installation and the manifest's exit paths
are one command: a PR that resolved without installing would ship a
command that does nothing. Two thirds of the diff is tests.

**Since the first review:**
- Java release arms resolved nothing, because the wrong tag infix was
used. Fixed.
- The manifest now has one write site: `status` is required, reads check
`schema_version`, and writes create the parent directory and replace the
file atomically.
- JS release arms land in `dist/vX.Y.Z`.
- `platform` is rejected as an SDK.
- The platform pin records the directory it was installed to.
- A first-arm failure is now `failed`, not `partial`.

**Since review of #668:**
- **Abbreviated commits are rejected.** A ref that resolves to a commit
that isn't a full SHA (in practice, an abbreviated SHA that no branch,
tag or PR head points at) now fails before anything installs: "give the
full commit SHA for …", with a `failed` manifest and exit code 1.
Before, `abc1234` and its own full SHA weren't recognised as one build,
so the commit was built twice and reported as `success`. Reusing a
`dist/` directory and detecting a moved ref now compare full SHAs
exactly.
- An empty ref list fails instead of writing a `success` manifest with
no arms. Every manifest written sets `metadata.sdk` and passes #668's
strict reader.
- Platform and `otdfctl` install failures are caught narrowly
(`PlatformInstallError`, `BenchPrepareError`, `OSError`). Anything else
aborts with its traceback, and the manifest records `partial`
"Preparation aborted", as arm installs already did. A platform whose
`.version` has no full commit is `partial`.

**Since the 2026-10-29 review:**
- An arm whose stale `dist/` cannot be removed (`OSError`) is now
`failed`/`partial` with the error, like the `otdfctl` pin, rather than
"Preparation aborted".
- If the *first* manifest write fails, the CLI no longer prints
"Manifest: …" pointing at an earlier run's file. `prepare_benchmark`
raises `BenchPrepareError` for that case. `RuntimeError` is no longer
caught around the whole run: `xtest/sdk` is located up front,
`--manifest` or not.
- The CLI tests now go through Typer (`CliRunner().invoke(install_app,
…)`), so a lost registration or renamed option fails them. They also
assert a clean `typer.Exit`, since `CliRunner` reports an uncaught
exception as exit 1 too.
- New tests:
- npm-only js releases: one version is `neutral` ("whose commit is
unknown"), two versions are not;
  - a dist of unknown provenance is emptied before rebuilding;
  - an arm `OSError`;
  - a failed final write.
- Comments: two were wrong and are now corrected:
- the repeated-builds example `v0.38.0 main` is in fact neutral, so it
is now `v0.38.0 main pr:123`;
- the `otdfctl` pin "never provisions the fixtures it is measured
against" contradicted `check_collision`.

Stale ones fixed: js tags are `sdk-vX.Y.Z`; the go fallback affects
`v0.30.x`, not "older tags"; the java Makefile sources
`java/<slug>.env`. The `# type: ignore` is gone, and the CLI help
example now uses `v0.38.0`/`v0.38.1`.
- README:
- says reuse trusts `.version`, and that a release `dist/` is reused as
is;
  - says a full SHA is not checked;
  - says `pytest --bench` doesn't read the manifest yet.

**Validation** (on this commit):
- `uv run pytest` 404 passed; ruff check, ruff format and pyright are
clean, with no `noqa` or `type: ignore`.
- Each new test was checked against a deliberate break of the code it
covers: the unregistered command, the missing `rmtree`, the uncaught arm
`OSError`, the unwrapped first write and the neutral message all fail at
least one test.
- Live, from an empty `xtest/sdk/go`:
- `install benchmark go v0.38.0 main --otdfctl v0.38.1` gave `success`,
exit 0. The arms were `v0.38.0` at `308002c2` (release) and `main` at
`ef874839` (source). `dist/main/.version` matches `git -C src/main
rev-parse HEAD`. The pin was `v0.38.1` at `99b51145`.
- The same command re-run gave `success` in 1.4 s, with nothing rebuilt.
- `install benchmark go 0e4b0d1 main` gave `failed`, "give the full
commit SHA for 0e4b0d1", with exit 1 and no arms.
- `--manifest` under a regular file gave "Error: cannot write the
manifest …", exit 1, and no "Manifest:" line.
- The platform pin (`--platform`) was not exercised live on this commit;
its last live run predates the review fixes.

## Stack ([DSPX-4372-t1
plan](#657))

Two parallel tracks from `main`; they share no imports and meet in stage
5.

| Part | PR | Base |
|---|---|---|
| 2a: bare-repo and js install fixes | #667 (merged) | `main` |
| 2b: manifest contract | #668 (merged) | `main` |
| 2c: `install benchmark` | **#626 (this PR)** | `main` |
| 2d: `otdf-local env` exports | #642 | 2c |
| 3a: arms as a tuple | #669 | `main` |
| 3b: two to four arms | #658 | 3a |
| 3c: pytest reads the manifest | #670 | 3b |
| 4: candidate-to-candidate conclusions | #659 | 3c |
| 5, 6: CI and report | #660, #661 | not yet moved onto these stacks |

#667 and #668 were split out of this PR so the fixes and the contract
can be reviewed on their own. Split out of an earlier version so they
could land first: #638, #639, #640 and #641, all merged. #647, the
golden-key configuration CI needs once `otdf-local` starts the platform,
is still open.

**Related, independent of this stack:** #684 makes go release installs
put a real `otdfctl` binary in `dist/`. Before it, a go release arm ran
`go run` on every call (about 370 ms wall and 1 s CPU, against 54 ms for
the binary), while source-built candidates ran a binary. That made
release baselines look slower than they are.

## Follow-ups

None of these blocks merge. All but the "Re-run" message are now noted
in the code or the README.

- **Go repository URL.** Go arms are all recorded under the monorepo
URL. The monorepo carries `otdfctl/v0.10.0`–`v0.29.0` at the archived
repo's commits, but not `v0.30.x`, which `resolve` answers from the
archived repo. That makes `v0.30.x` arms the ones recorded with a commit
the monorepo lacks. The fix needs `resolve` to report the repo it
matched.
- **Fallback on any error.** Go resolution falls back to the archived
otdfctl repo whenever the monorepo lookup fails, so a transient error
can resolve `main` or `pr:N` against the wrong repo. A source arm then
fails the read-back, but the error says "the ref moved".
- **Unhelpful "Re-run" advice.** "The ref moved… Re-run" is wrong for a
worktree that `is_mutable_ref` treats as fixed (full SHAs; web-sdk
`release/x.y.z` branches). Checkout never resets those, so a re-run
fails the same way. The message should name `src/<slug>` and say to
delete it.
- **Reuse trusts `.version`.** The SDK Makefiles rebuild into
`dist/<slug>` without touching `.version`, and `_built_commit` doesn't
check for a dirty worktree. The README now tells users to delete the
directory; a code check (`git status --porcelain`, a binary hash) would
be better.
- **Release dists reused as is.** A release arm adopts an existing
`dist/<tag>` (`install_release`'s skip), including a half-finished
install.
- **Full SHAs unverified.** A typo fails at checkout after the arms
before it have built, giving `partial`.
- **`--platform` not resolved up front.** It is not resolved before the
arms build: `--platform latest` becomes `service/vlatest` and fails
after every build. The release/source label for the pin is also a
`startswith("v")` guess; both installers build from source.

**Downstream:** #642 is rebased onto this commit with no conflicts (it
touches only `otdf-local` and `xtest/perf/README.md`). Its local-run
example uses `--otdfctl v0.30.0`, one of the `v0.30.x` tags above;
`v0.38.1` would avoid that.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **New Features**
* Added an `install benchmark` command to prepare SDK benchmarks from
ordered references, with optional platform and CLI tool artifacts.
* Benchmark preparation validates references and installation conflicts,
reuses matching builds, and records progress and results in a manifest.
Results distinguish successful, neutral, partial, and failed
preparations.
* **Documentation**
* Added guidance for preparing benchmarks and troubleshooting common
test failures.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
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.

1 participant