Repository navigation
fix(service-automation): a connection already marked reconnect required answers the cause it stores, and the refusal log names it - #29065
Conversation
…e; the refusal log names it Co-Authored-By: Claude <noreply@anthropic.com>
…ed answers the cause it stores, and the log names it Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e5d8d0175eb5dbbf183c7e7e79308d2a51e58ffa && git checkout e5d8d0175eb5dbbf183c7e7e79308d2a51e58ffa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c708de67b2c00c095afbf5fb36c6c093dbe85458 543224b14eabb7b0ddff6b1cc1c793b296eef1e8 && git checkout -B drift-repro c708de67b2c00c095afbf5fb36c6c093dbe85458 && git merge --no-ff 543224b14eabb7b0ddff6b1cc1c793b296eef1e8
node scripts/docs-audit/affected-docs.mjs --json c708de67b2c00c095afbf5fb36c6c093dbe85458
|
|
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt 席位核对ACCEPT · The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」. 席位意见:与 diff 相符。已标成「需要重新连接」的连接,之后每次调度都回答该行存下的原因(没存原因时才说「授权服务器拒绝」);刷新标记连接时的日志也改为这句原因,三条路径各说各的。状态、错误码、503 拒绝本身不变。
|
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt
Requested by Leehom · project thread
Fixes #28963
Clause-②: no
Metadata-protocol: none
维护者速读
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 markedreconnect_requiredanswers what it stores. A new privatemarkedReason(connector, record)returns the row'sstatusReason, 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) andrefreshOnce'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 fix(service-automation): a connection marked reconnect required says the cause that actually happened (#28863) #28955 introduced, so the first dispatch and every later one now answer the same sentence.onGrantRefusedcarries the reason.OAuth2GrantHandleDeps.onGrantRefusedis now(organizationId, reason):grantRefusedpasses the sentence it just stored. The interface is internal:oauth2-grant/is not re-exported from the package entry (src/index.tshas nooauth2-grantexport, 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 fix(service-automation): a connection marked reconnect required says the cause that actually happened (#28863) #28955's words. Stillwarn; it is distinct from the hold's own "… is degraded: …" line..changeset/28963-reconnect-required-stored-cause.md:patchon@objectstack/service-automation, both declaration lines.packages/spec/**; no docs page change (connectors.mdxalready says the connection'sstatusReasonnames the cause).Why
Clause-②: no. Nothing that was accepted is refused and nothing refused is accepted: the 503 stays a 503SERVICE_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:gets and arefresh, and from a second handle that never saw the marking (another replica, a restart). None saysrefused; no token request is made.no_clientbefore running the refresh): the dispatch answers that stored sentence. This is therefreshOncefirst check.reconnectRequiredReason's refusal sentence, atselectand under the lock (control; green on main too).invalid_granteach callonGrantRefused('org_a', stored)once, with the stored sentence naming that cause.Two existing cases now also read the second argument (
invalid_grantgets 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):[Automation] REASON.and nowhererefused the grant, and the instance is held with that same cause. The rig gainsidp.exchangeIssuesRefreshToken(default true, reset after each case).[Automation] connector instance 'crm' needs to be reconnected for organization 'org_a': its authorization server refused the grant.Red on main, green on head
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.Test Files 2 passed (2),Tests 46 passed (46).Ablation, one per site
Each through
node scripts/ablation-replace.mjs(anchor 1 → 0, blob moved, restored;git diff HEADempty after all four), running both files. The package's tests import./grant-handle.jsand./plugin.jsfromsrc/, so no build sits between the mutation and the run.selectmarkedReason(connector, record)→reconnectRequiredReason(connector, record.organizationId)refreshOncefirst checkmarkedReason(connector, current)→reconnectRequiredReason(connector, organizationId)grantRefusedcallbackonGrantRefused?.(record.organizationId, reason)→onGrantRefused?.(record.organizationId)`[Automation] ${reason}.`→ the old "the authorization server refused the grant of organization" lineAcceptance notes
plugin.ts, theholdConnectorUntilConnectedcall in the reconcile) already met ruling 1 on main, so it is unchanged: it readsstate.reconnectRequiredReason, the first marked row'sstatusReason, and falls back toreconnectRequiredReason(name, org)only when that row stores none. The new door case pins it: the hold names the no-refresh-token cause.onGrantRefusedkeeps its name although it now also fires for the two paths that never asked the server; renaming an internal callback is outside this card.connector-connection-door.test.ts(the no-leak case), updated here. Searched objectstack, objectui and hotcrm forrefused the grant of organization: no other hit.Verification
All readings at head
543224b1: the change (9ae5df9dtests,d40bcfaffix,d5ba4f70changeset) merged once withorigin/mainab085a04, a clean merge. One later main commit,0be247ce, touches onlyui/(plugin-detail).Package suite, typecheck, lint
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 testat543224b1:Test Files 256 passed (256),Tests 3303 passed (3303), lock VERDICT command-exit 0 (before the merge, atd5ba4f70: 255/255, 3289/3289).pnpm --filter @objectstack/service-automation typecheck(tsc + test-typecheck) atd5ba4f70: lock VERDICT command-exit 0. The merge touched no file of this diff.pnpm lintcommand scoped topackages/services/service-automation: exit 0.Derived gates (`dispatch-gates.mjs --commands`, 61 families)
543224b1;dispatch-gates --ran:61 derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED. Record:pm-orders/server/28963-gates-ran.txtin the project.d5ba4f70before the merge: 60 exit 0 andcheck:dual-build-cjs-loadsexit 3 (PREREQUISITE NOT MET, other packages'dist/missing); at543224b1it ran and exits 0.check-plugin-teardown-shape.mjs --self-testneeds its pinned fixture commit on this shallow clone (git fetch --depth=1 origin 621a4876…), then exits 0.pnpm lintand the CI-only families are CI's; see the PR's check-runs.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