Skip to content

fix(ci) [BRNS-DESK-033]: the artifact paths point where cargo actually writes - #65

Open
stefan-ssv-labs wants to merge 4 commits into
devfrom
fix/desk-033-034-artifact-paths-and-ledger-net
Open

fix(ci) [BRNS-DESK-033]: the artifact paths point where cargo actually writes#65
stefan-ssv-labs wants to merge 4 commits into
devfrom
fix/desk-033-034-artifact-paths-and-ledger-net

Conversation

@stefan-ssv-labs

@stefan-ssv-labs stefan-ssv-labs commented Aug 13, 2026

Copy link
Copy Markdown

The clean-room rebuild moved the cargo workspace root to the repo root, and the release workflows never learned it.

Ticket: BRNS-DESK-033 on the shared findings board.

This PR was two tickets and is now one. It also carried BRNS-DESK-034, the ledger-net separator fix — but #62 landed the identical fix first, in commit e87d525, written by the person who reported the bug. Dev's version is the better one and this branch now takes it verbatim. See The ticket that left at the bottom.

The defect

./Cargo.toml declares the workspace and lists the seven engine crates plus src-tauri as members, so every build output lands in repo-root target/. src-tauri/target/ does not exist and never has in this tree. Nine paths across release.yml and ci.yml still carried the old prefix, inherited from the original single-crate layout.

Requirement Change
macOS smoke-launch reads the built app release.yml:150
Notarize step finds the DMG release.yml:193
Final verify reads app + DMG release.yml:206-207
macOS release artifact uploads release.yml:260
Windows verify reads the release dir release.yml:299 ($exe at :301 derives from it)
Windows NSIS + MSI artifacts upload release.yml:335-336
Label-gated test DMG uploads ci.yml:323
The manual window check names a real directory scripts/eval/specs/01-window.yaml:8

dev-release.yml already used repo-root target/, and its runs publish real signed, notarized, installable artifacts — so every new path here is copied from a proven-correct reference rather than derived a second time. It is deliberately left byte-identical.

The last row is one line of prose beyond the ticket's stated .github/ surface: a manual-test note telling a human to watch src-tauri/target for HMR-retriggering writes. Same wrong fact, last tracked file repeating it. scripts/cef-dev-bundle.mjs:29 is left alone — it names the path in order to say it does not exist, which is correct.

Out of scope and correct as they stand: the Recall source paths under src-tauri/recall/ and src-tauri/binaries/, and cargo fmt/clippy --manifest-path src-tauri/Cargo.toml, which select the shell crate on purpose.

Reproduction

ls src-tauri/target on a clean checkout: no such directory. Every path in the table resolved into it. The macOS find … -print -quit sites fail quietly (empty string, step still exits 0); the upload-artifact sites fail loudly via if-no-files-found: error. That asymmetry is how it survived: the noisy site was never reached, because the quiet one had already stopped verifying anything.

Evidence

  • The label-gated Build macOS job ran and uploaded a real universal DMG from the corrected pathtest-build-macos, 99,861,209 bytes, with if-no-files-found: error armed. That job had never executed once in this tree before; its first run is green. This is the live proof the fix works, and it is the only end-to-end proof obtainable before a tag.
  • actionlint .github/workflows/*.yml — clean.
  • Zero src-tauri/target references remain in any tracked file except the comment stating it does not exist.
  • npm run check (svelte-check), lint:size, lint:imports, lint:manifests, lint:css-vars, npm run build — all pass. No manifest churn.
  • npm run eval -- --no-agent needs node scripts/dev/recall/stub-bundle-artifacts.mjs first, which is exactly what CI's Rust and eval jobs do. Without it, five cargo checks fail on any checkout of this tree: the declared externalBin binaries/recall-node is absent locally and tauri_build::build() hard-fails on a declared binary it cannot find. Unrelated to this branch, which contains no Rust and no tauri config.

What is not proven here

The end-to-end proof is a tagged release, which this PR cannot produce. release.yml fires on v* only, and no v* tag yet contains the workspace-root move (git tag --contains 2e0e247 is empty) — which is precisely why nothing has caught this. The eight release.yml paths are verified by inspection plus the fact that they are now byte-identical in scheme to dev-release.yml, whose runs publish real installers. The ci.yml path is verified by execution, above.

Shipping impact

No runtime code changes — nothing a user installs differs. The impact is that the next v* tag can produce a release at all. Before this, both build jobs would have died at their verify steps or at if-no-files-found: error, and publish (needs: [build, build-windows]) would never have run: a tag with no assets.

Regression provenance

Not a regression. 2e0e247 ("sync: 2026-08-06 tree snapshot") both added the root workspace Cargo.toml and imported these workflows verbatim from the original repo, where cargo was rooted at src-tauri/ and the paths were correct. The move made them dead without touching them. dev-release.yml was born later and got the correct paths; these two never did. Found by the PR #47 parity review, not by CI.

The ticket that left

BRNS-DESK-034 — the ledger-net audit comparing join()-built native paths against forward-slash literals — was fixed twice, independently, on the same day.

#62 (fix/windows-build, merged into dev as 17a3b21) carries commit e87d525, which does the identical thing this branch did: import relative and sep from node:path, and replace the process.cwd().length + 1 slice with relative(process.cwd(), p).split(sep).join("/"). Amir reported that bug and then fixed it inside the Windows lane that motivated it.

Dev's version is better and this branch now holds it byte-for-byte: it names the normalization repoPath() rather than inlining it in the .map, and it carries the explanation of why a phantom Windows-only failure is worse than no net at all. Merging dev conflicted on exactly that file and on nothing else; ci.yml merged clean, because #62 restructured the Rust job above the Build macOS upload, moving it without touching it.

One thing did not change with the merge: #62 does not make that test run on Windows. Its new Windows leg is the Rust matrix. The frontend job — the only npm test, so the only vitest — is still ubuntu-latest. So the normalization is correct-by-construction and locally mutation-checked in both directions, but its first real Windows execution still belongs to future work.

Follow-ups filed, not folded in

Each has its own row rather than widening an XS ticket. #62 changed the standing of the first two and neither is closed by it:

  1. [BRNS-DESK-035] Swatinem/rust-cache is given workspaces: src-tauri — the same stale assumption, in cache config rather than artifact paths. The action resolves its cached target dir as <workspaces>/target, so no compiled artifact is ever saved or restored and every Rust leg is a cold full rebuild. Windows: make dev build, compile and test on Windows #62 makes this cost more, not less: the Rust job is now a two-OS matrix, so there are two cold rebuilds per run.
  2. [BRNS-DESK-036] cargo fmt --check and clippy are scoped --manifest-path src-tauri/Cargo.toml, which selects the shell crate alone — all seven engine crates go unformatted and unlinted. Windows: make dev build, compile and test on Windows #62 explicitly declines to widen it, for the stated reason that the base is ~160 fmt-dirty sites and widening would land red.
  3. [BRNS-DESK-037] The ledger audit cannot see a builder written the idiomatic way. BUILDS_A_BLOCK matches only a marker in a string literal, but handover.ts:37 builds HANDOVER_PREFIX from the shared CONTEXT_MARKER constant — a live, wire-bound [context] block, absent from BUILDERS, invisible to the net. Untouched by Windows: make dev build, compile and test on Windows #62, which fixed how the audit compares paths, not what it can see. Same row covers code()'s block-comment stripper treating the /* inside import.meta.glob("./apps/*/app.json") as a comment opener, silently deleting a span of main.ts.

Merge method: squash (do not merge-commit or rebase-merge).

stefan-ssv-labs and others added 3 commits August 13, 2026 00:35
…ESK-033)

The rebuilt tree's cargo workspace is rooted at the repo root — `./Cargo.toml`
lists the seven engine crates and `src-tauri` as members — so every build
output lands in repo-root `target/`. Nine paths across `release.yml` and
`ci.yml` still carried `src-tauri/target/`, the prefix that was correct back
when `src-tauri` was the only crate and its manifest was the workspace root.

Nothing caught it because neither job runs by default. `Build macOS` is
label-gated on `build-macos`/`build-test`, and `release.yml` fires on a `v*`
tag, which the rebuilt tree has never cut. So the first release would have
found no app to sign, no app to notarize, and no DMG to upload — and
`if-no-files-found: error` is the only reason that surfaces as a red build
instead of a published release with nothing in it.

`dev-release.yml` already reads from repo-root `target/`, and its runs publish
real signed, notarized, installable artifacts. That makes it a proven-correct
reference rather than a second guess, so every new path here is the one it
already uses. It is deliberately left byte-identical.

Out of scope, and correct as they stand: `Swatinem/rust-cache`'s
`workspaces: src-tauri` (the reference workflow sets it too, so it is not a
differentiator), the Recall source paths under `src-tauri/recall/` and
`src-tauri/binaries/`, and `cargo fmt`/`clippy --manifest-path
src-tauri/Cargo.toml`, which select the shell crate on purpose.

Verified with `actionlint` and by grepping the workflow directory for any
surviving `src-tauri/target` reference. The end-to-end proof is a tagged
release, which this PR cannot produce; a `build-test` label on it exercises
the `ci.yml` half against a real universal build.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-DESK-034)

The ledger's net ends with an audit that walks `src/` and asserts the set of
files building a `[context]` block is exactly the set `BUILDERS` names. The
walk builds each path with `join()`, which emits the platform separator, and
then trimmed the working directory off with a raw slice — so on Windows it
compared `src\layout\panes\session\turn-context.ts` against the forward-slash
literals the Set is written in. Every entry unmatched, the whole audit red,
for a reason that has nothing to do with the ledger.

Normalize the discovered side only: `relative(process.cwd(), path)` then
`.split(sep).join("/")`. `BUILDERS` stays byte-for-byte what it was — it is
written in the repo's own notation, and the notation is the thing worth
keeping stable. No `os` gate: the assertion is meant to hold everywhere, and
skipping it on a platform is how it would rot there.

Verified both directions, which is what makes this a normalization and not a
way to make the test agree with itself. Dropping an unlisted `[context]`
builder into `src/` fails the audit and names it — in forward-slash form,
which is the fix doing its job. Removing an entry from `BUILDERS` fails it
too. Full `npm test` is green: 1668 vitest tests over 140 files, plus the
drive and eval suites.

The Windows run itself is still unproven — there is no Windows job in CI at
all, so this is correct-by-construction plus the local two-way check, and the
first real Windows execution belongs to the parity work that introduces one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…NS-DESK-033)

01-window's manual variant told the reader to watch for `src-tauri/target`
writes retriggering HMR. That directory has never existed in the rebuilt tree —
the workspace root is the repo root, so cargo writes to `target/`. The concern
itself is real and already handled (vite.config.ts ignores both `**/src-tauri/**`
and `**/target/**`); only the name was stale.

This is the same wrong fact the workflow paths carried, in the last tracked file
still repeating it. `scripts/cef-dev-bundle.mjs:29` is left alone: it names
`src-tauri/target` in order to say it does not exist, which is correct.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dev's

BRNS-DESK-034 was fixed twice, independently: here, and in Amir's Windows PR
(#62, commit e87d525), which reported the bug in the first place. Both do the
identical thing — import relative+sep from node:path and normalize each walked
path through relative(process.cwd(), p).split(sep).join("/") in place of the
process.cwd().length + 1 slice.

Dev's version wins outright. It names the normalization repoPath() instead of
inlining it in the .map, and it carries the explanation of why a phantom
Windows-only failure is worse than no net at all. This branch's copy of that
file is now byte-identical to dev, so the ledger-net half of this PR is empty
and what remains is BRNS-DESK-033 alone.

.github/workflows/ci.yml merged clean: #62 restructured the Rust job above the
Build macOS upload, which moved but did not touch it.
@stefan-ssv-labs stefan-ssv-labs changed the title fix: the artifact paths point where cargo actually writes, and the builder audit compares paths not separators (BRNS-DESK-033, BRNS-DESK-034) fix(ci) [BRNS-DESK-033]: the artifact paths point where cargo actually writes Aug 13, 2026

@stefan-ssv-labs stefan-ssv-labs left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ review-pr (self-review, cannot approve own PR): Clean — BRNS-DESK-033 requirements are met.

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