feat(git-sync): ask "what is this repository?" at create (trinity-enterprise#704) - #3022
Conversation
…rise#703)
Invariant G3: human and fleet work reaches the agent within a bound.
Nothing inside the container ever pulled; the 2026-09-24 audit found
agents up to 31 commits behind.
- agent server: a pull loop beside the push loop, gated per cycle on
the owner's pull_sync_enabled (read live, GIT_SYNC_PULL fallback),
interval GIT_SYNC_PULL_INTERVAL_SECONDS (defaults to the push one).
Under _REPO_LOCK, never while an execution runs (checked again right
before the tree is touched). Fast-forward, or rebase aborted on
conflict. Uncommitted edits are stashed explicitly and a pull that
collides with them is undone - not --autostash, which strands them
in the stash while reporting success.
- pull_sync_enabled + last_pull_at/_status/behind_after_pull on both
migration tracks; backfill on only where auto-sync is on; new github
agents get it at creation (source mode included), GIT_SYNC_PULL env
derived from the DB flag alone at recreate.
- GET/PUT /api/agents/{name}/git/pull-sync; sync-health persists the
pull outcome (bounded); Settings -> Git sync gains the toggle.
Stacked on #3020 (+#3015-#3018): merges after it.
Related to Abilityai/trinity-enterprise#703
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nto feature/ent703-pull-heartbeat
…erprise#704)
The UI and manifest half of the create-time kind that trinity-enterprise#705
added to the API.
- Create modal: AgentKindPicker asks "an agent" or "a deployment of a
codebase" exactly when the create binds git (a GitHub template from the list,
or a custom repo with the clone intent). Never for blank/local, copy, fork or
fork-to-own. Unasked, no kind is sent.
- After create: GitModeNotice renders the response's git_mode and says plainly
when an agent was created pull-only, with the reason.
- Git panel: GitBindingBadge ("Agent · own branch" / "Pull-only") from the new
db_config.source_mode on the git status.
- System manifest: per-agent `kind` is parsed (no unknown-key warning) and
passed to create.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eSQL (trinity-enterprise#703)
The inline comment on `pull_sync_enabled` sat between the comma and the
FOREIGN KEY clause. `_PG_TABLE_SUBS` strips that clause only when it directly
follows the comma, so the clause survived without its REFERENCES and
PostgreSQL rejected the table ("syntax error at or near ')'"), failing 18
requires_postgres tests in schema-parity. The comment moves above the column.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
merge-train: ejected from this train because it is stacked on #3020, which is ejected pending a ruling (see #3020). No conflict with #3021 (merge-tree clean). Mechanical fix to fold in while it waits: |
|
Resolve by running |
|
Resolve by merging |
…e notice (trinity-enterprise#704) The modal swapped to the post-create step only for a custom repo or a fork-to-own template, so a GitHub template picked from the list closed on create and GitModeNotice never rendered. That is the path the kind picker is shown on and the one most likely to come back pull-only, so the warning was lost exactly where it mattered. githubSourced now keys on the template's source; a local or blank create still closes straight away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
merge-train (2026-09-27): one commit pushed, the fix from my 09-25 note. This PR still merges after #3020, which it is stacked on. #3020 is waiting on the probe-versus-ownership ruling. |
…nto feature/ent703-pull-heartbeat # Conflicts: # src/backend/db/migrations.py # tests/registry.json
…ns, brings main to working branches (trinity-enterprise#703) PR #3021 review: - _with_stash wraps every post-stash step: a git child that times out (run_registered raises TimeoutExpired) goes back to the pre-pull HEAD and re-applies the stash; when that is impossible the error says the edits are kept in `git stash`. - reset --hard's return code is checked. A failed undo stops with an error naming it instead of leaving UU files behind silently. - Both cycles refuse to run over unmerged paths. The push cycle checks before `git add -A`, which would stage conflict markers and push them to origin as a successful sync. - The execution gate counts register_pending entries (#2433), so a turn queued on the chat lock or the headless pool is busy, not idle. - The pull reaps stale lock litter first, as the push cycle does; a pull-only agent has no push cycle to do it. - sync-state gains last_pull_error streak fields: consecutive pull failures / skips and last_successful_pull_at. Ruling on the intent question: a trinity/* working branch only ever pulled itself, so human pushes to main never arrived (G3). The cycle now also merges origin/main (via _get_pull_branch) into the working branch — a merge, not a rebase, since the branch is already pushed; a conflict is aborted and recorded. Nine real-repo tests, all red before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… + UI copy (trinity-enterprise#703) - Alembic 0076_pull_sync -> 0080_pull_sync <- 0079_telegram_group_context (one head, 81 revisions); SQLite entry re-appended after dev's. - The same migration pair adds agent_sync_state.last_pull_error, last_successful_pull_at, consecutive_pull_failures and consecutive_pull_skips, persisted by the sync-health poller (bounded, coerced; a success clears the error). ent#706/#707 read these, so they land here rather than as a second migration on the same table. - agent-runtime.md says three loops; the flow and requirement describe the main merge, the queued-turn gate, the unmerged-path refusal, what behind_after_pull measures, and that auto-sync agents already rebase in the push cycle. - Settings copy no longer hard-codes 15 minutes (GIT_SYNC_PULL_INTERVAL_SECONDS) and no longer claims the push cycle never touches the tree while the agent works. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui
…n (trinity-enterprise#704) Follows the #3020 ruling: the platform-wide GitHub token never earns the working-branch default. The picker no longer promises one for any token that can push, and no longer hard-codes the 15-minute interval. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Tests: |
AndriiPasternak31
left a comment
AndriiPasternak31
left a comment
There was a problem hiding this comment.
This is stacked on #3020 (its head 8b20c1714 is an ancestor), so I reviewed only this PR's own commits against it: 94301a8b4, a87f3f115, e83fe51f8, plus the 6d7285457 merge. The picker gating, the payload and the manifest passthrough are right. The problem is that the notice and the badge decide "pull-only" from source_mode alone, and fork-to-own agents are source_mode=true but do push. So both surfaces now tell every fork-to-own user something false.
-
[blocking]
src/frontend/src/components/GitModeNotice.vue:38,src/frontend/src/components/GitBindingBadge.vue:12: fork-to-own agents are shown as pull-only._apply_fork_to_ownsetsconfig.source_mode = True(crud.py:893), and_git_auto_sync_bakedstill bakes auto-push for it becausefork_upstreamis set. The agent pushes to its fork's default branch.- The create response for that path is
{kind: "agent", source_mode: true}.kinddefaults toagentbecause the picker is hidden on fork paths, so nokindis sent. - As a result,
fellBackis true and the post-create step shows the warning-coloured "Created pull-only". Both fork paths reach that step: the custom-repo fork intent did already, and sincea87f3f115, so does a featured fork-to-own template from the list. I mounted the notice with that payload and it renderedCreated pull-only / source_mode set explicitly. - On the Git panel, the badge for the same agent says "Pull-only: this agent tracks the branch and never pushes to it."
- Failure scenario: someone forks a template so it can keep its work, and the platform tells them it won't.
- Fix: have the backend say whether the agent pushes rather than inferring it from
source_mode. For example, add apushesflag togit_modefrom_git_auto_sync_baked, and ondb_configeither an equivalent flag or a fork marker. Then keyfellBack, the headline and the badge on that flag. Add a fork-to-own case tocreateAgentKind.spec.js. It would have caught this. - Related, and belongs in #3020: the reason shown for a fork create is "source_mode set explicitly", not "fork-to-own: the agent owns its fork". Pydantic v2 adds the attribute that
_apply_fork_to_ownassigns tomodel_fields_set, so the explicit-source_modebranch in_apply_agent_kind_defaultfires first and the fork branch is unreachable. I checked this in the test venv. Now that this PR shows the reason text to users, it matters more.
-
[should-fix]
src/backend/services/system_service.py:1114(export_manifest):kinddoes not round-trip.- This PR makes
kinda manifest key, but export never writes it. - Failure scenario: export a system whose members were deployed as
kind: deploymentand redeploy it. The creator's own token can push, so those members come back as agents with working branches and auto-push into the product repos they were meant to only pull from. - The #2373 D3 tests treat export→redeploy round-trip as the contract.
- Fix: emit
kind: deploymentfor a source-mode, non-fork member, or persistkind, and add an export assertion. If you'd rather not do it here, call the gap out insystem-manifest.mdand open a follow-up.
- This PR makes
-
[nit]
src/frontend/src/components/AgentKindPicker.vue:58: the "An agent" description is one ~60-word sentence joined by semicolons and dashes, with three separate caveats (own token, platform token doesn't count, use Fork for someone else's template). Splitting it into two short lines, or moving the caveats into the post-create notice, would make the choice readable at a glance. -
[nit] The
deploy_systemMCP tool describes the manifest format inline (src/mcp-server/src/tools/systems.ts:130-140) and doesn't mentionkind. An agent composing a manifest through MCP won't know the key exists. One line there would fix that.
What I checked
- Stack:
git merge-base --is-ancestoragainst #3020's head. Reviewed the own-commit diff only (17 files). - Backend, run in the test venv with seeds 12345 and 99999:
test_ent704_git_status_binding.py,test_2373_system_endpoints.py,test_ent125_resilient_system_deploy.py,test_1484_create_agent_characterization.py. 119 passed on both seeds. - Frontend:
createAgentKind.spec.jsplus the raw-colour and loading-gate ratchets, 30/30. I also ran a throwaway spec that mountedGitModeNotice/GitBindingBadgewith a fork-to-own payload, which confirmed finding 1. - CI: 23 pass, 5 skipped.
- Security:
git_modestill rides the create response only, not the/wsbroadcast. The newdb_config.source_modefield is behind the existing agent-scoped auth on git status. No token handling or PAT-gate change in this PR's own commits, and no schema change. - Design contract: only semantic
action-*/status-*tokens plus gray.
#3005 landed Alembic 0080_agent_skill_sets on 0079_telegram_group_context, the same parent this branch's 0080_pull_sync used, so merging as-is would leave two heads and `upgrade head` would apply nothing. Renumber the revision to 0081_pull_sync, chained on 0080_agent_skill_sets (file, revision, down_revision, docstring, the SQLite mirror note and the revision test). SQLite MIGRATIONS keeps dev's agent_skill_sets entry first, pull_sync after it. tests/registry.json keeps both entries. No logic change.
…nto feature/ent703-pull-heartbeat # Conflicts: # tests/unit/test_1484_create_agent_characterization.py
…p merges (trinity-enterprise#703) PR #3021 re-review: - A conflict merging main was recorded as "Auto-merging <file>": git prints it on stdout with an empty stderr. Unmerged paths are read before the abort; the error is "diverged: merge conflict with main (<files>)", and a failed merge --abort is named. - `git rebase` flattened the merge the pull made into copies of main's commits; both cycles now rebase with --rebase-merges. - A failed strict ahead/behind count fails the pull instead of reading as up to date. - A timed-out step is reset to the pre-pull HEAD even with nothing stashed; a reset blocked by the killed child's index.lock says so. - "Never runs while the agent is working" is check-then-act: the copy and docs now say "never starts" and document the admission window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui # Conflicts: # docs/memory/requirements/github.md
…rt round-trips kind (trinity-enterprise#704) PR #3022 review: - Fork-to-own agents are source_mode=true but auto-push to their own fork, so GitModeNotice showed "Created pull-only" and GitBindingBadge "Pull-only" for every fork. The create response's git_mode now carries `pushes` (the _git_auto_sync_baked predicate the env is baked from) and the git status's db_config carries `pushes` (not source_mode, or auto-sync on); both components key on it. Fork-to-own reads "Agent · own repo". - export_manifest writes `kind: deployment` for a pull-only git member, so export -> redeploy never hands a deployment the agent default. - The kind picker's agent description is split into two short lines. - deploy_system's manifest format documents `kind`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent703-pull-heartbeat # Conflicts: # tests/registry.json
…nto feature/ent704-create-kind-ui
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review of 09-28 addressed. 1. [blocking] Fork-to-own shown as pull-only.
2. [should-fix]
3. [nit] Picker description. Split into two short lines: what an agent is, then "Needs a repository you own and your own GitHub token (Settings), or it is created pull-only. For someone else's template, choose Fork." This now also reflects #3020's owner == token-login rule. 4. [nit] MCP Verification
🤖 Generated with Claude Code |
…, wrap header, name the auto-sync toggle limit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@obasilakis, your non-blocking points are addressed in the latest commit on this branch.
Also since your approval: the branch has merged the updated #3021, so it carries current dev and #3021's @AndriiPasternak31, the 09-28 changes-requested items were addressed in 🤖 Generated with Claude Code |
|
✅ Alembic head check clear — merging this PR into Previously flagged; resolved. Evaluated on the version line only — this PR also conflicts with Advisory — this check does not block merge. · head_sha: |
AndriiPasternak31
left a comment
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Thanks, the 09-28 points are addressed. Re-checked at d43337aa6.
-
[was blocking] Fork-to-own shown as pull-only: fixed.
git_mode.pushesis now the same_git_auto_sync_bakedpredicate the env is baked from (crud.py:1582-1593), so the notice and the container can't disagree.db_config.pushesis atrouters/git.py:135.GitModeNotice.vueandGitBindingBadge.vuekey onpushes, and fork-to-own reads "An agent that owns its repository" / "Agent · own repo". Mounted fork-to-own cases are increateAgentKind.spec.js. The unreachable fork reason is fixed upstream in #3020 (d8a9fd0f8) and asserted end to end intest_1484. -
[should-fix]
kindround-trip: fixed for the normal case, one residual.export_manifestwriteskind: deploymentfor source-mode members with auto-sync off (system_service.py:1194-1206), and the new round-trip test re-parses the export. The residual is the "Known limit" you documented insystem-manifest.md.PUT /git/auto-sync(routers/git.py:1188) has no source-mode guard, so an owner who turns auto-sync on for a deployment gets two wrong results:- The badge says "saves its work to its default branch" while the heartbeat refuses to push.
- Export drops
kind. A redeploy by a creator who owns the repo and has their own token then gets a working branch plus auto-push. That's the original failure, now reachable through one owner-side toggle.
Documenting it is fine for this PR, but please file the follow-up (a persisted fork marker on
agent_git_config, then reject the toggle for non-fork source-mode agents) and link it from the Known-limit paragraph. I couldn't find an open issue for it. -
[nit] Picker copy: fixed. Split into
desc+note(AgentKindPicker.vue:59-60). -
[nit] MCP
deploy_systemformat: fixed.kinddocumented (systems.ts:137).
Before merge (not code in this PR):
- Alembic head fork vs current dev. dev now has
0082_agent_sync_state_divergence(#3035). This branch carries #3021's0082_pull_sync, which also chains off0081_portal_messages_unread_idx, so that's two heads, andupgrade headwould apply nothing (Invariant #3; the alembic-head-watch comment shows it). The fix belongs in #3021: rechain0082_pull_sync→0083_…off0082_agent_sync_state_divergenceon both tracks. Then re-merge here. - CONFLICTING with dev:
requirements/github.md,db/migrations.py,db/schema.py,db/sync_state.py,db/tables.py,sync_health_service.py,tests/registry.json. Most of these come from the #3021/#3035 overlap.
What I ran (worktree at d43337aa6):
- Backend:
test_ent704_git_status_binding.py,test_2373_system_endpoints.py,test_1484_create_agent_characterization.py,test_ent125_resilient_system_deploy.py. 136 passed on seed 12345; also green on seed 99999. - Frontend:
createAgentKind.spec.jsplus the raw-color and loading-gate ratchets, 32/32.
| # that population (creation turns auto-sync on for a source-mode | ||
| # agent only when it forked); a working branch pushes even while | ||
| # its auto-sync is paused (operator Push). | ||
| "pushes": (not git_config.source_mode) or bool( |
There was a problem hiding this comment.
pushes = not source_mode or auto_sync_enabled is right today, but it's an inference. set_auto_sync_config (L1188) lets a deployment's owner flip it, and then this says "pushes" for an agent the heartbeat won't push from. Please link the follow-up issue in this comment.
| # mode AND auto-pushes) exports as a deployment, so a redeploy never | ||
| # hands it the agent default's working branch + auto-push. An agent | ||
| # that pushes, or has no git binding, exports no `kind` (the default). | ||
| try: |
There was a problem hiding this comment.
Same inference as git.py:135. With auto-sync toggled on, a deployment exports with no kind and redeploys as an agent. The known limit is documented, so this is fine for now. Please link the follow-up issue.
… revision to 0083 dev gained 0082_agent_sync_state_divergence (#3035), which adds its own agent_sync_state columns. Union both column sets across schema/tables/ sync_state/sync_health_service, chain 0083_pull_sync off dev's head, and order the SQLite entry after agent_sync_state_divergence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui
#2996's route census (merged to dev) requires every new route to be classified. The agent's pull loop reads GET .../git/pull-sync with its own key each cycle (AGENT_CALLABLE); the PUT is a setting write, so it takes Depends(require_person) like the other #2996 settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui
|
merge-train: not on this train. It is blocked on #3021, which it contains in full, including #3021's open data-loss path. This PR's own delta has no criticals, and its Items to consider once #3021 is fixed:
|
dev moved past 0082 (0083 to 0085 landed), so 0083_pull_sync forked the Alembic graph into two heads, which alembic-head-watch flagged. It is now 0086_pull_sync on 0085_ent720_email_identity, and the SQLite list keeps both sides, with dev's entries first and pull_sync after. The merge also brings #3107, the agent-server boot fix whose absence failed journey-smoke. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…estatement (#3021) dev gained 0086_metric_points_restatement off 0085, so this branch's 0086_pull_sync was a second head and alembic-head-watch failed. Renamed to 0087_pull_sync and re-parented; the SQLite list keeps dev's entry first, matching the Alembic order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui
dolho
left a comment
dolho
left a comment
There was a problem hiding this comment.
Review — CI fixed, one UX issue worth fixing before merge
CI: alembic-head-watch was red because dev gained 0086_metric_points_restatement off 0085, making the stacked 0086_pull_sync (from #3021) a second head. Fixed on #3021 (renamed to 0087_pull_sync, re-parented onto 0086_metric_points_restatement) and merged into this branch (0d4615924). Locally: check_alembic_heads → 1 head, check_alembic_parity PASS; submodule pointers equal dev's.
Overall: solid. pushes comes from one predicate (_git_auto_sync_baked), the Git panel/status rule and the export rule are exact complements, so fork-to-own no longer reads as pull-only. No new endpoint; the ent#162 PAT-tier gate in _apply_agent_kind_default is unchanged.
Medium — the kind picker offers an answer that cannot happen on list templates
CreateAgentModal.vue:438-441 (showKindPicker) shows the picker for every source === 'github' list template, defaulting to "An agent — saves its work to its own branch". But every list template is a catalog entry: crud.py:1595 passes catalog_template=True, and _apply_agent_kind_default (crud.py:981) then always returns pull-only ("a shared catalog template: pull-only. Fork it…"). So a creator with their own PAT picks the default and always lands on the amber "Created pull-only" notice. The agent option's note says "choose Fork", but non-required list templates have no fork option here.
Fix (either): show the picker only for github-custom + clone (the one path where "agent" can produce a working branch); or on the list path default to deployment and change the agent option's note to "Catalog templates are always pull-only — fork to keep this agent's work in git". createAgentKind.spec.js ("is asked for a GitHub template, defaults to an agent") pins the current behaviour and would move with it.
Low
- Export collapses "asked for agent, fell back" (
system_service.py:1194-1206): an agent createdkind=agentthat fell back to pull-only exports askind: deployment, so a redeploy after the creator adds a token stays pull-only. Safe direction and documented; worth a line in the PR description (or persistkindlater).
Nits
GitBindingBadge.vue:335— fork-to-own with auto-sync off shows "Pull-only … never pushes to it", but an operator Push may still work; "auto-sync off" would be truer.ImportValidationStep.vue:39— the notice sits between theh3and the copy-provenance line; putting it after keeps the heading and its subtitle together.GitModeNotice.vue:386— falls back to!source_modewhenpushesis absent (older backend), the exact misread this PR fixes; a neutral "unknown" headline would be more honest.- List GitHub templates now go through
ImportValidationStep(compatibility check after/info,CreateAgentModal.vue:287) — intended, but beyond the issue's stated scope; one line in the description.
Checked clean: semantic status-warning-* / action-primary-* tokens in both themes (raw-colour ratchet passes), native radio group in fieldset/legend, invalid manifest kind → 400 / preview failure, MCP systems.ts example updated (Invariant #13), backend tests cover fork-to-own pushes=True and the status binding. createAgentKind, rawColorRatchet, loadingGateRatchet specs: 32/32 pass.
🤖 Generated with Claude Code
…tarving each other (#3021 review) The two background loops started together on one interval and took _REPO_LOCK non-blocking, so the loser skipped silently every tick and the effective pull bound could quietly double. Background cycles now wait up to GIT_SYNC_LOCK_WAIT_SECONDS (default 120 s) for the lock, and the pull loop's first tick is half an interval after the push loop's. Operator endpoints keep their immediate 409. The pull's reset --hard undo re-checks that no execution started meanwhile; if one did, the tree is left as it is (conflict markers block the next push and pull) instead of discarding the turn's writes. merge --abort runs only when MERGE_HEAD exists, so a merge that refused to start no longer records "There is no merge to abort". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nto feature/ent704-create-kind-ui
|
merge-train (2026-10-02): not on this train. #3021 merges today. This PR rides the next train once fixed, and it will need Needs your call: your own 09:47 finding is still open at
Mechanical, for the same push:
Verified at this head: |
vybe
added
the
status-needs-fix
|
@dolho still outstanding from 10-02 (no push since the
|
…kind-ui # Conflicts: # docker/base-image/agent_server/auto_sync.py # docker/base-image/agent_server/routers/git.py # docs/memory/requirements/github.md # src/backend/db/migrations.py # src/frontend/src/components/GitSyncSettingsPanel.vue # tests/unit/test_1484_create_agent_characterization.py
…an matter (#3022 review) Every list template is a catalog entry, which _apply_agent_kind_default always makes pull-only, so the kind picker there defaulted to an outcome that could not happen. It now shows only for a custom-repo clone. Review nits: GitModeNotice stays neutral when the response lacks `pushes` instead of guessing from source_mode; GitBindingBadge says pull-only is about auto-sync; the notice sits after the provenance line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Follow-up to the 10-02 review:
Not done: the Low item about export collapsing "asked for agent, fell back to pull-only". It's still documented behaviour. 🤖 Generated with Claude Code |
github-actions
Bot
removed
the
status-needs-fix
/review — automated pre-landing review (at
|
| changed symbol / test file | executed by | live consumer | verdict |
|---|---|---|---|
showKindPicker / payload.kind (CreateAgentModal.vue:441,538) |
createAgentKind.spec.js (mounted, jsdom) |
CreateAgentModal.vue:266 |
✅ executed |
githubSourced → postCreate.gitMode |
createAgentKind.spec.js |
CreateAgentModal.vue:337 → ImportValidationStep |
✅ executed |
GitModeNotice.vue pushes/fellBack/headline |
createAgentKind.spec.js (mounted) |
ImportValidationStep.vue:50 |
✅ executed |
GitBindingBadge.vue |
createAgentKind.spec.js (mounted) |
GitPanel.vue:210 |
✅ executed |
db_config.source_mode / .pushes (routers/git.py:126-137) |
test_ent704_git_status_binding.py (calls the handler) |
GitPanel.vue:210 |
✅ executed |
git_mode["pushes"] (crud.py:1609-1613) |
test_1484_… ×3 through create_agent_internal |
GitModeNotice.vue:435 |
✅ executed |
manifest kind parse / deploy / export |
test_2373_… ×3, test_ent125_… ×1 (real functions) |
deploy_manifest → AgentConfig.kind |
✅ executed |
Source-text grep over the changed tests: no hits.
Fix mutations, each reverted in a scratch copy:
- (a) Re-widened
showKindPickerto include list GitHub templates. Red: "is not asked for a catalog GitHub template, which is always pull-only". - (b) Restored the
pushes ?? !source_modeguess inGitModeNotice. Red: "stays neutral when the response does not say whether the agent pushes".
The obasilakis 09-29 review already reported 11/11 mutations red for the earlier fixes.
Tests run at this head:
- Backend:
test_ent704_git_status_binding,test_2373_system_endpoints,test_ent125_resilient_system_deploy,test_1484_create_agent_characterization. 136 passed. - Frontend:
createAgentKind+rawColorRatchet+loadingGateRatchet+sourceTextRatchet. 42/42 passed.
Critical Findings (block merge)
None.
Informational Findings (review required)
[I1] Scope / conflict resolution: the dev merge reverted #3021's honest data-loss disclosure, so user-facing copy and docs now overclaim (Confidence: 9/10)
Files (all on dev now):
src/frontend/src/components/GitSyncSettingsPanel.vue:76docker/base-image/agent_server/routers/git.py:1180-1182, 1257-1258docker/base-image/agent_server/auto_sync.py:187-188docs/memory/requirements/github.md:~901
Evidence. This PR's merge diff replaces #3021's final wording (squash c3ba98a63) with the older pre-review text:
- agent is working or has a turn queued, and a conflicting pull is
- undone with the agent's edits put back. Edits made outside agent
- turns (uploads, terminal) while a pull runs are not protected. ...
+ agent is working or has a turn queued, and never discards its own
+ changes. ...
- - Refuses to run over unmerged paths. An undo never resets over a
- registered execution (`_safe_to_reset`); writes outside the process
- registry (Files API, docker exec, web terminal) made during the
- integrate window are not protected. ...
+ - Refuses to run over unmerged paths. Local work is never discarded; ...
The code did not change. _safe_to_reset (agent_server/routers/git.py:595) still says reset --hard "discards every tracked-file write since the stash" and guards only registered executions. Files API, terminal and docker exec writes made during the integrate window can therefore still be lost.
Issue: none of these lines are part of #704. They come from the conflict resolution in 806b538bf ("Merge remote-tracking branch 'origin/dev'"), which took the branch side instead of dev's side on the #3021 files. The author's 10-05 comment says each conflict was resolved "as dev's version plus this PR's own delta", but for these four hunks it wasn't. The Settings panel now makes the exact promise that #3021's review asked to be withdrawn.
Fix: a small follow-up PR against dev that restores c3ba98a63's text in all four places. git show c3ba98a63 -- <file> holds the intended wording. No code change is needed.
Why: an operator who trusts "never discards its own changes" may upload or terminal-edit during a pull and lose work without warning.
[I2] Design-system contract: GitBindingBadge.vue hand-rolls a badge instead of composing BaseBadge (Confidence: 8/10)
File: src/frontend/src/components/GitBindingBadge.vue:11-16
Evidence: <span ... class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium bg-gray-100 text-gray-700 dark:bg-gray-700 dark:text-gray-300". The contract (design-system-contract.md:27) lists BaseBadge and says "Never hand-roll a lookalike". components/base/BaseBadge.vue:32 has a neutral variant.
Suggestion: <BaseBadge variant="neutral" :title="title" data-testid="git-binding-badge">{{ label }}</BaseBadge>. vybe raised this on 10-02 and it is still unaddressed.
[I3] Docs: db_config.source_mode / pushes not documented in the API catalog (Confidence: 8/10)
File: docs/memory/architecture/api-endpoints.md has no entry for the new db_config fields on GET /api/agents/{name}/git/status. Grepping for git/status there finds no line that mentions them.
Suggestion: add one line: db_config carries source_mode and pushes (not source_mode or auto_sync_enabled, ent#704). Raised 10-02 and still open.
[I4] Known limit has no tracking issue (Confidence: 8/10)
File: docs/memory/feature-flows/system-manifest.md (Known limit paragraph) and routers/git.py:131-137.
Evidence: pushes = (not source_mode) or auto_sync_enabled infers fork-to-own. PUT /git/auto-sync accepts the toggle on a deployment, after which the badge says it saves to its default branch and export drops kind: deployment. No issue matching "fork marker" exists in either tracker, and the paragraph links none.
Suggestion: file the follow-up that AndriiPasternak31 and vybe asked for (persisted fork marker on agent_git_config, then reject the auto-sync toggle for non-fork source-mode agents) and link it as (#N) from both places.
[I5] Process: PR body links the issue with Related to, not Fixes (Confidence: 7/10)
Evidence: PR body line 36 reads Related to abilityai/trinity-enterprise#704. vybe asked on 10-03 for Fixes. The PR is merged, so ent#704's status-in-dev / closure now has to be set by hand.
[I6] Test hygiene: duplicated helper in test_ent704_git_status_binding.py (Confidence: 6/10)
File: tests/unit/test_ent704_git_status_binding.py:46-58. test_git_status_db_config_carries_the_binding re-inlines the monkeypatch that _status() (:33-42) already provides. Cosmetic only.
Prior review findings (status at 76c5470eb)
| Source | Finding | Status | Evidence |
|---|---|---|---|
10-02 review, Medium (GitHub shows it as dolho's COMMENTED review of 10-02 09:47; same content as the brief's "10-02 kind-picker" finding) |
Picker offered "An agent" on catalog list templates, which are always pull-only | ✅ Resolved | CreateAgentModal.vue:441 now reads form.template === 'github-custom' && importIntent.value === 'clone'. Mutation (a) above turns the new test red. crud.py:1595 catalog_template=bool(gh_template) confirms list templates are catalog. |
| 10-02 Low | Export collapses "asked agent, fell back" into kind: deployment |
Author's 10-05 comment says so. Documented in system-manifest.md. Safe direction. |
|
| 10-02 nit | Badge title "never pushes" was misleading | ✅ Resolved | GitBindingBadge.vue:383: "auto-sync does not push its work". Pinned by a test. |
| 10-02 nit | Notice placed between h3 and the provenance line |
✅ Resolved | ImportValidationStep.vue:48-50: after the provenance </p>. |
| 10-02 nit | Notice guessed !source_mode when pushes was absent |
✅ Resolved | GitModeNotice.vue:435 pushes ?? null gives the neutral "Created from GitHub". Mutation (b) turns it red. |
| 10-02 nit | List templates now go through ImportValidationStep (beyond scope) |
✅ Described | PR body and github-sync.md describe the post-create path. |
| 09-28 AndriiPasternak31 (blocking) | Fork-to-own shown as pull-only | ✅ Resolved (re-approved 09-29) | crud.py:1609-1613 git_mode.pushes from _git_auto_sync_baked; routers/git.py:136. Mounted fork cases in spec. |
| 09-28 (should-fix) | kind round-trip on export |
✅ Resolved, with a known limit | system_service.py:1194-1206. Limit tracked in I4. |
| 09-28 nits | Picker copy; MCP deploy_system doc |
✅ Resolved | AgentKindPicker.vue:247-248; systems.ts:137. |
| vybe 10-02 mechanical | BaseBadge; api-endpoints.md line; follow-up issue; Fixes in body |
❌ Open | I2, I3, I4, I5. |
| vybe 10-02 mechanical | dark:text-status-warning-300 → -400 |
GitModeNotice.vue:418. Contract :20 says dark status text is 400, but :9/:32 prescribe 300 text on a dark tinted ground, which is this case. Left to the design owner. |
|
| alembic-head-watch | Two heads | ✅ Clear | Bot reports one head (0088_skill_gate_requests). |
Clean Categories
- SQL & data safety: no SQL or schema change.
export_manifestreadsdb.get_git_configinsidetry/exceptwith a warning (system_service.py:1200-1206). - Race conditions: none introduced.
will_pushis computed once and reused (crud.py:1609,1624). - Auth boundaries: no new endpoint.
db_configfields ride the existing agent-scopedget_git_status.git_modestays on the create response only, not on/ws(feat: agent-reported structured reports via MCP + dashboard display #918). Manifestkindgoes through_apply_agent_kind_default, so the ent#705 PAT gate is unchanged. Out-of-rangekindgives aValueError(testtest_manifest_kind_outside_the_two_values_is_rejected). - Credential exposure: none. The test PAT is a placeholder.
- Enterprise disclosure (4.5): the guard pattern over added
docs/lines finds 0 hits. - Enum completeness:
kindisLiteral["agent","deployment"](models.py:1266), and_KNOWN_AGENT_KEYSwas updated. - Invariant feat: SMARTS trading pipeline with Telegram notifications and Miro visualization #13 (MCP):
systems.tsmanifest example updated. - Frontend tokens / ratchets: only gray,
action-primary-*andstatus-warning-*. Raw-colour and loading-gate ratchets pass. Nov-html.
Summary
- Critical: 0 — none found
- Informational: 6 — I1 should be fixed promptly as a small follow-up PR because the reverted copy is live on
dev; I2–I4 are the outstanding 10-02 mechanical items - Scope: drift (I1, from the conflict resolution)
Suggested learning / deferred debt: after a stacked PR's base squash-merges, resolving the follow-on dev merge "branch side" on the base's files silently reverts review-driven changes the squash carried. Diff each conflicted file against the squash commit (git diff <squash> HEAD -- <file>) before committing the merge. (#3022 / 806b538bf vs #3021 c3ba98a63)
🤖 Generated with Claude Code
…ollow-up) (#3226) The dev merge 806b538 in #3022 resolved its conflicts on the #3021 files by taking the branch side, which reverted #3021's (c3ba98a) review-driven wording back to "never discards its own changes" / "never discards local work". That is false: _safe_to_reset only spares registered executions, so Files API, web terminal and docker exec writes made while a pull integrates can still be lost. Restores #3021's wording in the four places (Git sync settings panel copy, _integrate_remote and _run_pull_once docstrings, the auto_sync pull-loop comment, requirements/github.md) and keeps everything #3022 legitimately added. Adds a mounted vitest assertion so the panel cannot regain the claim. No behaviour change. Related to #3022, #3021 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
5 participants
Summary
The UI and manifest half of trinity-enterprise#704. trinity-enterprise#705 (#3020) added the create-time
kind(agent/deployment) and thegit_moderesult to the API. This PR asks the question where people create agents, and shows the result.AgentKindPicker.vueasks "What is this repository?" with two answers, An agent or A deployment of a codebase.kindis sent, so the API default applies.ImportValidationSteprendersGitModeNotice.vuefrom the create response'sgit_mode. When an agent fell back to pull-only, it says so plainly and gives the reason (status-warning tokens).GitBindingBadge.vueshowsAgent · own branchorPull-only. It reads a newdb_config.source_modeonGET /api/agents/{name}/git/status.kind:is a recognised key, so it no longer triggers an unknown-key warning.deploy_manifestpasses it to create. An unsetkindstaysNone, so the create default applies.Tests
src/frontend/tests/unit/createAgentKind.spec.js(mounted, jsdom), 8 tests:createAgentpayloadgit_modereaching the post-create steptests/unit/test_2373_system_endpoints.py: manifestkindis parsed without a warning; an out-of-range value is rejected.tests/unit/test_ent125_resilient_system_deploy.py: the declaredkindreaches the create call; an undeclared one staysNone.tests/unit/test_ent704_git_status_binding.py:db_config.source_modefor both bindings.check:tokens. Related backend suites: 148 passed.Docs
feature-flows/github-sync.md: wherekindis chosen.feature-flows/system-manifest.md: the example showskind.requirements/github.md§11.17: extended, not renumbered, because feat(git-sync): the container pulls origin on its own (trinity-enterprise#703) #3021 adds §11.18.tests/registry.json: new entry.Related to abilityai/trinity-enterprise#704
🤖 Generated with Claude Code