Skip to content

fix(agent-dev): add-git-sync hooks stand down on a Trinity-deployed agent (trinity-enterprise#708) - #24

Open
dolho wants to merge 1 commit into
mainfrom
fix/add-git-sync-trinity-standdown
Open

fix(agent-dev): add-git-sync hooks stand down on a Trinity-deployed agent (trinity-enterprise#708)#24
dolho wants to merge 1 commit into
mainfrom
fix/add-git-sync-trinity-standdown

Conversation

@dolho

@dolho dolho commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

This is the marketplace half of the ruling on abilityai/trinity-enterprise#708 (2026-09-25): on an agent deployed to Trinity, the platform's auto-sync heartbeat owns commit-and-push durability; the add-git-sync hooks own it for local sessions only. Before this change, both ran git add -A on the same working tree, and neither knew about the other.

Closes Abilityai/trinity-skills#1 once mirrored into the library.

  • Every hook script stands down on Trinity. This covers Stop, Stop local-only, SessionStart and PreCompact. Each exits before touching git when TRINITY_BACKEND_URL and AGENT_NAME are both set, which is the Trinity agent container's own environment.
    • The check lives in the scripts, so a repo that carries the hooks behaves correctly wherever it runs.
    • The same repo keeps full hook behaviour in local sessions.
    • Re-running the skill updates older installs.
  • The .gitignore block drops the .claude/settings.json + !.claude/settings.json pair. Trinity no longer ignores that file: after ent#708 it keeps a copy out of a commit only when it registers container-only /opt/trinity/ hook paths. An existing negation line is harmless and stays.
  • The installer says plainly when it runs inside a Trinity container.
  • add-git-sync 1.4, agent-dev 1.16.6.

⚠️ Merge order

Merge after Abilityai/trinity#3019 (ent#708) is released. Until then, Trinity still ignores .claude/settings.json. The negation line this PR drops is what keeps the hook registrations tracked on a repo Trinity pushes to.

Test plan

  • On a real repo with a bare remote, with the Trinity variables set, all four hooks leave HEAD, the remote and the dirty tree untouched.
  • Without them, or with only one of the two set, the Stop hook commits and pushes as before.
  • Inside a live Trinity agent container, using its real environment: HEAD is unchanged.
  • bash -n passes on every template; the frontmatter parses.

🤖 Generated with Claude Code

@dolho @claude
fix(agent-dev): add-git-sync hooks stand down on a Trinity-deployed a…
736b0b2 
…gent (trinity-enterprise#708)

Ruling on trinity-enterprise#708 (2026-09-25): on a deployed agent the
platform's auto-sync heartbeat owns commit/push durability; the hooks
are for local sessions. Both used to run git add -A on the same tree.

- Every hook script (Stop, Stop local-only, SessionStart, PreCompact)
  exits before touching git when TRINITY_BACKEND_URL and AGENT_NAME are
  both set, the Trinity agent container's own environment. The check
  lives in the scripts, so a repo carrying them behaves correctly
  wherever it runs; re-running the skill updates older installs.
- The .gitignore block drops the .claude/settings.json + negation pair.
  Trinity no longer ignores that file (ent#708), so the escape hatch
  has nothing left to escape.
- The installer says plainly when it runs inside a Trinity container.
- add-git-sync 1.4; agent-dev 1.16.6.

Closes Abilityai/trinity-skills#1 once mirrored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dolho

dolho commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

/review Report

Branch: fix/add-git-sync-trinity-standdown → main (merge-base diff)
Files changed: 6 (+67 / −20)
Scope: CLEAN for Abilityai/trinity-skills#1. A residual outside the diff contradicts the ruling (I1).
Plan completion (trinity-skills#1 checklist):

  • Platform detection: DONE. There is a stand-down block at the top of all four templates/*.sh, and the installer prints a notice in Step 1.
  • Existing installs: CHANGED. The runtime check ships in the scripts, but old copies only pick it up when the skill is re-run (Step 3 diffs them). Until then, touch .git/NO_AUTOSYNC stops an old copy; that's documented in Notes. An old script can't learn a check it doesn't contain, so this is the honest limit.
  • Drop the negation step: DONE (.gitignore block).
  • Version, changelog, banner: DONE (1.4 and 1.16.6).

Execution coverage (Step 2.5)

Changed symbol Executed by Live consumer Verdict
stand-down block in git-sync.sh, git-sync-local.sh, git-session-start.sh, git-pre-compact.sh manual repro only (real repo + bare remote, env set / unset / one var set; and inside a live Trinity container) the Stop / SessionStart / PreCompact hooks of every agent that installs the skill ⚠️ no automated test, see I3
SKILL.md Steps 1, 5 and 8 — the wizard run 🛡 doc

Critical findings

None.

Informational findings

[I1] Residual: trinity:onboard still tells people the hooks are for Trinity-deployed scheduled agents (confidence 9/10)
File: plugins/trinity/skills/onboard/SKILL.md:983 (and :1020, :602–609)
Evidence:

  • :983: "Installs three hooks that auto-commit on session end… Ideal for Trinity-deployed agents running scheduled tasks."
  • :602–609 still teaches the .claude/settings.json + !.claude/settings.json escape hatch that this PR removes from add-git-sync.

After this PR the hooks exit immediately inside a Trinity container, so onboarding advertises the one place they no longer act.
Suggestion: in the same PR or a paired one on the trinity plugin, say the hooks are for local sessions and that the deployed agent uses Trinity's auto-sync (Settings → Git sync). Drop the negation paragraph once Abilityai/trinity#3019 ships, and bump trinity to 2.11.2.

[I2] Version collision with #23 (confidence 8/10)
File: plugins/agent-dev/.claude-plugin/plugin.json
This PR bumps 1.16.5 → 1.16.6. #23 (stacked on #22/#21) bumps agent-dev to 1.17.0. Both edit the same line.

[I3] No automated test for the stand-down (confidence 7/10)
The repo has no test harness for hook templates, and the evidence in the PR is a manual repro. Someone later editing git-sync.sh could move the block below git add -A without anything going red.
Suggestion: a tiny plugins/agent-dev/skills/add-git-sync/tests/standdown.sh that renders each template, runs it with and without the two variables against a temp repo and bare remote, and asserts HEAD and the remote are unchanged. That's the repro from the PR, committed, and later edits could run it.

Clean categories

  • Credential exposure: none; the scripts read only the presence of two variables and never print them.
  • Behaviour outside Trinity: unchanged. Both variables are required; with one set (tested) or none, the hooks commit and push exactly as before.
  • Idempotency: the installer's diff-and-ask on existing hooks still applies, and a re-run on a 1.3 install shows exactly the new block.
  • Escape hatches: .git/NO_AUTOSYNC and SELF_SELECT_MSG are unchanged; the stand-down check runs before them.
  • Merge order: stated in the PR (after trinity#3019 is released), and the reason is correct. Until then Trinity still ignores .claude/settings.json, and the negation is what keeps hook registrations tracked.

Low confidence (appendix)

  • (5/10) Stop and SessionStart now exit before reading the hook payload from stdin (they used to INPUT=$(cat) first). If the host ever reports EPIPE on an unread stdin as a hook error, it would show as noise in the container. git-pre-compact.sh never read stdin, which suggests the host tolerates it. Moving the block below INPUT=$(cat) in the two scripts that read it costs nothing.

Summary

  • Critical: 0
  • Informational: 3. I1 (onboard contradicts the ruling) is the one to fix alongside; I2 is a merge-order note; I3 is a test follow-up.
  • Scope: clean. The residual is in the sibling trinity plugin.

🤖 Generated with Claude Code

vybe added a commit that referenced this pull request Sep 30, 2026
@vybe @claude
fix(marketplace): Trinity dev 863240f3 truth sync — native asks, role…
16ea364 
… addressing, settings.json committable, working-branch default, skill-manager fence

Platform range dev 1a1deb2b..863240f3 (ops feed to 0378f2550 + 47 commits read source-direct).

- create-agent 2.1.2: custom 1.15 + review 1.9 — .claude/settings.json is no longer
  gitignored or required-ignored (ent#708 content guard); playbook-gap escalation via
  ask_operator/get_my_ask (ent#611); token tiers for the working-branch default (ent#705);
  report `to` role (ent#606); report guard matches the #2975 refusal wording
- trinity 2.11.2: onboard 6.4 (settings.json scaffold + escape hatch removed, create_agent
  `kind` + git_mode, autonomy person-only #2996, claude-opus-5-5 CLI floor, ask_operator,
  chain depth #2806), loop 1.10 (timeout is the owner's act), sync 2.7.2, connect 1.7
  (JWT-only key minting), README
- agent-dev 1.16.6: create-playbook 2.18 (no respond_to_operator_queue — person-only;
  get_my_ask; report `to`), add-pipeline 1.8 (tick/recover via ask_operator/get_my_ask,
  depth refusal halts), add-orchestrator 1.32 (reconcile-skill-map enforces the ent#596
  fence + set:<name> ent#530; orchestrate notify-as-role + depth + start_agent lines;
  sync-fleet-to-head null counts unknown #2105), agent-fleet-analysis 2.5

Open PRs #21-#24 untouched; #24 also sets agent-dev 1.16.6 — rebase to 1.16.7 on merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
obasilakis
obasilakis reviewed Oct 23, 2026
View reviewed changes

@obasilakis obasilakis left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main already ships this fix in 06679ae ("Trinity 1.0.0 truth sync"), which guards all four hook templates with [ -d /opt/trinity/hooks ]. This PR now conflicts in SKILL.md and plugin.json (1.16.6 here vs 1.23.1 on main), and the four templates merge cleanly, so a naive merge leaves each script with two guards.

Notes on the delta:

  1. .gitignore. The PR drops the .claude/settings.json + !.claude/settings.json pair. main keeps both on purpose: Trinity versions before 1.0 still ignore and untrack that file, which holds the hook registration. An agent installed from this PR and deployed to a pre-1.0 instance loses its hooks on the next platform push. Needs a team call on whether pre-1.0 is still supported.

  2. Detection. The PR keys on TRINITY_BACKEND_URL + AGENT_NAME. TRINITY_BACKEND_URL is set only when an MCP key exists (crud.py:2200-2207, lifecycle.py:1754-1757), never on the system agent (system_agent_service.py:413, pull_mode.py:119-125), and it is a container env var not written to any profile, so an SSH-started session likely does not see it. /opt/trinity/hooks comes from the base image (docker/base-image/Dockerfile:234) and is present in every container.

  3. Installer notice. The step 1 "this is a Trinity-deployed agent" note is the one piece main lacks. Worth keeping, switched to the /opt/trinity/hooks check.

Shell behaviour verified: all four templates pass bash -n; with both vars set HEAD and remote are unchanged; with one, none, or an empty TRINITY_BACKEND_URL they commit and push as before.

Suggest closing as superseded, or rebasing down to the installer notice only.

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

Reviewers

1 more reviewer
@obasilakis obasilakis obasilakis left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add-git-sync: stand down on a Trinity-deployed agent — the platform heartbeat owns durability there

2 participants

@dolho @obasilakis