Skip to content

fix(artifact): put binary macro before merge=ours in generated .gitattributes#916

Merged
DeusData merged 1 commit into
DeusData:mainfrom
alex-plosceac:fix/artifact-gitattributes-merge-ours
Jul 9, 2026
Merged

fix(artifact): put binary macro before merge=ours in generated .gitattributes#916
DeusData merged 1 commit into
DeusData:mainfrom
alex-plosceac:fix/artifact-gitattributes-merge-ours

Conversation

@alex-plosceac

@alex-plosceac alex-plosceac commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the attribute ordering in the auto-generated .codebase-memory/.gitattributes so the merge=ours conflict prevention it exists for actually engages.

The writer currently emits:

graph.db.zst merge=ours binary

gitattributes apply left to right, and the binary macro expands to -diff -merge -text, so the trailing macro unsets the merge=ours that precedes it:

$ git check-attr merge -- .codebase-memory/graph.db.zst
.codebase-memory/graph.db.zst: merge: unset

Concurrent artifact refreshes on two branches therefore hit a binary merge conflict instead of auto-resolving — the exact failure the generated file's own comment says it prevents. Reordering to graph.db.zst binary merge=ours fixes it:

$ git check-attr merge -- .codebase-memory/graph.db.zst
.codebase-memory/graph.db.zst: merge: ours

-diff and -text from the macro remain in effect.

How this was found: while investigating the v0.8.1 layout memory blow-up on a 155k-node repository (17.6GB peak RSS for a 50k-node layout; full evidence, lldb SIGKILL capture, and measurements confirming main is fixed are in #917), we committed the graph artifact for team bootstrap and noticed check-attr reporting merge: unset on the freshly generated file.

Verification, reproduce-first: extended artifact_gitattributes_created to pin the ordering (asserts the fixed order present, broken order absent). It fails on unmodified main (5914 passed / 1 failed) and passes with this change (5915 / 0, ASan+UBSan build).

Scope notes:

  • The writer is create-only (O_EXCL), so existing repos keep their old file; affected users need a one-line manual edit. Happy to add a fix-up pass if preferred.
  • Even with correct ordering, merge=ours needs merge.ours.driver=true in the clone's git config; the tool sets it where it runs, but teammates' fresh clones won't have it and degrade to a normal conflict. Possibly worth a docs note (out of scope here).

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test) — 5915 passed, 0 failed, 1 skipped
  • Lint passes (make -f Makefile.cbm lint-ci) — cppcheck, clang-format, NOLINT whitelist all green
  • New behavior is covered by a test (reproduce-first for bug fixes) — artifact_gitattributes_created extended; RED on main, GREEN with fix

…tributes

gitattributes apply left to right and the binary macro expands to
-diff -merge -text, so the generated line

    graph.db.zst merge=ours binary

leaves the merge attribute UNSET (git check-attr merge reports
"unset"), and concurrent artifact refreshes produce a binary merge
conflict instead of auto-resolving -- the exact failure this file is
generated to prevent. Reordering to

    graph.db.zst binary merge=ours

makes check-attr report merge: ours while keeping -diff/-text intact.

Verified on a real repository: before the reorder git check-attr merge
-- .codebase-memory/graph.db.zst printed "unset"; after, "ours".

Extends artifact_gitattributes_created to pin the ordering so the
regression cannot return.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Alex <plosceac.alexandre@gmail.com>
@alex-plosceac alex-plosceac requested a review from DeusData as a code owner July 6, 2026 15:24
@DeusData DeusData added bug Something isn't working stability/performance Server crashes, OOM, hangs, high CPU/memory priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 7, 2026
@DeusData

DeusData commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the focused artifact attributes fix. Triage: artifact workflow bug, normal priority.

Review should confirm the generated attributes order does what the comment promises across Git versions and that no other artifact file behavior changes. The git check-attr proof is exactly the right evidence to include.

@DeusData DeusData added this to the 0.9.1-rc milestone Jul 8, 2026
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Verified and correct — gitattributes apply left to right, the binary macro expands to -diff -merge -text, so the trailing macro was unsetting the very merge=ours the file exists to provide. The git check-attr evidence and the order-pinning regression test are exactly what we want. Clean scope, green CI.

One gap to consider (fine as a follow-up if you prefer): ensure_gitattributes is create-only (O_CREAT|O_EXCL), so every repo that already generated the file keeps the broken merge=ours binary line forever — the fix only reaches fresh checkouts. Since the file is fully auto-generated by us, a tiny self-heal would close that: when the file exists and its content contains the exact known-broken generated line, rewrite it with the corrected one (and leave any user-modified file untouched). If you'd rather keep this PR minimal, say so and we'll merge as-is and track the self-heal separately.

@DeusData DeusData merged commit 3ea4bb0 into DeusData:main Jul 9, 2026
19 checks passed
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Merged as-is — thank you! The self-heal for already-generated files is tracked as a follow-up; feel free to pick it up if you fancy it, otherwise we'll get to it. Great catch on the macro ordering.

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

Labels

bug Something isn't working priority/normal Standard review queue; useful PR with ordinary maintainer urgency. stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants