diff --git a/packages/mosaic/framework/defaults/AGENTS.md b/packages/mosaic/framework/defaults/AGENTS.md index ca3117a2..0af5914d 100755 --- a/packages/mosaic/framework/defaults/AGENTS.md +++ b/packages/mosaic/framework/defaults/AGENTS.md @@ -43,6 +43,8 @@ overwritten on upgrade. (Layer model: `constitution/LAYER-MODEL.md`.) | Secrets / vault usage | `guides/VAULT-SECRETS.md` | | Tool/credential reference (service CLIs, wrappers) | `guides/TOOLS-REFERENCE.md` | | Memory protocol (OpenBrain capture/recall) | `guides/MEMORY.md` | +| Seat identity, git credentials, token slots | `guides/SEAT-IDENTITY.md` | +| Reaching another agent (fleet comms) | `guides/FLEET-COMMS.md` | ## Subagent Model Selection (Cost — Hard Rule) diff --git a/packages/mosaic/framework/guides/CODE-REVIEW.md b/packages/mosaic/framework/guides/CODE-REVIEW.md index 938915d8..29b2e064 100755 --- a/packages/mosaic/framework/guides/CODE-REVIEW.md +++ b/packages/mosaic/framework/guides/CODE-REVIEW.md @@ -15,8 +15,82 @@ Merge strategy enforcement (HARD RULE): - Merge to `main` MUST be squash-only. - Use `~/.config/mosaic/tools/git/pr-merge.sh -n {PR_NUMBER} -m squash --expect-head {approved_full_sha}` (or PowerShell equivalent). +An estate MAY carry a documented exception for a repository whose gates are commit hooks rather +than review. Such an exception belongs in that estate's own working copy of this guide, is +scoped to the named repository, and is never precedent for a second one. + +**Do not use `pr-review.sh` or `issue-comment.sh` to post a verdict** (mosaicstack#1280). Both +ignore the resolved identity and attribute the record to `mos-dt-0`, and the read-back verifies +green, so the failure is invisible from inside the tool. Post through a direct authenticated API +call as your own seat, or hand the verdict to the requesting seat. Handing it over is a +legitimate delivery path, not a fallback. + +## Evidence Discipline (applies to every finding) + +The checklist below says what to look at. This section says when you are allowed to believe what +you saw. Every rule here was earned by a wrong conclusion that reached a report. + +1. **A finding is a claim about behavior.** State the failing input, the path taken, and the + wrong result. "This looks fragile" is not a finding. +2. **A green check is not a result until you have shown it could go red.** Run the control. A + `0`, an empty result, or a column of identical values with no failing counterpart is a + non-result, and reporting it as a pass is the most common way a review misses the defect it + was called for. +3. **Measurement and explanation are separate sentences.** Report the command and its output, + then, as its own sentence, what you think it means. +4. **Never widen the case you measured.** If you checked one path, the finding covers one path. +5. **Reproduce a reported failure before recording it, and say which tree you measured.** Two + correct measurements of two different trees disagree without either being wrong. +6. **Verify by content on the ref that ships**, never by ancestry of a local sha. A rebase mints + new shas; a commit being an ancestor of something local proves nothing about the remote. + Compare by digest against `origin/`. +7. **Confidence is part of the finding.** "I could not reproduce this" is a usable review + comment. A confident guess is not. +8. **Author is not reviewer** (Gate-16). Do not review your own work, or work you shaped closely + enough to be a co-author of. Say so and hand it back. + +### Measuring a shell suite + +Earned on mosaicstack#1311, 2026-08-18. Each of these produced a wrong conclusion first. + +9. **`cmd | tail; echo rc=$?` reports `tail`'s exit code, not `cmd`'s.** It reads as a pass when + the command failed. Redirect to a file and check `rc` directly, or use `${PIPESTATUS[0]}`. +10. **Under `set -o pipefail`, a missed glob makes `ls` exit 2**, the pipeline inherits it, and + `set -e` kills the run. Iterate a glob with a `for` loop and an `-e` test instead of piping + `ls`. +11. **A suite that exits nonzero with ZERO output is an environment question, not a defect in + the code under review.** The usual cause is a sourced dependency that is absent, so `set -e` + kills the first case before anything prints. Extract whole tool trees — `tools/git` alone is + missing `tools/_lib/credentials.sh`. Isolate the variable and prove it by adding only that + back. +12. **`git -C ` in a directory that is not itself a repo answers from the enclosing repo.** + A scratch tree under `~/.mosaic` reports `~/.mosaic`'s HEAD, not the PR's, and every + conclusion drawn from it describes the wrong tree. Confirm `git rev-parse --show-toplevel` + is the tree you think it is before trusting any git output. + +### Feedback Categories + +- **Blocker**: must fix before merge (security, bugs, test failures) +- **Should Fix**: important but not blocking (code quality, minor issues) +- **Suggestion**: optional improvement (style preference, nice-to-have) +- **Question**: seeking clarification + ## Review Checklist +Reviewer seats split this checklist by class rather than duplicating it. A seat reviews its own +sections in full and may raise anything it notices outside them as a Suggestion, never as a +Blocker on someone else's ground. + +| Reviewer class | Owns | +|---|---| +| `rev-code-*` | 1 Correctness, 3 Testing, 4 Code Quality, 4a TypeScript, 5 Documentation, 6 Performance, 7 Dependencies | +| `rev-security-*` | 2 Security, 2a OWASP | + +Where two seats of the same class review the same change, they review independently and compare +after. A second seat that reads the first seat's findings before measuring is a proofreader, not +a second opinion. + + ### 1. Correctness - [ ] Code does what the issue/PR description says @@ -53,7 +127,7 @@ Merge strategy enforcement (HARD RULE): - [ ] Tests cover happy path AND error cases - [ ] Situational tests cover all impacted change surfaces (primary gate) - [ ] Tests validate required behavior/outcomes, not only internal implementation details -- [ ] TDD was applied when required by `~/.config/mosaic/guides/QA-TESTING.md` +- [ ] TDD was applied when required by `guides/QA-TESTING.md` - [ ] Coverage meets 85% minimum - [ ] Tests are readable and maintainable - [ ] No flaky tests introduced @@ -82,7 +156,7 @@ Merge strategy enforcement (HARD RULE): ### 5. Documentation - [ ] Complex logic has explanatory comments -- [ ] Required docs updated per `~/.config/mosaic/guides/DOCUMENTATION.md` +- [ ] Required docs updated per `guides/DOCUMENTATION.md` - [ ] Public APIs are documented - [ ] Private/internal APIs are documented - [ ] API input/output schemas are documented @@ -126,13 +200,6 @@ git diff main...HEAD - Distinguish between blocking issues and suggestions - Be constructive, not critical of the person -### Feedback Categories - -- **Blocker**: Must fix before merge (security, bugs, test failures) -- **Should Fix**: Important but not blocking (code quality, minor issues) -- **Suggestion**: Optional improvements (style preferences, nice-to-haves) -- **Question**: Seeking clarification - ### Review Comment Format ``` diff --git a/packages/mosaic/framework/guides/FLEET-COMMS.md b/packages/mosaic/framework/guides/FLEET-COMMS.md new file mode 100644 index 00000000..0c8ac539 --- /dev/null +++ b/packages/mosaic/framework/guides/FLEET-COMMS.md @@ -0,0 +1,86 @@ +# Fleet Comms Guide + +How one seat reaches another on a host. The mechanism is the framework's; the sessions and +sockets are per-host, so measure yours rather than trusting an example. + +`mosaic ` would normally inject the addressing block from the roster. Where the composer +is unavailable, or where the roster is stale, this guide is the substitute. + +## Measure the fleet; do not trust the roster + +`fleet/roster.yaml` is a declaration of intent, not an observation. It routinely names a socket +that was never created, lists seats that are not running, and omits seats that are — this was +measured on a live host on 2026-08-18 and all three were true at once. Find out what is actually +up before addressing anyone: + +```bash +tmux list-sessions +tmux list-panes -a -F '#{session_name} #{pane_current_command} #{pane_current_path}' +``` + +The pane command tells you the runtime. A pane showing `bash` is an idle shell with no agent +attached — a send there lands in a shell prompt and is not read by anyone. + +Use the **default socket**. Do not pass `-L mosaic-fleet` on the strength of the roster. + +## Sending + +```bash +~/.config/mosaic/tools/tmux/agent-send.sh -s -C -m "" +``` + +`-s` also accepts `session:window.pane`. `-f ` sends a file body; stdin works too. + +### Classes + +`-C` takes exactly one of these. Anything else exits 3. + +| Class | Use for | +|---|---| +| `terminal-log` | log only; never needs the agent's attention | +| `actionable` | a decision, blocker, gate, or question needing an answer | +| `human` | relayed from a human operator | +| `reaction` | an ack or acknowledgement token | +| `digest` | machine wake, coalescible | + +An absent class is treated as `actionable` by consumers, which is the fail-safe direction. Prefer +naming it anyway. + +### Addressing preamble + +The wire format is `[ -> class=] `. Flip it when you reply — the tool +sends, it does not auto-reply. + +### Exit codes + +| rc | Meaning | +|---|---| +| 0 | delivered or queued | +| 1 | target session not found | +| 2 | text reached the pane but is **still a draft** | +| 3 | usage error (bad class, missing `-s`) | + +**Never retry on rc=2.** The message is in the target pane; retrying double-sends it. Confirm +instead: + +```bash +tmux capture-pane -p -t :0.0 | tail -20 +``` + +rc=2 is the normal result when the target is an idle pi seat. + +## Durable comms + +tmux delivery is host-local and does not survive a pane. Anything that must outlive the session +goes through the estate's durable comms protocol — a committed `comms/` tree in an estate repo, +with its own README. Use it for cross-host messages, verdicts, and anything a later session needs +to find. + +## Handing work across seats + +1. **A verdict handed to the requesting seat is a legitimate delivery path**, and the required one + for anything `pr-review.sh` would otherwise post (see `guides/CODE-REVIEW.md`). +2. **Address the seat, not the runtime.** A seat name is a session name; whether it runs claude, + pi or codex is not the sender's business. +3. **Say what you measured, not just what you concluded** — the receiving seat cannot see your + terminal. diff --git a/packages/mosaic/framework/guides/SEAT-IDENTITY.md b/packages/mosaic/framework/guides/SEAT-IDENTITY.md new file mode 100644 index 00000000..5890089e --- /dev/null +++ b/packages/mosaic/framework/guides/SEAT-IDENTITY.md @@ -0,0 +1,126 @@ +# Seat Identity & Credentials Guide + +Every agent that touches a Mosaic-managed git host acts as a named seat with its own credential. +This guide is how that works on a host, and what an agent must never do with it. + +The mechanism below is the framework's. The specific paths, seats and stores are per-host: +measure yours before trusting any of them. + +## The rule + +**One seat, one identity, one token file.** A seat never borrows another seat's credential, never +falls back to a shared owner account, and never carries a second copy of its own token. A second +copy is drift, and drift surfaces as the stale copy returning 401 — which reads as a revoked +token and sends whoever debugs it somewhere else entirely. + +A credential refusal is correct behavior, not a bug to route around. If git refuses with a +fail-closed diagnostic, the fix is to provision or correct *your* identity. Escalate; do not +substitute. + +## How a credential is resolved + +Find the helper the way **git** does, not with `command -v`. Git runs whatever +`credential.helper` names, and on a Mosaic host that is an absolute path — so a PATH lookup +answers a different question and the two disagree the moment the PATH copy is removed. It was +removed here on 2026-08-18. + +```bash +git config --get-all credential.helper # every helper, in the order git tries them +``` + +Git tries **each** configured helper in turn until one supplies a credential. A fail-closed +helper supplies nothing, so a second helper configured behind it silently becomes the one that +answers. When you care which binary serves a credential, read the whole list. +`~/.mosaic/fleet/bin/lib-credential-helper.sh` does this correctly and handles all three forms +git accepts (absolute path, `!command`, bare name resolved on PATH). + +The helper resolves the identity in this order: + +1. `$MOSAIC_GIT_IDENTITY` +2. `git config --get mosaic.gitIdentity` +3. the username git supplied on stdin + +It maps the host to a store prefix — `git.mosaicstack.dev` to `gitea-mosaicstack`, +`git.uscllc.com` to `gitea-usc`. Any other host is declined quietly with rc=0, which is not an +error and raises no escalation. + +Then it chooses **one** of two stores, and reads exactly one file: + +``` +brain_home = ${MOSAIC_BRAIN_HOME:-$HOME/.mosaic} + +seat — when $brain_home/fleet/agents// EXISTS + $brain_home/fleet/agents//secrets/-.token +service — otherwise + ~/.config/mosaic/secrets/gitea-tokens/-.token +``` + +**There is no precedence between the two and no fallback from one to the other.** The existence +of the seat directory decides it. A seat that has a directory and an empty slot fails closed; it +does not reach the service store. That is the intended behavior — the alternative is an agent +silently acting as somebody else. + +If the file is unreadable the helper **fails closed**: it refuses, spools a record, and attempts +a fleet notification. It does not fall back to a shared account. That fallback is what made +`usc/uconnect#3084` unattributable, and it was removed deliberately. + +Verify the helper you actually have: + +```bash +h=$(git config --get credential.helper) +grep -c 'FAIL CLOSED' "$h" # expect >= 1 +grep -c 'fleet/agents' "$h" # expect >= 1; 0 means it predates mosaicstack#1311 +``` + +## Where a seat's token lives + +The seat slot is the **only** copy: + +``` +~/.mosaic/fleet/agents//secrets/-.token real file, mode 600 +``` + +The framework store at `~/.config/mosaic/secrets/gitea-tokens/` holds tokens for **service +identities only** — identities with no seat directory. A seat's token does not belong there. + +Before mosaicstack#1311 the deployed helper knew only the service store, and seats were bridged +with a symlink from the store into the slot. **Those bridges are gone** (removed 2026-08-18 by +`~/.mosaic/fleet/bin/migrate-credentials-to-seat-slots.sh`) and must not be recreated. A symlink +is not how a system finds a credential; the helper resolving the right store is. + +`.principal` and `.scopes` beside the token are grant records, not secrets. They are tracked. The +`.token` never is. + +### Provisioning a new seat + +1. Create `~/.mosaic/fleet/agents//secrets/` mode 700. +2. Write `.principal` (the Gitea login) and `.scopes` (the granted scopes), mode 600. +3. Jason mints the token into the seat slot, mode 600. Agents do not mint their own. +4. Symlink the framework store entry to the seat slot. +5. Verify with an authenticated `GET /user` and confirm the returned login is the seat, **not the + minting account**. Record the date in `ENTITY.md`. Never record the value. + +Until step 3, the seat is unminted and its git writes fail closed. That is the designed state and +is safe to launch in — the seat is told at launch so it does not discover it mid-task. + +## Acting as yourself + +Name the identity on every invocation: + +```bash +MOSAIC_GIT_IDENTITY= git push +git -c user.name= -c user.email=@mosaicstack.dev commit -m "..." +``` + +**Never persist `git config mosaic.gitIdentity` inside a `~/src/stack` worktree.** Every worktree +of that clone shares one `.git/config`, so a persisted identity there silently rewrites the +identity of every other seat working in that clone. The per-invocation form has no exception. + +## Handling + +1. **Never print a token value.** Compare by SHA-256 digest, or write ``. +2. **Never stage a `.token`, `secrets.json`, or `ENTITY.md`.** Stage explicit paths and **never + `git add -A`** — `secrets/*.principal` and `secrets/*.scopes` are covered by no ignore rule. +3. **Never place a token in an environment variable** in an interactive session. A `declare -x` + dump has leaked the whole environment to a terminal before. +4. **No real credential or operator data on a sandbox VM, ever.** diff --git a/packages/mosaic/framework/templates/agent/fragments/conditional-loading.md b/packages/mosaic/framework/templates/agent/fragments/conditional-loading.md index e8b2f574..74556487 100644 --- a/packages/mosaic/framework/templates/agent/fragments/conditional-loading.md +++ b/packages/mosaic/framework/templates/agent/fragments/conditional-loading.md @@ -15,3 +15,5 @@ | Infrastructure/DevOps | `~/.config/mosaic/guides/INFRASTRUCTURE.md` | | QA/Testing | `~/.config/mosaic/guides/QA-TESTING.md` | | Secrets management (Vault) | `~/.config/mosaic/guides/VAULT-SECRETS.md` | +| Seat identity / git credentials | `~/.config/mosaic/guides/SEAT-IDENTITY.md` | +| Reaching another agent (fleet comms) | `~/.config/mosaic/guides/FLEET-COMMS.md` |