You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 2af0ae2
Browse filesBrowse the repository at this point in the historyBrowse files
fix(service-automation): a connection already marked reconnect required answers the cause it stores, and the refusal log names it (#29065)
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XAJtxVwrJVr4YFeLFREMkGX)_Fixes#28963
Clause-②: no
Metadata-protocol: none
## 维护者速读
-
**改了什么**:连接器连接被标成「需要重新连接」后,之后的每一次调度都回答该行存下的原因(上一张卡让每条刷新路径存下真实原因:服务器拒绝、没发
refresh token、没有可用
client)。以前只有标记它的那一次说对,之后每次(换副本、重启、重试)都说「授权服务器拒绝了授权」。刷新标记连接时写的服务器日志也改为这句存下的原因,三条路径各说各的原因。
- **为什么改**:管理员从调度拒绝和服务器日志里读到的原因,在没发 refresh token 或没有 client
的情况下是错的(#28955 开发者发现,seat 1 立卡)。
- **风险与回滚**:低。只改拒绝文字和一行日志;503
`SERVICE_UNAVAILABLE`、状态值、错误码、写入都不变。没存原因的行仍回答原来的拒绝句。回滚即 revert。
-
**先看**:`packages/services/service-automation/src/oauth2-grant/grant-handle.ts`
的 `markedReason` 及其两处调用;`plugin.ts` 的 `onGrantRefused`
日志行;`grant-handle.test.ts` 末尾的 `[#28963]` 四条。
- **两分钟验证**:`pnpm --filter @objectstack/service-automation exec vitest
run src/oauth2-grant/grant-handle.test.ts
src/connector-connection-door.test.ts`,预期 `Tests 46 passed (46)`(main 上
6 条红)。
- **需要你做的**:无,席位落地。
## What changes
- **`grant-handle.ts`: a connection already marked `reconnect_required`
answers what it stores.** A new private `markedReason(connector,
record)` returns the row's `statusReason`, and the default refusal
sentence (`reconnectRequiredReason(connector, organizationId)`,
unchanged) only when the row stores none. Both places that refuse a
marked row use it: `select` (every dispatch's first read) and
`refreshOnce`'s first check (the re-read under the cluster lock, which
sees a row another replica marked while this one waited). The stored
reasons are the per-path sentences PR #28955 introduced, so the first
dispatch and every later one now answer the same sentence.
- **`onGrantRefused` carries the reason.**
`OAuth2GrantHandleDeps.onGrantRefused` is now `(organizationId,
reason)`: `grantRefused` passes the sentence it just stored. The
interface is internal: `oauth2-grant/` is not re-exported from the
package entry (`src/index.ts` has no `oauth2-grant` export, and no other
package names it), so no published member changes.
- **`plugin.ts`: the log line is that reason.** The warn line was
"connector instance 'X': the authorization server refused the grant of
organization 'Y'; it needs to be reconnected there" on all three refresh
paths. It is now `[Automation] ` + the stored reason, which names the
instance, the organization and the cause in PR #28955's words. Still
`warn`; it is distinct from the hold's own "… is degraded: …" line.
- **Changeset** `.changeset/28963-reconnect-required-stored-cause.md`:
`patch` on `@objectstack/service-automation`, both declaration lines.
- No new status value, error code or published member; no
`packages/spec/**`; no docs page change (`connectors.mdx` already says
the connection's `statusReason` names the cause).
**Why `Clause-②: no`.** Nothing that was accepted is refused and nothing
refused is accepted: the 503 stays a 503 `SERVICE_UNAVAILABLE`, only its
message on an already-marked row and one log line's text change.
## Tests
Four new cases at the end of `src/oauth2-grant/grant-handle.test.ts`
(`[#28963]`), on the file's in-memory connection store:
- **A row marked because no refresh token was issued** answers the
stored no-refresh-token sentence on the first dispatch, on two later
`get`s and a `refresh`, and from a second handle that never saw the
marking (another replica, a restart). None says `refused`; no token
request is made.
- **A row another replica marked while this one waited for the lock**
(the lock double marks it `no_client` before running the refresh): the
dispatch answers that stored sentence. This is the `refreshOnce` first
check.
- **A marked row that stores no cause** still answers
`reconnectRequiredReason`'s refusal sentence, at `select` and under the
lock (control; green on main too).
- **Each refresh path tells the plugin its own cause:** no refresh
token, no client and `invalid_grant` each call `onGrantRefused('org_a',
stored)` once, with the stored sentence naming that cause.
Two existing cases now also read the second argument (`invalid_grant`
gets the refusal message; the kept-client case gets the no-client
sentence).
`src/connector-connection-door.test.ts` (real plugin, real ObjectQL on
SQLite, local OAuth server):
- **New:** a connect whose code exchange issues no refresh token, then a
dispatch the upstream answers 401. The row stores the no-refresh-token
sentence, the 503 message equals it, the log carries `[Automation]
REASON.` and nowhere `refused the grant`, and the instance is held with
that same cause. The rig gains `idp.exchangeIssuesRefreshToken` (default
true, reset after each case).
- **Updated:** the no-leak case's "the logs did record it" assertion
reads the refused path's new line, `[Automation] connector instance
'crm' needs to be reconnected for organization 'org_a': its
authorization server refused the grant`.
<details><summary>Red on main, green on head</summary>
- **Main** (the tests at `9ae5df9d`, before the fix): `Tests 6 failed |
40 passed (46)`. The reds: the two existing cases and the per-path case
(`expected "vi.fn()" to be called with arguments: [ 'org_a', …(1) ]`),
the no-refresh-token and replica cases (`Expected: "…its authorization
server issued no refresh token…"` / `"…keeps no client to refresh
with…"`, `Received: "…its authorization server refused the grant
(revoked, or expired with no usable refresh)…"`), and the door case
(`expected '…' to contain '[Automation] connector instance \'crm…'`).
The no-stored-cause control is green both ways.
- **Head:** `Test Files 2 passed (2)`, `Tests 46 passed (46)`.
</details>
<details><summary>Ablation, one per site</summary>
Each through `node scripts/ablation-replace.mjs` (anchor 1 → 0, blob
moved, restored; `git diff HEAD` empty after all four), running both
files. The package's tests import `./grant-handle.js` and `./plugin.js`
from `src/`, so no build sits between the mutation and the run.
| Site | Mutation | Result |
|---|---|---|
| `select` | `markedReason(connector, record)` →
`reconnectRequiredReason(connector, record.organizationId)` | 1 failed:
the no-refresh-token "every later dispatch" case |
| `refreshOnce` first check | `markedReason(connector, current)` →
`reconnectRequiredReason(connector, organizationId)` | 1 failed: the
replica-marked-under-lock case |
| `grantRefused` callback | `onGrantRefused?.(record.organizationId,
reason)` → `onGrantRefused?.(record.organizationId)` | 3 failed: the
per-path case and the two updated cases |
| plugin log line | `` `[Automation] ${reason}.` `` → the old "the
authorization server refused the grant of organization" line | 2 failed:
the new door case and the no-leak case |
</details>
## Acceptance notes
- **The plugin's hold-reason fallback (`plugin.ts`, the
`holdConnectorUntilConnected` call in the reconcile) already met ruling
1 on main**, so it is unchanged: it reads
`state.reconnectRequiredReason`, the first marked row's `statusReason`,
and falls back to `reconnectRequiredReason(name, org)` only when that
row stores none. The new door case pins it: the hold names the
no-refresh-token cause.
- **`onGrantRefused` keeps its name** although it now also fires for the
two paths that never asked the server; renaming an internal callback is
outside this card.
- **Who read the old log text:** only
`connector-connection-door.test.ts` (the no-leak case), updated here.
Searched objectstack, objectui and hotcrm for `refused the grant of
organization`: no other hit.
## Verification
All readings at head `543224b1`: the change (`9ae5df9d` tests,
`d40bcfaf` fix, `d5ba4f70` changeset) merged once with `origin/main`
`ab085a04`, a clean merge. One later main commit, `0be247ce`, touches
only `ui/` (plugin-detail).
<details><summary>Package suite, typecheck, lint</summary>
- Dependency closure: `pnpm turbo run build
--filter='@objectstack/service-automation...' --concurrency=2`, `Tasks:
32 successful, 32 total`, lock VERDICT command-exit 0.
- `pnpm --filter @objectstack/service-automation test` at `543224b1`:
`Test Files 256 passed (256)`, `Tests 3303 passed (3303)`, lock VERDICT
command-exit 0 (before the merge, at `d5ba4f70`: 255/255, 3289/3289).
- `pnpm --filter @objectstack/service-automation typecheck` (tsc +
test-typecheck) at `d5ba4f70`: lock VERDICT command-exit 0. The merge
touched no file of this diff.
- The repo's `pnpm lint` command scoped to
`packages/services/service-automation`: exit 0.
</details>
<details><summary>Derived gates (`dispatch-gates.mjs --commands`, 61
families)</summary>
- All 61 exit 0 at `543224b1`; `dispatch-gates --ran`: `61 derived
famil(ies) accounted for — 61 run, 0 NOT-MEASURED`. Record:
`pm-orders/server/28963-gates-ran.txt` in the project.
- The same list at `d5ba4f70` before the merge: 60 exit 0 and
`check:dual-build-cjs-loads` exit 3 (PREREQUISITE NOT MET, other
packages' `dist/` missing); at `543224b1` it ran and exits 0.
`check-plugin-teardown-shape.mjs --self-test` needs its pinned fixture
commit on this shallow clone (`git fetch --depth=1 origin 621a487…`),
then exits 0.
- Repo-wide `pnpm lint` and the CI-only families are CI's; see the PR's
check-runs.
</details>
`test:repo`: not run locally (no repo-project test or derived list names
it); CI's Test Core is the record. Dogfood battery: not owed (no `!`, no
narrowing arm) and not run.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5nz6egXzQp6jxykFAm)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(service-automation): a connection already marked "reconnect required" answers the cause it stores, and the refusal log names it (#28963)
6
+
7
+
Clause-②: no — the text of a dispatch refusal on an already-marked connection and of one log line. No accept set, status value, error code or export changes meaning.
8
+
Metadata-protocol: none
9
+
10
+
A refresh that marks a connector connection `reconnect_required` stores a reason naming its cause: the authorization server refused the grant, it issued no refresh token, or the connection keeps no client to refresh with. Two places still said the server refused the grant whatever had happened.
11
+
12
+
-**Every later dispatch on the marked connection** answers the reason the row stores, the same sentence the first dispatch answered. Before, only the dispatch that marked the row named the cause; every one after it (another replica, a restart, a retry) said the authorization server refused the grant. A row that stores no reason still answers the refusal sentence.
13
+
-**The server log line** written when a refresh marks a connection is now that stored reason, so it names the cause on each of the three refresh paths. Before, it said "the authorization server refused the grant" on all three.
14
+
- Unchanged: the 503 `SERVICE_UNAVAILABLE` refusal itself, the reason each path stores, the instance's "reconnect required" hold, and every status, code and write.
0 commit comments