feat(cli): non-interactive Slack connect/disconnect + sync round-trip fixes (#8935)

* feat(cli): non-interactive Slack connect/disconnect

Extract create_slack_workspace_artifacts / create_slack_instance_artifacts
from the browser OAuth callbacks and expose them via two new endpoints that
accept a pre-minted xoxb bot token:

- POST /w/{workspace}/workspaces/connect_slack (admin)
- POST /oauth/connect_slack_instance (super-admin)

Both produce bit-for-bit identical DB state to the UI browser flow.

Wire three CLI commands as thin wrappers:
- wmill workspace connect-slack
- wmill workspace disconnect-slack
- wmill instance connect-slack

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(cli): round-trip stability for workspace settings handlers

wmill sync push was destroying UI-configured error_handler/success_handler
state on every deploy. Two orthogonal bugs:

(a) pushWorkspaceSettings called editErrorHandler with `path: undefined`
    when the YAML lacked the handler block, which the backend treats as a
    clear — so syncing settings.yaml that didn't mention the handler wiped
    the DB row. Fix: skip the call entirely when absent from YAML.

(b) edit_error_handler omitted muted_on_cancel / muted_on_user_path when
    false, but the CLI always sends them, causing perpetual deepEqual
    drift and a spurious editErrorHandler call on every sync push. Fix:
    always persist both booleans.

migrateToGroupedFormat now preserves explicit `null` on
error_handler / success_handler as a "clear remote" signal distinct from
absence. Widen ErrorHandlerConfig | null / SuccessHandlerConfig | null to
make this explicit in the type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(cli): sync support for workspace-level Slack OAuth override

Add slack_oauth_client_id and slack_oauth_client_secret to the v2 tarball
export and to pushWorkspaceSettings, so the workspace-level OAuth override
is now fully managed as code through settings.yaml.

Semantics:
  - both defined and truthy → setWorkspaceSlackOauthConfig (upsert)
  - both defined but falsy (e.g. empty strings) and remote has a value
    → deleteWorkspaceSlackOauthConfig
  - either omitted → leave remote alone ("not managed by git")

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(cli): normalize workspace settings sync to "omit = clear"

Earlier commits on this branch introduced an "omit = keep" rule for
error_handler / success_handler / slack_oauth_client_{id,secret} that
diverged from every other workspace setting (webhook, deploy_to, etc. all
treat YAML as canonical: absence = clear). Normalize:

- v2 tarball always emits these 4 fields (null when remote is NULL) so
  round-trip is bijective and settings.yaml is a complete snapshot.
- pushWorkspaceSettings drops the absent-from-YAML guards; YAML is
  canonical. Absence and explicit null both clear the remote — same rule
  as every other field.
- set_slack_oauth_config / delete_slack_oauth_config now fire
  handle_deployment_metadata so UI mutations reach git-sync-enabled
  workspaces' committed settings.yaml.

Policy for users: pull before push (same as every other setting). On first
post-upgrade pull, explicit `null` keys appear for any workspace whose
handlers / oauth override are unset — one-time YAML diff, no semantic
change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(cli): add unit + integration coverage for Slack settings sync

Unit tests (settings_unit.test.ts): cover migrateToGroupedFormat preserving
explicit `null` on error_handler / success_handler, and passthrough of
slack_oauth_client_id / _secret (both populated and null values).

Integration tests (slack_settings_sync.test.ts, skipped on CI per the same
convention as datatable_settings_sync.test.ts): exercise the full backend
via withTestBackend to verify

  1. pull emits null for unset error_handler / success_handler /
     slack_oauth_client_id / _secret;
  2. round-trip with all-null handlers is idempotent;
  3. push of populated slack_oauth_config upserts;
  4. omitting the slack_oauth keys from YAML clears remote (universal
     "omit = clear" rule);
  5. explicit null error_handler in YAML clears remote;
  6. round-trip preserves a populated error_handler exactly, including the
     always-persisted muted_on_cancel / muted_on_user_path booleans.

Also feature-gates `use crate::oauth2_oss::workspace_connect_slack` and its
route registration behind `cfg(feature = "oauth2")`: the import caused a
build failure on subsets of the workspace without the oauth2 feature,
surfaced by the integration test harness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: bump ee-repo-ref to 59b6123

Pins windmill-ee-private to the tip of branch alp/slack_cli, which
contains the companion EE changes (helper extraction, non-interactive
Slack connect handlers, git-sync for Slack settings mutations).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Update SQLx metadata

* chore: regenerate system prompts for new slack CLI commands

Captures the new workspace connect-slack, workspace disconnect-slack,
and instance connect-slack commands in the auto-generated files that
CI enforces via system_prompts/check-freshness.sh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to b4a5ca11e3b96ff03793c2bd396dbc1fe6ea1022

This commit updates the EE repository reference after PR #550 was merged in windmill-ee-private.

Previous ee-repo-ref: d7e44d0519327ec9077625130365e887826f324b

New ee-repo-ref: b4a5ca11e3b96ff03793c2bd396dbc1fe6ea1022

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
This commit is contained in:
Alexander Petric
2026-04-24 19:14:08 +02:00
committed by GitHub
parent a1a73309fd
commit 95d4c6a94d
20 changed files with 874 additions and 45 deletions
@@ -15,7 +15,7 @@
]
},
"nullable": [
true
null
]
},
"hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55"
@@ -0,0 +1,23 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT EXISTS (SELECT 1\n FROM workspace_settings\n WHERE workspace_id <> $1\n AND slack_command_script IS NOT NULL\n AND slack_team_id = $2\n AND (SELECT slack_command_script IS NOT NULL FROM workspace_settings WHERE workspace_id = $1))\n ",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "exists",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
null
]
},
"hash": "68601ae5830fd8ccf016b03db2168414d44eca6d4bf1da92f0a30a96512743c2"
}
@@ -1,23 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT EXISTS (SELECT 1\n FROM workspace_settings\n WHERE workspace_id <> $1\n AND slack_command_script IS NOT NULL\n AND slack_team_id = $2\n AND (SELECT slack_command_script IS NOT NULL FROM workspace_settings WHERE workspace_id = $1))\n ",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "exists",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
null
]
},
"hash": "f955a01779f5441efc6aa9364b24c79b3cbc6413046c3b6099d19f675d8a395b"
}
+1 -1
View File
@@ -1 +1 @@
680885a4e8c8de5185650cddeb56b926e722718f
b4a5ca11e3b96ff03793c2bd396dbc1fe6ea1022
@@ -967,6 +967,18 @@ async fn set_slack_oauth_config(
tx.commit().await?;
handle_deployment_metadata(
&authed.email,
&authed.username,
&db,
&w_id,
DeployedObject::Settings { setting_type: "slack_oauth_config".to_string() },
Some("Slack OAuth config set".to_string()),
false,
None,
)
.await?;
Ok(format!("Slack OAuth config set for workspace {}", &w_id))
}
@@ -1001,6 +1013,18 @@ async fn delete_slack_oauth_config(
tx.commit().await?;
handle_deployment_metadata(
&authed.email,
&authed.username,
&db,
&w_id,
DeployedObject::Settings { setting_type: "slack_oauth_config".to_string() },
Some("Slack OAuth config deleted".to_string()),
false,
None,
)
.await?;
Ok(format!(
"Slack OAuth config deleted for workspace {}",
&w_id
@@ -2803,18 +2827,19 @@ async fn edit_error_handler(
}
}
// Always persist `muted_on_cancel` and `muted_on_user_path` (including
// false values) so that a YAML round-trip via `wmill sync pull && wmill
// sync push` is stable instead of re-firing `editErrorHandler` on every
// push (the CLI sends `false` defaults and deepEqual would otherwise
// mismatch an omitted-on-write shape against an always-sent-by-CLI one).
let mut error_handler = serde_json::json!({
"path": path,
"muted_on_cancel": ee.muted_on_cancel,
"muted_on_user_path": ee.muted_on_user_path,
});
if let Some(extra_args) = &ee.extra_args {
error_handler["extra_args"] = extra_args.clone();
}
if ee.muted_on_cancel {
error_handler["muted_on_cancel"] = serde_json::json!(true);
}
if ee.muted_on_user_path {
error_handler["muted_on_user_path"] = serde_json::json!(true);
}
sqlx::query!(
"UPDATE workspace_settings SET error_handler = $1 WHERE workspace_id = $2",
+60
View File
@@ -3566,6 +3566,37 @@ paths:
schema:
type: string
/w/{workspace}/workspaces/connect_slack:
post:
summary: connect slack (non-interactive; pre-minted bot token)
operationId: connectSlack
tags:
- workspace
parameters:
- $ref: "#/components/parameters/WorkspaceId"
requestBody:
description: connect slack with a pre-minted bot token
required: true
content:
application/json:
schema:
type: object
required:
- bot_token
- team_id
- team_name
properties:
bot_token:
type: string
description: xoxb-... bot token obtained at api.slack.com/apps
team_id:
type: string
team_name:
type: string
responses:
"200":
description: status
/w/{workspace}/workspaces/run_slack_message_test_job:
post:
summary: run a job that sends a message to Slack
@@ -5730,6 +5761,35 @@ paths:
schema:
type: string
/oauth/connect_slack_instance:
post:
summary: connect slack instance (non-interactive; pre-minted bot token)
operationId: connectSlackInstance
tags:
- oauth
requestBody:
description: connect slack at the instance level with a pre-minted bot token
required: true
content:
application/json:
schema:
type: object
required:
- bot_token
- team_id
- team_name
properties:
bot_token:
type: string
description: xoxb-... bot token obtained at api.slack.com/apps
team_id:
type: string
team_name:
type: string
responses:
"200":
description: status
/oauth/connect_callback/{client_name}:
post:
summary: connect callback
+14
View File
@@ -53,6 +53,20 @@ pub fn workspaced_service() -> Router {
Router::new()
}
#[cfg(not(feature = "private"))]
pub async fn workspace_connect_slack() -> Result<http::status::StatusCode, error::Error> {
Err(error::Error::BadRequest(
"Slack only available on enterprise".to_string(),
))
}
#[cfg(not(feature = "private"))]
pub async fn connect_slack_instance() -> Result<http::status::StatusCode, error::Error> {
Err(error::Error::BadRequest(
"Slack only available on enterprise".to_string(),
))
}
#[cfg(all(feature = "oauth2", not(feature = "private")))]
pub use windmill_oauth::{AllClients, BasicClientsMap, ClientWithScopes};
+5
View File
@@ -12,6 +12,8 @@ use windmill_api_workspaces::workspaces::{build_copilot_settings_state, Instance
use crate::ai::{invalidate_ai_request_cache_for_workspace, AIConfig};
use crate::db::ApiAuthed;
#[cfg(feature = "oauth2")]
use crate::oauth2_oss::workspace_connect_slack;
use crate::teams_oss::{
connect_teams, edit_teams_command, run_teams_message_test_job,
workspaces_list_available_teams_channels, workspaces_list_available_teams_ids,
@@ -72,6 +74,9 @@ pub fn workspaced_service() -> Router {
)
.route("/critical_alerts/mute", post(mute_critical_alerts));
#[cfg(feature = "oauth2")]
let router = router.route("/connect_slack", post(workspace_connect_slack));
#[cfg(all(feature = "stripe", feature = "enterprise"))]
{
crate::stripe_oss::add_stripe_routes(router)
+14 -3
View File
@@ -278,9 +278,11 @@ struct SimplifiedSettings {
webhook: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
deploy_to: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
// Always serialize (including as `null`) so that `wmill sync pull` emits
// these fields in settings.yaml unconditionally. Makes round-trip
// bijective: YAML is the source of truth, absence/null = "clear remote",
// mirroring every other workspace setting.
error_handler: Option<Value>,
#[serde(skip_serializing_if = "Option::is_none")]
success_handler: Option<Value>,
#[serde(skip_serializing_if = "Option::is_none")]
ai_config: Option<serde_json::Value>,
@@ -307,6 +309,9 @@ struct SimplifiedSettings {
slack_name: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
slack_command_script: Option<String>,
// Always serialize (see note above on error_handler / success_handler).
slack_oauth_client_id: Option<String>,
slack_oauth_client_secret: Option<String>,
}
// V1 format: Legacy flat format for backward compatibility (matches main branch exactly)
@@ -372,6 +377,8 @@ struct SettingsRow {
slack_team_id: Option<String>,
slack_name: Option<String>,
slack_command_script: Option<String>,
slack_oauth_client_id: Option<String>,
slack_oauth_client_secret: Option<String>,
}
pub(crate) async fn tarball_workspace(
@@ -998,7 +1005,9 @@ pub(crate) async fn tarball_workspace(
datatable,
slack_team_id,
slack_name,
slack_command_script
slack_command_script,
slack_oauth_client_id,
slack_oauth_client_secret
FROM workspace_settings
LEFT JOIN workspace ON workspace.id = workspace_settings.workspace_id
WHERE workspace_id = $1"#,
@@ -1028,6 +1037,8 @@ pub(crate) async fn tarball_workspace(
slack_team_id: row.slack_team_id.clone(),
slack_name: row.slack_name.clone(),
slack_command_script: row.slack_command_script.clone(),
slack_oauth_client_id: row.slack_oauth_client_id.clone(),
slack_oauth_client_secret: row.slack_oauth_client_secret.clone(),
};
serde_json::to_value(settings)
.map(|v| serde_json::to_string_pretty(&v).ok())
+14 -1
View File
@@ -26,6 +26,7 @@ import {
pushInstanceGroups,
pushInstanceUsers,
} from "../user/user.ts";
import { connectSlackInstance } from "./slack.ts";
import {
add as workspaceSetup,
addWorkspace,
@@ -825,6 +826,18 @@ const command = new Command()
"--instance <instance:string>",
"Name of the instance, override the active instance",
)
.action(getConfig as any);
.action(getConfig as any)
.command("connect-slack")
.description(
"Non-interactively connect Slack at the instance level using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: global_settings 'slack' row + encrypted f/slack_bot/global_bot_token variable and resource in the admins workspace."
)
.option("--bot-token <bot_token:string>", "Slack bot token (xoxb-...)", { required: true })
.option("--team-id <team_id:string>", "Slack team id", { required: true })
.option("--team-name <team_name:string>", "Slack team name", { required: true })
.option(
"--instance <instance:string>",
"Instance profile to connect against (defaults to the active instance)"
)
.action((opts: any) => connectSlackInstance(opts));
export default command;
+55
View File
@@ -0,0 +1,55 @@
import { colors } from "@cliffy/ansi/colors";
import * as log from "../../core/log.ts";
import { setClient } from "../../core/client.ts";
import * as wmill from "../../../gen/services.gen.ts";
import { getActiveInstance, allInstances, Instance } from "./instance.ts";
async function resolveInstance(
instanceName: string | undefined
): Promise<Instance> {
if (instanceName) {
const match = (await allInstances()).find((i) => i.name === instanceName);
if (!match) {
throw new Error(`No local instance profile named ${instanceName}`);
}
return match;
}
const activeName = await getActiveInstance({});
if (!activeName) {
throw new Error(
"No active instance. Run 'wmill instance add' or pass --instance."
);
}
const match = (await allInstances()).find((i) => i.name === activeName);
if (!match) {
throw new Error(`Active instance ${activeName} not found in config`);
}
return match;
}
export async function connectSlackInstance(opts: {
instance?: string;
botToken: string;
teamId: string;
teamName: string;
}) {
const instance = await resolveInstance(opts.instance);
setClient(
instance.token,
instance.remote.substring(0, instance.remote.length - 1)
);
await wmill.connectSlackInstance({
requestBody: {
bot_token: opts.botToken,
team_id: opts.teamId,
team_name: opts.teamName,
},
});
log.info(
colors.bold.underline.green(
`Slack connected at instance ${instance.name} (team ${opts.teamName} / ${opts.teamId})`
)
);
}
+47
View File
@@ -0,0 +1,47 @@
import { GlobalOptions } from "../../types.ts";
import { colors } from "@cliffy/ansi/colors";
import * as log from "../../core/log.ts";
import { requireLogin } from "../../core/auth.ts";
import { resolveWorkspace } from "../../core/context.ts";
import * as wmill from "../../../gen/services.gen.ts";
export async function connectSlack(
opts: GlobalOptions & {
botToken: string;
teamId: string;
teamName: string;
}
) {
await requireLogin(opts);
const workspace = await resolveWorkspace(opts);
await wmill.connectSlack({
workspace: workspace.workspaceId,
requestBody: {
bot_token: opts.botToken,
team_id: opts.teamId,
team_name: opts.teamName,
},
});
log.info(
colors.bold.underline.green(
`Slack connected to workspace ${workspace.workspaceId} (team ${opts.teamName} / ${opts.teamId})`
)
);
}
export async function disconnectSlack(opts: GlobalOptions) {
await requireLogin(opts);
const workspace = await resolveWorkspace(opts);
await wmill.disconnectSlack({ workspace: workspace.workspaceId });
log.info(
colors.bold.underline.green(
`Slack disconnected from workspace ${workspace.workspaceId} (slack_team_id / slack_name cleared). ` +
`To also remove the bot token variable/resource/folder/group, delete the corresponding files from the local sync folder and run 'wmill sync push'. ` +
`To remove the workspace-level OAuth override (if any), set slack_oauth_client_id/_secret to '' in settings.yaml and push.`
)
);
}
+15 -1
View File
@@ -17,6 +17,7 @@ import { setClient } from "../../core/client.ts";
import { requireLogin } from "../../core/auth.ts";
import { createWorkspaceFork, deleteWorkspaceFork } from "./fork.ts";
import { mergeWorkspaces } from "./merge.ts";
import { connectSlack, disconnectSlack } from "./slack.ts";
import * as wmill from "../../../gen/services.gen.ts";
@@ -787,6 +788,19 @@ const command = new Command()
.option("--exclude <items:string>", "Comma-separated kind:path items to exclude")
.option("--preserve-on-behalf-of", "Preserve original on_behalf_of/permissioned_as values")
.option("-y --yes", "Non-interactive mode (deploy without prompts)")
.action(mergeWorkspaces as any);
.action(mergeWorkspaces as any)
.command("connect-slack")
.description(
"Non-interactively connect Slack to the active workspace using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: workspace_settings fields, g/slack group, f/slack_bot folder, and the encrypted bot token variable + resource at f/slack_bot/bot_token."
)
.option("--bot-token <bot_token:string>", "Slack bot token (xoxb-...)", { required: true })
.option("--team-id <team_id:string>", "Slack team id", { required: true })
.option("--team-name <team_name:string>", "Slack team name", { required: true })
.action(connectSlack as any)
.command("disconnect-slack")
.description(
"Clear slack_team_id / slack_name on the active workspace (marks the workspace as disconnected). Does NOT remove the bot token variable/resource/folder/group — delete those from the local sync folder and run 'wmill sync push' to tear them down. Does NOT remove the workspace-level OAuth override — set slack_oauth_client_id/_secret to '' in settings.yaml and push."
)
.action(disconnectSlack as any);
export default command;
+51 -9
View File
@@ -35,10 +35,11 @@ export interface SuccessHandlerConfig {
}
export interface SimplifiedSettings {
// Grouped format (current)
// Grouped format (current). Explicit `null` on error_handler / success_handler
// signals "clear the remote value"; `undefined` means "not managed by git".
auto_invite?: AutoInviteConfig;
error_handler?: ErrorHandlerConfig;
success_handler?: SuccessHandlerConfig;
error_handler?: ErrorHandlerConfig | null;
success_handler?: SuccessHandlerConfig | null;
// Other fields
webhook?: string;
@@ -56,6 +57,8 @@ export interface SimplifiedSettings {
slack_team_id?: string;
slack_name?: string;
slack_command_script?: string;
slack_oauth_client_id?: string;
slack_oauth_client_secret?: string;
}
// Legacy settings interface for reading old settings.yaml files
@@ -83,6 +86,8 @@ interface LegacySimplifiedSettings {
slack_team_id?: string;
slack_name?: string;
slack_command_script?: string;
slack_oauth_client_id?: string;
slack_oauth_client_secret?: string;
}
// Helper to convert legacy flat settings to new grouped format
@@ -104,6 +109,8 @@ export function migrateToGroupedFormat(settings: any): SimplifiedSettings {
if (settings.slack_team_id !== undefined) result.slack_team_id = settings.slack_team_id;
if (settings.slack_name !== undefined) result.slack_name = settings.slack_name;
if (settings.slack_command_script !== undefined) result.slack_command_script = settings.slack_command_script;
if (settings.slack_oauth_client_id !== undefined) result.slack_oauth_client_id = settings.slack_oauth_client_id;
if (settings.slack_oauth_client_secret !== undefined) result.slack_oauth_client_secret = settings.slack_oauth_client_secret;
// Handle auto_invite: check if already grouped or needs migration
if (settings.auto_invite && typeof settings.auto_invite === "object") {
@@ -117,8 +124,12 @@ export function migrateToGroupedFormat(settings: any): SimplifiedSettings {
};
}
// Handle error_handler: check if already grouped or needs migration
if (settings.error_handler && typeof settings.error_handler === "object") {
// Handle error_handler: check if already grouped or needs migration.
// Preserve explicit null as a signal to clear the remote handler (distinct
// from absent = "not managed by git, leave remote alone").
if (settings.error_handler === null) {
result.error_handler = null;
} else if (settings.error_handler && typeof settings.error_handler === "object") {
result.error_handler = settings.error_handler;
} else if (typeof settings.error_handler === "string") {
// Legacy format (error_handler was a string path)
@@ -129,8 +140,10 @@ export function migrateToGroupedFormat(settings: any): SimplifiedSettings {
};
}
// Handle success_handler: check if already grouped or needs migration
if (settings.success_handler && typeof settings.success_handler === "object") {
// Handle success_handler: same semantics.
if (settings.success_handler === null) {
result.success_handler = null;
} else if (settings.success_handler && typeof settings.success_handler === "object") {
result.success_handler = settings.success_handler;
} else if (typeof settings.success_handler === "string") {
// Legacy format (success_handler was a string path)
@@ -197,6 +210,8 @@ export async function pushWorkspaceSettings(
slack_team_id: remoteSettings.slack_team_id,
slack_name: remoteSettings.slack_name,
slack_command_script: remoteSettings.slack_command_script,
slack_oauth_client_id: remoteSettings.slack_oauth_client_id,
slack_oauth_client_secret: remoteSettings.slack_oauth_client_secret,
};
} catch (err) {
throw new Error(`Failed to get workspace settings: ${err}`);
@@ -268,7 +283,10 @@ export async function pushWorkspaceSettings(
});
}
// Handle error_handler using grouped format
// Handle error_handler using grouped format. YAML is canonical:
// absent / null → clear remote; present object → upsert.
// (Same "omit = clear" rule as every other workspace setting. Pull always
// emits the field as null when remote is NULL, so round-trip is bijective.)
if (!deepEqual(localSettings.error_handler, settings.error_handler)) {
log.debug(`Updating error handler...`);
const localErrorHandler = localSettings.error_handler;
@@ -283,7 +301,7 @@ export async function pushWorkspaceSettings(
});
}
// Handle success_handler using grouped format
// Handle success_handler using grouped format. Same semantics as error_handler.
if (!deepEqual(localSettings.success_handler, settings.success_handler)) {
log.debug(`Updating success handler...`);
const localSuccessHandler = localSettings.success_handler;
@@ -401,6 +419,30 @@ export async function pushWorkspaceSettings(
},
});
}
// Workspace-level Slack OAuth override. YAML is canonical (same rule as
// every other setting): both present → upsert; anything else → delete.
// Pull always emits both fields as null when remote is NULL.
if (
localSettings.slack_oauth_client_id != settings.slack_oauth_client_id ||
localSettings.slack_oauth_client_secret != settings.slack_oauth_client_secret
) {
log.debug(`Updating slack oauth config...`);
if (
localSettings.slack_oauth_client_id &&
localSettings.slack_oauth_client_secret
) {
await wmill.setWorkspaceSlackOauthConfig({
workspace,
requestBody: {
slack_oauth_client_id: localSettings.slack_oauth_client_id,
slack_oauth_client_secret: localSettings.slack_oauth_client_secret,
},
});
} else {
await wmill.deleteWorkspaceSlackOauthConfig({ workspace });
}
}
}
export async function pushWorkspaceKey(
+10
View File
@@ -5519,6 +5519,11 @@ sync local with a remote instance or the opposite (push or pull)
- \`-o, --output-file <file:string>\` - Write YAML to a file instead of stdout
- \`--show-secrets\` - Include sensitive fields (license key, JWT secret) without prompting
- \`--instance <instance:string>\` - Name of the instance, override the active instance
- \`instance connect-slack\`
- \`--bot-token <bot_token:string>\` - Slack bot token (xoxb-...)
- \`--team-id <team_id:string>\` - Slack team id
- \`--team-name <team_name:string>\` - Slack team name
- \`--instance <instance:string>\` - Instance profile to connect against (defaults to the active instance)
### job
@@ -5867,6 +5872,11 @@ workspace related commands
- \`--exclude <items:string>\` - Comma-separated kind:path items to exclude
- \`--preserve-on-behalf-of\` - Preserve original on_behalf_of/permissioned_as values
- \`-y --yes\` - Non-interactive mode (deploy without prompts)
- \`workspace connect-slack\` - Non-interactively connect Slack to the active workspace using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: workspace_settings fields, g/slack group, f/slack_bot folder, and the encrypted bot token variable + resource at f/slack_bot/bot_token.
- \`--bot-token <bot_token:string>\` - Slack bot token (xoxb-...)
- \`--team-id <team_id:string>\` - Slack team id
- \`--team-name <team_name:string>\` - Slack team name
- \`workspace disconnect-slack\`
`,
};
+57
View File
@@ -210,4 +210,61 @@ describe("migrateToGroupedFormat", () => {
expect(result.slack_name).toBe("my-team");
expect(result.slack_command_script).toBe("u/admin/slack_handler");
});
// error_handler / success_handler: null must round-trip through migration so
// that `wmill sync push` can forward it to the backend as a "clear remote"
// signal. Before this fix, null fell through the truthy-object + string
// branches and ended up as undefined, making an explicit `error_handler: null`
// in settings.yaml a no-op under the universal "omit = clear" rule.
test("preserves explicit null error_handler", () => {
const result = migrateToGroupedFormat({ name: "ws", error_handler: null });
expect(result.error_handler).toBeNull();
expect("error_handler" in result).toBe(true);
});
test("preserves explicit null success_handler", () => {
const result = migrateToGroupedFormat({ name: "ws", success_handler: null });
expect(result.success_handler).toBeNull();
expect("success_handler" in result).toBe(true);
});
test("preserves both null handlers alongside a populated handler", () => {
const result = migrateToGroupedFormat({
name: "ws",
error_handler: { path: "u/admin/err", muted_on_cancel: false },
success_handler: null,
});
expect(result.error_handler).toEqual({
path: "u/admin/err",
muted_on_cancel: false,
});
expect(result.success_handler).toBeNull();
});
// slack_oauth_client_id / slack_oauth_client_secret are the workspace-level
// OAuth override. Pull always emits them (null when DB is NULL), and push
// forwards whatever is in YAML — both present upserts, anything else deletes.
test("copies slack_oauth_client_id / _secret through (populated)", () => {
const result = migrateToGroupedFormat({
name: "ws",
slack_oauth_client_id: "1234567890.1234567890",
slack_oauth_client_secret: "abcdef0123456789",
});
expect(result.slack_oauth_client_id).toBe("1234567890.1234567890");
expect(result.slack_oauth_client_secret).toBe("abcdef0123456789");
});
test("copies slack_oauth_client_id / _secret through (null)", () => {
const result = migrateToGroupedFormat({
name: "ws",
slack_oauth_client_id: null,
slack_oauth_client_secret: null,
});
// migrateToGroupedFormat only copies fields that are `!== undefined`;
// null is a valid value and must flow through so the push comparison sees
// "local is null, remote is null → equal" instead of treating absence as
// "not in YAML".
expect(result.slack_oauth_client_id).toBeNull();
expect(result.slack_oauth_client_secret).toBeNull();
});
});
+446
View File
@@ -0,0 +1,446 @@
/**
* Slack settings sync tests.
*
* Covers the "omit = clear" normalization applied to error_handler,
* success_handler, slack_oauth_client_id, and slack_oauth_client_secret:
*
* - pull always emits these 4 fields (null when remote is NULL) so that
* settings.yaml is a complete snapshot of remote state.
* - push treats the YAML as canonical: absence and explicit null both clear;
* a populated value upserts.
* - Round-trip (pull → push with no edits) is idempotent.
*
* Mirrors the structure of `datatable_settings_sync.test.ts`. Requires a live
* backend (skipped on CI by `shouldSkipOnCI()`).
*/
import { expect, test } from "bun:test";
import { writeFile, readFile } from "node:fs/promises";
import { parse, stringify } from "yaml";
import { withTestBackend } from "./test_backend.ts";
import { shouldSkipOnCI } from "./cargo_backend.ts";
import { addWorkspace } from "../workspace.ts";
// When the remote has no error_handler / success_handler / slack_oauth_client_*
// configured, a sync pull should still emit the keys — as `null` — so that the
// YAML is self-describing and round-trip is bijective.
test.skipIf(shouldSkipOnCI())(
"Slack sync: pull emits null for unset handlers + oauth override",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_pull_null_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
// Clear any prior state from other tests.
await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/edit_error_handler`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({}),
},
);
await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/edit_success_handler`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({}),
},
);
await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/slack_oauth_config`,
{ method: "DELETE" },
);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const settingsYaml = await readFile(`${tempDir}/settings.yaml`, "utf-8");
const settings = parse(settingsYaml) as Record<string, unknown>;
// All four keys MUST be present and null — not absent.
expect("error_handler" in settings).toBe(true);
expect(settings.error_handler).toBeNull();
expect("success_handler" in settings).toBe(true);
expect(settings.success_handler).toBeNull();
expect("slack_oauth_client_id" in settings).toBe(true);
expect(settings.slack_oauth_client_id).toBeNull();
expect("slack_oauth_client_secret" in settings).toBe(true);
expect(settings.slack_oauth_client_secret).toBeNull();
});
},
);
// After pull, pushing back without any edits must not mutate remote state
// (no churn on muted_on_* booleans, no spurious editErrorHandler calls).
test.skipIf(shouldSkipOnCI())(
"Slack sync: round-trip with all-null handlers is idempotent",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_roundtrip_null_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/slack_oauth_config`,
{ method: "DELETE" },
);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const pushResult = await backend.runCLICommand(
["sync", "push", "--yes"],
tempDir,
);
expect(pushResult.code).toEqual(0);
// Remote state must match the pre-push snapshot.
const settingsResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/get_settings`,
);
expect(settingsResp.ok).toBe(true);
const settings = await settingsResp.json();
expect(settings.error_handler ?? null).toBeNull();
expect(settings.success_handler ?? null).toBeNull();
expect(settings.slack_oauth_client_id ?? null).toBeNull();
expect(settings.slack_oauth_client_secret ?? null).toBeNull();
});
},
);
// Push with an explicit slack_oauth_client_id / _secret pair upserts the
// workspace-level OAuth override; the subsequent pull reflects the new state.
test.skipIf(shouldSkipOnCI())(
"Slack sync: push of populated slack_oauth_config upserts",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_push_oauth_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/slack_oauth_config`,
{ method: "DELETE" },
);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const settingsYaml = await readFile(`${tempDir}/settings.yaml`, "utf-8");
const localSettings = parse(settingsYaml) as Record<string, unknown>;
localSettings.slack_oauth_client_id = "1234567890.1234567890";
localSettings.slack_oauth_client_secret = "deadbeefcafebabe";
await writeFile(
`${tempDir}/settings.yaml`,
stringify(localSettings),
"utf-8",
);
const pushResult = await backend.runCLICommand(
["sync", "push", "--yes"],
tempDir,
);
expect(pushResult.code).toEqual(0);
const settingsResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/get_settings`,
);
expect(settingsResp.ok).toBe(true);
const settings = await settingsResp.json();
expect(settings.slack_oauth_client_id).toEqual("1234567890.1234567890");
expect(settings.slack_oauth_client_secret).toEqual("deadbeefcafebabe");
});
},
);
// Deleting the two slack_oauth keys from settings.yaml (leaving them absent)
// must clear the remote — same rule as every other workspace setting.
test.skipIf(shouldSkipOnCI())(
"Slack sync: omitting slack_oauth_config from YAML clears remote",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_omit_clear_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
// Pre-seed the workspace with a workspace-level OAuth override.
const setResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/slack_oauth_config`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
slack_oauth_client_id: "1111111111.2222222222",
slack_oauth_client_secret: "feedfacefeedfacefeedface",
}),
},
);
expect(setResp.ok).toBe(true);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const settingsYaml = await readFile(`${tempDir}/settings.yaml`, "utf-8");
const localSettings = parse(settingsYaml) as Record<string, unknown>;
// Remove both keys entirely — "omit = clear" under the universal rule.
delete localSettings.slack_oauth_client_id;
delete localSettings.slack_oauth_client_secret;
await writeFile(
`${tempDir}/settings.yaml`,
stringify(localSettings),
"utf-8",
);
const pushResult = await backend.runCLICommand(
["sync", "push", "--yes"],
tempDir,
);
expect(pushResult.code).toEqual(0);
const settingsResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/get_settings`,
);
expect(settingsResp.ok).toBe(true);
const settings = await settingsResp.json();
expect(settings.slack_oauth_client_id ?? null).toBeNull();
expect(settings.slack_oauth_client_secret ?? null).toBeNull();
});
},
);
// Explicit null in YAML must also clear — same outcome as omission; the two
// representations converge on the "clear remote" semantic.
test.skipIf(shouldSkipOnCI())(
"Slack sync: explicit null error_handler clears remote",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_null_clear_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
// Pre-seed an error handler on the workspace.
const setResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/edit_error_handler`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
path: "script/hub/19741/workspace-or-schedule-error-handler-slack",
extra_args: null,
muted_on_cancel: false,
muted_on_user_path: false,
}),
},
);
expect(setResp.ok).toBe(true);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const settingsYaml = await readFile(`${tempDir}/settings.yaml`, "utf-8");
const localSettings = parse(settingsYaml) as Record<string, unknown>;
expect(localSettings.error_handler).not.toBeNull(); // sanity: it was populated
// Explicit null — must clear.
localSettings.error_handler = null;
await writeFile(
`${tempDir}/settings.yaml`,
stringify(localSettings),
"utf-8",
);
const pushResult = await backend.runCLICommand(
["sync", "push", "--yes"],
tempDir,
);
expect(pushResult.code).toEqual(0);
const settingsResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/get_settings`,
);
expect(settingsResp.ok).toBe(true);
const settings = await settingsResp.json();
expect(settings.error_handler ?? null).toBeNull();
});
},
);
// Round-trip for a populated error_handler: pull should capture it exactly
// (including the always-persisted muted_on_* booleans), and re-push without
// edits should be a no-op.
test.skipIf(shouldSkipOnCI())(
"Slack sync: round-trip preserves populated error_handler",
async () => {
await withTestBackend(async (backend, tempDir) => {
const testWorkspace = {
remote: backend.baseUrl,
workspaceId: backend.workspace,
name: "slack_handler_roundtrip_test",
token: backend.token,
};
await addWorkspace(testWorkspace, {
force: true,
configDir: backend.testConfigDir,
});
const handlerPath =
"script/hub/19741/workspace-or-schedule-error-handler-slack";
const extraArgs = { channel: "test", slack: "$res:f/slack_bot/bot_token" };
const setResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/edit_error_handler`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
path: handlerPath,
extra_args: extraArgs,
muted_on_cancel: false,
muted_on_user_path: false,
}),
},
);
expect(setResp.ok).toBe(true);
await writeFile(
`${tempDir}/wmill.yaml`,
`defaultTs: bun
includes:
- "**"
includeSettings: true`,
"utf-8",
);
const pullResult = await backend.runCLICommand(
["sync", "pull", "--yes"],
tempDir,
);
expect(pullResult.code).toEqual(0);
const settingsYaml = await readFile(`${tempDir}/settings.yaml`, "utf-8");
const localSettings = parse(settingsYaml) as Record<string, unknown>;
const localHandler = localSettings.error_handler as Record<string, unknown>;
expect(localHandler).not.toBeNull();
expect(localHandler.path).toEqual(handlerPath);
expect(localHandler.extra_args).toEqual(extraArgs);
// Always-persist booleans (fix 4b) ensure round-trip stability.
expect(localHandler.muted_on_cancel).toEqual(false);
expect(localHandler.muted_on_user_path).toEqual(false);
// Push unchanged — remote must be byte-identical.
const pushResult = await backend.runCLICommand(
["sync", "push", "--yes"],
tempDir,
);
expect(pushResult.code).toEqual(0);
const settingsResp = await backend.apiRequest!(
`/api/w/${backend.workspace}/workspaces/get_settings`,
);
expect(settingsResp.ok).toBe(true);
const settings = await settingsResp.json();
expect(settings.error_handler).toEqual({
path: handlerPath,
extra_args: extraArgs,
muted_on_cancel: false,
muted_on_user_path: false,
});
});
},
);
@@ -261,6 +261,11 @@ sync local with a remote instance or the opposite (push or pull)
- `-o, --output-file <file:string>` - Write YAML to a file instead of stdout
- `--show-secrets` - Include sensitive fields (license key, JWT secret) without prompting
- `--instance <instance:string>` - Name of the instance, override the active instance
- `instance connect-slack`
- `--bot-token <bot_token:string>` - Slack bot token (xoxb-...)
- `--team-id <team_id:string>` - Slack team id
- `--team-name <team_name:string>` - Slack team name
- `--instance <instance:string>` - Instance profile to connect against (defaults to the active instance)
### job
@@ -609,4 +614,9 @@ workspace related commands
- `--exclude <items:string>` - Comma-separated kind:path items to exclude
- `--preserve-on-behalf-of` - Preserve original on_behalf_of/permissioned_as values
- `-y --yes` - Non-interactive mode (deploy without prompts)
- `workspace connect-slack` - Non-interactively connect Slack to the active workspace using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: workspace_settings fields, g/slack group, f/slack_bot folder, and the encrypted bot token variable + resource at f/slack_bot/bot_token.
- `--bot-token <bot_token:string>` - Slack bot token (xoxb-...)
- `--team-id <team_id:string>` - Slack team id
- `--team-name <team_name:string>` - Slack team name
- `workspace disconnect-slack`
+10
View File
@@ -2020,6 +2020,11 @@ sync local with a remote instance or the opposite (push or pull)
- \`-o, --output-file <file:string>\` - Write YAML to a file instead of stdout
- \`--show-secrets\` - Include sensitive fields (license key, JWT secret) without prompting
- \`--instance <instance:string>\` - Name of the instance, override the active instance
- \`instance connect-slack\`
- \`--bot-token <bot_token:string>\` - Slack bot token (xoxb-...)
- \`--team-id <team_id:string>\` - Slack team id
- \`--team-name <team_name:string>\` - Slack team name
- \`--instance <instance:string>\` - Instance profile to connect against (defaults to the active instance)
### job
@@ -2368,6 +2373,11 @@ workspace related commands
- \`--exclude <items:string>\` - Comma-separated kind:path items to exclude
- \`--preserve-on-behalf-of\` - Preserve original on_behalf_of/permissioned_as values
- \`-y --yes\` - Non-interactive mode (deploy without prompts)
- \`workspace connect-slack\` - Non-interactively connect Slack to the active workspace using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: workspace_settings fields, g/slack group, f/slack_bot folder, and the encrypted bot token variable + resource at f/slack_bot/bot_token.
- \`--bot-token <bot_token:string>\` - Slack bot token (xoxb-...)
- \`--team-id <team_id:string>\` - Slack team id
- \`--team-name <team_name:string>\` - Slack team name
- \`workspace disconnect-slack\`
`;
@@ -266,6 +266,11 @@ sync local with a remote instance or the opposite (push or pull)
- `-o, --output-file <file:string>` - Write YAML to a file instead of stdout
- `--show-secrets` - Include sensitive fields (license key, JWT secret) without prompting
- `--instance <instance:string>` - Name of the instance, override the active instance
- `instance connect-slack`
- `--bot-token <bot_token:string>` - Slack bot token (xoxb-...)
- `--team-id <team_id:string>` - Slack team id
- `--team-name <team_name:string>` - Slack team name
- `--instance <instance:string>` - Instance profile to connect against (defaults to the active instance)
### job
@@ -614,4 +619,9 @@ workspace related commands
- `--exclude <items:string>` - Comma-separated kind:path items to exclude
- `--preserve-on-behalf-of` - Preserve original on_behalf_of/permissioned_as values
- `-y --yes` - Non-interactive mode (deploy without prompts)
- `workspace connect-slack` - Non-interactively connect Slack to the active workspace using a pre-minted bot token (xoxb-...). Produces the same artifacts as the UI OAuth flow: workspace_settings fields, g/slack group, f/slack_bot folder, and the encrypted bot token variable + resource at f/slack_bot/bot_token.
- `--bot-token <bot_token:string>` - Slack bot token (xoxb-...)
- `--team-id <team_id:string>` - Slack team id
- `--team-name <team_name:string>` - Slack team name
- `workspace disconnect-slack`