[freeqaz]

Problem

On an Installation with no Slack proxy (neither Helm slackProxy.enabled nor api.channelDirectoryProxyUrl), the production API's default-deny egress never lets the Slack Channel Driver's auth.test reach Slack. Create Agent provisioning and Agent deploy of a correctly bound Slack channel (whole ${SLACK_BOT_TOKEN} / ${SLACK_APP_TOKEN}) answered 503 CHANNEL_CREDENTIAL_UNAVAILABLE "Channel credential validation is temporarily unavailable. Retry before deploying." A retry can never succeed, and nothing named the setting the Slack guide requires (dogfood finding 2521).

Change

• Production composition builds the Slack Channel Driver with a new constructor option, expectProxy. The ChannelDriver contract does not change.
• With expectProxy and no proxy URL, a credential check whose auth.test request gets no answer from Slack raises SlackRouteNotConfiguredError. "No answer" means the fetch rejected with a refused or unreachable connection, a DNS failure (ENOTFOUND, EAI_AGAIN), a connect timeout, or the 8 s budget running out. Any other fetch rejection (a redirect refused by redirect: "error", a TLS failure, an invalid header value) keeps the retryable 503. SlackRouteNotConfiguredError is a ResourceStateConflictError, so the API answers 409 RESOURCE_CONFLICT with this fixed text:

  OCC could not reach Slack, and the API has no Slack proxy. If the API has another approved route to Slack, try again. Otherwise an administrator must set Helm slackProxy.enabled or api.channelDirectoryProxyUrl; see the Slack integration guide.

• The message keeps "try again" because the API cannot tell a missing route from an open route that timed out (see Risks). It names the guide instead of linking it: the error contract caps messages at 256 characters.
• Precedent for the status: other "not configured on this Installation" answers use 409 with a fixed message that names the fix (RepositoryDriverNotConfiguredError, finding 2362; ServiceAccountDriverNotConfiguredError). The Console already shows a named 409 as written, with no Retry button, on Create Agent and on the Agent page.
• Unchanged:
  • Any HTTP answer from Slack (5xx, 429, invalid_auth, ...) and any fetch rejection other than the ones listed above keep today's errors, including the retryable 503.
  • A route an operator approved instead of a proxy still validates (production settings: "another approved network route").
  • Development composition, which sets no expectProxy, is unchanged.
  • Binding and token-prefix checks still answer first, and no token text is echoed.
• Directory lookup is unchanged: OCC wraps every lookup error as CHANNEL_DIRECTORY_*.
• Docs: Slack Channel Driver reference (credential validation), Slack guide (proxy section), channel directory flow.

Verification

• tests/integration/occ-api.test.mjs "Slack validation with no Slack proxy names the setting at provision and deploy" runs the real API routes with the Driver built as production builds it. It checks that a wrong token role still answers 400 first, a connection timeout (the rejection shape Node's fetch gives) gets the 409 at provision with no Agent created, an HTTP 503 from Slack stays 503 CHANNEL_CREDENTIAL_UNAVAILABLE, a working route passes validation with auth.test actually called, and deploy of a saved Agent gets the same 409.
• tests/conformance/slack-channel-directory.test.mjs "...name the missing proxy only when production expects one and Slack never answered" drives Node's own fetch at local endpoints, so each rejection has its real shape. A refused connection and an expired time budget give the named refusal. A refused 302 redirect and an invalid header byte in the token stay unavailable. Development stays unavailable, and a configured proxy that refuses its tunnel stays unavailable.
• Both tests fail on main (503 instead of 409; no SlackRouteNotConfiguredError) and on this PR's previous head 277166da2c (old message; the redirect and header cases got the 409). Two classifier mutants (always/never "unanswered") fail the conformance test. The tests pass 20/20 at 2 CPUs on a remote test VM. The existing Slack validation tests pass. tsc --build, eslint and prettier are clean on the touched files.

Risks

• Only the production path with no proxy URL changes.
• Open direct route. A production Installation can reach Slack with no proxy: GitHub, Google or OIDC sign-in with the default empty egressCidrs allows API TCP 443 to any address. That is a common setup, not a rare one. There, a transient failure (Slack slower than the 8 s budget, a DNS EAI_AGAIN, or a multi-account check that used up the shared budget) now gets this 409 instead of the retryable 503. The message covers that case ("If the API has another approved route to Slack, try again"), but the administrator also reads proxy settings they may not need, and the Console shows no Retry button for a 409; resubmitting works.
• The exact fix is for Helm to tell the API whether its egress is closed, and to set expectProxy only then. It is a follow-up, not part of this PR.
• Draft Draft implementation: managed outbound HTTPS for dedicated Gateways #2518 also edits slack.ts. A later merge may need to carry the option across.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

This PR names the Slack proxy settings when production credential validation fails before receiving an HTTP response.

Example: Provision a Slack-enabled Agent with valid token bindings, no API proxy URL, and blocked Slack egress.

• Before: 503 CHANNEL_CREDENTIAL_UNAVAILABLE asks the user to retry.
• After: 409 RESOURCE_CONFLICT names slackProxy.enabled and api.channelDirectoryProxyUrl.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	Useful, focused work with meaningful API coverage, but the HTTP-response classification defect needs repair.
Proof confidence	🦐 gold shrimp (3/6)	This collaborator-authored diagnostic PR is outside the ordinary contributor proof gate; supplied validation exercises OCC routes in process with injected Slack responses, and linked Gateway UI media does not demonstrate this change.
Patch quality	🦪 silver shellfish (2/6)	1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Slack-enabled Agent provisioning or deployment asks users to retry when the Installation lacks the API network route that an administrator must configure.
Reason: The maintainer-authored direction replaces an ineffective retry instruction with the existing operator settings, while retaining credential checks and the selected Driver boundary.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep this PR open: the diagnostic improvement is useful and maintainer-directed, but HTTP redirects are incorrectly classified as unanswered requests.

Priority: P2
Reviewed head: 277166da2c3e2b936912292193f6a53a7ca226e5

Before merge

• Preserve unavailable errors for refused HTTP redirects (P2) - With production's expectProxy enabled and no proxy URL, an auth.test response of 301, 302, 307, or 308 causes fetch to reject because the request uses redirect: "error". This catch turns that received HTTP response into SlackUnansweredError, so provisioning and deployment return 409 with instructions to configure a proxy instead of the existing 503 CHANNEL_CREDENTIAL_UNAVAILABLE. The existing real-fetch redirect test covers these responses only without expectProxy, so it misses the regression. Use manual redirect handling so the existing non-success response path rejects redirects without following them, and extend that test to cover production options.
• Explain or restore the original intent of HTTP redirect error classification - fd1e433: fix(credentials): refuse ip and localhost oauth2 token_url hosts and slack redirects #2439 deliberately made redirects unavailable without forwarding credentials; this PR promises HTTP-answer behavior remains unchanged but reclassifies those rejections.

Findings

• [P2] Preserve unavailable errors for refused HTTP redirects — apps/controller/src/drivers/channel/slack.ts:259-260

Provenance

• HTTP redirect error classification changes intended behavior without addressing why it exists (fd1e433: fix(credentials): refuse ip and localhost oauth2 token_url hosts and slack redirects #2439 deliberately made redirects unavailable without forwarding credentials; this PR promises HTTP-answer behavior remains unchanged but reclassifies those rejections.)

Tests

• Missing end-to-end proof: No supplied shipped-server run demonstrates the new response; the added regression tests are reported to fail on main and pass on the head, but that comparison was not independently executed.

Agent review details

How this fits together

Production composition configures the Slack Channel Driver; OCC authorizes Secret access, invokes credential validation before provisioning or deployment, and translates errors into API responses.

flowchart TD
  A[Provision or deploy request] --> B[OCC authorization]
  B --> C[Secret Driver callback]
  C --> D[Slack credential validation]
  E[Production proxy settings] --> D
  D --> F[Slack auth test]
  F --> G[API result]

Loading

Technical review

Best possible solution:

Keep the existing request owner, expose redirect responses without following them, and reserve the missing-proxy diagnostic for transport failures.

Do we have a high-confidence way to reproduce the issue?

Current main maps Slack fetch failures to the retryable credential-unavailable error even without a configured proxy; source inspection establishes the path, and no target code was run.

Is this the best way to solve the issue?

The existing Driver is the right owner, but rejected HTTP redirects must remain distinguishable from requests that receive no response.

Full review comments:

• [P2] Preserve unavailable errors for refused HTTP redirects — apps/controller/src/drivers/channel/slack.ts:259-260
  With production's expectProxy enabled and no proxy URL, an auth.test response of 301, 302, 307, or 308 causes fetch to reject because the request uses redirect: "error". This catch turns that received HTTP response into SlackUnansweredError, so provisioning and deployment return 409 with instructions to configure a proxy instead of the existing 503 CHANNEL_CREDENTIAL_UNAVAILABLE. The existing real-fetch redirect test covers these responses only without expectProxy, so it misses the regression. Use manual redirect handling so the existing non-success response path rejects redirects without following them, and extend that test to cover production options.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3918205bc18f.

Provenance checked

• Request-time Slack credential validation changes intended behavior with a stated reason (756b02c: Validate Slack token roles and bot credentials before API deployment #568 placed bounded provider validation before admission, with exact Secret authorization and sanitized failures.)
• Production Slack proxy composition and documentation keeps the original intent (ee48017: feat(deploy): add OpenClaw and Codex installation profiles #555 supplied optional managed CONNECT routing with narrow API egress; this PR explains the diagnostic change when that route is absent.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

• node --test --test-name-pattern='Slack credential checks refuse redirects|Slack credential checks name the missing proxy' tests/conformance/slack-channel-directory.test.mjs
• node --test --test-name-pattern='Slack validation with no Slack proxy' tests/integration/occ-api.test.mjs
• pnpm lint

What I checked:

• Introduced error classification: The pinned diff changes every fetch rejection into SlackUnansweredError, which production validation translates into the missing-proxy conflict when no proxy URL is configured. (apps/controller/src/drivers/channel/slack.ts:259, 277166da2c3e)
• Existing redirect scenario: The existing test uses a real HTTP server and fetch for 301, 302, 307, and 308 responses, expecting unavailable and no request to the redirect target; it does not enable expectProxy. (tests/conformance/slack-channel-directory.test.mjs:415, 277166da2c3e)
• Redirect refusal provenance: fix(credentials): refuse ip and localhost oauth2 token_url hosts and slack redirects #2439 deliberately refused redirects to prevent bot-token forwarding and specified unavailable as their result. (apps/controller/src/drivers/channel/slack.ts:248, fd1e433eb30b)
• Accepted diagnostic tradeoff: The collaborator-authored PR explicitly accepts reporting proxy guidance when an approved direct route is temporarily down; that tradeoff requires no additional owner decision. (277166da2c3e)
• Coverage boundary: The added API case exercises real OCC routes through an in-process application with an injected Slack transport, covering provisioning refusal, unchanged HTTP 503 handling, and deployment refusal. (tests/integration/occ-api.test.mjs:8719, 277166da2c3e)
• Linked proof belongs to separate work: The three inspected screenshots show Gateway network settings from Draft implementation: managed outbound HTTPS for dedicated Gateways #2518, not this diagnostic; its video preparation failed with 'ffprobe failed: spawnSync ffprobe ENOENT', and no browser playback tool was available.

Likely related people:

• kevinlin-openai: Raw commit 756b02c adds apps/controller/src/drivers/channel/slack.ts:97 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 756b02cec5b4; files: apps/controller/src/drivers/channel/slack.ts)
• Steven Lee: Raw commit 2a8d6cb adds apps/controller/src/drivers/channel/slack.ts:78 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 2a8d6cb405c8; files: apps/controller/src/drivers/channel/slack.ts)
• Free Wortley: Raw commit fd1e433 adds apps/controller/src/drivers/channel/slack.ts:206 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: fd1e433eb30b; files: apps/controller/src/drivers/channel/slack.ts)

Labels

Label changes:

• add P2: The PR improves a bounded Slack provisioning diagnostic; the introduced defect gives incorrect recovery guidance without changing credential acceptance.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P2: The PR improves a bounded Slack provisioning diagnostic; the introduced defect gives incorrect recovery guidance without changing credential acceptance.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦐 gold shrimp and patch quality is 🦪 silver shellfish.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 25, 2026, 7:07 PM ET / 23:07 UTC.