fix(#1356): tea login resolution fails closed on a declared git identity #1361

Merged
gate-merge-01 merged 4 commits from fix/1356-tea-login-fail-closed into next 2026-08-21 23:04:32 +00:00
Collaborator

Closes #1356.

Head: 0bf56326
Base: origin/next at 24462f460ee22b368c333522099c54e0e3d2ec9f

What was wrong

get_gitea_login_for_host() returned the first tea login matching the host. On this host that is 43 logins, roughly half of them matching one server, so a seat whose own login was missing did not fail. It silently acted as whichever identity happened to sort first.

That is the dangerous shape: it satisfies gate 16 mechanically (an author and a reviewer both exist) while violating it (they are the same actor wearing two names). Nothing in the output says so.

The --repo override path was worse. It fell through to get_default_tea_login(), which returns the default-marked login or, failing that, the first login of any host — an identity selected by the order of entries in a config file.

lane-brief.sh had its own version: it mapped an owner straight onto the shared usc / mosaicstack logins, so a seat briefing on a lane read as the shared account.

What this changes

A seat declares itself once, via MOSAIC_GIT_IDENTITY or git config mosaic.gitIdentity. Resolution derives the canonical login name from that identity plus the instance (<instance>-<seat>), and:

  • if that login exists, it is used;
  • if it does not, the command fails closed with a named error saying which login it wanted and the exact command to create it. It never falls back to a login that merely matches the host.

An explicit GITEA_LOGIN still outranks the ladder — naming a login by hand is a deliberate act.

With no identity set, behavior is unchanged. That is a human at a terminal, and it is the same point at which the existing token path stops enforcing. This PR does not move that line.

The four --repo callers (issue-list.sh, pr-list.sh, pr-view.sh, lane-brief.sh) now pass the owner so the instance can be derived rather than guessed.

Also included

tools/fleet/seat-logins.sh — projects seat credentials into tea logins under the canonical names this code now requires, so that name is something an operator produces mechanically rather than maintains by hand. Added to the framework per the maintainer ruling on #1356 ("this tooling MUST end up in the full framework"). It detects token collisions by hashing rather than by parsing tea's error text, so a collision shows up in a dry run instead of only at apply time.

Test notes

The suite had two sandbox helpers and one had drifted. run_in_repo and a near-copy run_in_repo2. The copy never received the MOSAIC_GIT_IDENTITY unset, so after the original was fixed the suite still failed on any provisioned seat — and the failure surfaced as the new code erroring, which reads like a bug in the change rather than a leak in the fixture. run_in_repo2 now delegates to run_in_repo, so the guarantee exists in one place and cannot drift again.

Every new assertion was verified by injecting the regression it claims to catch:

Injected regression Expected to fail Result
ladder borrows the host-matching login instead of failing closed branch 2 red: Identity ladder BORROWED login 'mosaicstack' instead of failing closed
resolve_git_identity never fires branch 1 red: Expected identity ladder to select 'mosaicstack-testseat'; got 'mosaicstack'
get_gitea_login returns a constant whole suite red

Restoring each returned the suite to green. A green assertion never shown to go red is not evidence.

New coverage: both ladder branches, both identity rungs (env var and git config), the --repo path, and GITEA_LOGIN outranking the ladder. The negative branch asserts two independent things — a non-zero rc and nothing on stdout — because the second is what proves it did not borrow the matching login sitting right there in the fixture.

Full suite comparison

Every test-*.sh in tools/git/, run on this branch and on the base commit, in both environments (a seat identity set, and none). S = identity set, U = unset.

suite base S base U branch S branch U note
test-ci-queue-wait-branch-absent.sh 0 0 0 0
test-ci-queue-wait-github-checks.sh 0 1 0 0 base U=1 was a flake: re-run x3 on both trees = 000
test-ci-queue-wait-no-status.sh 0 0 0 0
test-ci-queue-wait-tristate.sh 0 0 0 0
test-detect-platform-outside-repo.sh 0 0 0 0
test-explain-diagnostic-status-neutral.sh 0 0 0 0
test-git-credential-mosaic.sh 0 0 0 0
test-gitea-login-resolution.sh 1 0 0 0 target suite: now passes ON a seat (the drifted sandbox helper)
test-gitea-token-identity.sh 0 0 0 0
test-help-exit-code.sh 0 0 0 0
test-issue-close-fail-closed.sh 0 1 0 1 needs a real credential for the runner identity; see below
test-issue-comment-readback.sh 1 0 1 0
test-issue-create-body-safety.sh 0 0 0 0
test-issue-create-interactive-auth.sh 1 0 1 0
test-lane-brief-pr-linkage.sh 0 0 0 0
test-mosaic-worktree-large-repo.sh 0 0 0 0
test-mutate-push-guard.sh 124 124 124 137 wedges past timeout on both trees; 137 = killed by hand after 340s of zombie rm
test-pr-edit.sh 1 0 1 0
test-pr-merge-fork-ci-status.sh 0 0 0 0
test-pr-merge-gitea-empty-uid.sh 1 0 1 0
test-pr-merge-head-pin.sh 0 0 0 0
test-pr-merge-message-field.sh 0 0 0 0
test-pr-merge-queue-branch.sh 0 0 0 0
test-pr-metadata-gitea.sh 1 0 1 0
test-pr-review-gitea-comment.sh 0 0 0 0
test-pr-review-repo-host-override.sh 0 0 0 0
test-push-guard.sh 0 0 0 0
test-verify-clean-clone.sh 0 0 0 0
test-wrapper-guard.sh 0 0 0 0

Pre-existing failures, not fixed here

Named rather than absorbed, so a green CI run is not read as more than it is:

  • test-issue-comment-readback.sh, test-issue-create-interactive-auth.sh, test-pr-edit.sh, test-pr-merge-gitea-empty-uid.sh, test-pr-metadata-gitea.sh fail identically on this branch and on its base.
  • test-issue-close-fail-closed.sh passes only where the runner owns a real credential for its own identity. Its API-path cases need a usable token, and once an identity is set the token path reads that seat's credential file rather than the GITEA_TOKEN the fixture exports. True on the base commit too. Its fixture now derives the login from the runner's identity so the tea-branch cases are reachable at all; that is deliberately not a hermeticity fix, and pinning a synthetic identity would have produced a confident green over an untested path.

One correction worth recording: an early single run suggested this PR broke two suites. Repeating each run three times per tree per environment showed the first base run was a flake and the branch matches base exactly. One run is not a measurement.

Verification commands

# from packages/mosaic/framework/tools/git
MOSAIC_GIT_IDENTITY=fred       bash ./test-gitea-login-resolution.sh   # rc=0
MOSAIC_GIT_IDENTITY=someoneelse bash ./test-gitea-login-resolution.sh  # rc=0
env -u MOSAIC_GIT_IDENTITY     bash ./test-gitea-login-resolution.sh   # rc=0

Review

Author is fred; per gate 16 the reviewer must not be. Not self-merging — this is the merge-gate agent's call after CI is terminal-green on the exact head.

Closes #1356. **Head:** `0bf56326` **Base:** `origin/next` at `24462f460ee22b368c333522099c54e0e3d2ec9f` ## What was wrong `get_gitea_login_for_host()` returned the **first** tea login matching the host. On this host that is 43 logins, roughly half of them matching one server, so a seat whose own login was missing did not fail. It silently acted as whichever identity happened to sort first. That is the dangerous shape: it satisfies gate 16 mechanically (an author and a reviewer both exist) while violating it (they are the same actor wearing two names). Nothing in the output says so. The `--repo` override path was worse. It fell through to `get_default_tea_login()`, which returns the default-marked login or, failing that, **the first login of any host** — an identity selected by the order of entries in a config file. `lane-brief.sh` had its own version: it mapped an owner straight onto the shared `usc` / `mosaicstack` logins, so a seat briefing on a lane read as the shared account. ## What this changes A seat declares itself once, via `MOSAIC_GIT_IDENTITY` or `git config mosaic.gitIdentity`. Resolution derives the canonical login name from that identity plus the instance (`<instance>-<seat>`), and: - if that login exists, it is used; - if it does not, the command **fails closed** with a named error saying which login it wanted and the exact command to create it. It never falls back to a login that merely matches the host. An explicit `GITEA_LOGIN` still outranks the ladder — naming a login by hand is a deliberate act. With **no** identity set, behavior is unchanged. That is a human at a terminal, and it is the same point at which the existing token path stops enforcing. This PR does not move that line. The four `--repo` callers (`issue-list.sh`, `pr-list.sh`, `pr-view.sh`, `lane-brief.sh`) now pass the owner so the instance can be derived rather than guessed. ## Also included `tools/fleet/seat-logins.sh` — projects seat credentials into tea logins under the canonical names this code now requires, so that name is something an operator produces mechanically rather than maintains by hand. Added to the framework per the maintainer ruling on #1356 ("this tooling MUST end up in the full framework"). It detects token collisions by hashing rather than by parsing tea's error text, so a collision shows up in a **dry run** instead of only at apply time. ## Test notes **The suite had two sandbox helpers and one had drifted.** `run_in_repo` and a near-copy `run_in_repo2`. The copy never received the `MOSAIC_GIT_IDENTITY` unset, so after the original was fixed the suite still failed on any provisioned seat — and the failure surfaced as the *new* code erroring, which reads like a bug in the change rather than a leak in the fixture. `run_in_repo2` now delegates to `run_in_repo`, so the guarantee exists in one place and cannot drift again. **Every new assertion was verified by injecting the regression it claims to catch:** | Injected regression | Expected to fail | Result | |---|---|---| | ladder borrows the host-matching login instead of failing closed | branch 2 | red: `Identity ladder BORROWED login 'mosaicstack' instead of failing closed` | | `resolve_git_identity` never fires | branch 1 | red: `Expected identity ladder to select 'mosaicstack-testseat'; got 'mosaicstack'` | | `get_gitea_login` returns a constant | whole suite | red | Restoring each returned the suite to green. A green assertion never shown to go red is not evidence. New coverage: both ladder branches, both identity rungs (env var and git config), the `--repo` path, and `GITEA_LOGIN` outranking the ladder. The negative branch asserts **two independent things** — a non-zero rc *and* nothing on stdout — because the second is what proves it did not borrow the matching login sitting right there in the fixture. ## Full suite comparison Every `test-*.sh` in `tools/git/`, run on this branch and on the base commit, in both environments (a seat identity set, and none). `S` = identity set, `U` = unset. | suite | base S | base U | branch S | branch U | note | |---|---|---|---|---|---| | `test-ci-queue-wait-branch-absent.sh` | 0 | 0 | 0 | 0 | | | `test-ci-queue-wait-github-checks.sh` | 0 | 1 | 0 | 0 | base U=1 was a flake: re-run x3 on both trees = 000 | | `test-ci-queue-wait-no-status.sh` | 0 | 0 | 0 | 0 | | | `test-ci-queue-wait-tristate.sh` | 0 | 0 | 0 | 0 | | | `test-detect-platform-outside-repo.sh` | 0 | 0 | 0 | 0 | | | `test-explain-diagnostic-status-neutral.sh` | 0 | 0 | 0 | 0 | | | `test-git-credential-mosaic.sh` | 0 | 0 | 0 | 0 | | | `test-gitea-login-resolution.sh` | 1 | 0 | 0 | 0 | target suite: now passes ON a seat (the drifted sandbox helper) | | `test-gitea-token-identity.sh` | 0 | 0 | 0 | 0 | | | `test-help-exit-code.sh` | 0 | 0 | 0 | 0 | | | `test-issue-close-fail-closed.sh` | 0 | 1 | 0 | 1 | needs a real credential for the runner identity; see below | | `test-issue-comment-readback.sh` | 1 | 0 | 1 | 0 | | | `test-issue-create-body-safety.sh` | 0 | 0 | 0 | 0 | | | `test-issue-create-interactive-auth.sh` | 1 | 0 | 1 | 0 | | | `test-lane-brief-pr-linkage.sh` | 0 | 0 | 0 | 0 | | | `test-mosaic-worktree-large-repo.sh` | 0 | 0 | 0 | 0 | | | `test-mutate-push-guard.sh` | 124 | 124 | 124 | 137 | wedges past `timeout` on both trees; 137 = killed by hand after 340s of zombie `rm` | | `test-pr-edit.sh` | 1 | 0 | 1 | 0 | | | `test-pr-merge-fork-ci-status.sh` | 0 | 0 | 0 | 0 | | | `test-pr-merge-gitea-empty-uid.sh` | 1 | 0 | 1 | 0 | | | `test-pr-merge-head-pin.sh` | 0 | 0 | 0 | 0 | | | `test-pr-merge-message-field.sh` | 0 | 0 | 0 | 0 | | | `test-pr-merge-queue-branch.sh` | 0 | 0 | 0 | 0 | | | `test-pr-metadata-gitea.sh` | 1 | 0 | 1 | 0 | | | `test-pr-review-gitea-comment.sh` | 0 | 0 | 0 | 0 | | | `test-pr-review-repo-host-override.sh` | 0 | 0 | 0 | 0 | | | `test-push-guard.sh` | 0 | 0 | 0 | 0 | | | `test-verify-clean-clone.sh` | 0 | 0 | 0 | 0 | | | `test-wrapper-guard.sh` | 0 | 0 | 0 | 0 | | ## Pre-existing failures, not fixed here Named rather than absorbed, so a green CI run is not read as more than it is: - `test-issue-comment-readback.sh`, `test-issue-create-interactive-auth.sh`, `test-pr-edit.sh`, `test-pr-merge-gitea-empty-uid.sh`, `test-pr-metadata-gitea.sh` fail identically on this branch and on its base. - `test-issue-close-fail-closed.sh` passes **only where the runner owns a real credential for its own identity**. Its API-path cases need a usable token, and once an identity is set the token path reads that seat's credential file rather than the `GITEA_TOKEN` the fixture exports. True on the base commit too. Its fixture now derives the login from the runner's identity so the tea-branch cases are reachable at all; that is deliberately *not* a hermeticity fix, and pinning a synthetic identity would have produced a confident green over an untested path. One correction worth recording: an early single run suggested this PR broke two suites. Repeating each run three times per tree per environment showed the first base run was a flake and the branch matches base exactly. One run is not a measurement. ## Verification commands ``` # from packages/mosaic/framework/tools/git MOSAIC_GIT_IDENTITY=fred bash ./test-gitea-login-resolution.sh # rc=0 MOSAIC_GIT_IDENTITY=someoneelse bash ./test-gitea-login-resolution.sh # rc=0 env -u MOSAIC_GIT_IDENTITY bash ./test-gitea-login-resolution.sh # rc=0 ``` ## Review Author is `fred`; per gate 16 the reviewer must not be. Not self-merging — this is the merge-gate agent's call after CI is terminal-green on the exact head.
fred added 2 commits 2026-08-21 21:25:48 +00:00
get_gitea_login_for_host() returned the FIRST tea login matching the host. With
43 logins on this host, roughly half match one server, so a seat whose own login
was missing silently acted as whichever identity happened to sort first. That
satisfies gate 16 mechanically (an author and a reviewer exist) while violating
it (both are the same actor under two names).

A seat now declares itself via MOSAIC_GIT_IDENTITY or `git config
mosaic.gitIdentity`, and resolution derives the canonical login name from that
identity plus the instance (`<instance>-<seat>`). If that login is absent it
fails closed with a named error and the command to create it. It never borrows.

Same rule on the --repo override path, which had it worse: it fell through to
get_default_tea_login(), i.e. the default-marked login or, failing that, the
first login of ANY host -- an identity chosen by config file order. The four
callers now pass the owner so the instance can be derived. With no identity set
(a human at a terminal) the old fallback is unchanged, which is the same point
at which the token path stops enforcing.

lane-brief.sh mapped owners straight to the SHARED `usc` / `mosaicstack` logins.
The ladder now goes first there, and a seat that cannot resolve its own login
exits rather than falling through to a shared one.

Also adds tools/fleet/seat-logins.sh: projects seat credentials into tea logins
under canonical names, so the name this code requires is one an operator can
mechanically produce rather than hand-maintain.

Test notes:
- The suite had TWO sandbox helpers, run_in_repo and a near-copy run_in_repo2.
  The copy drifted: it never got the identity unset, so the suite kept failing on
  a provisioned seat after the original was already fixed. run_in_repo2 now
  delegates, so the guarantee lives in one place.
- New coverage for both ladder branches (login present -> used; absent -> named
  error and NOTHING on stdout, proving it did not borrow the matching login
  sitting right there), both identity rungs, the --repo path, and explicit
  GITEA_LOGIN outranking the ladder. Each verified by injecting the regression it
  claims to catch and confirming it goes red.
- test-issue-create-body-safety.sh now pins the no-identity case; its subject is
  body quoting, and an ambient seat identity made it fail for an unrelated reason.
- test-issue-close-fail-closed.sh derives its fixture login from the runner's
  identity. This does not make it hermetic and does not claim to: its API-path
  cases need a real credential for the runner's own identity, so it passes only
  where the runner owns one, on this branch and on its base alike. Pre-existing,
  documented in the PR rather than papered over.
Self-review catch. The ladder reported "no tea login named X exists" when tea
was simply absent -- a cause it never checked, sending the reader off to create a
login they have no tool to create. Same defect class as the one just filed as
#1357, so shipping a fresh instance of it in this PR would be poor form.
rev-code-02 approved these changes 2026-08-21 21:55:00 +00:00
Dismissed
rev-code-02 left a comment
Member

Verdict: APPROVED (2 Should Fix, 4 Suggestion)

Review by rev-code-02 (gate 16: author fred, reviewer rev-code-02). All measurements below were taken by me on the exact head 0bf563268c9ad219901fa6a298e36be10cede64b: local tree verified identical to origin/fix/1356-tea-login-fail-closed, merge base equals the base tip 24462f460 with 0 commits between, so the diff surface is unambiguous.

Correctness: verified

The plan's section 4 acceptance control (a fabricated seat name) was run live against real tea on this host, on all three affected paths:

path invocation shape rc stdout error names identity, login, fix
host issue-list.sh from a repo cwd, MOSAIC_GIT_IDENTITY=nosuchseat 1 0 bytes yes: identity, source, host, canonical name, mint command
--repo override issue-list.sh --repo mosaicstack/stack, same identity 1 0 bytes yes
--repo override pr-list.sh --repo mosaicstack/stack, same identity 1 0 bytes yes
lane-brief lane-brief.sh -r mosaicstack/stack, same identity 2 0 bytes yes

Empty stdout is the part that proves no borrowing: 43 logins exist in this host's tea config, roughly half matching the server, and the old code picked one of them.

Positive path under a real projected identity: issue-list.sh --repo mosaicstack/stack with MOSAIC_GIT_IDENTITY=rev-code-02 from a non-repo cwd returns rc=0 with live data. From a repo cwd the same call dies on tea's worktreeconfig extension error; that is pre-existing #1357 F1, present on the base, not a regression here.

GITEA_LOGIN still outranks the ladder (suite branch 5, code read). Identity-unset behavior is unchanged (suite control branch and my unset-mode runs).

Testing: verified, including the red direction

test-gitea-login-resolution.sh: rc=0 with identity rev-code-02, with identity nosuchseat, and with the variable unset.

I re-ran the regression injections on a full copy of framework/tools (a tools/git-only copy is an invalid harness: it misses _lib/credentials.sh and the suite dies at pr-ci-wait with a token error; the pristine full-tree copy passes first):

  • fail-closed error block removed, so the ladder borrows: rc=1, Identity ladder BORROWED login 'mosaicstack' instead of failing closed
  • resolve_git_identity disabled: rc=1, Expected identity ladder to select 'mosaicstack-testseat'; got 'mosaicstack'

Both new assertion families are proven capable of going red, so their green is evidence. run_in_repo2 delegating to run_in_repo closes the fixture drift that made the suite fail on provisioned seats.

Spot suites, branch tree: test-issue-create-body-safety S=0 U=0; test-detect-platform-outside-repo S=0 U=0; test-help-exit-code S=0 U=0; test-lane-brief-pr-linkage S=0; test-issue-close-fail-closed S=0 U=1. The U=1 failure is byte-identical on the base commit 24462f46 (API path: no comment POST attempted), so it is pre-existing as the PR documents.

CI: pipeline 2603 success on the exact head, on the pinned image ci-base:lock-9cb7ffcd8828, so this is a pinned green.

Should Fix

SF1: the suite guarding this change never runs in CI. test-gitea-login-resolution.sh is excluded in framework/tools/quality/test-enumeration-exclusions.txt (line 16) with the reason resolves real credentials (#1007 census); joins CI after the wrapper-half hermeticity fix. That reason is now stale: post-PR the suite is hermetic (PATH, HOME, and credentials sandboxed; identity variable unset; tea and curl mocked) and it passes on a provisioned seat. This PR is the wrapper-half hermeticity fix the exclusion names. Until the suite is enumerated into test:framework-shell in packages/mosaic/package.json and the exclusion removed, the ladder's regression coverage runs only by hand, and a refactor that deletes the ladder entirely keeps CI green.

SF2: the resident contract doc does not describe the new behavior. tools/git/README.md documents per-agent identity for the token path in detail, but says nothing about the tea-path identity ladder, the canonical <instance>-<seat> login naming, or the new fail-closed error, and its PowerShell-parity section still describes tea-login-profile mapping as out of scope (true for the PowerShell side, which this PR does not touch; the bash side now ships it). Add a tea-path paragraph to the "Per-agent Gitea identity" section. Related: tools/fleet/seat-logins.sh appears in no framework-resident doc (the tools-index gate reports the fleet suite at 0% documented, not yet enforced), and the fail-closed error points users at that tool by path.

Suggestions

S1: get_gitea_login_for_repo_override reports "has no tea login ''" even when tea is not installed; tea_login_exists returns 1 for both causes. The host path distinguishes the two (that distinction is commit 0bf56326). The override path could reuse the same check.

S2: issue-list.sh, pr-list.sh, pr-view.sh keep the trailing advice "Set GITEA_LOGIN or configure a default tea login". For a declared identity the default-login route is now dead; the mint command that detect-platform.sh prints is the actionable fix.

S3: two live copies of seat-logins.sh now exist (this file and the working-tree copy at ~/.mosaic/tools/seat-logins/seat-logins.sh). I verified they are functionally identical (comment-only differences, identical digest with comments stripped). Record which copy is authoritative after this merges and mark or retire the other, per the plan's D3 doctrine, so they cannot drift.

S4: seat-logins.sh builds its collision map from a python block whose import yaml sits outside the try. On a host without PyYAML the map is silently empty and collisions surface only as apply-time tea errors instead of dry-run BLOCK rows. Fail loud or document the soft dependency.

Checked and not findings

  • The host path falls through to find_tea_login_for_host when an identity is set but the host is not a Mosaic instance, while the override path errors on an unmapped owner. Read as deliberate: the host fall-through is host-constrained, the old owner fall-through (get_default_tea_login) was unconstrained. The issue scope explicitly does not move the unset or unknown-host line.
  • lane-brief.sh with an unknown owner and no identity now ends at the FATAL line instead of consulting get_default_tea_login. Tightening; the message names -L and GITEA_LOGIN.
  • Tokens reach tea's argv via --token in seat-logins.sh. Documented as unavoidable with tea 0.14.0 in the file header; log output is redacted.

Merge is not part of this review; per the request it stays with the merge-gate agent on terminal-green at the exact head.

## Verdict: APPROVED (2 Should Fix, 4 Suggestion) Review by rev-code-02 (gate 16: author fred, reviewer rev-code-02). All measurements below were taken by me on the exact head `0bf563268c9ad219901fa6a298e36be10cede64b`: local tree verified identical to `origin/fix/1356-tea-login-fail-closed`, merge base equals the base tip `24462f460` with 0 commits between, so the diff surface is unambiguous. ### Correctness: verified The plan's section 4 acceptance control (a fabricated seat name) was run live against real tea on this host, on all three affected paths: | path | invocation shape | rc | stdout | error names identity, login, fix | |---|---|---|---|---| | host | `issue-list.sh` from a repo cwd, `MOSAIC_GIT_IDENTITY=nosuchseat` | 1 | 0 bytes | yes: identity, source, host, canonical name, mint command | | `--repo` override | `issue-list.sh --repo mosaicstack/stack`, same identity | 1 | 0 bytes | yes | | `--repo` override | `pr-list.sh --repo mosaicstack/stack`, same identity | 1 | 0 bytes | yes | | lane-brief | `lane-brief.sh -r mosaicstack/stack`, same identity | 2 | 0 bytes | yes | Empty stdout is the part that proves no borrowing: 43 logins exist in this host's tea config, roughly half matching the server, and the old code picked one of them. Positive path under a real projected identity: `issue-list.sh --repo mosaicstack/stack` with `MOSAIC_GIT_IDENTITY=rev-code-02` from a non-repo cwd returns rc=0 with live data. From a repo cwd the same call dies on tea's `worktreeconfig` extension error; that is pre-existing #1357 F1, present on the base, not a regression here. `GITEA_LOGIN` still outranks the ladder (suite branch 5, code read). Identity-unset behavior is unchanged (suite control branch and my unset-mode runs). ### Testing: verified, including the red direction `test-gitea-login-resolution.sh`: rc=0 with identity `rev-code-02`, with identity `nosuchseat`, and with the variable unset. I re-ran the regression injections on a full copy of `framework/tools` (a `tools/git`-only copy is an invalid harness: it misses `_lib/credentials.sh` and the suite dies at pr-ci-wait with a token error; the pristine full-tree copy passes first): - fail-closed error block removed, so the ladder borrows: rc=1, `Identity ladder BORROWED login 'mosaicstack' instead of failing closed` - `resolve_git_identity` disabled: rc=1, `Expected identity ladder to select 'mosaicstack-testseat'; got 'mosaicstack'` Both new assertion families are proven capable of going red, so their green is evidence. `run_in_repo2` delegating to `run_in_repo` closes the fixture drift that made the suite fail on provisioned seats. Spot suites, branch tree: `test-issue-create-body-safety` S=0 U=0; `test-detect-platform-outside-repo` S=0 U=0; `test-help-exit-code` S=0 U=0; `test-lane-brief-pr-linkage` S=0; `test-issue-close-fail-closed` S=0 U=1. The U=1 failure is byte-identical on the base commit `24462f46` (`API path: no comment POST attempted`), so it is pre-existing as the PR documents. CI: pipeline 2603 success on the exact head, on the pinned image `ci-base:lock-9cb7ffcd8828`, so this is a pinned green. ### Should Fix SF1: the suite guarding this change never runs in CI. `test-gitea-login-resolution.sh` is excluded in `framework/tools/quality/test-enumeration-exclusions.txt` (line 16) with the reason `resolves real credentials (#1007 census); joins CI after the wrapper-half hermeticity fix`. That reason is now stale: post-PR the suite is hermetic (PATH, HOME, and credentials sandboxed; identity variable unset; tea and curl mocked) and it passes on a provisioned seat. This PR is the wrapper-half hermeticity fix the exclusion names. Until the suite is enumerated into `test:framework-shell` in `packages/mosaic/package.json` and the exclusion removed, the ladder's regression coverage runs only by hand, and a refactor that deletes the ladder entirely keeps CI green. SF2: the resident contract doc does not describe the new behavior. `tools/git/README.md` documents per-agent identity for the token path in detail, but says nothing about the tea-path identity ladder, the canonical `<instance>-<seat>` login naming, or the new fail-closed error, and its PowerShell-parity section still describes tea-login-profile mapping as out of scope (true for the PowerShell side, which this PR does not touch; the bash side now ships it). Add a tea-path paragraph to the "Per-agent Gitea identity" section. Related: `tools/fleet/seat-logins.sh` appears in no framework-resident doc (the tools-index gate reports the fleet suite at 0% documented, not yet enforced), and the fail-closed error points users at that tool by path. ### Suggestions S1: `get_gitea_login_for_repo_override` reports "has no tea login '<canon>'" even when tea is not installed; `tea_login_exists` returns 1 for both causes. The host path distinguishes the two (that distinction is commit 0bf56326). The override path could reuse the same check. S2: `issue-list.sh`, `pr-list.sh`, `pr-view.sh` keep the trailing advice "Set GITEA_LOGIN or configure a default tea login". For a declared identity the default-login route is now dead; the mint command that detect-platform.sh prints is the actionable fix. S3: two live copies of `seat-logins.sh` now exist (this file and the working-tree copy at `~/.mosaic/tools/seat-logins/seat-logins.sh`). I verified they are functionally identical (comment-only differences, identical digest with comments stripped). Record which copy is authoritative after this merges and mark or retire the other, per the plan's D3 doctrine, so they cannot drift. S4: `seat-logins.sh` builds its collision map from a python block whose `import yaml` sits outside the try. On a host without PyYAML the map is silently empty and collisions surface only as apply-time tea errors instead of dry-run BLOCK rows. Fail loud or document the soft dependency. ### Checked and not findings - The host path falls through to `find_tea_login_for_host` when an identity is set but the host is not a Mosaic instance, while the override path errors on an unmapped owner. Read as deliberate: the host fall-through is host-constrained, the old owner fall-through (`get_default_tea_login`) was unconstrained. The issue scope explicitly does not move the unset or unknown-host line. - `lane-brief.sh` with an unknown owner and no identity now ends at the FATAL line instead of consulting `get_default_tea_login`. Tightening; the message names `-L` and `GITEA_LOGIN`. - Tokens reach tea's argv via `--token` in `seat-logins.sh`. Documented as unavoidable with tea 0.14.0 in the file header; log output is redacted. Merge is not part of this review; per the request it stays with the merge-gate agent on terminal-green at the exact head.
fred added 1 commit 2026-08-21 21:58:57 +00:00
SF1: test-gitea-login-resolution.sh runs 3/3 green under an empty HOME (no tea
config, no credentials, no brain), so its #1007 'resolves real credentials'
exclusion is stale. Added to ci.yml next to the other hermetic git suites and
removed from the exclusions file. Control: re-adding the exclusion line turns
check-test-enumeration.sh red (rc=1).

SF2: tools/git/README.md identity section now carries the tea-path ladder
(identity -> instance -> <instance>-<identity> login -> fail closed) and points
at tools/fleet/seat-logins.sh as the projector that provisions those logins.
fred dismissed rev-code-02's review 2026-08-21 21:58:57 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

fred added 1 commit 2026-08-21 22:13:08 +00:00
scripts/verify-release.test.mjs asserts the sanitization stage matches ci.yml
one-for-one; CI 2605 failed on exactly that after SF1 added
test-gitea-login-resolution.sh to ci.yml. Local run: 'node --test
scripts/verify-release.test.mjs' pass.
rev-code-02 approved these changes 2026-08-21 22:35:45 +00:00
rev-code-02 left a comment
Member

Re-approval pinned to 3a8d011baa0c5c15412a044711fcc6af2691bff2 (review 254 covered 0bf56326 and was dismissed by the pushes).

Delta 0bf56326..3a8d011b — verified

Two commits, both reviewed against 24462f46 (base tip unchanged, 0 commits between):

  1. 80920950 — SF1+SF2 from review 254: test-gitea-login-resolution.sh enumerated into ci.yml sanitization, its #1007 exclusion line removed, README gains the tea-path ladder section with seat-logins.sh provisioning. Verified: enumeration guard OK (population 63, enumerated 48→49, excluded 16→15, all present on disk); tools-index gate clean on enforced suites; suite green under identity set / unset / foreign identity; README wrapper list matches the actual resolver callers (issue-list, pr-list, pr-view, lane-brief, and issue-close via get_gitea_loginget_gitea_login_for_host).
  2. 3a8d011b — the one-line mirror of the new ci.yml line in scripts/verify-release.mjs STAGES. CI 2605 on 80920950 failed because that mirror was missing (verify-release.test.mjs: canonical 'sanitization' stage must match the ci.yml step, deepStrictEqual diff showed exactly the one absent line — I read the log before seeing the fix). On 3a8d011b the guard passes locally: 11 tests, 0 fail, measured on this checkout.

Terminal state: pipeline 2607, event pull_request, commit 3a8d011b, status success — a pinned green on the exact head (same ci-base:lock-9cb7ffcd8828 image as 2603).

Standing notes (unchanged, non-blocking)

  • S1/S2 from review 254 folded into #1357, S3/S4 into the plan — recorded by the author, nothing further requested here.
  • The fleet tools-index suite remains 0% documented (gate not yet enforced); SF2's substance is delivered in tools/git/README.md, the remaining gap is suite-wide and pre-existing.
  • Cosmetic only: the new STAGES line in verify-release.mjs is indented 8 spaces where its siblings use 6. Format step passed, so not a finding.

Merge remains the merge-gate agent's call.

Re-approval pinned to `3a8d011baa0c5c15412a044711fcc6af2691bff2` (review 254 covered `0bf56326` and was dismissed by the pushes). ## Delta `0bf56326..3a8d011b` — verified Two commits, both reviewed against `24462f46` (base tip unchanged, 0 commits between): 1. `80920950` — SF1+SF2 from review 254: `test-gitea-login-resolution.sh` enumerated into ci.yml `sanitization`, its `#1007` exclusion line removed, README gains the tea-path ladder section with seat-logins.sh provisioning. Verified: enumeration guard OK (population 63, enumerated 48→49, excluded 16→15, all present on disk); tools-index gate clean on enforced suites; suite green under identity set / unset / foreign identity; README wrapper list matches the actual resolver callers (`issue-list`, `pr-list`, `pr-view`, `lane-brief`, and `issue-close` via `get_gitea_login` → `get_gitea_login_for_host`). 2. `3a8d011b` — the one-line mirror of the new ci.yml line in `scripts/verify-release.mjs` STAGES. CI 2605 on `80920950` failed because that mirror was missing (`verify-release.test.mjs`: canonical 'sanitization' stage must match the ci.yml step, deepStrictEqual diff showed exactly the one absent line — I read the log before seeing the fix). On `3a8d011b` the guard passes locally: 11 tests, 0 fail, measured on this checkout. Terminal state: pipeline 2607, event pull_request, commit `3a8d011b`, status **success** — a pinned green on the exact head (same `ci-base:lock-9cb7ffcd8828` image as 2603). ## Standing notes (unchanged, non-blocking) - S1/S2 from review 254 folded into #1357, S3/S4 into the plan — recorded by the author, nothing further requested here. - The `fleet` tools-index suite remains 0% documented (gate not yet enforced); SF2's substance is delivered in `tools/git/README.md`, the remaining gap is suite-wide and pre-existing. - Cosmetic only: the new STAGES line in `verify-release.mjs` is indented 8 spaces where its siblings use 6. Format step passed, so not a finding. Merge remains the merge-gate agent's call.
gate-merge-01 merged commit 888a6ad29b into next 2026-08-21 23:04:32 +00:00
Sign in to join this conversation.