Conversation
Trinity persists every agent's remote as `<scheme>://oauth2:<PAT>@<host>/<org>/<repo>.git`, which puts the platform GitHub token in `.git/config` on the workspace volume and — because git expands the stored URL into `git-remote-https`'s argv — in the container's process table on every fetch and push. From argv it reaches `ps`, the orphan sweep's reaped-cmdline logging, Vector, the host log files and the logs API, i.e. another agent's LLM context. Git speaks the credential protocol to a helper over stdin/stdout, which never reaches argv and is never persisted. This is the helper plus its two string builders; the producers still build token URLs and are converted next. Three things are load-bearing and each is proven, not asserted (git 2.50.1): - Registered as `trinity`, NOT the filename. Git prepends `git-credential-` to any helper value that is not an absolute path, so registering `git-credential-trinity` resolves to `git-credential-git-credential-trinity`: `git: 'credential-git-credential-trinity' is not a git command`. The helper would never run, and with token-free URLs that is a silent fleet-wide fetch/push outage. - Registered UNSCOPED, host-checked inside the helper. TRINITY_GIT_BASE_URL is a runtime value, so a `credential.<base>.helper` baked at image-build time could only ever name github.com and a self-hosted install would get no helper. Resolving the origin at request time also writes nothing to `~/.gitconfig`, which sits in the agent's repo root and is not ignored. - `.env` FIRST, baked env second — the inverse of startup.sh, deliberately. startup.sh runs at boot where baked env is the freshly-recreated truth; the helper runs in steady state where a rotation (#1967) or a per-agent PAT set after creation (#1264) has live-injected the new token into `.env` while Config.Env is immutable without a recreate. Baked-env-first would authenticate with the revoked token forever. The sweep that reaches existing containers lands with the remediation commit. It probes the helper by EXIT CODE only: `git credential fill` prints the credential on stdout, and this output crosses the docker exec boundary into the platform log. Not closed: an agent reading its own credential. Root ownership is integrity, not confidentiality. That is trinity-enterprise#558 (AAuth). Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four producers built `<scheme>://<userinfo>@<host>/<repo>.git`, not the two the first pass found. All four are converted or deleted, so there is now exactly one remote-URL builder in the product and it carries no credential. 1. `git_service._git_remote_url` — DELETED. Its three call sites (`update_remote_pat`, `rebind_origin_and_push`, `initialize_git_in_container`) use `_credentialless_remote_url`. 2. `startup.sh` CLONE_URL — one form, credential-less. The clone, the fetch and the push authenticate through the `trinity` credential helper. 3. `template_service.clone_github_repo` — DELETED. It passed a token URL as argv to `subprocess.run` on the BACKEND HOST: same bug class, one layer out. No production callers (an export and two test mocks). 4. `skill_service._authenticated_url` — split into `_normalized_url` (no credential) and `_auth_pat_for` (the host decision, unchanged from PR #1901's parse-based check), with the PAT carried in the git child's environment. This one was LIVE: git wrote the spliced URL to `origin`, so the platform PAT sat at rest in `/data/skills-library/*/.git/config` — on the `~/trinity-data` host bind mount, and therefore in every backup and snapshot of it. Ordering is the whole design: **no path strips a credential it has not already replaced.** - `update_remote_pat` writes the token to `.env` over `docker exec` (which works while the agent server is wedged, restarting or OOM — the HTTP write it belts does not), runs the sweep, and only re-points `origin` once a credential provably resolves. - `initialize_git_in_container` SEEDS the credential before anything writes a remote, which is what stops it creating the orphan class it was named for: it used to push with the resolved platform PAT while baking no git env, persisting no row and writing no `.env`, leaving the agent's only credential inside its own origin URL. - `startup.sh`'s origin rewrite becomes conditional. Unconditional was safe only while the replacement URL also carried a token; for an orphan agent it would be destruction of the last credential, at container start, before any backend sweep could harvest it. - `configure_push_remote`'s gate widens from `GITHUB_PAT` to "a credential resolves" — narrowly: the helper reads `.env`, baked env, and the harvest file, which only exists for an agent whose own URL already carried a push credential. An ent#123 tokenless agent resolves nothing and stays blackholed. The harvest deliberately does NOT write `GITHUB_PAT`, which startup.sh exports as GH_TOKEN/GITHUB_TOKEN — that would be a grant. Also: `_agent_has_write_credentials`'s premise ("the global tier is deliberately excluded — a global PAT never reaches a tokenless container's remote") is what this change falsifies, so it is no longer the whole predicate. `_agent_can_push` consults it first and the helper's own ladder second, by exit code, only for agents that already look tokenless. The ent#109 rebind push stays in-container — the history lives only on the workspace volume — but runs as ROOT with the user's PAT in the exec environment, so `/proc/<pid>/environ` is unreadable by the `developer`-uid agent for the 120s push window. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three reachers, no new recurring service: - `sweep_fleet_git_remote_tokens` — a boot-time one-shot. After this change no code path produces a token-bearing remote, so there is no recurring producer for a recurring loop to chase. This is also the only reacher that covers a `restart: unless-stopped` container the Docker daemon brings back after a HOST REBOOT, which never passes through `start_agent_internal`. Fail-open lease, and why that is right here is stated at the function: a duplicate pass of an idempotent, refuse-rather-than-destroy sweep is cheap; never remediating an install with no Redis is not. - `spawn_git_remote_token_scrub` at `start_agent_internal` — the #2069 fire-and-forget shape, deliberately WITHOUT its `auto_sync_enabled` gate. Whether an agent auto-syncs has nothing to do with whether its `.git/config` holds a token, and copying that gate would silently skip every agent that does not. - `startup.sh`'s conditional per-restart rewrite (previous commit). A refusal is queued, not just logged: one operator-queue alert per agent per UTC day, so an operator learns that an agent still holds an embedded credential that Trinity would not remove without a replacement. `PROTECTED_KEYS` gains `GIT_CONFIG_KEY_*` / `GIT_CONFIG_VALUE_*` (by prefix — an unbounded family), `GIT_CONFIG_NOSYSTEM`, `GIT_ASKPASS` and `GIT_PROXY_COMMAND`. `GIT_CONFIG_COUNT` was already listed but its SLOTS were not, so the guard covered the count and left the payload writable — and a slot can set `core.sshCommand`, `core.pager`, `diff.external` or `credential.helper`. `NOSYSTEM` becomes load-bearing here: `/etc/gitconfig` is now where the helper is registered, alongside the #1595 gc guards, so one agent-written `.env` line would disable both. Tests: `tests/unit/test_ent615_token_free_remotes.py`, 66 cases. The helper and the sweep are SHELL, so these EXECUTE them — a Python assertion on script text passes against a script that cannot run, which is exactly the CRIT-1 failure. Mutation-checked: reverting the helper name, the `.env`-first ladder, the exact-host check, the harvest-before-strip order, the root exec, or the credential-less re-point each turns tests red. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…c-health polling (#2742) Trinity Rule #1 — the requirements/architecture delta lands before the code. What the docs now say: - requirements/github.md §11.9 gains "Lock-free status read" and "Stuck-lock report, never a runtime delete", and the stale-lock hygiene bullet records that the boot reap now speaks. §11.8's SyncHealthService bullet names the leader lease AND its honest side effect (time-to-sync_failing ~90s → ~180s). - architecture/agent-lifecycle.md is the single home: why the status read took index.lock ~2x/min in every workspace, what replaced it, and why the runtime delete was cut on measurement rather than hardened. - architecture/agent-runtime.md gets a one-line pointer (no second home). - architecture/background-services.md's Sync Health row states the lease, its fail direction and the reason (a feed that raises sync_failing must not go dark when Redis does) — per that file's header rule. - feature-flows/git-sync-health.md: §1a announced boot reap + the observe-only runtime report with the three measurements that decide it; a new §2a for the agent status handler with the two distinct bounds (35s caller / 90s computation) and the ~130s child-budget arithmetic that replaces the doc's wrong "~30s worst case"; Files Touched, Testing, Operator Controls; and a Known Limitations rewrite that retires the "still blocks the event loop" bullet and replaces it with the residuals this change does NOT close. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main.py starts SyncHealthService in EVERY uvicorn worker and prod runs `--workers 2`, with no lease — unlike opqueue:leader (#1632), monitoring:leader (#1464), skills:sync:leader and canary:leader. So every git-enabled agent was asked for /api/git/status twice a minute, and each ask runs a 30s credentialed `git fetch origin` inside the container. `synchealth:leader` in the #1464 shape: SET NX EX, own-lease-only refresh, release from stop(), acquire + transition logs at the top of _poll_cycle with an early return for non-leaders. Two deliberate divergences from the copied shape: - Release is a single compare-and-delete EVAL, not GET-then-DEL. The non-atomic form lets a worker whose lease lapsed between the two calls delete a sibling's FRESH grant — two pollers for a whole cycle. EVAL is outside the -@dangerous categories denied to the backend/scheduler ACL users. - Fail direction is OPEN and stated in the docstring: Redis down means every worker polls, i.e. today's behaviour. Failing closed would darken the only feed that ever raises sync_failing, exactly when infra is already degraded. The lease has an honest side effect and it is named, not buried. db.upsert_sync_state INCREMENTS consecutive_failures per failed upsert, so it is NOT idempotent: two unleased workers reached sync_failing in ~90s, one leader takes ~180s. Asserted in TestLeaderLeaseAlertTiming — including the load-bearing fact itself (the same payload polled twice increments twice), so the test cannot pass against an idempotent upsert and prove nothing. Also here (same file, same subject): SYNC_HEALTH_POLL_INTERVAL_SECONDS, read at CALL time in the _maintenance_timeout_seconds shape, parse-guarded and positive-clamped, with the default DELIBERATELY unchanged at 60s — the AC is written as "the 60s poll" and the sampler evidence was taken there. poll_interval becomes a property so the module-level singleton (built at import) still honours the env; `is not None` rather than truthiness so the tests' poll_interval=0 keeps meaning "one cycle then exit". The `service` fixture now stubs get_breaker_redis -> None: with a local Redis it would otherwise leave a real 30s lease behind and the NEXT test's fresh service would lose the election and silently poll nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`git status --porcelain` takes .git/index.lock on EVERY invocation to refresh the index — whether or not a rewrite follows — for a window that scales with index size (0.6ms at one file, 12.8-15.4ms at 20k; 334-473ms measured in a real container). The backend polls /api/git/status every 60s for every git-enabled agent, so the platform was taking that lock ~2x/min in every workspace, outside _REPO_LOCK, racing the agent's own `git add`. `git --no-optional-locks status --porcelain` on exactly that one call site. Scoped deliberately: the auto-sync cycle's status (under the repo lock, right after `git add -A`, proceeding to commit) and the sync/pull bodies keep the plain form — they are lock-serialized and they WANT the refreshed stat cache. Flag form rather than GIT_OPTIONAL_LOCKS=0 because run_registered has no env= kwarg, the env form would silently change the mutating sites too, and only the argv is assertable in a test. Verified across a clean index, a stat-dirty index, core.untrackedCache, --untracked-files=all, an fsmonitor hook and a repo with no .git/index: zero lock sightings and zero rewrites in every case. The flag closes only the index-lock half, so every status child now routes through run_registered (#1595's seam): a sweep tick straddling the 30s `git fetch` still SIGKILLs it, and a killed fetch orphans FETCH_HEAD.lock / packed-refs.lock — which NO reaper covers, not even startup.sh's (its find is scoped to refs/ and logs/). run_registered accepts neither capture_output nor text, so both kwargs are DELETED at all ten sites; a name-only substitution would be a TypeError ten times over. Three of those ten are the shared helpers _compute_ahead_behind, _get_pull_branch and _persist_last_remote_sha, which are also reached from _conflict_response (the 409 arm of every locked endpoint), sync_to_github and pull_from_github. So the conversion sweep-registers three children on the MUTATING paths too. That is desirable — those are the children a repack-length operation exposes longest — but it is stated and pinned by a test rather than left for a reviewer to find. Also here, because the diff was already moving these lines: - remote_url is now unconditionally redact_url_userinfo'd. The old shape special-cased @github.com and returned everything else VERBATIM, so any non-github.com remote (GHES, GitLab, a host-rewritten origin) put a live `https://oauth2:<PAT>@host/...` in the response body — proxied unmodified by git_service.get_git_status to the UI and the MCP tool. ent#615 owns the broader class; this is the one line of it in this diff. - _read_sync_state_file gates on st_size (64 KiB) BEFORE reading. That file is fully agent-authored and merged wholesale, and the backend reads every agent concurrently once a minute, so an unbounded read_text() OOMs both sides. - The comment beside _REPO_LOCK now says what it does NOT exclude: the agent's own git and the backend's docker exec sites. Misreading that is what produced a plan to treat a successful non-blocking acquire as evidence of quiescence. _compute_git_status is extracted verbatim as one blocking callable (the handler calls it directly for now) so the next commit can put the coalescing in front of it without also moving the body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three callers converge on this one route — the backend's 60s sync-health poll, the UI git panel's own 60s poll while open, and the MCP get_git_status tool — and each computation runs a 30s `git fetch origin` against one repo. The issue observed two overlapping fetches 0.9s apart. A guard on the poller alone would not have satisfied AC2. Now the first caller starts the computation and every other caller awaits the SAME future. The slot is keyed on the resolved home path, check-and-set is atomic because there is no `await` between the test and the assignment, and the agent server is single-process (agent_server/main.py calls uvicorn.run(app, ...) with no workers=) — the BACKEND is not, which is what the Redis lease is for. Coalescing happens ON THE LOOP, and only the computation takes a thread. This is the load-bearing shape choice, not a style preference: asyncio.to_thread uses the loop's DEFAULT executor — min(32, cpu+4) = 6 threads on a 2-vCPU agent — and services/headless_executor.py records that ctx.terminate, auto-sync and pipe-close deliberately stay on that same pool. Parking followers there is the #2433 starvation class, where a burst of status callers stalls EXECUTION TERMINATION. Five callers must cost exactly one to_thread, and a test asserts it so the rejected shape cannot come back. Two bounds, deliberately distinct numbers for distinct things: - _STATUS_FOLLOWER_WAIT_SECONDS = 35 is a CALLER bound, at or just past the point every real client has already given up (poller 10s, git_service 30s); a longer wait can only produce work nobody awaits. Timeout is a 504. - _STATUS_LEADER_DEADLINE_SECONDS = 90 is a COMPUTATION bound. The child timeouts sum to ~130s nominal before run_registered's post-killpg drain, and a slow leader costs no follower threads but DOES hold the slot, so every caller in that window 504s. The docstring carries the arithmetic; the flow doc's old "~30s worst case" (which is where a 60s follower bound came from) is corrected in the docs commit. asyncio.shield is load-bearing: a follower that times out or disconnects must not cancel the computation everyone else is waiting on. And the future is ALWAYS resolved — to_thread -> run_in_executor -> _WorkItem.run catches BaseException and calls set_exception — so there is no hand-rolled set_result/set_exception pair and no "leader vanished" fallback. The comment says not to add one. `computed_at` admits what coalescing costs: a follower arriving at t=29s of a 30s leader run is served a 29-second-old snapshot, visible as "1 ahead" right after a successful push. Stamping the age is cheaper and more honest than claiming coalescing changes nothing. TTL caching was rejected — serving late followers a fresh run reintroduces the overlapping fetch AC2 forbids. No 409 on status by design: it is a read, and _with_repo_lock on it would make every poll a contended write and flap the agent `unreachable`. D5: test_1920's walk now covers docker/base-image/agent_server/ as well as src/backend — Invariant #5, "a guard that walks only one of the two trees is not a guard" (ent#314). Proven by planting an nx=True in an agent-server module and watching it fail. That tree gets no allowlist row on purpose: it issues no nx=True set and cannot (agents are not on the platform network, so Redis is unreachable from one), and #2742's coalescing is a different class entirely. Its one-home property is asserted directly instead, beside a meta-assertion that fails loudly if the agent-server root ever moves out from under the walk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lock (#2742) AC3 has two halves and only one of them was missing. The RECOVERY already existed and is provably safe: startup.sh reaps index.lock at container start, where "no git process is running" is definitional because the PID namespace is empty. What was missing was the OBSERVABLE half — the block was `rm -f`, silent whether or not it removed anything, so the one moment the platform reliably heals a wedge produced no evidence that it had. startup.sh now tests-then-removes, echoes a line naming each lock it cleared (Vector captures it), and drops ~/.trinity/lock-recovery.json. It also reaps index.lock under .git/modules/* and .git/worktrees/*, which NOTHING covered before — its find is scoped to refs/ and logs/ — so a submodule or linked-worktree wedge used to survive every restart. Proven by extracting the shipped block and running it against fixtures: /verify-local's agent stage boots local:test-echo, so startup.sh never executes there and trusting the stage would prove nothing. _record_lock_recovery folds that marker into sync_state.last_lock_recovery on the next status read, then deletes it so the episode is reported once rather than every minute forever. It goes through a dedicated metrics-only writer: _write_sync_state_file unconditionally stamps last_sync_at = now, which would mark a never-synced agent as freshly synced and turn its dashboard dot green. _index_lock_stuck REPORTS a currently-present lock and never unlinks it. The runtime delete was cut on measurement, not on taste: - st_size == 0 is the signature of a LIVE writer, not an abandoned one — git creates the lock with O_EXCL before walking the worktree and writes the new index into it only at the very end. Measured 100% of a healthy `git add -A`'s life: 3.9s on 450MB, 29s at 60k files, 155s under a clean filter. - st_mtime is stamped at create and never advances, so age measures the IN-FLIGHT OPERATION. At t=+130s a healthy add reads size=0 age=130s — both naive gates satisfied. - A wrong unlink is permanent and strictly worse than the wedge: git renames by PATH, so a second git's in-flight file gets promoted onto .git/index, the corrupting process exits rc=0 with empty stderr, and the 0-byte index is cleared by nothing — not the boot reap, not _reap_stale_git_litter, not `git reset`. - _REPO_LOCK would not have helped: it excludes this server's own auto-sync cycle and nothing else, not the agent's own `git add`, which is the premise of this issue. So detection is two-point inode stability — the same (st_ino, st_mtime_ns, st_size) unchanged across >=3 status reads spanning >=15 min — on time.monotonic(), which a forward NTP step or a live migration cannot move. The tunable is the sighting COUNT, not a wall-clock age. Three properties that are each a defect if dropped: - It resolves the REAL gitdir. `.git` is a FILE for a linked worktree and for a submodule, both creatable by the agent in one command, and assuming a directory makes the observer look where the lock provably is not. Candidates cover <gitdir>/index.lock, modules/*/index.lock and worktrees/*/index.lock. A symlinked .git is skipped outright. - It takes NO repo lock. An lstat needs no mutual exclusion, and holding _REPO_LOCK across the observation would make a status poll a brand-new source of 409 agent_busy on an operator's POST /api/git/sync. - It is wrapped end to end in its own except OSError. It runs inside _compute_git_status's try, whose tail is HTTPException(500), and _fetch_git_status treats any non-200 as None and writes nothing — so one EACCES would silently stop the agent's sync-health row advancing and sync_failing would never fire either. An observability path must never be able to darken the feed it feeds. The sighting ledger is in memory, deliberately NOT in sync-state (a deviation from the plan, on safety grounds): a per-tick read-modify-write into the agent-authored document would race the auto-sync writer and could drop consecutive_failures or last_sync_status — the observability path corrupting the feed. It needs no durability either, since the only thing that clears a genuinely wedged lock is the container restart that also clears the ledger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The backend half of AC3, plus the AC4 concurrency proofs.
_coerce_lock_recovery / _coerce_lock_stuck rebuild both new fields from values
the backend has checked, and never pass the agent's dict through. That is not
belt-and-braces: sync-state.json is agent-authored, the agent server merges it
wholesale (merged.update(data)), so last_lock_recovery is fully agent-controlled
even on an agent where nothing ever reaped anything — and
git_service.get_git_status proxies response.json() UNMODIFIED to the UI and the
MCP tool.
Three guards, each for a named failure:
- isinstance(value, str) before parsing: parse_iso_timestamp raises
AttributeError, not ValueError, on a non-str.
- except (ValueError, TypeError) around the parse AND the window comparison: a
valid-but-naive ISO parses cleanly and then raises TypeError on aware > naive,
and that raise lands in _sync_agent AFTER the upsert where
gather(return_exceptions=True) swallows it — so the symptom is a lost alert
with no traceback. The same guard with the same comment already exists in the
file this record comes from.
- An explicit UTC offset is REQUIRED. startup.sh always stamps Z, so a naive
value did not come from us, and the house posture over agent-authored JSON is
to reject rather than guess.
The window is [now - 1h, now + 60s], with the hour deliberately a floor rather
than 2x the poll interval: startup.sh stamps `at` at container boot and the
agent server folds it in on its FIRST status read, and a cold boot can put
several minutes between those two. The clamp's job is to reject nonsense — the
far-future `at` that would otherwise be "newer" forever — and the DEDUP is what
stops a real record repeating.
Dedup is against the last OBSERVED value. The rejected alternative ("at newer
than the prior row's last_check_at") is not a dedup at all: last_check_at is
re-stamped to now on EVERY upsert, so 9999-01-01 would be newer on every tick,
per agent, for the life of the process — a WARNING asserting a platform action
that never happened. Both halves are tested, so neither can pass by rejecting
everything.
index_lock_stuck is edge-triggered and re-arms after the lock clears. No
operator-queue item and no DB column: the report is a diagnosis, not a decision.
The agent-supplied `path` is dropped at the boundary — it is composed from a
.git the agent can point anywhere and adds nothing to a fleet-level WARNING.
AC4 ships in three phases, strongest first:
- The GATE is deterministic AND genuinely concurrent: a real
`git status --porcelain` child is SIGSTOP-frozen the instant it takes
.git/index.lock, and the agent's own `git add` — a second real process — then
fails rc!=0 with index.lock in stderr. 10/10 locally. SIGSTOP is the only way
to hold that window open: git runs hooks and filters OUTSIDE the index lock
(an fsmonitor hook sleeping 1s stretches status to 1279ms while the lock
window stays 0.8ms). SIGCONT is in a `finally` — --timeout-method=signal
raises inside the test and a leaked frozen child would hold the lock for the
rest of the session. Its twin proves the flagged argv cannot be caught at all.
- The lock-sighting sampler (AC1, earlier commit) is the property.
- The threaded witness through the real get_git_status() route is
self-validating and non-gating: its control arm must reproduce an index.lock
failure IN THIS RUN or the test skips. Measured through this route the duty
cycle is ~0.7% (about 95% of each iteration is git fetch), so the margin is
roughly one failure per run — it skips ~2 runs in 5 here, which is exactly the
point: it can never pass without having demonstrated it can fail.
Writer rounds write time.time_ns() and failures are classified by
"index.lock" in stderr, never by return code: `git commit` exits 1 with EMPTY
stderr when a round writes content identical to the previous one, which under
check=True is indistinguishable from a lock failure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tail steps: /update-tests and /sync-feature-flows. Two gaps the plan's test matrix listed and the implementation had not yet covered, both now closed: - test_leader_deadline_releases_the_slot — a wedged leader must release the in-flight slot rather than monopolise the read. It holds no follower threads on this design, but it does hold the slot, so every caller in that window 504s. - An AST guard pinning _STATUS_LEADER_DEADLINE_SECONDS to the status body's REAL child-timeout budget. They are both 90s today (10 rev-parse + 10 status + 10 log + 30 fetch + 10 merge-base + 10 log + 10 remote get-url). Adding a child, or widening one, now fails here instead of showing up as an unexplained 504 in production — and the flow doc's arithmetic is guarded with it. Plus the ordering invariant between the two bounds (caller < computation). Flow-doc sync — one home per feature, so the two adjacent docs get pointers, not copies: - github-sync.md documents GET /api/git/status's own contract, so its endpoint row, its _persist_last_remote_sha row (whose child is now sweep-registered on the LOCKED sync path too), its stale "line 249-250" call-site reference, and its revision history all needed the #2742 delta. - mcp-git-tools.md: the MCP tool proxies the agent payload verbatim, so it gains computed_at / lock_recovery / index_lock_stuck with no signature change — and its callers now coalesce with the poller and the UI panel rather than stacking a third overlapping git fetch. - feature-flows.md's Git Sync Health category row and git-sync-health.md's index_lock_stuck field list corrected to match what the code actually emits. The test-runner catalog entry lives in the private .claude submodule and is committed THERE, on its own branch, deliberately WITHOUT bumping this repo's gitlink — `git add -A` stages that gitlink silently under `diff.ignoresubmodules=all`, which is what ejected #2606 from a merge train. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…est (#2742) Public repo. The fixture token was never real and is too short to match GitHub's own secret-scanning format, but a `ghp_` prefix is exactly what the repo's pre-commit checklist tells a reviewer to grep for, and the test's point is that URL userinfo is stripped — not that it is a GitHub PAT specifically. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…emotes Trinity Rule #1 works backwards here — the code landed first because the mechanism had to be proven by execution before it could be described honestly (the helper NAME and the resolution ORDER were both wrong in the first design and only a real git told us). - Requirements: `github.md` §11.16 (the full requirement, six FRs), plus FR-4 and FR-5 corrected in place. FR-5's parenthetical — "never the global tier, which cannot reach a tokenless container" — is a sentence this change makes FALSE, so it is rewritten rather than left standing. `security.md` §20.9b (the standing rule, the chain it closes, and what it does NOT close). `credentials.md` §3.8 (the ladder as the git credential source of record — `.env` first, and why rung 3 is neither `.env` nor the DB row). - Architecture: `architecture/security.md` Container Security (the helper, the three properties that each cost an iteration, and the honest "root ownership is integrity, not confidentiality" line); `agent-lifecycle.md` (the rebind push's root exec, and why it cannot move to the backend host); `agent-runtime.md` (the `PROTECTED_KEYS` prefix rule). Core `architecture.md` is unchanged — no Architecture-Map path moved. - Feature flows: `github-sync.md` gains a Credential-free remotes section and four corrected passages (the unconditional restart rewrite is now conditional; the write-credentials predicate has a second tier); `github-repo-initialization.md`'s accepted-risk block — "PAT is visible in git remote URL inside container" — is retired, and this change is its changelog; `credential-injection.md` records `.env` as the first rung and why the harvest is not written there. - `docs/migrations/GIT_REMOTE_TOKEN_SCRUB_2026-09.md`: the operator runbook — what the sweep does, what `harvested` / `refused` / `gitmodules_hits` mean, the `.gitmodules` case it cannot fix, and the standing advice to rotate the platform token after adoption (mandatory rather than advisory if any agent reports a `.gitmodules` hit). - `tests/registry.json`: both new files, and the #1967 entry's own "`.env` is not where git authenticates from" premise corrected — that is the sentence this issue inverts. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`/sync-feature-flows` over the branch diff. Three carried claims this change makes false, which is the class the sweep exists to catch: - `template-processing.md` printed the PAT-bearing `CLONE_URL` and the old PAT-gated clone condition (ent#123 had already retired the gate). - `github-sync.md`'s branch-support table pointed at `template_service.clone_github_repo()`, deleted here — it put a token URL on the backend host's **argv** and had no production callers. - `skills-library-sync.md` printed the splice itself. That one was live: git wrote the spliced URL to `origin`, so the platform PAT was at rest in `/data/skills-library/*/.git/config` — on the `~/trinity-data` host bind mount, and so in every backup of it. The host DECISION it documents is unchanged and still parse-based (PR #1901): an `http.extraHeader` goes to whatever host git connects to, so "is this host ours" still gates whether the credential travels. Its status allow-list and the scrubbers stay exactly as they are — a stored source row can still carry userinfo of its own. Two gained a seam worth naming: - `async-docker-operations.md`: `execute_command_in_container` now takes `environment` (Exec Create body, **not argv** — argv is the leak) and `user` (root, so `/proc/<pid>/environ` is unreadable by the `developer`-uid agent). Also records the pre-existing trap that its `timeout` is accepted and forwarded nowhere, and why a caller needs BOTH bounds. - `agent-lifecycle.md`: the two fire-and-forget start hooks, and why the ent#615 one is deliberately not behind #2069's `auto_sync_enabled` gate. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Not cosmetic. Each of these states, as a fact a later reader will rely on, something that was true before ent#615 and is false after it — which is the class of stale comment that makes the next change wrong. - `github_pat_propagation_service`'s module docstring said "`.env` is not where git authenticates from", with the reasoning that made #1967's fix correct. That premise is exactly what this issue inverts, and deliberately: the helper reads `.env` FIRST, because a rotation does not recreate the container, so `Config.Env` keeps the revoked token until the next recreate. The old text is kept in past tense with the inversion stated under it, rather than deleted — it is why the current shape is what it is. - `_apply_pat_to_agent`'s docstring now says why the HTTP `.env` write stays primary even though `update_remote_pat` writes the same line over `docker exec`: only the HTTP path runs `sync_process_env()`, and only the exec path works while the agent server is wedged. - `routers/git.py`'s set-PAT note ranked `remote_updated` above `env_updated` because "the live git process authenticates from the remote URL". Both now mean the agent works immediately, so both say so. Also simplifies a test fixture that had become a lambda calling a lambda. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…urvives Two silent failure modes found by reviewing my own diff. **The exec amplification is on the rotation path, not the boot pass.** The bound was written where it looked needed — the fleet one-shot — and the caller that actually needed it is `propagate_github_pat`, which gathers over the WHOLE FLEET with no bound of its own. This change takes each agent from one exec to three, so a 50-agent rotation is 150 execs against the fixed 6-thread `_docker_executor` the whole backend shares (`to_thread` draws from it too, #2433), each holding its thread for up to the in-container timeout. The bound moves to `scrub_git_remote_tokens` itself, so every caller — rotation, start hook, boot pass — inherits one, and a concurrent boot pass cannot out-run a rotation. **A bare `create_task` is GC-collectable mid-flight** (#1083). The boot sweep sleeps 20 s before doing any work and runs exactly once per boot, so a collected task is a remediation that silently never happened. It now holds a strong ref, the same remedy `main._first_run_seed_task` documents — and the scheduling moved into the service, because inlining it in `_schedule_staggered_services` pushed that phase past the 100-line threshold `test_1028_lifespan_phases.py` guards. Splitting is what that guard asks for. Four tests, including that the start hook did NOT inherit #2069's `auto_sync_enabled` gate — on a fleet of read-only template agents, that gate would skip most of them. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found reviewing the diff. A `skill_sources` row may hold userinfo of its own — `_adopt_legacy_clone` writes rows with no validation, and `reject_embedded_credentials` only guards NEW writes — and moving the platform PAT from the URL into `http.extraHeader` left BOTH in play for such a row: libcurl's basic auth from the URL and our header. That is exactly the double-credential shape ent#347 documented as rejected by GitHub, spelled differently, so those rows would have kept failing after a change that reads as though it fixed them. `_clone_target` composes the two halves and carries the rule neither can: **when we are going to send a credential, the URL must not carry one** — and when we are NOT, the stored userinfo is left exactly as it is, because for a row whose own token is its only credential, stripping it is this issue's own cardinal sin applied to the skills library. It resolves `_normalized_url` through `self`, not the class: that is the seam the ent#237/ent#332 fixtures override per instance to let a local fixture repo path through, and a `cls.`-qualified call bypasses it silently — which is how 23 tests went red on the first attempt. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… released Two findings from `/review` over the branch diff. **The guard had this issue's own failure mode, one level up.** It scanned `ast.JoinedStr` only, so the same defect written as concatenation, `%`-format or `.format()` would have shipped green — and a guard with a known blind spot is the ent#314 lesson again (a scan that walks one shape is not a scan). It now flattens five expression shapes, and the self-test is parametrized over SIX literal spellings of the defect — including the exact strings the three deleted producers used — plus five shapes it must stay quiet on, four of them the scrubbers this change deliberately keeps. Still zero allowlist entries. **The boot lease was taken and left to expire.** `acquire` never waits, so a losing worker has already returned by the time the winner finishes; holding the lease for its full 900 s TTL bought nothing and silently skipped the sweep on a deliberate restart inside the window — the one moment an operator is most likely to want it. Released in a `finally`. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n pitfall (#2742) Moving the status handler off the event loop removed a mutual exclusion that sync_to_github/pull_from_github were providing by never awaiting. _REPO_LOCK was added for the cycle, so it does not cover a newly threaded path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nts (#2742) Re-measured inside a real agent container on the ext4 workspace volume. The penalty tracks content bytes re-hashed, not index size: ~1x on 42491 files of ~1 MB, ~390x on 1500 files of 294 MB. The single 29x figure was taken outside the fleet and sat between the two regimes, describing neither. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o read `/autoplan` reads this ledger before planning, so both entries are written for that reader. 1. **A guard that knows only the CURRENT spelling of a defect certifies the next spelling of it.** Both halves of this issue hit it: registering the credential helper under its filename produces a helper that silently never runs — a fleet-wide fetch/push outage that every source-level assertion agrees is fine — and the AC1 producer guard walked f-strings only, so the same defect written as concatenation or `%`-format would have shipped green under a guard whose whole job was to prevent it. 2. **A comment that states the premise a change inverts is load-bearing documentation, and it goes stale silently.** Four sat downstream of #1967's "`.env` is not where git authenticates from" — including an accepted-risk block that this change is the changelog for. None is reachable by grepping the symbol that changed; they name a property of the system, so grep the SENTENCE. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…2742) `SYNC_HEALTH_POLL_INTERVAL_SECONDS` was read by `sync_health_service` at call time — a property rather than an import-time copy, specifically so an operator override is honoured — and then reached no container: it was absent from `docker-compose.yml`, `docker-compose.prod.yml` and `.env.example` alike. The knob shipped inert. Proven by rendering rather than grepping: before, the var does not appear in `docker compose config`'s backend environment at all. This is the #1056 packaging class (`VOIP_*`), which `test_ent237_skill_source_env_packaging.py` records as having already recurred seven times. Prod compose launches standalone — no base-compose merge and no `env_file:` on the backend service — so the explicit `environment:` list is the only route in, and wiring dev alone would not have carried over. The `${VAR:-60}` pass-through form is the correct one here: a cadence has no "disable" sentinel, so unset and empty must both land on the unchanged 60 s default. `TestPollIntervalReachesTheContainer` asserts the FORM, not mere presence, and pins the compose default against `DEFAULT_POLL_INTERVAL` rather than a literal so the two cannot drift into a container that polls at a different rate from a laptop. Verified to have teeth by deleting the prod wiring and watching it go red. Found by /validate-pr §4.9 on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…2742) `b6757ba66` wired `SYNC_HEALTH_POLL_INTERVAL_SECONDS` into `docker-compose.yml`, `docker-compose.prod.yml` and `.env.example` — and missed `docker-compose.hosted.yml`, the pull-only twin every marketplace and managed-host channel actually runs. `test_2280_hosted_compose_parity` caught it in CI: deterministic, all three head seeds, clean in all three base seeds. That is the bug reproducing inside its own repair. The commit being fixed exists because a knob never reached the container; it then failed to reach the container that ships to marketplace installs. The hosted file's own header names the class and lists its five prior victims (#1039, #1056, #1707, #1871, #2381) — this is the sixth, and the first to be caught by the guard rather than by an operator. The line is byte-identical to prod's, since #2280 compares the backend `environment` list wholesale. No hosted-specific assertion is added here: that guard is strictly stronger than anything this file could restate, and two guards over one fact drift apart. The reasoning is recorded in the test's docstring so the next reader does not "helpfully" add the redundant one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hip is not a boundary The two blockers `/review` + `/cso --diff` raised against this branch, both reproduced before they were fixed. [C1] On a hardened install the sweep did nothing and reported success. `agent_full_capabilities=false` gives the container RESTRICTED_CAPABILITIES, which withholds DAC_OVERRIDE and FOWNER — so root inside it is subject to ordinary permission checks against the 0700 `developer`-owned home. Two things followed. The strip's config writes are both `|| true` while `scrubbed` was incremented unconditionally, so a write that failed still reported a removal: a false `remotes_scrubbed` is strictly worse than a refusal, because the operator acts on it and stops looking. And when the exec could not traverse the tree at all, `find` enumerated nothing, the probe failed, and the report was all zeros with exit 0 — which is exactly what a healthy, already-clean agent reports, so nothing distinguished "nothing to do" from "could not even look". The counter is now earned: the sweep re-reads each key after writing and counts only what is really gone, and whatever survived is a refusal, which already alarms. `root_readable` is the discriminator for the second, and an unreadable pass files its own operator-queue family — separate from the refusal, because the two need different operator action and one daily-stable id would let whichever fired first suppress the other all day. Nothing is destroyed in either case; the token stays where it already was. What changes is that the platform stops certifying a remediation that did not happen. [C2] This branch documented a security property the platform does not have. "The agent cannot rewrite what platform git executes" was written into `architecture/security.md`, `requirements/security.md` and three code comments — while the base image's own `usermod -aG sudo developer` grants `NOPASSWD:ALL`, so an agent that wants to rewrite the helper or `/etc/gitconfig` can sudo and do it. The claim was already false on dev; this branch is what promoted it from an unstated assumption into a documented invariant in the file the next reviewer reads first. All five sites now say what root ownership actually buys — that nothing rewrites those paths BY ACCIDENT — and a source guard keeps the falsified sentence out, paired with a test that fails if the sudo grant is ever removed, which would make the corrected wording the stale one. Each fix is mutation-checked: reverting the earned counter, the readability probe, the alarm branch or the corrected wording each turns a named test red. The permission-bit tests skip under uid 0, where the bits mean nothing, and both arms carry a positive control. Refs Abilityai/trinity-enterprise#615 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… a failure (#2795) Two independent gaps stacked up, so once a room fanned a message out there was no way to interrupt any agent short of waiting for the turn timeout. **1. The room's tiles never offered Stop.** `PortalWorkCard` has always rendered a Stop button; `PortalRoom.vue` simply never handed it `:can-stop` / `@stop`. Wired to the Work tab's own store action — `stopItem` re-checks the server's verdict, calls the same portal terminate route, treats a 404 as the lost race rather than a refusal, and refetches so CANCELLED comes back from the server instead of being written optimistically. Two surfaces, one cancel path. **2. The server said those rows were unstoppable.** `can_stop` gated on `kind in ("turn", "delegated")`, and a room wake projects as `room` — so the Work tab listed the run and hid the only control that would have ended it. That widening is not cosmetic: the terminate route's own gates are `_require_roster(agent)` and `execution_belongs_to_caller` (agent match + `source_user_email` match), and `_wake_agent` satisfies both by construction — every wake runs through `execute_task(..., source_user_email=<the poster>)` on an agent that is a room participant, which on the Workspace can only be an agent already on the poster's roster. The route accepted these rows all along. `test_the_projection_and_the_terminate_route_agree` now evaluates both predicates against one row so the claim cannot rot. The kind list became a named ALLOWLIST (`STOPPABLE_KINDS`) rather than gaining a third literal: an unrecognised trigger projects as `other` and must stay unstoppable. `loop` is still excluded — a loop is stopped from the Loops tab, where stopping the LOOP is what the person means. Nothing else about the gate moves: "only the person who started the run may stop it" is untouched, and stopping one participant's execution leaves the others alone (the fan-out is sequential, so the next agent is woken after the cancel returns). **3. A cancel read as a fault.** `_wake_agent` treated CANCELLED exactly like FAILED: it posted "<agent> could not respond (no response)." — the surface blaming the agent for something the reader themselves asked for — and dropped the cached resume handle. That drop exists for a DEAD handle; a cancel is no evidence of one, and dropping it makes the next turn pay for a cold context rebuild. CANCELLED now posts "<agent>'s turn was stopped." and keeps the handle. The read cursor is still not advanced, so the delta the stopped turn never answered is re-delivered on the next wake. **Escape** gets a rule of its own rather than being scoped out: a room fans out to several agents, so `soleStoppableItem` stops the turn only when there is exactly one to stop, and is a no-op otherwise — guessing by position destroys work somebody is still waiting for. In practice the fan-out is sequential, so a room normally has one live row and Escape behaves as it does in a 1:1. It goes through `shouldCancelOnEscape` with the typeahead and add-agent popups as overlays, so ent#155's "anything nearer the keystroke wins" rule is unchanged. Also guards the live-work `v-if`/`v-else-if` chain with an AST test. Not hypothetical: the first draft of this change inserted the stop-error line between two of its arms and silently repointed the "…is thinking…" fallback at `stopError`. The SFC compiled and every other test passed — the #2794 defect, committed inside its own sibling fix. `roomComposerChain.spec.js` pins the same hazard one region down. Tests: `tests/unit/test_2795_room_stop.py` (17) and `src/frontend/tests/unit/roomStopWork.spec.js` (17, incl. a negative-tested chain guard). Full frontend suite 2922 green; the room/work backend suites 152 green. Related to #2795 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…2794) Attaching a file in a 1:1, then @mentioning a second agent, correctly moved the conversation to a room and left the file behind. `PortalConversation` uploads a dropped file straight into the CURRENT agent's inbox as it is attached, and the `escalate-to-room` event carried only `{ agents, message }` — so the person had watched a chip confirm the upload and believed both agents had it, while only the original one ever did and the room showed no trace of a file at all. The rule the issue states is the one this follows: whatever a user could do inside a room, escalating into one must produce the same result. A room-native drop is one upload per participant, so an escalation owes exactly that to the participants that have not already received the file — no more (the origin agent must not get two copies) and no less. - `usePortalFileDrop` keeps the `File` handle on each entry, so the same bytes can reach a second destination without asking the person to pick the file again, and exposes `settled()` so a caller can wait for an in-flight batch. Overlapping drops now CHAIN rather than race: two batches firing together is the request burst the sequencing already existed to avoid, and `settled()` could otherwise resolve while an earlier batch was still going. - `send()` awaits `settled()` before escalating and emits the entries with the message. Waiting is the honest branch of the AC and the last moment it is possible, since the composer is about to unmount. It deliberately does NOT clear the chips: on success the component unmounts as the room opens, and on failure the shell already hands the text back and the chips are still standing beside it — the recovery AC with no new plumbing. - `onEscalateToRoom` fans each carried file out to the participants that do not already have it, BEFORE posting the message — the message is what wakes the mentioned agent, and a turn that starts before the file is in its inbox cannot see the thing it was asked about. Per-agent failures are collected rather than aborting the carry. - The room then SAYS what arrived, for whom, and what did not: a file that missed a participant is named per file and per agent ("attach it again here to retry"), and a file that never finished uploading in the 1:1 is named too. Never silently dropped. Decidable rules live in the new pure `components/portal/portalAttachments.js` (`vitest.config.js` pins `environment: 'node'` with no mount harness); the SFCs are dispatchers over it. The origin agent is excluded BY NAME, not by position — the shell builds `agents` as `[origin, ...mentioned]` and a plan trusting that order would double-send the day it changes — and the notice reads its recipients off the plan rather than re-deriving them from `agents`. ## Two adjacent defects, found on the way Escalating lands attachments in a room, and attaching in a room was broken. **The room composer rendered on the wrong condition.** It shipped as `<form v-else>` chained to the "this conversation has ended" line (ent#358) — render the composer unless the room is closed. `v-else` binds to the immediately preceding ELEMENT, and three changes since have each inserted a conditional in between (the batch notice and the attachment chips in ent#524, the budget banner in #2620), so the chain ended on `attachments.length`. Two live defects in one expression: attaching a file to a room REPLACED the composer, and a closed room rendered a live composer directly under the line saying it had ended. The composer now carries `v-if="!isClosed"` — a `v-else` is a promise about whatever happens to sit above it, and this neighbourhood has broken that promise three times. `roomComposerChain.spec.js` had pinned the broken state as the contract, so it is rewritten to pin the OUTCOME: the composer names its own condition, no composer form is chained at all, the chips render beside the composer rather than instead of it, and a closed room still says so. **The room never cleared its chips.** It accumulated every chip it had ever drawn, describing files delivered several messages ago as though they were still pending. It now clears after a successful send, the 1:1's rule. Tests: `src/frontend/tests/unit/roomEscalationAttachments.spec.js` (33) plus the rewritten chain spec. Full frontend suite 2943 green, raw-colour and loading-gate ratchets included. Related to #2794 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
… flow doc (#2794) /review on the branch surfaced two real findings and one stale doc. **Re-entry during the settle wait.** The escalation now AWAITS the in-flight uploads, and `input.value` is cleared BEFORE that await — so the composer is empty and live for seconds rather than one microtask. A second Enter in that window cleared the newly typed text and emitted a second escalation, which `Portal.vue`'s own `escalating` flag then dropped on the floor: message gone, no error, and no composer left to recover it from. `escalatingNow` guards it, held separately from `sending` (which means "a turn is running" and is read by the header, the Stop control and the reattach poller), and released in a `finally` on BOTH paths — a flag left set would outlive a FAILED escalation and leave the composer the shell had just restored permanently dead. **The carry notice outlived its message.** It describes the message that created the room, and sat under the composer for every later message too. The room's own send retires it. It cannot fire early: the escalation's first post is made by the shell, not by the room. **Doc.** `workspace-agents-at-the-centre.md` owns the ent#524 upload gesture — its destination table and its "uploads run sequentially" contract both moved. Adds the escalation destination, the chaining/`settled()` rule, the before-the-post ordering, and the two adjacent composer-chain defects. Full frontend suite 2945 green. Related to #2794 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…e defect (#2794) The #2794 class, worth the ledger because it recurred inside its own sibling fix during the same session: the room composer's `v-else` was correct when written, three later inserts stole it, and the guard added afterwards pinned the broken adjacency as the contract for three months. Related to #2794 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…2795) /review finding on this branch. `_wake_agent` tells a user cancel from a failure by reading `execute_task`'s returned status — exact on a current agent image, which relabels its own 504/502/500 to a `cancelled` 200 when its process registry says the turn was terminated (#679 F3). An OLDER image re-raises: `execute_task` writes FAILED, that write loses the CAS to the CANCELLED the terminate route already wrote, and returns FAILED anyway. The room would then post "<agent> could not respond (no response)." for a stop the reader had just asked for — the exact AC #4 violation this PR exists to fix — and drop a resume handle that was never bad. The 1:1 is immune for a reason worth copying carefully: it never trusted the return value either, it remembers the cancel client-side (`cancelledExecutionIds`). A room has no such memory, so it asks the row. Three properties: the re-read is scoped to the branch where it can change the answer (the first draft fired on every terminal — a test now pins the successful-reply path at zero reads); it is fail-OPEN, so an unreadable row leaves the returned status in force; and it only runs on a path that has already lost an LLM turn. Tests: 24 in `tests/unit/test_2795_room_stop.py` (was 17), covering the old-image cancel, a genuine failure, both no-read paths, and both fail-open paths. Related to #2795 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
#2795) Found reviewing this branch: `_wake_agent` read `execute_task`'s returned status to tell a cancel from a failure, which is exact only while the agent image relabels its own cancel terminals. On an older image the FAILED write loses the CAS to the terminate route's CANCELLED and returns FAILED anyway. Related to #2795 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
#2794) Operator testing found the hole: attach a file through the rail's **Files** panel, @mention a second agent, and nothing was carried — and because the composer held no attachments, not even a notice saying so. Verified on the live instance: the file reached the 1:1's agent and no other. There are two upload surfaces and only one of them is the composer. `PortalRailFiles.vue::uploadBatch` sends straight to its own "Send to" target and keeps no pending state at all, so `attachments` was empty at send time and the carry had nothing to work with. The two are indistinguishable to someone who just wants to attach a file, and the rail is the more discoverable of them. `clientPortal.uploadDocument` is the ONE funnel all three surfaces already share (#2582 says so and relies on it), so the record goes there: a carry log of uploads that have not yet gone out with a message. `mergeCarrySources` unions it with the composer's own entries, deduped on `name + size` — not on the `File` reference, which would double-carry every composer upload, since a composer attachment passes through the same funnel and therefore appears in both views. The composer entry wins a tie: it holds the live per-file outcome the chip is rendering, so a chip that FAILED stays failed and is reported as not carried rather than being masked by a same-named log entry. The boundary is drawn exactly where the composer clears its chips — on mount (files from a previous visit are not pending), after a sent turn, and after an escalation consumes them (so a second escalation in the same conversation cannot carry them twice). That is the same rule the chips already follow, applied to the surface that has no chips. The log retains `File` objects, so it is bounded three ways and the tightest wins: 15 minutes, 20 entries, 64 MiB — evicting oldest. A single file over the byte cap is kept anyway; evicting it would silently drop the one file the person cares about, which is the failure this whole issue is about. Tests: 50 in `roomEscalationAttachments.spec.js` (was 35) — the merge rules, the three prune bounds, and the boundary sites. Full frontend suite 2960 green. Related to #2794 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…admin claim, first-run overlay, platform keys, canvas lifecycle, abilities 2.0 Every page under docs/user-docs/ was audited against the routers, views, feature flows and today's merges. 78 pages updated, 61 verified unchanged; FAQ 431 → 470 questions with the index regenerated from headings. Landed since the morning sync and now documented: - browser admin claim on 1-Click / installer installs (no generated password, claim at /setup, ADMIN_PASSWORD_SOURCE=browser) and the one first-run overlay that replaces the dashboard card ladder, with its re-run entry; "restart the backend" corrected to a container recreate wherever .env must be re-read - platform keys (Claude, GitHub, Resend, Gemini) set and validated in Settings, env vars as the fallback, across credentials / voice / VoIP / image / avatars - canvas lifecycle: delete, Manage, pin, search, the per-agent cap, share links, PDF, open canvas as turn context, the two-fact header; full REST + MCP surface - abilities marketplace 2.0: retired domain wizards removed everywhere, plugin versions and skill counts corrected, TRINITY_PLATFORM_PLUGINS opt-out Corrections found by the audit: the Permissions tab direction was reversed; subscribe_to_event and initialize_github_sync signatures were wrong; the image generation endpoint path was wrong; two removed routes (decrypt-and-inject, process triggers) were still listed and the startup auto-import documented as working; Slack / Nevermined / Telegram / WhatsApp setup steps and Settings labels now match the UI; per-public-link Connect Slack, the Workspace speaker toggle, webhook rate limits and the skills-library legacy-adoption behaviour are new sections. Verification: relative-link check 0 broken (FAQ index anchors included); issue-number / codename grep 0 hits outside the historical archives; public-safety greps and the enterprise-docs-guard pattern 0 hits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…te the carry (#2794) Operator reproduced it twice on the live instance: attach a file to Analyst through the rail's Files panel, open Analyst's chat, @mention a second agent — no carry, and no notice either. Found by instrumenting the live Pinia store, and proven both ways: with the boundary the log entry survives but `uploadsCarriedAt` is stamped the moment the chat mounts and the carry finds nothing; without it the entry is still there at escalation and the file reaches both inboxes. The rail is a SIBLING of the stage (ent#474) and survives every navigation, so "attach from wherever you are, then open the chat you want to escalate from" is the ordinary gesture — and `onMounted`'s `markUploadsCarried` consumed exactly that upload. A thread switch, ⌘J and an agent switch all remount this component, so one boundary broke several gestures, and it broke them SILENTLY: an empty carry set produces no notice, which is the same silence the issue exists to fix. The rule it was reaching for — "files from a previous visit must not ride along" — is already covered twice: `CARRY_MAX_AGE_MS` bounds staleness, and the log is plain Pinia state, so a page load starts it empty regardless. Mounting a component was never evidence that anything had been SENT. The two things that genuinely consume a pending upload are a message going out and an escalation taking it, and both already mark it themselves. The replacing test asserts the ABSENCE at the mount site and pins the consume points as a whole-file count, so a third one cannot be added quietly. Full frontend suite 2961 green; verified live on the operator's exact flow. Related to #2794 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpvcfgWkmQPD7DrDLmATTf
…rolled p (#2795) A failed verb surfaces an `InlineError` next to its control and persists until dismissed (design-system contract, principle 18). The refused-cancel line was a hand-rolled `<p role="status">` with no dismiss; the sibling surface for the same verb already does it right (`PortalWork.vue:43`, same `stopError` ref). `role="alert"` comes with the primitive, which is the correct semantic for a problem the person must notice. The AST guard locates the element by its static `data-testid`, which the component node still carries, so `roomStopWork.spec.js`'s "the stop-error line sits OUTSIDE the chain" is unchanged and still bites. merge-train: mechanical, per the merge-train note on the PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… on a problem (#2794) Three mechanical corrections to the notice added by this PR: - The dismiss control was a hand-rolled underline link. Buttons are `BaseButton` (design-system contract, Primitives first) — ghost/sm, the variant every other dismiss-shaped control in the portal already uses. - `role="status"` is polite, so the problem arm — files that did NOT travel — was announced as a passing remark. It now reads `alert` when `carryNotice.problem` and stays `status` otherwise. - `vi` was imported and never used in the spec. Markup, testids and the source guards are otherwise untouched. merge-train: mechanical, per the merge-train note on the PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 15, 2026
Merged
|
Resolve by merging |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE. This is an integration surface, not a change. It exists to run the full suite over five PRs together, because every member was tested against
devand never against its siblings. It is closed and deleted once the members merge individually.Members, in merge order:
Conflicts resolved on the train (member-vs-member, so they could not be resolved on a member branch):
docs/memory/learnings.mdanddocs/memory/feature-flows/github-sync.mdappend collisions, both sides kept;tests/registry.jsonrebuilt from the three git stages, 247 base + 2 + 2 = 251 entries, no reformatting churn.No closing keywords here on purpose — the members carry those, and this branch must promote and close nothing.
🤖 Generated with Claude Code