Artifact [95b0fcc6b4]

Artifact 95b0fcc6b4c5773b9505117b494cda18d760de456335c2e00617bc5cc6993d4a:

• File repo_sync_test.go — part of check-in [18bfbc166c] at 2026-07-27 23:31:22 on branch trunk — fix(sync): derive the project code, and write artifact visibility inside the storing transaction (#204)

  * fix(sync): derive the project code from the repo instead of panicking (#199)

  Supplying User/Password made SyncOpts.ProjectCode mandatory in practice: buildLoginCard passed the caller's raw value straight to computeLogin, which asserts it is non-empty, so an authenticated Sync on a repo whose project code the caller had not repeated took the process down. Nothing in the public API signalled that a password made the field required, and a server embedding the library died with it.

  buildRequest already fell back to the repo's stored project-code for the push/pull cards; that fallback was local to one function, which is exactly how the login path drifted out of step with it. Resolve the code once when the session is built — caller value first, the repo's own config second — and let buildRequest and buildLoginCard both read it. A code that genuinely cannot be determined now returns a descriptive error from the site that needs it rather than panicking.

  Sync's deferred result-not-nil assertion fired during panic unwinding, where err and result are both nil, replacing the original cause with its own message. The invariant only holds on a normal return, so it no longer asserts while a panic is in flight.

  * fix(sync): write received artifacts' visibility inside the storing transaction (#200)

  Receiving an artifact cost three write transactions, not one. The blob was stored in a transaction, its dephantomize cascade committed another, and then applyPrivateStatus re-derived the rid on the pool and issued a bare DELETE FROM private -- an implicit transaction of its own, with its own BEGIN IMMEDIATE and fsync, for a row that a transaction microseconds earlier had guaranteed did not exist. Every igot card announcing an artifact already held cost one more. Measured against a real fossil peer: 1509 statements executed outside a transaction to receive 1504 artifacts.

  That is minutes of pure commit overhead on a large repository -- roughly 200,000 write transactions for a 67k-artifact pull -- and it is fragile as well as slow. Each of those writes contests the database's single write lock independently, so N artifacts means N chances to lose that contest and fail outright rather than one chance to wait at a single BEGIN.

  Visibility now travels with the content it describes: storeReceivedFile takes the visibility a private card announced and records it inside the transaction that stores the blob, using the rid that transaction already has. The status is now atomic with the content rather than a separate write that could be interrupted between the two. The igot path buffers its updates and writes them once per round. The same three-transaction shape on the server's receive path goes with it.

  The same real-fossil pull now executes 2 statements outside a transaction for those 1504 artifacts, down from 1509. db.DB.PoolExecs makes that budget assertable, which is how the regression test pins O(rounds) rather than O(artifacts) without needing a large corpus.

  On the failure originally reported: the "database is locked" error came from a harness that copied a live SQLite database out from under a running server without its WAL, so that specific lock holder is not a product defect and this does not claim to fix it. The transaction storm above is a real performance and robustness defect on its own evidence, and is what this changes.

  * test(sync): pin the transaction budget, not just writes outside a transaction

  The budget tests measured PoolExecs, which counts statements issued outside a transaction. That pins one half of the defect and misses the other: moving a per-artifact write from the pool into its own WithTx leaves PoolExecs unchanged while every artifact still pays a BEGIN IMMEDIATE and an fsync, and still contests the single write lock on its own. That is the cost issue #200 actually measured -- roughly three write transactions per received file, about 203,000 for one clone-sized pull.

  Verified by mutation: reverting the batched visibility flush to one transaction per update left both budget tests green. They now count transactions too, via a new db.DB.WriteTxns counter, and the same mutation fails with "began 51 write transactions for 50 igot cards over 1 rounds; want <= 4".

  Also stop the contention test from starving the pull it is meant to observe. The rival connection re-acquired the write lock immediately on release, so on a loaded machine it could hold the pull's single BEGIN past busy_timeout and fail the test even when the batching under test is perfect -- which is what happened on CI. It now yields between bursts, modelling another process doing occasional writes rather than testing SQLite's fairness. (user: [email protected], size: 1310) [annotate] [blame] [check-ins using]

package libfossil

import (
	"context"
	"path/filepath"
	"testing"

	_ "github.com/danmestas/go-libfossil/internal/testdriver"
	"github.com/danmestas/go-libfossil/internal/xfer"
)

// Supplying credentials must not make SyncOpts.ProjectCode mandatory — the
// *Repo being synced already knows its own project code (issue #199).
func TestRepoSyncWithCredentialsAndNoProjectCode(t *testing.T) {
	r, err := Create(filepath.Join(t.TempDir(), "repo.fossil"), CreateOpts{User: "test"})
	if err != nil {
		t.Fatalf("Create: %v", err)
	}
	defer r.Close()

	var sawLogin bool
	transport := TransportFunc(func(_ context.Context, payload []byte) ([]byte, error) {
		req, err := xfer.Decode(payload, xfer.ContentTypeCompressed)
		if err != nil {
			return nil, err
		}
		for _, c := range req.Cards {
			if _, ok := c.(*xfer.LoginCard); ok {
				sawLogin = true
			}
		}
		return (&xfer.Message{}).Encode()
	})

	defer func() {
		if p := recover(); p != nil {
			t.Fatalf("Sync panicked: %v", p)
		}
	}()
	res, err := r.Sync(context.Background(), transport, SyncOpts{
		Push:     true,
		User:     "syncer",
		Password: "pw123",
	})
	if err != nil {
		t.Fatalf("Sync: %v", err)
	}
	if res == nil {
		t.Fatal("Sync returned nil result on success")
	}
	if !sawLogin {
		t.Fatal("no login card reached the transport")
	}
}