pkg: resolve the remote's default branch instead of fabricating "main" (#879)

[InauguralPhysicist]

lib/pkg.eigs:297 hardcoded the tag when --pkg add was given none:

tag is "main"
if (len of arg_list) >= 3:
    tag is arg_list[2]

So the clone ran git clone --depth 1 --branch main and failed outright on any repository whose default branch is master, trunk or develop:

warning: Could not find remote branch main to clone.
fatal: Remote branch main not found in upstream origin

The part that made it unrecoverable

The fabricated tag was persisted into eigs.json before the clone was attempted. That ordering is deliberate and correct — the manifest write is what makes add recoverable by re-running install — but it meant the recovery path was poisoned too. The project was left naming a branch that does not exist, and --pkg install could never fix it.

The fix

PACKAGE_SPEC.md:60 already specified "default branch if omitted", so an omitted tag now means exactly that:

• no --branch on the clone — git picks the remote's own default;
• the manifest records no tag key rather than a guess;
• the lockfile still pins the resolved commit, which is what makes install reproducible — so dropping the guess costs nothing in determinism;
• --pkg add reports which default branch it resolved to.

One clone_args helper is shared by add / install / update so the three cannot drift on what "no tag" means. Worth noting: install had the identical hardcoded --branch and would have failed the same way on a no-tag dep, so fixing only add would have moved the failure one command downstream.

Verification

Against a real local repo whose default branch is master:

$ eigenscript --pkg add alice/mylib file:///…/msource
Added alice/mylib -> file:///…/msource @ master (default branch) (96d8edd9)

$ cat eigs.json
{… "deps":{"alice/mylib":{"git":"file:///…/msource"}}}      # no fabricated tag

$ rm -rf eigs_modules && eigenscript --pkg install            # the path the bug destroyed
  installed alice/mylib @ 96d8edd9

tests/test_pkg_fetch.sh gains a master-branch remote and five checks: add succeeds, the manifest records no guessed tag, install recovers from the manifest alone, verify passes, and an explicit tag is still recorded and honored. Section [95] goes 6 → 11 checks.

• Suite: 3809/3809

lib/pkg.eigs is pure EigenScript, so there's no C to sanitize; the suite runs it under every variant CI builds.

Closes #879

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

cmd_install can still write "tag": null into the lockfile for no-tag deps when it has to create a new lock entry, which is inconsistent with the new “omit tag” behavior and is addressed by the suggested fix in review comments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Updates the EigenScript package manager so an omitted tag means “use the remote’s default branch” (per docs/PACKAGE_SPEC.md), instead of hardcoding "main" and persisting that fabricated tag into eigs.json (which previously made failed adds unrecoverable via --pkg install).

Changes:

• Centralizes clone argument construction in clone_args() so add/install/update share the same “no tag” semantics (no --branch).
• Stops writing a guessed "tag" key to eigs.json (and lockfile) when the user omits a tag; --pkg add now reports the resolved default branch name.
• Extends the fetch test to cover a master-default “remote” and updates the suite’s expected check counts.

File summaries

File	Description
lib/pkg.eigs	Implements default-branch behavior when tag is omitted; shares clone arg construction; improves --pkg add reporting.
tests/test_pkg_fetch.sh	Adds coverage for cloning/installing from a repo whose default branch is master, and asserts manifest omits "tag".
tests/run_all_tests.sh	Updates section [95] expected check/pass counts to match new test assertions.
docs/PACKAGE_SPEC.md	Clarifies that omitted tags remain omitted in eigs.json and clone runs without --branch.
CHANGELOG.md	Documents the fix and its recoverability implications.

Review details

Suppressed comments (1)

lib/pkg.eigs:404

• When a dep omits "tag" in eigs.json, tag can be null here; cloning is fine (clone_args treats null/"" as default-branch), but if cmd_install has to create a new lock entry it will currently write "tag": null. That makes lockfile entries inconsistent with cmd_add’s no-tag behavior and may introduce a non-string tag value into the lockfile.

        ok is run_git of (clone_args of [git_url, target, tag])   # #879
        if ok == 0:
            throw of f"install failed: clone for {name} failed"

        locked_commit is ""

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟢 Ready to approve

The functional change is well-scoped, matches the package spec, and is covered by new end-to-end tests for the previously unrecoverable scenario.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

lib/pkg.eigs:270

• The fetch_dep header comment still says it returns only {"commit", "tree"}, but the function now also returns a "branch" field. This mismatch can confuse callers/readers (and makes the comment incorrect).

# Clone <git_url> at <tag> into eigs_modules/<name>. Recreates the
# target dir to keep `add` idempotent — a re-add against the same
# name picks up the new url/tag instead of mixing trees. Returns
# {"commit": sha, "tree": tree_sha} on success, {} on failure.

lib/pkg.eigs:311

• fetch_dep now shells out to git one extra time (git_current_branch) on every fetch, even when the caller provided an explicit tag and the branch name is never used. This adds avoidable subprocess overhead in loops (e.g., --pkg update over many deps).

    return {"commit": commit, "tree": tree, "branch": git_current_branch of target}

tests/test_pkg_fetch.sh:205

• The new "omitted tag" manifest shape is now exercised for add/install/verify, but --pkg update isn’t covered for a dep whose manifest entry has no "tag" key. Adding a regression check here would prevent future drift/regressions for the no-tag path in update.

# The recovery path the old bug destroyed: reinstall from the manifest alone.
rm -rf eigs_modules
INSTALL_M=$("$EIGS" --pkg install 2>&1) || {
    echo "  FAIL: --pkg install must recover a dep with no tag"
    echo "$INSTALL_M"
    exit 1
}
cat > mapp.eigs <<'EOF'
import mylib
print of mylib.mylib_greet
EOF
MAPP_OUT=$("$EIGS" mapp.eigs 2>&1)
if [ "$MAPP_OUT" != "hello from a master-branch repo" ]; then
    echo "  FAIL: reinstalled master-branch dep should be usable — got '$MAPP_OUT'"
    exit 1
fi
echo "  PASS: --pkg install recovers a no-tag dep (was: unrecoverable)"

VERIFY_M=$("$EIGS" --pkg verify 2>&1) || {
    echo "  FAIL: --pkg verify should pass for a no-tag dep"
    echo "$VERIFY_M"
    exit 1
}
echo "  PASS: --pkg verify passes for a no-tag dep"

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.