fix(session): hold the refresh-token rotation lock in the kernel

.agents/skills/code-review/SKILL.md

@@ -1,12 +1,12 @@
 ---
 name: code-review
-description: Review the changes since a fixed point (commit, branch, tag, or merge-base) along two axes — Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/PRD asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to "review since X".
+description: Review the changes since a fixed point (commit, branch, tag, or merge-base) along two axes — Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/spec asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to "review since X".
 ---
 
 Two-axis review of the diff between `HEAD` and a fixed point the user supplies:
 
 - **Standards** — does the code conform to this repo's documented coding standards?
-- **Spec** — does the code faithfully implement the originating issue / PRD / spec?
+- **Spec** — does the code faithfully implement the originating issue / spec?
 
 Both axes run as **parallel sub-agents** so they don't pollute each other's context, then this skill aggregates their findings.
 
@@ -28,7 +28,7 @@ Look for the originating spec, in this order:
 
 1. Issue references in the commit messages (`#123`, `Closes #45`, GitLab `!67`, etc.) — fetch via the workflow in `docs/agents/issue-tracker.md`.
 2. A path the user passed as an argument.
-3. A PRD/spec file under `docs/`, `specs/`, or `.scratch/` matching the branch name or feature.
+3. A spec file under `docs/`, `specs/`, or `.scratch/` matching the branch name or feature.
 4. If nothing is found, ask the user where the spec is. If they say there isn't one, the **Spec** sub-agent will skip and report "no spec available".
 
 ### 3. Identify the standards sources
@@ -57,8 +57,6 @@ Each smell reads *what it is* → *how to fix*; match it against the diff:
 
 ### 4. Spawn both sub-agents in parallel
 
-Send a single message with two `Agent` tool calls. Use the `general-purpose` subagent for both.
-
 **Standards sub-agent prompt** — include:
 
 - The full diff command and commit list.

.agents/skills/setup-matt-pocock-skills/SKILL.md

@@ -37,7 +37,7 @@ Lead each section with the recommended answer so the user can accept it in a wor
 
 **Section A — Issue tracker.**
 
-> Explainer: The "issue tracker" is where issues live for this repo. Skills like `to-tickets`, `triage`, `to-spec`, and `qa` read from and write to it — they need to know whether to call `gh issue create`, write a markdown file under `.scratch/`, or follow some other workflow you describe. Pick the place you actually track work for this repo.
+> Explainer: The "issue tracker" is where issues live for this repo. Skills like `to-tickets`, `triage`, and `to-spec` read from and write to it — they need to know whether to call `gh issue create`, write a markdown file under `.scratch/`, or follow some other workflow you describe. Pick the place you actually track work for this repo.
 
 Default posture: these skills were designed for GitHub. If a `git remote` points at GitHub, propose that. If a `git remote` points at GitLab (`gitlab.com` or a self-hosted host), propose GitLab. Otherwise (or if the user prefers), offer:
 

.agents/skills/setup-matt-pocock-skills/issue-tracker-github.md

@@ -1,6 +1,6 @@
 # Issue tracker: GitHub
 
-Issues and PRDs for this repo live as GitHub issues. Use the `gh` CLI for all operations.
+Issues and specs for this repo live as GitHub issues. Use the `gh` CLI for all operations.
 
 ## Conventions
 

.agents/skills/setup-matt-pocock-skills/issue-tracker-gitlab.md

@@ -1,6 +1,6 @@
 # Issue tracker: GitLab
 
-Issues and PRDs for this repo live as GitLab issues. Use the [`glab`](https://gitlab.com/gitlab-org/cli) CLI for all operations.
+Issues and specs for this repo live as GitLab issues. Use the [`glab`](https://gitlab.com/gitlab-org/cli) CLI for all operations.
 
 ## Conventions
 

.agents/skills/setup-matt-pocock-skills/issue-tracker-local.md

@@ -1,6 +1,6 @@
 # Issue tracker: Local Markdown
 
-Issues and specs (you may know a spec as a PRD) for this repo live as markdown files in `.scratch/`.
+Issues and specs for this repo live as markdown files in `.scratch/`.
 
 ## Conventions
 

.agents/skills/to-spec/SKILL.md

@@ -4,7 +4,7 @@ description: Turn the current conversation into a spec and publish it to the pro
 disable-model-invocation: true
 ---
 
-This skill takes the current conversation context and codebase understanding and produces a spec (you may know this document as a PRD). Do NOT interview the user — just synthesize what you already know.
+This skill takes the current conversation context and codebase understanding and produces a spec. Do NOT interview the user — just synthesize what you already know.
 
 The issue tracker and triage label vocabulary should have been provided to you — run `/setup-matt-pocock-skills` if not.
 

.agents/skills/triage/SKILL.md

@@ -73,7 +73,7 @@ Show counts and a one-line summary per item. Let the maintainer pick.
 
 3. **Verify the claim.** Before any grilling, check that the claim holds up. For a bug, reproduce it from the reporter's steps. For a PR, confirm the diff does what it claims — check it out, run the relevant tests or commands. Report what happened: confirmed (with code path), failed, or insufficient detail (a strong `needs-info` signal). A confirmed verification makes a much stronger agent brief.
 
-4. **Grill (if needed).** If the request needs fleshing out, run the `/grilling` and `/domain-modeling` skills together — grill it into shape one question at a time, sharpening domain terms and updating `CONTEXT.md`/ADRs inline as decisions land.
+4. **Grill (if needed).** If the request needs fleshing out, run the `/grilling` and `/domain-modeling` skills together — grill it into shape a round of questions at a time, sharpening domain terms and updating `CONTEXT.md`/ADRs inline as decisions land.
 
 5. **Apply the outcome:**
    - `ready-for-agent` — post an agent brief comment ([AGENT-BRIEF.md](AGENT-BRIEF.md)).

.agents/skills/wayfinder/SKILL.md

@@ -14,7 +14,7 @@ Wayfinder is **planning** by default: each ticket resolves a decision, and the m
 
 ## Refer by name
 
-Every map and ticket is an issue, so it has a **name** — its title. In everything the human reads — narration, the map's Decisions-so-far — refer to it by that name, never by a bare id, number, or slug. A wall of `#42, #43, #44` is illegible; names read at a glance. The id and URL don't vanish — a name wraps its link — but they ride *inside* the name, never stand in for it.
+Every map and ticket is an issue, so it has a **name** — its title. In everything the human reads — narration, the map's Decisions-so-far — refer to it by that name, never by a bare id, number, or slug. A wall of `#42, #43, #44` is illegible; names read at a glance. The id and URL don't vanish — a name wraps its link — but they ride _inside_ the name, never stand in for it.
 
 ## The Map
 
@@ -72,12 +72,12 @@ The answer isn't part of the body — it's recorded on resolution (see [Work thr
 
 ## Ticket Types
 
-Every ticket is either **HITL** — human in the loop, worked *with* a human who speaks for themselves — or **AFK**, driven by the agent alone. A HITL ticket only resolves through that live exchange; the agent never stands in for the human's side of it (a grilling agent that answers its own questions has broken this).
+Every ticket is either **HITL** — human in the loop, worked _with_ a human who speaks for themselves — or **AFK**, driven by the agent alone. A HITL ticket only resolves through that live exchange; the agent never stands in for the human's side of it (a grilling agent that answers its own questions has broken this).
 
 - **Research** (AFK): Reading documentation, third-party APIs, or local resources like knowledge bases to surface a fact a decision waits on. Resolved by a `/research` **subagent**. Use when knowledge outside the current working directory is required.
 - **Prototype** (HITL): Raise the fidelity of the discussion by making a cheap, rough, concrete artifact to react to — an outline, a rough take, a stub, or UI/logic code via the /prototype skill. Links the prototype as an asset. Use when "how should it look" or "how should it behave" is the key question.
-- **Grilling** (HITL): Conversation via the /grilling and /domain-modeling skills, one question at a time. The default case.
-- **Task** (HITL or AFK): Manual work that must happen before a *decision* can be made — nothing to decide, prototype, or research, but the discussion is blocked until it's done. Signing up for a service so its API can be judged, provisioning access, moving data so its shape can be seen. This is the one type that *does* rather than decides — and it earns its place by unblocking a decision, not by delivering the destination. The agent drives it alone where it can (AFK); otherwise it hands the human a precise checklist (HITL). Resolved when the work is done; the answer records what was done and any resulting facts (credentials location, new URLs, row counts) later tickets depend on.
+- **Grilling** (HITL): Conversation. The default case. Always invoke the /grilling and /domain-modeling skills.
+- **Task** (HITL or AFK): Manual work that must happen before a _decision_ can be made — nothing to decide, prototype, or research, but the discussion is blocked until it's done. Signing up for a service so its API can be judged, provisioning access, moving data so its shape can be seen. This is the one type that _does_ rather than decides — and it earns its place by unblocking a decision, not by delivering the destination. The agent drives it alone where it can (AFK); otherwise it hands the human a precise checklist (HITL). Resolved when the work is done; the answer records what was done and any resulting facts (credentials location, new URLs, row counts) later tickets depend on.
 
 ## Fog of war
 

.github/workflows/pr-checks.yml

@@ -103,6 +103,22 @@ jobs:
 
           go vet ./...
 
+      - name: Cross-build the shell for every supported platform
+        # The gate below runs on Linux only, so a file behind a build tag for
+        # another platform is never compiled by it. The session lock is the
+        # first such seam, and a Windows-only file that stops compiling would
+        # otherwise reach a release unnoticed.
+        if: needs.changes.outputs.shell == 'true'
+        shell: bash
+        run: |
+          set -euo pipefail
+
+          for target in windows/amd64 darwin/amd64 darwin/arm64; do
+            echo "Building for ${target}"
+            GOOS="${target%%/*}" GOARCH="${target##*/}" go build ./...
+            GOOS="${target%%/*}" GOARCH="${target##*/}" go vet ./...
+          done
+
       - name: Lint shell
         if: needs.changes.outputs.shell == 'true'
         uses: golangci/golangci-lint-action@v9

docs/adr/0005-os-advisory-lock-for-session-rotation.md

@@ -0,0 +1,73 @@
+# ADR 0005: OS Advisory Lock for Session Rotation
+
+**Status:** Accepted
+
+Refresh-token rotation must stay single-writer across concurrent `wso2`
+invocations: a rotating issuer retires the token it was presented, so two
+processes refreshing one session leave the store holding a token the issuer has
+already replaced, and the next command refuses `auth.login_required`. The lock
+that guarantees this is an OS advisory lock — `flock(2)` on Unix,
+`LockFileEx` on Windows, reached through `golang.org/x/sys` behind a
+`tryLock` seam. The kernel ties ownership to an open file descriptor and
+releases it when the process exits however it exits, so an abandoned lock is
+impossible by construction and there is no staleness to detect.
+
+Two consequences look like defects and are not.
+
+**The lock file is created once and never unlinked** — releasing closes the
+descriptor and leaves the file in place. Removing it is the original bug in a
+new costume: a waiter that already holds a descriptor to that inode would lock
+a file no longer at the path, and a second process creating a fresh inode there
+would lock successfully alongside it. The files are empty and bounded by the
+number of credential references, so nothing accumulates that matters. Do not
+add cleanup.
+
+**A held lock reports `auth.login_required`**, not a distinct busy code,
+because the stable problem-code list is closed and the recovery — retry the
+command — is identical. Whether that list may grow is a separate open question
+and not a property of the locking mechanism.
+
+## Considered Options
+
+- The original design made the lock the *existence* of a file, with an
+  `O_EXCL` create to claim it and an age-based takeover so a crashed process
+  could not block the reference forever. `O_EXCL` is atomic, but the takeover
+  was `Stat` then `Remove` then create, with nothing atomic across the three:
+  two processes that both saw the same abandoned file could each evict the
+  other's live lock and both enter the critical section. No amount of
+  repair fixes a heuristic whose job disappears entirely under a kernel lock.
+- `github.com/gofrs/flock` covers both platforms behind one API and would have
+  avoided a build-tagged seam, but it would add a genuinely new module to a
+  graph that deliberately carried four direct dependencies at the time of this
+  decision. `golang.org/x/sys` was already present as an indirect dependency at
+  the version required, so promoting it to direct adds nothing to the graph and
+  costs roughly thirty lines per platform.
+- A blocking acquire would be simpler than a non-blocking attempt on a retry
+  loop, but it discards the bounded wait: the shell reports a busy session as a
+  typed, recoverable refusal rather than hanging on a peer that may never
+  finish.
+
+## Consequences
+
+The waiting deadline moved from 5 to 45 seconds. Under the previous
+implementation a waiter never truly blocked on a healthy holder, so 5 seconds
+was never tested against a real critical section. That section now spans a
+token refresh round trip to the issuer, which the `auth` package bounds at 30
+seconds, so the waiter's patience has to stay strictly greater than that bound:
+at equal values a holder that is merely slow outlives the waiter, and an
+ordinary wait becomes a spurious refusal. The two constants live in packages
+that cannot import each other, so the coupling is carried by a comment on each
+— raising one means raising the other.
+
+During an upgrade, an old binary and a new one do not interlock. The old one
+takes the lock by creating the file and releases it by removing the file, which
+the new one is holding a kernel lock on; once that file is gone, the next new
+invocation creates a fresh inode at the same path and locks it successfully
+alongside the first. The window is any machine where two CLI versions coexist —
+a pinned install, a CI image mid-rollout — and it closes once the old binary is
+gone. Nothing in the design can prevent it, because the old binary's behavior is
+already fixed; it is recorded here so the double-entry is recognised as the
+rollout artifact it is rather than a defect in this lock.
+
+Advisory locks are unreliable on NFS-mounted filesystems. The state root is a
+local user directory and a networked home directory is out of scope.

go.mod

@@ -7,11 +7,11 @@ require (
 	github.com/go-jose/go-jose/v4 v4.1.4
 	github.com/zalando/go-keyring v0.2.8
 	golang.org/x/oauth2 v0.36.0
+	golang.org/x/sys v0.27.0
 )
 
 require (
 	github.com/danieljoos/wincred v1.2.3 // indirect
 	github.com/godbus/dbus/v5 v5.2.2 // indirect
-	golang.org/x/sys v0.27.0 // indirect
 	google.golang.org/protobuf v1.36.11 // indirect
 )

internal/auth/session/lock.go

@@ -17,8 +17,6 @@
 package session
 
 import (
-	"errors"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"time"
@@ -27,11 +25,18 @@ import (
 const (
 	// lockRetryInterval is how often a waiting writer re-attempts the lock.
 	lockRetryInterval = 25 * time.Millisecond
-	// lockDeadline is how long a writer waits before giving up.
-	lockDeadline = 5 * time.Second
-	// lockStaleAfter is the age past which a held lock is presumed abandoned
-	// by a crashed process and taken over.
-	lockStaleAfter = 30 * time.Second
+	// lockDeadline is how long a writer waits before giving up. The critical
+	// section spans a token refresh round trip to the issuer, so the wait has
+	// to outlast a slow deployment rather than a local file operation.
+	//
+	// It must stay strictly greater than auth.grantDeadline, which bounds that
+	// round trip: at equal values a holder that is merely slow outlives the
+	// waiter's patience, and an ordinary wait becomes a spurious refusal. The
+	// constant is not derived from auth.grantDeadline because auth imports this
+	// package, so the coupling is stated here instead — raising one means
+	// raising the other. The margin covers the keyring read and write that
+	// bracket the round trip inside the same critical section.
+	lockDeadline = 45 * time.Second
 )
 
 // WithLock runs fn while holding the per-reference advisory file lock.
@@ -49,32 +54,32 @@ func (s Store) WithLock(ref string, fn func() error) error {
 	return fn()
 }
 
-// acquireLock takes the advisory lock at path, retrying until the deadline.
+// acquireLock takes the kernel advisory lock on the file at path, retrying the
+// non-blocking attempt until the deadline.
 //
-// The lock is the existence of the file: O_EXCL creation is atomic on every
-// filesystem the shell targets. A lock file older than lockStaleAfter is
-// treated as abandoned — removed once, then creation is retried like any
-// other attempt.
+// The lock is held by the kernel against the open file descriptor, not by the
+// existence of the file, so the file is created once and never unlinked:
+// release closes the descriptor and leaves the file in place. Removing it
+// would reintroduce the race it replaced — a waiter holding a descriptor to
+// the old inode and a newcomer creating a fresh one at the same path would
+// both lock successfully. See ADR 0005.
 func acquireLock(path string) (release func(), err error) {
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600)
+	if err != nil {
+		return nil, lockFailed()
+	}
 	deadline := time.Now().Add(lockDeadline)
-	tookOverStale := false
 	for {
-		file, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
-		if err == nil {
+		locked, lockErr := tryLock(file)
+		if lockErr != nil {
 			_ = file.Close()
-			return func() { _ = os.Remove(path) }, nil
-		}
-		if !errors.Is(err, fs.ErrExist) {
 			return nil, lockFailed()
 		}
-		if !tookOverStale {
-			if info, statErr := os.Stat(path); statErr == nil && time.Since(info.ModTime()) > lockStaleAfter {
-				tookOverStale = true
-				_ = os.Remove(path)
-				continue
-			}
+		if locked {
+			return func() { _ = file.Close() }, nil
 		}
 		if time.Now().After(deadline) {
+			_ = file.Close()
 			return nil, lockBusy()
 		}
 		time.Sleep(lockRetryInterval)

internal/auth/session/lock_unix.go

@@ -0,0 +1,45 @@
+// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com).
+//
+// WSO2 LLC. licenses this file to you under the Apache License,
+// Version 2.0 (the "License"); you may not use this file except
+// in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//go:build unix
+
+package session
+
+import (
+	"errors"
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+// tryLock takes an exclusive advisory lock on file without blocking. It
+// reports whether the lock was taken; a peer already holding it is contention,
+// not an error, and is reported as (false, nil).
+//
+// The kernel ties the lock to the open file description and drops it when the
+// process exits however it exits, so a lock held by a crashed process cannot
+// outlive it.
+func tryLock(file *os.File) (bool, error) {
+	err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB)
+	switch {
+	case err == nil:
+		return true, nil
+	case errors.Is(err, unix.EWOULDBLOCK):
+		return false, nil
+	default:
+		return false, err
+	}
+}