Skip to content

Commit a76bb7e

Browse files
committed
fix(cts): act on the review of the 0.42.3 reconciliation (CIP-3727)
Four of the five findings. The fifth is declined, with a reason. Two are staleness the precedence flip left behind — it made upstream's `help` win, which moved the dashboard link out of `message` and onto `url`, and two shipped documents still told a reader to look in the message: - `skills/stash-auth/SKILL.md` claimed "nothing upstream attaches a URL to them, which is why the dashboard link is written into `message`". That is false as of 0.42.3, contradicted this PR's own changeset, and contradicted the paragraph twenty lines above it that points at `failure.url`. Skills ship in the `stash` tarball, so the wrong sentence lands in a customer's repo. - `EncryptionError.authCode`'s JSDoc said the same thing in a published `.d.ts`. One is a real asymmetry, and it falsified a claim in the changeset. `code` is the one carried key with a CLOSED type, and the wasm entry screened it in `toFailure` but not on the thrown-init path, which reached the caller through `carryDiagnostics` alone — a deliberately structural copy, so that it cannot drop a field protect-ffi adds next. The two seams of the same entry therefore disagreed about a check the native entry applies to both, and a fetch failing inside a JS auth strategy could hand back `code: 'ECONNRESET'` wearing `ProtectErrorCode`. The screen now runs once, inside `carryDiagnostics`, so it cannot be forgotten at a third call site. `authCode` is deliberately untouched: that set is open and a newer code must still arrive. One is a code/prose disagreement of exactly the kind the CLI's two tables exist to prevent. `stash env`'s profile-load arm already rendered a terminal hint — "logging in again will not clear this" — while hardcoding `not_logged_in`, so an agent read the code, ran `stash auth login`, and arrived back. It now routes through `authFailureCliCode` like the renewal arm. DECLINED: `stash auth login` reports `USAGE_LIMIT_EXCEEDED` on `--json` while `stash env` reports `usage_limit_exceeded`. Real, but `login`'s code for an auth failure has always been `failure.type` verbatim (`EXPIRED_TOKEN` and the rest), which predates this work and is documented in `skills/stash-cli`. Collapsing the two spellings changes an existing machine-readable contract for every code, not just these, and belongs in its own change rather than riding along here. Claude-Session: https://claude.ai/code/session_01PS9J6pu3FmmQvJHxwTVJUc
1 parent 0fe8a2d commit a76bb7e

7 files changed

Lines changed: 104 additions & 8 deletions

File tree

‎.changeset/usage-limit-refusal-guidance.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ supplies one naming the same destinations.
2626
```typescript
2727
const result = await client.encrypt(value, { column, table })
2828
if (result.failure?.authCode === 'USAGE_LIMIT_EXCEEDED') {
29-
// Stop retrying. `result.failure.message` names dashboard.cipherstash.com.
29+
// Stop retrying. `result.failure.message` says what to do about it,
30+
// `result.failure.url` is where — the link is never folded into the message.
3031
}
3132
```
3233

@@ -66,8 +67,10 @@ diagnosis, and no longer answer a billing refusal with "run `stash auth login`
6667
and try again" — a fresh login cannot mint a credential that is being withheld
6768
on billing grounds. `stash env` reports the two terminal refusals under their
6869
own codes, `usage_limit_exceeded` and `org_not_provisioned`, rather than
69-
`session_invalid`: `code` is the only machine-readable field on the stream, so
70-
reporting a stale session sends an agent round a re-login loop it cannot win.
70+
`session_invalid` or `not_logged_in`: `code` is the only machine-readable field
71+
on the stream, so reporting a stale session sends an agent round a re-login loop
72+
it cannot win. Both of its failure arms do this — the one that loads the device
73+
session as well as the one that renews it.
7174

7275
The `--json` error envelope gains an optional `hint`, carrying that remedy. It
7376
was previously printed only on the interactive path, which left the dashboard

‎packages/cli/src/commands/env/__tests__/env.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,40 @@ function lastJson(): Record<string, unknown> {
178178
return JSON.parse(logSpy.mock.calls.at(-1)?.[0] as string)
179179
}
180180

181+
describe('envCommand — a terminal refusal at profile load', () => {
182+
/**
183+
* `fromProfile()` itself fails — no session on disk, or one CTS will not
184+
* honour.
185+
*/
186+
function stubStrategyFailure(type: string, message: string) {
187+
authMock.DeviceSessionStrategy.fromProfile.mockReturnValue({
188+
failure: { type, error: new Error(message) },
189+
})
190+
}
191+
192+
// The profile-load arm renders a terminal hint too, so its `code` has to
193+
// agree with it. It used to report `not_logged_in` while the hint said
194+
// logging in again would not help.
195+
it('reports the refusal code rather than not_logged_in', async () => {
196+
stubStrategyFailure('USAGE_LIMIT_EXCEEDED', 'Over the limit.')
197+
198+
await expectExit(envCommand({ name: 'x', json: true }), 1)
199+
200+
expect(lastJson()).toMatchObject({
201+
status: 'error',
202+
code: 'usage_limit_exceeded',
203+
})
204+
})
205+
206+
it('still reports not_logged_in for an ordinary profile failure', async () => {
207+
stubStrategyFailure('NOT_AUTHENTICATED', 'No profile.')
208+
209+
await expectExit(envCommand({ name: 'x', json: true }), 1)
210+
211+
expect(lastJson()).toMatchObject({ status: 'error', code: 'not_logged_in' })
212+
})
213+
})
214+
181215
describe('envCommand — a CTS refusal at session renewal', () => {
182216
/**
183217
* `fromProfile()` succeeds (there IS a session on disk) and the renewal is

‎packages/cli/src/commands/env/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,14 @@ async function mintCredentials(keyName: string): Promise<MintedCredentials> {
342342
// 1. Device session from ~/.cipherstash (written by `stash auth login`).
343343
const strategyResult = DeviceSessionStrategy.fromProfile()
344344
if (strategyResult.failure) {
345+
// `authFailureCliCode` for the same reason as the renewal arm below: this
346+
// arm already calls `authFailureHint`, so it can print "logging in again
347+
// will not clear this" — and pairing that with a hardcoded
348+
// `not_logged_in` is the exact code/prose disagreement the two tables
349+
// exist to prevent. An agent reads `code`, runs `stash auth login`, and
350+
// arrives back here.
345351
throw new MintError(
346-
'not_logged_in',
352+
authFailureCliCode(strategyResult.failure, 'not_logged_in'),
347353
`Not logged in: ${authFailureMessage(strategyResult.failure)}`,
348354
authFailureHint(strategyResult.failure, LOGIN_HINT),
349355
)

‎packages/stack/__tests__/wasm-inline-auth-failure.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,36 @@ describe('a usage-limit refusal at wasm-inline client init', () => {
234234
expect(error.message).toContain('[encryption]:')
235235
})
236236

237+
it('does not let a foreign `code` reach the thrown init error', async () => {
238+
// `code` is the one carried key with a CLOSED type, and the init path
239+
// reached the caller through `carryDiagnostics` alone — which copies keys
240+
// structurally, on purpose, so it cannot drop a field protect-ffi adds
241+
// next. That left this seam accepting anything: a fetch failing inside a
242+
// JS auth strategy rejects with `ECONNRESET`, and the caller got it
243+
// wearing `ProtectErrorCode`. `toFailure` had always screened it, so the
244+
// two seams of this entry disagreed about the same check.
245+
const error = await initFailure(
246+
Object.assign(new Error('socket hang up'), {
247+
code: 'ECONNRESET',
248+
authCode: 'SOME_FUTURE_REFUSAL',
249+
}),
250+
)
251+
252+
expect(error).not.toHaveProperty('code')
253+
// The open set is untouched: a code newer than this build still arrives.
254+
expect(error.authCode).toBe('SOME_FUTURE_REFUSAL')
255+
})
256+
257+
it('keeps a real protect-ffi code on the thrown init error', async () => {
258+
const error = await initFailure(
259+
Object.assign(new Error('bad config'), {
260+
code: 'UNSUPPORTED_CONFIG_VERSION',
261+
}),
262+
)
263+
264+
expect(error.code).toBe('UNSUPPORTED_CONFIG_VERSION')
265+
})
266+
237267
it('rescues a bare-object refusal at init too', async () => {
238268
const error = await initFailure(usageLimitObject())
239269

‎packages/stack/src/errors/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ export interface EncryptionError {
4949
* that `type` cannot express — `USAGE_LIMIT_EXCEEDED` above all, which means
5050
* the organisation is over its billing allowance and no amount of retrying
5151
* or key rotation will clear it. `message` already carries the remedy in
52-
* prose (including the dashboard URL); this field is for branching on it.
52+
* prose, and {@link EncryptionError.url} carries the link that goes with it —
53+
* the two are halves of one remedy, and the link is never folded into the
54+
* message. This field is for branching on the condition.
5355
*
5456
* ```typescript
5557
* if (result.failure?.authCode === 'USAGE_LIMIT_EXCEEDED') {

‎packages/stack/src/wasm-inline.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,24 @@ function carryDiagnostics<T extends object>(source: unknown, target: T): T {
602602
// it beats turning a failure into a rejection.
603603
}
604604
}
605+
606+
// `code` is the one copied key with a CLOSED type, so it is the one key the
607+
// structural copy above cannot be trusted with. Everything else is either
608+
// open (`authCode`) or untyped prose (`help`, `url`), but `code` is declared
609+
// `ProtectErrorCode`, and Node stamps `ECONNRESET` / `MODULE_NOT_FOUND` onto
610+
// its own errors — so a rejection from a fetch inside a JS auth strategy
611+
// would otherwise republish a foreign string wearing that type.
612+
//
613+
// `toFailure` has always run `readErrorCode` for exactly this reason; the
614+
// thrown-init path reached the caller through `carryDiagnostics` alone and
615+
// did not, which left the two seams of this entry disagreeing about a check
616+
// the native entry applies to both. Dropping an unrecognised code (rather
617+
// than mapping it to `UNKNOWN`) matches `readErrorCode` — see its docblock
618+
// for why those two are different statements.
619+
if ('code' in target && readErrorCode(target) === undefined) {
620+
delete (target as { code?: unknown }).code
621+
}
622+
605623
return target
606624
}
607625

‎skills/stash-auth/SKILL.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,12 @@ same code rides on the thrown error: `(err as { authCode?: string }).authCode`.
135135
Two of those fields are new alongside `authCode`. `help` is the remedy text the
136136
failure arrived with — already folded into `message`, so you rarely need to read
137137
it separately — and `url` is a link the diagnostic carried, which is *not* folded
138-
in and reaches you only as `failure.url`. Neither is populated for the two codes
139-
above today: nothing upstream attaches a URL to them, which is why the dashboard
140-
link is written into `message` rather than left to `url`.
138+
in and reaches you only as `failure.url`.
139+
140+
**For the two terminal codes, read both.** As of `stack-auth` 0.42.3 CipherStash
141+
attaches a `help` *and* a `url` to each, so `message` carries the prose and the
142+
link is on `failure.url` and nowhere else. Rendering `message` alone tells a
143+
user to upgrade their plan without telling them where.
141144

142145
One shape to know: a refusal met by `LockContext.identify()` is an HTTP response
143146
rather than a thrown error, so its failure carries `type` and `authCode` but no

0 commit comments

Comments
 (0)