fix(git-sync): keep container-only .claude/settings.json out by content, not name (trinity-enterprise#708)

[dolho]

Summary

Implements the ruling on abilityai/trinity-enterprise#708, recorded on the issue:

1. Durability owner on a deployed agent: the platform heartbeat. The marketplace add-git-sync hooks are for local sessions. The marketplace half is filed as Abilityai/trinity-skills#1: the skill detects the platform and stands down, and drops its negation step.
2. The .claude/settings.json ignore is narrowed to its actual damage. The file leaves the canonical ignore list, removed as well from the agent guide's fence and the 14 bundled templates' copies. What bug: base image bakes container-only hook paths into ~/.claude/settings.json, git sync commits it, external clones of the agent repo are bricked #2036 fixed was one content: absolute /opt/trinity/ hook paths, which brick any clone made outside the container. Every platform commit path now runs a guard after staging. If the index copy is refused (an /opt/trinity/ hook command, a credential-bearing key, or content that is not UTF-8 JSON), the guard leaves it alone when it equals HEAD (already in history; untracking would only commit a deletion), keeps the HEAD copy when HEAD has no /opt/trinity/ hook, and untracks the file only when there is no HEAD copy or HEAD is a pre-bug: base image bakes container-only hook paths into ~/.claude/settings.json, git sync commits it, external clones of the agent repo are bricked #2036 hook-path leak, which is then deleted from the remote on the next commit. The working-tree file is never touched.

Commit path	Guard
heartbeat _run_auto_sync_once	agent_server/routers/git.py::_guard_container_only_settings
operator Push sync_to_github	same
initialize_git_in_container (backend, docker exec shell)	gitignore.CONTAINER_ONLY_SETTINGS_GUARD, a shell twin of the same rule

• The heartbeat now commits only when something is staged (_has_staged_changes, the same rule Push already used). A guarded-out file stays untracked on disk, and the old git status --porcelain test would otherwise have turned every later cycle into an empty-commit failure.
• AC4 (legacy copies): startup.sh removes the legacy copy only on an exact byte match with the managed file, so older variants survive on long-lived volumes. The content guard covers them wherever they are.

⚠️ What existing agents see after upgrade

• Old ignore line. Agents created before this PR carry .claude/settings.json in their .gitignore. The .gitignore merge (Push, start, creation) drops it only inside a container whose agent server has the guard (it greps /app/agent_server/routers/git.py for _guard_container_only_settings). A container still on a pre-guard base image keeps the ignore until it is recreated on the new image, because its heartbeat and Push would commit an unchecked file.
• One commit, once the line is gone. Every agent already has a platform-written ~/.claude/settings.json (extraKnownMarketplaces: abilityai, enabledPlugins: trinity@abilityai); user and project settings are the same file because HOME is the repo root (refactor: agent git repo root is $HOME — 22 gitignore patterns exist only to compensate, and every new home-writing feature repeats the dance #1703). After the line is dropped, an auto-syncing agent commits its current settings once, unless the guard refuses them. The platform content is portable and byte-stable across restarts, so there is no churn. Accepted by the operator.

Changes

• src/backend/services/git_service/gitignore.py: pattern removed with the ent#345/ent#708 history; CONTAINER_ONLY_SETTINGS_GUARD added
• src/backend/services/git_service/provisioning.py: guard spliced after both git add . steps in initialize
• docker/base-image/agent_server/routers/git.py: _guard_container_only_settings and _has_staged_changes; wired into the heartbeat and Push
• config/agent-templates/*/.gitignore (14): the line removed so they stay a subset of the canonical list (the bug(templates): sage/scout/scribe ship no .gitignore — every agent created from them is born with 4 hard security findings #1908 guard)
• Docs: TRINITY_COMPATIBLE_AGENT_GUIDE.md fence, git-sync-health.md §1c (the ruling and the guard), architecture/agent-lifecycle.md, requirements/github.md
• Tests:
  • tests/unit/test_ent708_settings_json_guard.py (19, new): the same matrix on real repos for both the Python guard and the shell twin (clean file commits; a harmful untracked file stays out and on disk; a harmful edit of a clean tracked file restores the clean copy; a harmful copy already in HEAD is untracked; fresh repo; no-op). It also runs the heartbeat end to end: harmful content never reaches the remote and the next cycle makes no empty commit; a clean file syncs; a legacy committed copy is deleted from the remote. AST wiring checks cover add < guard < commit in both agent paths and the guard after both initialize adds.
  • test_2036_claude_settings_leak.py: rewritten to the new contract; a clean project settings.json now stages without any negation.

Test Plan

• 18 related suites green with random ordering: guard, 2036, 2529 precedence, github_init_gitignore (doc ↔ constant), 1908 bundled templates, auto-sync, 1595, 2069, ent345, 2742, git_service package, ent123, ent109, dual ahead/behind, protected paths, read-only, credential allowlist, ent183
• Mutation: with the agent router and provisioning reverted, 11 of 19 new tests fail. The 8 still green are the shell-twin matrix (it reads the unreverted constant, so it tests the constant itself) and the pattern check.
• Live on local dev: an agent on a base image built from this branch (:latest untouched), with a real repo in its home.
  • Heartbeat: content/b.md pushed, remote kept the clean settings copy, local harmful file untouched, and the next cycle made no commit.
  • The real sync_to_github: Synced to main, files_changed: 1, remote settings still clean.
  • Both logged kept .claude/settings.json out of the commit (restored).
  • Throwaway agent, volume, rows and image cleaned up.
• lint_sys_modules, lint_root_test_placement, enterprise-docs-guard pattern clean

Related to abilityai/trinity-enterprise#708 (cross-repo: this doesn't auto-close; close it at release)

🤖 Generated with Claude Code

[vybe]

merge-train: ejected from this train — rides the next one, rebased on a dev that contains #3016.

1. Conflict with fix(git-sync): auto-sync heartbeat fetches and rebases before push; refuses on a shared source-mode branch (#3011) #3016 in docker/base-image/agent_server/routers/git.py (both edit the auto-sync helpers). A source conflict between siblings means the later one re-applies after the first lands.
2. Unguarded commit path: reset_to_main_preserve_state_impl (~git.py:2111-2113, reachable via the reset route and the MCP reset_to_main_preserve_state tool) does git add -A → commit → force-push with no _guard_container_only_settings call. Reproduced: a harmful .claude/settings.json lands in HEAD there, which the old ignore prevented. Needs the guard plus a test that drives this path.
3. Needs a ruling: in an agent the repo root is $HOME, so .claude/settings.json is also Claude Code's user settings file. With the ignore gone, anything written there (e.g. an env block holding an API key, or apiKeyHelper) is committed and pushed; the guard only checks for /opt/trinity/. Was that accepted in the ent#708 ruling?

Minor: src/backend/services/compatibility/spec.py:46-49 docstring still lists the file in the canonical ignore list. Tests are green (30 + 298 passed).

[github-actions]

✅ Nightly unit-suite clean when this PR is merged into dev, all 3 seeds (head_sha: 60093e52ca189441daa220a50f9d2c04cc30a645).

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train (2026-09-27): three commits pushed, both follow-ups from my 09-25 note agreed with the operator.

1. 14564b407 merges dev. The routers/git.py conflict is two helper blocks added at the same spot: fix(git-sync): auto-sync heartbeat fetches and rebases before push; refuses on a shared source-mode branch (#3011) #3016's (_is_missing_remote_ref, _is_shared_source_branch, _rebase_onto_remote) and this PR's (_guard_container_only_settings, _has_staged_changes), kept side by side. _run_auto_sync_once merged cleanly. It now runs the source-mode refusal, then git add -A, then the guard, then the staged-only commit check, then fetch, rebase and lease-protected push. tests/registry.json was rebuilt from the merge stages.
2. 2eb2e052c makes reset_to_main_preserve_state_impl run the guard; it commits and force-pushes. A sweep of every stage-and-commit path found this to be the only unguarded one (heartbeat, operator Push and the backend's initialize_git_in_container already were). Three tests drive the reset path in a real repo, and all three were red before the fix: the harmful file landed in HEAD.
3. 9f6dc3012: because HOME is the repo root, this file is also Claude Code's user settings. The guard now also refuses a copy carrying a credential-bearing top-level key: env, apiKeyHelper, awsAuthRefresh, awsCredentialExport, gcpAuthRefresh or otelHeadersHelper (from the Claude Code settings reference). It also refuses a copy that isn't a JSON object, since that can't be checked, so this fails closed. The log line names the keys, never the values. The shell twin in gitignore.py runs the same rule through the container's python3, and a parity test pins the key lists together. The platform-written file (extraKnownMarketplaces, enabledPlugins) still commits, and a test proves it.

test_ent708_settings_json_guard.py: 58 passed. Across 55 related files (git, sync, gitignore, #2036, #3010, #3011, #2107, reset): 1,114 passed, 0 failed.

Behaviour change to know about: any non-empty env block keeps the file out, harmless variables included, and so does an empty or invalid-JSON file, which used to commit. A template that ships a harmless env in .claude/settings.json will have that file held back. Not done: the docstring at compatibility/spec.py:46-49 still lists the file as ignored.

[dolho]

Pushed c0c60573b, the last open item from the 09-25 and 09-27 notes. The compatibility/spec.py G-002 docstring no longer lists .claude/settings.json among the ignored patterns, and says why: ent#708 replaced the ignore with a content guard.

The guard on reset_to_main_preserve_state, the refusal of credential-bearing keys and the dev merge are already in from the 09-27 commits. CI is green and the PR is mergeable.

[AndriiPasternak31]

Moving from a name-based ignore to a content guard is the right call, and the guard's own matrix is solid. I checked it against real repos, though, and it deletes a template's committed settings.json from the remote whenever that file has a harmless env block. It also doesn't reach any agent that already exists, because the old ignore line survives the .gitignore merge. Both need fixing before this lands.

1. [blocking] docker/base-image/agent_server/routers/git.py:829-840: a committed, harmless settings.json gets deleted from the remote by an unattended cycle.
   When the index copy is refused and the HEAD copy is refused too, the guard runs git rm --cached, and that commits a deletion. env is refused whenever it is non-empty, and the /opt/trinity/ check is a plain substring match. So this fires on ordinary template content: "env": {"BASH_DEFAULT_TIMEOUT_MS": "600000"}, DISABLE_TELEMETRY, MCP_TIMEOUT, or a permissions.deny entry like Read(/opt/trinity/**).
   Repro: I committed and pushed a template-style settings.json with that env block, made no other change, and ran _run_auto_sync_once. It made a new commit, Trinity auto-sync: …, whose only change is .claude/settings.json | 11 -----------, and pushed it. The deny-mention variant does the same. The same branch runs in reset_to_main_preserve_state_impl, which force-pushes, and in the shell twin at initialize, where it becomes the "Initial commit". So a template's project settings, which this PR exists to preserve, get removed from the agent's branch with only a container log line to show for it. vybe's 09-27 note says such a file is "held back". It is actually deleted from a remote that already had it.
   Fix: only untrack when the refused content is new. If the index copy equals HEAD, leave it alone, because nothing leaks that isn't already in history. Keep "untrack a HEAD copy" for the /opt/trinity/ hook-path case, which is the pre-#2036 heal this PR wants. Add a test with a committed harmless env plus an idle heartbeat that asserts no new commit.

2. [should-fix] src/backend/services/git_service/gitignore.py:229 (_GITIGNORE_SUPERSEDED_LINES): removing the pattern is a no-op for every existing agent.
   The merge strips only lines that are currently managed. .claude/settings.json is no longer in that list, so on an agent whose .gitignore came from the old list, the next merge moves the line into the user region instead of dropping it. I checked this against real repos: I ran the base merge and then the PR's merge on the same repo. Afterwards git check-ignore -v reports .gitignore:45:.claude/settings.json, sitting between the default block and the protected floor as if it were the user's own rule. That covers every agent created or pushed since #2069. Their template hooks stay dropped. The "expect one commit per auto-syncing agent after upgrade" warning and the §1c text don't describe what will actually happen. The file already documents this migration step for the retired .trinity/ line and for marker strings (~line 311).
   Adding the line to _GITIGNORE_SUPERSEDED_LINES is not free, though. The merge runs from the backend via docker exec on Push, start and creation, including on containers still running a pre-PR base image. Those have no guard, so the old Push or heartbeat would then commit a legacy /opt/trinity/ copy or a credential. Pick one of two options and say so in the PR: supersede the line only for agents on a base image that has the guard, or accept that existing agents keep the ignore and correct the PR description and git-sync-health.md §1c.

3. [should-fix] docker/base-image/agent_server/routers/git.py:818-819: a non-UTF-8 settings.json stops all syncing, and the two copies of the guard disagree about it.
   _blob calls run_registered with strict decoding, so git show of a non-UTF-8 blob raises UnicodeDecodeError before _settings_refusal_reason ever runs. Repro: {"note":"caf\xe9"} plus an unrelated w.md. The heartbeat returns failed 'utf-8' codec can't decode byte 0xe9… on every cycle, so nothing else syncs until someone fixes the file, and Push fails the same way. The backend's shell version handles the same bytes by refusing the file (python exits non-zero), which is the fail-closed behaviour the comment promises. Fix: pass errors="replace" (the #2957 precedent) or decode bytes yourself, so the file falls into the existing "not valid JSON" refusal. Add that case to the shared matrix.

4. [nit] docker/base-image/agent_server/routers/git.py:841: an untracked file that gets refused is re-added by git add -A and refused again on every 15-minute cycle, so the same WARNING is logged forever. Nothing records it in sync-state.json, so an operator never sees that the file isn't being synced. Worth a sync-state field, or log once per content hash.

What I checked

• Read the full diff and the prior merge-train and author comments. The reset-path guard, the credential keys and the spec.py docstring from earlier rounds are all in. There is no other stage-and-commit path in agent_server or startup.sh. The provisioning write probe is --allow-empty in a scratch repo.
• Ran 13 related suites (ent708, 2036, 2529, 1908, github_init_gitignore, auto_sync, ent345, 2069 ×2, 2742, the reset_preserve_state pair, 1966) with pytest-randomly: seed 12345 gave 358 passed and 1 skipped, seed 99999 gave 359 passed. The local venv is Python 3.14, not CI's 3.13.
• Tried three mutations. Using k in data instead of data.get(k), going back to the old status.stdout.strip() check, and restoring any HEAD copy were each caught (1, 1 and 3 tests failed).
• Findings 1–3 are reproduced on real temporary repos with a bare remote, driving the real _run_auto_sync_once and the real .gitignore merge builder from both trees. CI on c0c60573b is green.

[dolho]

Addressed the 09-28 review in 3d79be192. Each item is below with its fix and the test that covers it.

1. [blocking] Committed harmless settings got deleted. A refused index copy that equals HEAD is now left alone, with no log line and no commit. If the refused copy differs from HEAD, the HEAD copy is kept (git reset). The file is untracked only when there's no HEAD copy, or when HEAD registers an /opt/trinity/ hook, which is the pre-bug: base image bakes container-only hook paths into ~/.claude/settings.json, git sync commits it, external clones of the agent repo are bricked #2036 heal. /opt/trinity/ now matches only inside the hooks subtree, so a permissions.deny rule that names the path is portable in both new and committed copies. The shell twin in gitignore.py uses the same rule through a second checker, _SETTINGS_HEAL_CHECK. It fails safe: if it errors, it keeps the HEAD copy instead of committing a deletion. The heartbeat, Push and reset all share _guard_container_only_settings.
   Tests: your repro runs on a real repo with a bare remote. A committed env (BASH_DEFAULT_TIMEOUT_MS, DISABLE_TELEMETRY) or a Read(/opt/trinity/**) deny, followed by an idle _run_auto_sync_once, makes no new commit, and the remote keeps the file. The reset force-push keeps it too. The matrix now also covers, for both the Python guard and the shell twin: a committed copy is left alone (env and deny), a new deny mention commits, and a refused edit of a refused HEAD copy restores HEAD. test_a_committed_credential_copy_is_untracked became ..._is_left_in_history_not_deleted: untracking cannot un-leak history, only rotation can.
2. [should-fix] Existing agents kept the old ignore. I took your first option: the line is superseded only on agents whose base image has the guard. _GITIGNORE_GUARD_GATED_SUPERSEDED_LINES = (".claude/settings.json",) is stripped by the merge only when grep -qsF _guard_container_only_settings /app/agent_server/routers/git.py succeeds inside the container. A pre-guard image keeps the line until it is recreated. The probe fails safe. The list stays out of _GITIGNORE_MANAGED_LINES so the shadowed-negation oracle doesn't claim a line that is still legitimate on old images. Tests: build a .gitignore with the old list, then merge. On a guarded probe, git check-ignore stops matching. On a pre-guard probe, it still matches. A pin checks that the probe token is the real agent-server function. §1c and the PR description's upgrade warning now describe this.
3. [should-fix] A non-UTF-8 file stopped all syncing. git show now reads with errors="surrogateescape", and a copy that won't re-encode is refused as "not valid UTF-8". The shell twin decodes stdin strictly (sys.stdin.buffer.read().decode()). Both are in the shared matrix, plus an end-to-end test: a caf\xe9 settings file next to w.md gives heartbeat success, w.md is pushed, and the settings file is not.
4. [nit] The same WARNING was logged every cycle. It is now logged once per (repo, action, content sha256), in a bounded set. The sync-state.json field is deferred. It would make this visible to an operator, and it's worth a follow-up if you want it.

The compatibility/spec.py G-002 docstring was already fixed in c0c60573b.

Verification: test_ent708_settings_json_guard.py gives 76 passed (19 new or changed, all red before the fix). The 26 related suites (ent708, 2036, 2529, 1908, 2069 ×2, github_init_gitignore, auto_sync, 3010, 3011, 2742 ×2, 1595 ×2, 1966, reset_preserve_state ×2, ent345, ent123, 953, data_paths, protected_paths, 1028, 1704, 2107) gave 621 passed with --randomly-seed=12345 and 621 passed with --randomly-seed=99999.

[obasilakis]

Approving.

Checked locally on 60093e52 (py3.13): the related suites (ent708, 2036, 2529, 1908, 2069 x2, github_init_gitignore, auto-sync, reset_preserve_state x2, ent345, 3010, 3011) pass 358/358 on seeds 12345 and 99999.

Mutation check on each fix from the 09-28 review: hooks-only marker (3 red), shell-twin keep-HEAD (10 red), non-UTF-8 read (2 red), guard-gated .gitignore strip (1 red), log-once (1 red). The head == staged early return survives mutation because the fallback git reset on an identical index is a no-op; only the log line and return value differ. Asserting None there would pin it, optional.

Heartbeat, Push and reset all commit only on staged changes, so a guarded-out file never produces an empty commit.

After merge, ent#708 needs status-in-dev set by hand (cross-tracker ref).

All four points from this review are addressed in 3d79be1 (equal-to-HEAD copy left alone and hooks-only marker, guard-gated removal of the old ignore line, non-UTF-8 file refused instead of failing sync, log once per content). Re-verified with tests and mutations and approved; dismissing to unblock merge.