feat: build Boris and Oliver helpers from main source and bundle into app - #218
Conversation
|
SummaryCoverage spans successful and failure-recovery helper builds, app packaging and signing, helper discovery and priority rules, recipe rendering and publishing fallbacks, and security and concurrency edge cases. Overall behavior is healthy across normal and degraded conditions, with platform-specific macOS interface behavior not fully exercised. Merge with caution — overlapping helper builds can publish a mismatched set of engines while still reporting success, creating a medium-severity release-integrity risk. The remaining findings do not indicate additional merge-blocking behavior. Tests run by ItoTip Reply with @itoqa to send us feedback on this test run. |
| return 1 | ||
| fi | ||
|
|
||
| tmp="$(mktemp -d)" |
There was a problem hiding this comment.
Concurrent builds create mismatched helpers
What failed: Starting two helper builds together left Oliver from one build and Boris containing slices from both builds. The expected result was one complete, internally consistent helper pair with no cross-build mixing.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: A release built while two helper jobs overlap can contain a mismatched helper pair. The app may fail or behave incorrectly even though the build reports success.
- Steps to Reproduce:
- Prepare two valid Boris and Oliver source trees with distinct provenance markers.
- Start two
make helperscommands at the same time, each using one source tree and the samedist/helpersdestination. - Wait for both commands to finish, then inspect the Boris and Oliver output files.
- Compare the provenance markers and confirm that the pair contains output from both source trees even though both commands returned success.
- Stub / mock content: The test used deterministic local Zig and lipo stand-ins to mark each build's source and timing; no application network requests or production data were used.
- Code Analysis: Scripts/helpers.sh:107-117 loops over
oliverandborisand callsbuild_helperindependently for each invocation. Inbuild_helperat lines 73-82, the compiler output uses a privatemktempdirectory, butlipowrites the combined binary to the shared$HELPERS/$name.tmppath and then moves it to the shared$HELPERS/$namepath. The host fallback at lines 91-98 copies directly to the same shared final path, while cleanup at lines 99-103 can remove that path. There is no lock around the invocation and no invocation-specific staging directory or pair-level publication. Makefile:33-35 exposes this script directly throughmake helpers, so two independent commands share these paths. The PR diff specifically introduced thehelperstarget and the changedbuild_helperpublication flow in Scripts/helpers.sh, making the changed publication behavior the direct cause. A targeted fix is to hold a lock for the whole helper build, or stage both helper outputs under a unique directory and replace the destination set only after both tools finish. - Why this is likely a bug: The failure is not only a test artifact: the script's shared destination names are real paths consumed by later app-bundling and helper-resolution flows, and separate invocations can write them concurrently. The recorded run reproduced the organic overlap condition with two successful commands and found mixed A/B provenance in the final outputs, with no temporary files left to reveal the problem. Because the final files remain executable and the command exits 0, downstream users can receive a wrong helper pair without a clear error. Serializing the script or atomically publishing an invocation's complete pair would prevent the specific corruption without requiring a broad redesign.
Relevant code
Scripts/helpers.sh:73-85
tmp="$(mktemp -d)"
...
lipo -create "$arm/bin/$name" "$x86/bin/$name" -output "$HELPERS/$name.tmp" ...
mv "$HELPERS/$name.tmp" "$HELPERS/$name"Scripts/helpers.sh:90-104
tmp="$(mktemp -d)"
if (cd "$src" && "$ZIG" build --prefix "$tmp" ...); then
cp "$tmp/bin/$name" "$HELPERS/$name"
...
rm -f "$HELPERS/$name"Scripts/helpers.sh:107-117
for tool in oliver boris; do
...
if src="$(find_or_fetch_source "$tool" "$src_override")"; then
build_helper "$tool" "$src"
fi
doneMakefile:33-35
helpers:
@$(BASH) Scripts/helpers.sh "$(DIST)"Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Concurrent builds create mismatched helpers**
**What failed:** Starting two helper builds together left Oliver from one build and Boris containing slices from both builds. The expected result was one complete, internally consistent helper pair with no cross-build mixing.
- **Impact:** A release built while two helper jobs overlap can contain a mismatched helper pair. The app may fail or behave incorrectly even though the build reports success.
- **Steps to reproduce:**
1. Prepare two valid Boris and Oliver source trees with distinct provenance markers.
2. Start two `make helpers` commands at the same time, each using one source tree and the same `dist/helpers` destination.
3. Wait for both commands to finish, then inspect the Boris and Oliver output files.
4. Compare the provenance markers and confirm that the pair contains output from both source trees even though both commands returned success.
- **Stub / mock content:** The test used deterministic local Zig and lipo stand-ins to mark each build's source and timing; no application network requests or production data were used.
- **Code analysis:** Scripts/helpers.sh:107-117 loops over `oliver` and `boris` and calls `build_helper` independently for each invocation. In `build_helper` at lines 73-82, the compiler output uses a private `mktemp` directory, but `lipo` writes the combined binary to the shared `$HELPERS/$name.tmp` path and then moves it to the shared `$HELPERS/$name` path. The host fallback at lines 91-98 copies directly to the same shared final path, while cleanup at lines 99-103 can remove that path. There is no lock around the invocation and no invocation-specific staging directory or pair-level publication. Makefile:33-35 exposes this script directly through `make helpers`, so two independent commands share these paths. The PR diff specifically introduced the `helpers` target and the changed `build_helper` publication flow in Scripts/helpers.sh, making the changed publication behavior the direct cause. A targeted fix is to hold a lock for the whole helper build, or stage both helper outputs under a unique directory and replace the destination set only after both tools finish.
- **Why this is likely a bug:** The failure is not only a test artifact: the script's shared destination names are real paths consumed by later app-bundling and helper-resolution flows, and separate invocations can write them concurrently. The recorded run reproduced the organic overlap condition with two successful commands and found mixed A/B provenance in the final outputs, with no temporary files left to reveal the problem. Because the final files remain executable and the command exits 0, downstream users can receive a wrong helper pair without a clear error. Serializing the script or atomically publishing an invocation's complete pair would prevent the specific corruption without requiring a broad redesign.
**Relevant code:**
`Scripts/helpers.sh:73-85`
~~~bash
tmp="$(mktemp -d)"
...
lipo -create "$arm/bin/$name" "$x86/bin/$name" -output "$HELPERS/$name.tmp" ...
mv "$HELPERS/$name.tmp" "$HELPERS/$name"
~~~
`Scripts/helpers.sh:90-104`
~~~bash
tmp="$(mktemp -d)"
if (cd "$src" && "$ZIG" build --prefix "$tmp" ...); then
cp "$tmp/bin/$name" "$HELPERS/$name"
...
rm -f "$HELPERS/$name"
~~~
`Scripts/helpers.sh:107-117`
~~~bash
for tool in oliver boris; do
...
if src="$(find_or_fetch_source "$tool" "$src_override")"; then
build_helper "$tool" "$src"
fi
done
~~~
`Makefile:33-35`
~~~make
helpers:
@$(BASH) Scripts/helpers.sh "$(DIST)"
~~~
Summary
zig build -Doptimize=ReleaseSafeintoContents/Helpersfor bothmake appand Xcode builds.OliverLocatorandBorisLocatorto resolve built helper binaries during development, CLI runs (banal doctor), and test execution.BorisLocatorTestsandOliverClientTests.Verification
swift testpasses 100% (all suites).lipo -infoverifies universal fat binaries.make appsuccessfully bundles and signs helper binaries intoContents/Helpers.make smokepasses end-to-end.swift run banal doctorverifies Boris and Oliver detection.