mosaicstack/stack
14
0
Fork 1
Code Issues 315 Pull Requests 11 Actions Packages Projects Releases 15 Wiki Activity

git wrapper tests inherit MOSAIC_GIT_IDENTITY and disagree on it: no ambient value passes all 29 #1353

New Issue
Closed
opened 2026-08-21 14:19:26 +00:00 by fred · 0 comments
fred commented 2026-08-21 14:19:26 +00:00
Collaborator
Copy Link
Copy Source

What is broken

The 29 wrapper tests in packages/mosaic/framework/tools/git/ do not control
MOSAIC_GIT_IDENTITY. They inherit whatever the invoking shell exports into their sandbox
HOME, and they disagree about which value they need, so no single ambient state passes
the suite.

Measured on next at a480ee83

Two full passes over all 29 tests, same tree, same machine, differing only in the variable:

test MOSAIC_GIT_IDENTITY=fred exported env -u MOSAIC_GIT_IDENTITY
test-gitea-login-resolution.sh FAIL pass
test-issue-comment-readback.sh FAIL pass
test-issue-create-interactive-auth.sh FAIL pass
test-pr-edit.sh FAIL pass
test-pr-merge-gitea-empty-uid.sh FAIL pass
test-pr-metadata-gitea.sh FAIL pass
test-issue-close-fail-closed.sh pass FAIL
other 22 pass pass

6 fail set, 1 fails unset, and the failing sets are disjoint. Best achievable from a
fixed ambient value is 28/29 in either direction.

Mechanism, both directions

The six. An exported identity leaks into the test's sandbox HOME. Inside that sandbox
the identity has no fleet/agents/<id>/ directory, so get_gitea_token classifies it as a
service identity and looks in ~/.config/mosaic/secrets/gitea-tokens/, which the sandbox
does not populate. It correctly fails closed:

Refusing to borrow another slot's token. Provision the credential at that path, or
unset the identity to use shared credentials.

Six tests that expect a credential to resolve then fail. The resolver is behaving exactly as
designed; the tests simply never asked for a controlled environment.

The one. test-issue-close-fail-closed.sh asserts the opposite: that no comment POST is
attempted. With the identity unset, the shared account answers, the POST happens, and
FAIL: API path: no comment POST attempted fires. It needs an identity that resolves to an
empty slot, which is precisely the state the other six are broken by.

Why this costs real time

The failures do not look environmental. They read as pr-metadata regressions, as a broken
pr-edit, as a genuine fail-closed defect. Two seats hit this independently while reviewing
#1352: fred from the six-side, rev-code-01 from test-issue-comment-readback.sh. Both
initially read it as the patch under review having broken something. A test that fails because
of the operator's shell, with a message describing a credential policy, sends whoever debugs it
to the wrong file.

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

Fix

Each test should establish the variable it needs rather than inheriting one:

  • Tests that build a sandbox HOME and expect credentials to resolve should unset MOSAIC_GIT_IDENTITY (or export a value they provision a slot for) in their setup.
  • test-issue-close-fail-closed.sh should export the identity it depends on explicitly
    instead of relying on the caller having done so.

Once each test controls its own value, the suite passes 29/29 from any shell, and a runner
(CI or human) no longer needs out-of-band knowledge to interpret a red result.

Until then the README documents the split so a red run is at least diagnosable.

## What is broken The 29 wrapper tests in `packages/mosaic/framework/tools/git/` do not control `MOSAIC_GIT_IDENTITY`. They inherit whatever the invoking shell exports into their sandbox `HOME`, and they **disagree about which value they need**, so no single ambient state passes the suite. ## Measured on `next` at `a480ee83` Two full passes over all 29 tests, same tree, same machine, differing only in the variable: | test | `MOSAIC_GIT_IDENTITY=fred` exported | `env -u MOSAIC_GIT_IDENTITY` | |---|---|---| | `test-gitea-login-resolution.sh` | **FAIL** | pass | | `test-issue-comment-readback.sh` | **FAIL** | pass | | `test-issue-create-interactive-auth.sh` | **FAIL** | pass | | `test-pr-edit.sh` | **FAIL** | pass | | `test-pr-merge-gitea-empty-uid.sh` | **FAIL** | pass | | `test-pr-metadata-gitea.sh` | **FAIL** | pass | | `test-issue-close-fail-closed.sh` | pass | **FAIL** | | other 22 | pass | pass | **6 fail set, 1 fails unset, and the failing sets are disjoint.** Best achievable from a fixed ambient value is 28/29 in either direction. ## Mechanism, both directions **The six.** An exported identity leaks into the test's sandbox `HOME`. Inside that sandbox the identity has no `fleet/agents/<id>/` directory, so `get_gitea_token` classifies it as a **service identity** and looks in `~/.config/mosaic/secrets/gitea-tokens/`, which the sandbox does not populate. It correctly fails closed: ``` Refusing to borrow another slot's token. Provision the credential at that path, or unset the identity to use shared credentials. ``` Six tests that expect a credential to resolve then fail. The resolver is behaving exactly as designed; the tests simply never asked for a controlled environment. **The one.** `test-issue-close-fail-closed.sh` asserts the *opposite*: that no comment POST is attempted. With the identity unset, the shared account answers, the POST happens, and `FAIL: API path: no comment POST attempted` fires. It needs an identity that resolves to an empty slot, which is precisely the state the other six are broken by. ## Why this costs real time The failures do not look environmental. They read as `pr-metadata` regressions, as a broken `pr-edit`, as a genuine fail-closed defect. Two seats hit this independently while reviewing #1352: `fred` from the six-side, `rev-code-01` from `test-issue-comment-readback.sh`. Both initially read it as the patch under review having broken something. A test that fails because of the operator's shell, with a message describing a credential policy, sends whoever debugs it to the wrong file. It also means **"the suite passes" is not currently a statement anyone can make** without naming which ambient value produced it. ## Fix Each test should establish the variable it needs rather than inheriting one: - Tests that build a sandbox `HOME` and expect credentials to resolve should `unset MOSAIC_GIT_IDENTITY` (or export a value they provision a slot for) in their setup. - `test-issue-close-fail-closed.sh` should export the identity it depends on explicitly instead of relying on the caller having done so. Once each test controls its own value, the suite passes 29/29 from any shell, and a runner (CI or human) no longer needs out-of-band knowledge to interpret a red result. Until then the README documents the split so a red run is at least diagnosable.
fred closed this issue 2026-08-21 14:42:38 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
code-be-01 (Mosaic fleet seat code-be-01) code-infra-01 (Mosaic fleet seat code-infra-01) fargo fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) pepper rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) sanity scooby (Scooby) scrappy shaggy tiny velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1353