Skip to content

BAD CODE: Think shells out to git directly instead of using git-warp's public API #36

Description

@flyingrobots

Observation

Think's storage substrate is git-warp, and Think writes no Git objects of its own — commit, commit-tree, write-tree, hash-object and update-ref appear nowhere in src/. Yet Think still invokes git directly:

Site Operation
src/git.js ensureGitRepo git init
src/git.js setFsmonitorDisabled git config core.fsmonitor false
src/git.js checkUpstream git ls-remote
src/git.js readOptionalGitConfig git config --get
src/git.js pushWarpRefs git push refs/warp/<graph>/*
src/project-context.js runGitString read-only context queries

Plus THINK_GIT_CONFIG_ARGS, which prepends -c core.fsmonitor=false to every invocation, and NON_INTERACTIVE_PUSH_ENV, which hand-manages credential-prompt suppression.

Why this is bad

Each of these is Think reasoning about Git semantics it has no business owning, and the failure mode is not theoretical. The identity defect fixed in #34 came from exactly this shape: ensureGitRepo wrote user.name/user.email into whatever directory it was handed, because that was the only channel through which the commit layer would read an identity. It silently rewrote the committer identity of any repository Think was pointed at, including a developer's own source checkout, and every commit made by hand there afterwards was misattributed.

That workaround existed because a capability was missing upstream. Reaching around the dependency rather than through it converted a missing feature into data corruption in someone else's repository.

The remaining direct calls carry the same latent risk: Think is guessing at Git behaviour that git-warp already encapsulates, and every guess is a place where Think's model and git-warp's model can drift apart silently.

Suggested direction

Establish the rule that Think uses git-warp's public API and nothing else, then close the gaps that currently prevent it:

  • repository creation (init) and core.fsmonitor policy
  • ref transport (push of refs/warp/*) and reachability checks (ls-remote)
  • read-only repository context queries

Where git-warp lacks a public surface for one of these, the fix belongs upstream — as with git-stunts/plumbing#13, which restores git's ability to read the operator's own configuration rather than having consumers inject identity.

Enforcement once the surface exists: see the companion COOL IDEA for a lint ratchet.

References

  • Remove Think's Git identity entirely #34 — the identity defect this shape produced
  • git-stunts/plumbing#13 — upstream fix for the missing capability
  • docs/method/backlog/bad-code/CORE_hexagonal-store-boundary.md — related boundary concern

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions