Decide deliberately: should `up`'s agent-config files be pushed to the sandbox? (gitignoring them in #677 changed it by side effect)

Aperta
#683 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
35/100
Tipo di issue
Funzionalità
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
git, typescript
Ambito
tooling

Direzione di ricerca

Read packages/mocactl/src/sbx/push.ts around planPush and the ignore handling, then compare it with packages/mocactl/test/sbx-up.test.ts and the fixture in test/helpers/sbx.ts. Decide which listed paths should reach the sandbox and record that decision; done means the test, its comment, and docs/sbx/user-guide.md consistently describe the chosen behavior.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

question

Summary

#677 added the per-developer agent wiring to this repository's .gitignore (/.bob/, /.mcp.json, /opencode.json, /opencode.jsonc, /AGENTS.md, /.sbx/). A side effect, not stated in the PR: those files stop being pushed to the sandbox, from the second up in a repository onward.

Nothing is broken today (see "Why this is not a bug" below). But the behaviour changed as a consequence of an ignore-list edit rather than as a decision, there is no test that pins it either way, and a comment in the tree now describes the old behaviour. This issue is to settle it deliberately and write it down.

Filed from the review of #677. Not a merge blocker — #677 is approved and this is independent of its Bob fix.

The mechanism

planPush collects files with (packages/mocactl/src/sbx/push.ts:367):

git ls-files -z --cached --others --exclude-standard

Its own docstring says what that means (push.ts:359-360): "tracked files (even under an ignored directory) and untracked ones that are not ignored". None of these paths is tracked in this repository, so once ignored they are untracked and excluded, and drop out of the push.

Demonstrated in a clone of this branch:

$ # with #677's ignore lines in place, after `up` has written the files
$ git ls-files --cached --others --exclude-standard -- AGENTS.md .mcp.json opencode.json .bob
$ # (nothing)

$ # the same files, ignore rules overridden
$ git ls-files --others -- AGENTS.md .mcp.json opencode.json .bob
.bob/mcp.json
.mcp.json
AGENTS.md
opencode.json

The first up in a fresh repository is unaffected either way: agent config is written after the push by design (sbx/sync.ts:326-327 — "it is written after the push, so the push carries the developer's tree"), so the files do not exist yet at push time. It is every later up/push that differs.

Note .sbx/ is not part of this question: planPush skips it unconditionally via isBindingPath (push.ts:391, "the binding is the laptop's"), so its ignore line changes no behaviour at all.

Why this is not a bug

Worth stating plainly, because it sets the stakes low:

  • The in-sandbox agent does not read these files. Its steering comes from MCP_INSTRUCTIONS, which the MCP server delivers over the protocol (packages/sandbox-client/src/mcp-server.ts:71). The sbx:begin/sbx:end block in AGENTS.md/CLAUDE.md is the same text, written for people and for OpenCode on the laptop. So not pushing AGENTS.md does not leave the sandbox-side agent unsteered.
  • The files are laptop-side wiring. They name one developer's sandbox and differ per working tree, which is exactly the reasoning in #677's .gitignore comment.
  • A pulled patch must never write them back anyway. AGENT_DIRS/AGENT_FILES (push.ts:158+) already refuse that, since each one makes the laptop start a program.

So the live question is not "is this broken" but "is this what we want, and is it written down".

What is inconsistent right now

  1. A test comment now describes behaviour that no longer holds in this repo. packages/mocactl/test/sbx-up.test.ts:250-251:

    // The 7 files `up` pushed, and the 7 agent-configuration files it wrote after its push.
    expect(h.err()).toMatch(new RegExp(`pushed 14 files \\(0\\.0 MiB\\) to ${NEW}`));
    

    This still passes, because the fixture repository has no such ignore lines (test/helpers/sbx.ts:221). It therefore asserts the old behaviour, and cannot catch a change in the new one. A reader comparing it against this repository will get the wrong idea.

  2. Nothing pins the intended behaviour. Whichever way we decide, there is currently no test that would fail if the other happened.

  3. The guide has a related sentence that now reads oddly. docs/sbx/user-guide.md:663: "new files under a directory your own .gitignore ignores are not protected: they may not come back." With these entries present, an AGENTS.md an agent authors in the sandbox is silently dropped by the next push. For five machine-generated paths that is clearly fine; AGENTS.md is the one a human might hand-write and expect to survive.

Options

A. Keep them out of the push (ratify the new behaviour)

Pros

  • Matches the intent already written in .gitignore: this is per-developer wiring that points at one developer's sandbox, so shipping it into the sandbox has no purpose.
  • The sandbox never needs them (MCP instructions arrive over the protocol).
  • Slightly smaller pushes; one less way for laptop-specific paths to reach a shared place.
  • Consistent with .sbx/, which is already skipped unconditionally.
  • Cheapest: a comment fix plus a test.

Cons

  • The sandbox's /workspace is then not a faithful copy of the developer's tree, which is the mental model the guide sets up ("a copy of your repository at /workspace").
  • A hand-written AGENTS.md in a repository that gitignores it is silently dropped — surprising, and only documented obliquely at user-guide.md:663.
  • Behaviour now depends on each repository's .gitignore, so up behaves differently in moca than in a repository that does not ignore these. That is implicit, not declared.

Work: fix the sbx-up.test.ts:250 comment; add a test with the ignore lines present asserting the files are not pushed; one line in the guide.

B. Always push them (restore the old behaviour explicitly)

Re-include them the way build-output re-inclusion already works: -x patterns beat every ignore file (push.ts:493-498).

Pros

  • /workspace stays a faithful copy, matching the documented model.
  • up's behaviour no longer varies with the repository's .gitignore.
  • Keeps the property the existing 14-file test describes, so that test and its comment stay honest.
  • An in-sandbox agent that does read AGENTS.md (a future agent, or a developer running one by hand in the VM) finds it.

Cons

  • Pushes laptop-specific wiring into the sandbox for no current consumer — .mcp.json and .bob/ name mocactl paths meaningless inside the VM.
  • Mild smell: deliberately overriding the developer's own .gitignore, which --exclude-standard exists to honour.
  • More moving parts than A, in the push path, which is the riskier code.

Work: add the four paths to the re-include set; a test that they arrive despite being ignored.

C. Split the difference — push AGENTS.md/CLAUDE.md, not the machine-generated JSON

Pros

  • Targets the only file with human-authored content and a plausible in-sandbox reader.
  • Keeps genuinely laptop-only wiring (.mcp.json, .bob/, opencode.json) out.

Cons

  • A third rule to explain and remember; the least predictable of the three.
  • CLAUDE.md is tracked and AGENTS.md is not, so they would reach the sandbox by different routes — confusing.
  • Still leaves "does /workspace mirror my tree?" answered with "partly".

Work: as B, narrowed; plus the explanation.

Recommendation

A, with the documentation and tests that A requires. The sandbox has no consumer for these files, the MCP server already carries the steering text, and A agrees with the reasoning #677 wrote into .gitignore. The cost of A is honesty work — a stale comment, a missing test, one guide sentence — not a behaviour change.

If anyone expects /workspace to be byte-identical to the laptop tree, that argues for B and should be said now, because it is the assumption the guide's wording invites.

Checklist, once decided

  • Record the decision (here, or in the SBX0 spec §6.2/§6.3 if it counts as spec)
  • Fix the sbx-up.test.ts:250 comment so it matches reality
  • Add a test with the ignore lines present that pins the chosen behaviour
  • Say it in docs/sbx/user-guide.md, near the §6.3 table or the user-guide.md:663 note
Lingua principale
TypeScript
Stelle
12
Fork
12
Merge medio
9h 27m
PR unite (30g)
164

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di rossoctl/moca

Tutte le issue di rossoctl/moca

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.