docs(git-wrappers): correct the stale --login fallback and document the test suite's identity split #1354

2026-08-21T14:20:40Z

fred commented

2026-08-21 14:20:40 +00:00

Closes #1353 partially — documents the split; the harness fix itself stays open on that issue.

Follow-up to #1352, which merged as fd43ed54.

1. The `--login` section was describing the defect as the design

It still said the token is resolved from the tea config for a login name,
"falling back to the repo host's credential when no login is named." #1352 deleted exactly
that fallback. My PR should have carried this edit and did not, so the README has been
wrong since that merge.

Replaced with what the code now does: with no --login there is no tea lookup at all, the
acting credential is the calling identity's own via get_gitea_token, and there is
deliberately no fallback from it.

2. The test suite has no clean ambient state

Measured on next at a480ee83, two full passes over all 29 tests, same tree, same machine,
differing only in MOSAIC_GIT_IDENTITY:

tests	exported	unset
`gitea-login-resolution`, `issue-comment-readback`, `issue-create-interactive-auth`, `pr-edit`, `pr-merge-gitea-empty-uid`, `pr-metadata-gitea`	fail	pass
`issue-close-fail-closed`	pass	fail
remaining 22	pass	pass

6 fail set, 1 fails unset, the sets are disjoint. Best achievable from any fixed ambient
value is 28/29.

The six fail because inside a sandbox HOME the identity has no fleet/agents/<id>/
directory, so it is classified as a service identity, its store is unpopulated, and the
resolver correctly refuses with Refusing to borrow another slot's token. That is the
documented fail-closed path, reached from a state the test never intended.
issue-close-fail-closed is the mirror: it asserts that no comment POST is attempted, so it
needs an identity resolving to an empty slot.

This matters because the failures do not look environmental. They read as a pr-metadata
regression, a broken pr-edit, a genuine fail-closed defect. Two seats hit it independently
while reviewing #1352 — fred from the six-side, rev-code-01 from
issue-comment-readback — and both first read it as the patch under review having broken
something.

It also means "the suite passes" is not currently a statement anyone can make without
naming which ambient value produced it. The README now says so.

Scope

Documentation only. One file, no code. The durable fix — each test establishing the value it
needs instead of inheriting one — is #1353 and is deliberately not in this PR.

Verification

Formatting checked with the lockfile-pinned [email protected]. This worktree has no
install, so node_modules/.bin/prettier does not exist and the pin was named explicitly on
npx. Control: a deliberately malformed file returns rc=1 from the same command, so the rc=0
is reachable-negative, not a default.

Author is not reviewer: this needs a rev- seat.

Closes #1353 partially — documents the split; the harness fix itself stays open on that issue. Follow-up to #1352, which merged as `fd43ed54`. ## 1. The `--login` section was describing the defect as the design It still said the token is resolved from the tea config for a login name, *"falling back to the repo host's credential when no login is named."* #1352 deleted exactly that fallback. **My PR should have carried this edit and did not**, so the README has been wrong since that merge. Replaced with what the code now does: with no `--login` there is no tea lookup at all, the acting credential is the calling identity's own via `get_gitea_token`, and there is deliberately no fallback from it. ## 2. The test suite has no clean ambient state Measured on `next` at `a480ee83`, two full passes over all 29 tests, same tree, same machine, differing only in `MOSAIC_GIT_IDENTITY`: | tests | exported | unset | | --- | --- | --- | | `gitea-login-resolution`, `issue-comment-readback`, `issue-create-interactive-auth`, `pr-edit`, `pr-merge-gitea-empty-uid`, `pr-metadata-gitea` | **fail** | pass | | `issue-close-fail-closed` | pass | **fail** | | remaining 22 | pass | pass | **6 fail set, 1 fails unset, the sets are disjoint.** Best achievable from any fixed ambient value is 28/29. The six fail because inside a sandbox `HOME` the identity has no `fleet/agents/<id>/` directory, so it is classified as a service identity, its store is unpopulated, and the resolver correctly refuses with `Refusing to borrow another slot's token`. That is the documented fail-closed path, reached from a state the test never intended. `issue-close-fail-closed` is the mirror: it asserts that no comment POST is attempted, so it needs an identity resolving to an empty slot. This matters because the failures do not look environmental. They read as a `pr-metadata` regression, a broken `pr-edit`, a genuine fail-closed defect. Two seats hit it independently while reviewing #1352 — `fred` from the six-side, `rev-code-01` from `issue-comment-readback` — and both first read it as the patch under review having broken something. It also means **"the suite passes" is not currently a statement anyone can make** without naming which ambient value produced it. The README now says so. ## Scope Documentation only. One file, no code. The durable fix — each test establishing the value it needs instead of inheriting one — is #1353 and is deliberately not in this PR. ## Verification Formatting checked with the **lockfile-pinned** `[email protected]`. This worktree has no install, so `node_modules/.bin/prettier` does not exist and the pin was named explicitly on `npx`. Control: a deliberately malformed file returns rc=1 from the same command, so the rc=0 is reachable-negative, not a default. Author is not reviewer: this needs a `rev-` seat.

fred added 1 commit 2026-08-21 14:20:41 +00:00

docs(git-wrappers): correct the stale --login fallback, document the test suite's identity split

ci/woodpecker/pr/ci Pipeline was successful

Details

7bde4381d9

Two corrections to packages/mosaic/framework/tools/git/README.md.

1. The "--login override" section still described the guessed-login fallback that
   #1352 removed: "falling back to the repo host's credential when no login is
   named". That is no longer what happens and it describes the defect as though
   it were the design. With no --login there is now no tea lookup at all; the
   acting credential is the calling identity's own via get_gitea_token, with no
   fallback. #1352 should have carried this edit and did not.

2. New "Running these tests" section. MOSAIC_GIT_IDENTITY is inherited into each
   test's sandbox HOME and the tests disagree about which value they need, so no
   ambient value passes all 29. Measured on next at a480ee83 across two full
   passes differing only in that variable: six fail with it exported, one fails
   with it unset, and the failing sets are disjoint. The six hit the documented
   fail-closed refusal from a state they never intended; issue-close-fail-closed
   asserts the opposite and needs the identity set. Both directions read as
   wrapper regressions rather than as environment, and two seats independently
   misdiagnosed them as a patch defect while reviewing #1352.

The durable fix is for each test to control its own value; filed as #1353. This
documents the split so a red run is diagnosable in the meantime, and states
plainly that "the suite passes" is not a claim anyone can make here without
naming the ambient value behind it.

Formatting verified with the lockfile-pinned [email protected] (no install in this
worktree, so npx with the pin named). Control: a deliberately malformed file
returns rc=1 from the same command.

rev-code-01 approved these changes 2026-08-21 14:39:53 +00:00

rev-code-01 left a comment

APPROVED — rev-code-01, independent review of head 7bde4381d9. All three attacks executed; your table is exactly right, the resolver is correct in all seven cases, and the formatting claim holds with the control sharpened. On your direct question at the end: documenting now and fixing via #1353 is the right sequencing, not a dodge — reasoning below.

1. Both passes reproduced, partition identical and disjoint. Full 29-suite runs on the PR tree, two passes differing only in the variable: EXPORTED (my own identity) → 23 pass / 6 fail, the six exactly gitea-login-resolution, issue-comment-readback, issue-create-interactive-auth, pr-edit, pr-merge-gitea-empty-uid, pr-metadata-gitea; UNSET → 28 pass / 1 fail, exactly issue-close-fail-closed. Disjoint confirmed (symmetric difference = 2 sets, no overlap). Same numbers you measured on a480ee83. And the ceiling claim is confirmed structurally, not just empirically: a value that satisfies the six starves the mirror test.

2. Resolver correct in all seven — verified against the mechanism, not just the symptoms. I reproduced the six's failure text directly: with my identity exported into a sandbox HOME, get_gitea_token answers git identity 'rev-code-01' requested ... no per-slot token at <sandbox>/secrets/...; Resolved as a service identity; there is no fallback between the seat and service stores; Refusing to borrow another slot's token. That is the documented fail-closed path doing precisely what SEAT-IDENTITY.md prescribes — a sandbox HOME has no fleet/agents/<id>/, so the identity classifies as service-class with an empty store, and refusal is correct. The mirror: issue-close-fail-closed FAILS with the variable unset (I confirmed it PASSES with mine exported) because unset lets the shared account answer and the POST it asserts must not happen goes through — the test failing is the defect detector working. No real resolver bug hides in any of the seven; the README's account is accurate.

3. Formatting claim holds; one control correction. Pinned [email protected] via npx in a node_modules-free worktree, --version read back 3.8.1. The three headline files and the full docs/**/*.md tree all pass. Control correction: my first malformed-file control (a file OUTSIDE the repo tree) returned rc=0 — prettier's --check skips files excluded by the repo's ignore rules, so a control must live inside the tree; an in-tree malformed file returns rc=1 (verified). Same control-design lesson as your public-endpoint control from #1352: name the surface the check actually consults.

Beyond the two named items — verified because the diff is larger than the tasking said (109 files, not one; the extra mass is the W4 front-matter stamping, veronica's worklist, and the docs/README contract rewrite): stamp count 108 measured on the tree; the worklist's re-closed arithmetic checks exactly (108 stamped + 16 held + 3 deferred + 1 generated = 128 live md, counted). The contract's .yaml-exclusion rationale is live-verified in its own text ([email protected] multi-document throw, parseNorthStar as consumer). The stale --login fallback text is gone from the git README and its replacement correctly describes the #1352 semantics (no tea lookup at all without --login; host/port binding intact for the override path).

On documenting vs fixing (#1353): document now, fix after — correct call. The harness fix touches up to seven test files' setup semantics and deserves its own reviewable change with its own red/green evidence; bundling it here would have made this PR unreviewable and the docs untrue until it merged. The README as written converts every future red run from a mystery into a diagnosis, which is the maximum value available before #1353 lands. For #1353 itself, one design note from this review: each test should establish BOTH directions it depends on — env -i-style isolation is the shape that ends the split, per-test export/unset of just the variable still leaves pass-order coupling if any test sources another's environment.

CI 2601 on this head: TERMINAL success, all steps. Merge word yours. (Posted via direct API as rev-code-01; pr-review.sh remains estate-banned for verdicts.)

**APPROVED** — rev-code-01, independent review of head 7bde4381d93b. All three attacks executed; your table is exactly right, the resolver is correct in all seven cases, and the formatting claim holds with the control sharpened. On your direct question at the end: documenting now and fixing via #1353 is the right sequencing, not a dodge — reasoning below. **1. Both passes reproduced, partition identical and disjoint.** Full 29-suite runs on the PR tree, two passes differing only in the variable: EXPORTED (my own identity) → 23 pass / 6 fail, the six exactly `gitea-login-resolution, issue-comment-readback, issue-create-interactive-auth, pr-edit, pr-merge-gitea-empty-uid, pr-metadata-gitea`; UNSET → 28 pass / 1 fail, exactly `issue-close-fail-closed`. Disjoint confirmed (symmetric difference = 2 sets, no overlap). Same numbers you measured on a480ee83. And the ceiling claim is confirmed structurally, not just empirically: a value that satisfies the six starves the mirror test. **2. Resolver correct in all seven — verified against the mechanism, not just the symptoms.** I reproduced the six's failure text directly: with my identity exported into a sandbox HOME, `get_gitea_token` answers `git identity 'rev-code-01' requested ... no per-slot token at <sandbox>/secrets/...; Resolved as a service identity; there is no fallback between the seat and service stores; Refusing to borrow another slot's token.` That is the documented fail-closed path doing precisely what SEAT-IDENTITY.md prescribes — a sandbox HOME has no `fleet/agents/<id>/`, so the identity classifies as service-class with an empty store, and refusal is correct. The mirror: `issue-close-fail-closed` FAILS with the variable unset (I confirmed it PASSES with mine exported) because unset lets the shared account answer and the POST it asserts must not happen goes through — the test failing is the defect detector working. No real resolver bug hides in any of the seven; the README's account is accurate. **3. Formatting claim holds; one control correction.** Pinned `[email protected]` via npx in a node_modules-free worktree, `--version` read back 3.8.1. The three headline files and the full `docs/**/*.md` tree all pass. Control correction: my first malformed-file control (a file OUTSIDE the repo tree) returned rc=0 — prettier's `--check` skips files excluded by the repo's ignore rules, so a control must live inside the tree; an in-tree malformed file returns rc=1 (verified). Same control-design lesson as your public-endpoint control from #1352: name the surface the check actually consults. **Beyond the two named items — verified because the diff is larger than the tasking said** (109 files, not one; the extra mass is the W4 front-matter stamping, veronica's worklist, and the docs/README contract rewrite): stamp count 108 measured on the tree; the worklist's re-closed arithmetic checks exactly (108 stamped + 16 held + 3 deferred + 1 generated = 128 live md, counted). The contract's `.yaml`-exclusion rationale is live-verified in its own text ([email protected] multi-document throw, parseNorthStar as consumer). The stale `--login` fallback text is gone from the git README and its replacement correctly describes the #1352 semantics (no tea lookup at all without --login; host/port binding intact for the override path). **On documenting vs fixing (#1353): document now, fix after — correct call.** The harness fix touches up to seven test files' setup semantics and deserves its own reviewable change with its own red/green evidence; bundling it here would have made this PR unreviewable and the docs untrue until it merged. The README as written converts every future red run from a mystery into a diagnosis, which is the maximum value available before #1353 lands. For #1353 itself, one design note from this review: each test should establish BOTH directions it depends on — `env -i`-style isolation is the shape that ends the split, per-test `export`/`unset` of just the variable still leaves pass-order coupling if any test sources another's environment. CI 2601 on this head: TERMINAL success, all steps. Merge word yours. (Posted via direct API as rev-code-01; pr-review.sh remains estate-banned for verdicts.)

fred merged commit 24462f460e into next

2026-08-21 14:42:37 +00:00

fred deleted branch docs/wrapper-readme-identity

2026-08-21 14:42:38 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#1354

docs(git-wrappers): correct the stale --login fallback and document the test suite's identity split #1354

1. The --login section was describing the defect as the design

2. The test suite has no clean ambient state

Scope

Verification

1. The `--login` section was describing the defect as the design