Skip to content

fix(git): restore git->readcredentials so appl/cmd builds - #556

Closed
Ticed wants to merge 1 commit into
infernode-os:masterfrom
Ticed:fix/git-readcredentials
Closed

fix(git): restore git->readcredentials so appl/cmd builds#556
Ticed wants to merge 1 commit into
infernode-os:masterfrom
Ticed:fix/git-readcredentials

Conversation

@Ticed

@Ticed Ticed commented Aug 25, 2026

Copy link
Copy Markdown

Summary

  • appl/cmd/git/push.b:163 calls git->readcredentials, which is declared in no .m interface, so mk install under appl/cmd fails on master
  • declare it in module/git.m, implement it beside getremoteurl in appl/lib/git/git.b
  • semantics taken from the two call sites, not invented: reads <gitdir>/credentials, trims, returns that one line or nil
  • the tracked push.dis was built while the function still existed, so only a from-source build hits this

Test plan

  • cd appl/cmd/git; mk install — reproduced push.b:163: 'readcredentials' is not a member of 'git' of type Git on 90b1365, clean after
  • cd appl/cmd; mk install — was exit(1), now exits 0
  • rebuilt dis/cmd/git/push.dis is byte-identical to the committed one (md5 b64151bb27c6d3cb4fcb015d97c9521b), so this restores the removed signature rather than inventing one
  • macOS ARM64
  • /tests/runner.dis not run — no test covers push

Checklist

  • No .dis committed from appl/ or tests/
  • No secrets or credentials included

Two things I noticed here and did not fix: no CI builds the Limbo tree from source, which is why this survived; and after mk install in appl/cmd, five tracked .dis files do not reproduce from their sources — auditget, countfs, newuser, nsaudit, trfs. Everything else rebuilt byte-identically. Happy to open either as its own issue.

@Ticed
Ticed requested a review from pdfinn as a code owner August 25, 2026 19:26
push.b calls git->readcredentials(gitdir), which is declared in no .m
interface, so mk install in appl/cmd fails. The tracked push.dis was built
when the function existed, which is why only a from-source build hits it.

The rebuilt dis/cmd/git/push.dis is byte-identical to the committed one,
so this restores the removed signature rather than inventing a new one.
@Ticed
Ticed force-pushed the fix/git-readcredentials branch from e6f841f to 2f4929a Compare August 25, 2026 19:29
@pdfinn

pdfinn commented Aug 31, 2026

Copy link
Copy Markdown
Member

Closing this — the diagnosis is right and the fix landed, just by the other
route. Reasoning, since the difference matters.

You found a real break: push.b:163 called git->readcredentials, which no
.m declared, so appl/cmd/git had not compiled since 2026-07-02. Your note
that "the tracked push.dis was built while the function still existed, so
only a from-source build hits this" is exactly right, and it's the reason it
sat unnoticed for two months — nothing ever rebuilt the whole tree. That
observation is a good part of why #559/#560 went the way they did.

Where this differs is the remedy. readcredentials wasn't lost by accident —
it was removed deliberately by #338, fix(security): attenuate each tool invocation namespace:

-readcredentials(gitdir: string): string
-	fd := sys->open(gitdir + "/credentials", Sys->OREAD);
-	readcredentials: fn(gitdir: string): string;

The point of removing it was that Git is a module an agent tool can load, so
a credentials read inside it is reachable from a context that should not have
it. Restoring it to module/git.m and appl/lib/git/git.b puts that back,
and #338 left no note saying push.b still depended on it — so this looks
like a plain omission when it's actually a deliberate removal with one caller
left behind.

#559 fixed the same break the other way: the read moved into
appl/cmd/git/push.b as a private function. The command runs in its own
namespace, so whether .git/credentials is visible is decided by the
namespace rather than by library policy — which is the shape AGENTS.md asks
for — and the Git module stays clean for anything that loads it.

That's on master now, so appl/cmd/git builds, and this branch is
CONFLICTING against it.

Two things from your PR that did carry over: the reproduction
(cd appl/cmd/git; mk install) is now what CI does on every PR
(tools/verify-dis-build.sh), so a source that stops compiling fails the
build instead of hiding behind committed bytecode. And the semantics you
derived from the call sites — read <gitdir>/credentials, trim, return the
one line or nil — are what the version in push.b implements.

If you think the credential read belongs back in the library rather than the
command, that's a real design question and worth an issue against #338's
reasoning rather than a build fix — happy to argue it there.

@pdfinn pdfinn closed this Aug 31, 2026
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.

2 participants