-
Notifications
You must be signed in to change notification settings - Fork 26
fix(block-kit): resolve CLI before validation #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "slack": patch | ||
| --- | ||
|
|
||
| Make Block Kit validation resolve and use the Slack CLI before falling back to curl. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,22 @@ Never use a block type, element, or field you have not seen on a live page. | |
|
|
||
| --- | ||
|
|
||
| ## Resolve Tooling | ||
|
|
||
| Before choosing the Fast Path, Modification Mode, or full workflow, use the | ||
| `slack:slack-cli` skill, **Step 1: Detect the Slack CLI**, to resolve the public | ||
| Slack CLI command once. Keep the result as either `SLACK_CMD` or an explicit | ||
| reason the CLI is unavailable. This is a capability check only; do not | ||
| authenticate, validate, or write Slack CLI configuration during this preflight. | ||
| For Block Kit validation, stop after the standard-path and `PATH` probes in | ||
| that detection step. If neither resolves the CLI, record it as unavailable and | ||
| use curl; do not propose installation or ask about an alias unless the developer | ||
| independently asked to configure the Slack CLI. | ||
|
|
||
| All later validation and preview steps must reuse this result. | ||
|
|
||
| --- | ||
|
|
||
| ## Fast Path (for clear, specific requests) | ||
|
|
||
| If the developer's request is specific enough to determine both the target surface and the desired layout, collapse Steps 1-4 into a single pass: | ||
|
|
@@ -178,15 +194,40 @@ The authoritative reference for this method (its parameters, auth requirements, | |
|
|
||
| ### 5a. Build the validation request | ||
|
|
||
| Prefer the Slack CLI when it's available, since it reuses the slack-cli skill's CLI detection and needs no token wrangling. If the CLI isn't installed, fall back to curl. Both call the same public method and return the same response, so Step 5b applies either way. | ||
| Use the tooling result resolved before layout work. If `SLACK_CMD` resolved, you | ||
| MUST attempt validation with the Slack CLI. Use curl only when the CLI was not | ||
| found or its API command cannot run after the handling below. A Slack API | ||
| validation response with `"ok": false` means the CLI ran successfully; fix the | ||
| payload and retry with the CLI rather than switching transports. | ||
|
|
||
| **Path A: Slack CLI (preferred).** | ||
|
|
||
| Use the `slack:slack-cli` skill, **Step 1: Detect the Slack CLI**, to check whether the public CLI is installed and resolve its command (`SLACK_CMD`). | ||
| Use the `slack:slack-cli` skill, **Step 4: Calling Web API Methods (`slack api`)**, | ||
| for the argument-passing contract. The canonical `blocks.validate` invocation | ||
| below is already known: attempt it directly without running `api --help`, | ||
| `--version`, or generic help first. | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the narrow exception to generic help-driven command discovery: |
||
|
|
||
| If the CLI is available, use the `slack:slack-cli` skill, **Step 4: Calling Web API Methods (`slack api`)**, to invoke it. That step covers the `SLACK_CMD api <method> key=value …` syntax. Run `SLACK_CMD api --help` first to confirm the syntax **and the flag that skips authentication**. `blocks.validate` needs no token, so call it without authentication. Don't hard-code that flag from memory; read it from the help output so this stays correct if it's ever renamed. Pass the payload as a positional `key=value` argument: `blocks=<JSON array>` for messages, or `view=<JSON view object>` for modals and home tabs. | ||
| ```bash | ||
| $SLACK_CMD api blocks.validate --no-auth 'blocks=[...]' | ||
| $SLACK_CMD api blocks.validate --no-auth 'view={...}' | ||
| ``` | ||
|
|
||
| **Path B: curl (fallback, when the CLI isn't installed).** | ||
| If the command cannot start because the host blocks writes to the Slack CLI's | ||
| config or log path, ask for the host's normal narrowly scoped permission and | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fallback boundary received the closest security review. A config/log denial may retry the identical command only through normal scoped host approval; if approval is denied, validation stops rather than using curl to route around the operator's decision. Semantic API and shared network failures likewise cannot be hidden by changing transport. |
||
| retry the identical command. Never activate a permission bypass. If permission | ||
| is denied or unavailable, preserve the error, report that validation did not | ||
| run, and stop; do not use another transport to evade that boundary. If the | ||
| canonical attempt reports an incompatible CLI, record its version and | ||
| diagnostic help output before deciding that the API command cannot execute; do | ||
| not silently invent a different CLI invocation. For another non-permission | ||
| execution failure, use curl only as an explicitly disclosed fallback. | ||
|
|
||
| **Path B: curl (fallback).** | ||
|
|
||
| Use curl only when the resolved preflight state says the CLI is absent, or the | ||
| CLI call cannot execute after the policy-compliant handling above. Do not use | ||
| curl to hide payload errors, Slack API errors, or a network/service failure that | ||
| would affect either transport. Retain the exact fallback reason for Step 6. | ||
|
|
||
| POST to the endpoint with the **Bash tool**. The API uses form-urlencoded encoding, so pass the JSON directly as the parameter value. | ||
|
|
||
|
|
@@ -241,7 +282,17 @@ When validation fails: | |
|
|
||
| ## Step 6: Deliver the Final Output | ||
|
|
||
| Present the validated payload, then help the developer put it to use. | ||
| Present the validated payload, then help the developer put it to use. State | ||
| whether validation ran through the Slack CLI or curl. If it ran through curl, | ||
| state the recorded reason the CLI path was unavailable or unable to execute. | ||
|
|
||
| Use one of these stable disclosures: | ||
|
|
||
| - `Validation transport: Slack CLI (<resolved command>).` | ||
| - `Validation transport: curl (Slack CLI unavailable: <reason>).` | ||
|
|
||
| If validation did not succeed, state `Validation status: not validated | ||
| (<reason>).` Never imply success from a transport attempt alone. | ||
|
|
||
| ### Send it | ||
|
|
||
|
|
@@ -253,13 +304,14 @@ Help the developer view their layout with the **Block Kit Builder**. Prefer the | |
|
|
||
| **Path A: Slack CLI (preferred).** | ||
|
|
||
| Use the `slack:slack-cli` skill, **Step 1: Detect the Slack CLI**, to check whether the public CLI is installed and resolve its command (`SLACK_CMD`). | ||
|
|
||
| If the CLI is available, run `SLACK_CMD blocks preview --help` to see how to pass the blocks and open the preview. The command loads the blocks into the Block Kit Builder in the developer's browser. | ||
| Reuse the `SLACK_CMD` recorded in **Resolve Tooling**. If it is available, run | ||
| `SLACK_CMD blocks preview --help` to see how to pass the blocks and open the | ||
| preview. The command loads the blocks into the Block Kit Builder in the | ||
| developer's browser. | ||
|
|
||
| Because you run the CLI non-interactively, this command also needs a `--team` flag. Resolve the team ID with the `slack:slack-cli` skill, **Step 2: Command Discovery via Help**, whose "Resolving `--app` and `--team` values" guidance covers running `SLACK_CMD auth list`; Any authenticated workspace works for a preview, if several are available, pick one and mention which you used rather than blocking on the choice. | ||
|
|
||
| **Path B: Block Kit Builder link (fallback, when the CLI isn't installed).** | ||
| **Path B: Block Kit Builder link (fallback, when Resolve Tooling recorded the CLI as unavailable).** | ||
|
|
||
| Offer the Block Kit Builder link so the developer can paste the JSON in and tweak visually: `https://app.slack.com/block-kit-builder`. Builder needs an object (`{ "blocks": [...] }` or a full view object), not a bare array. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| from pathlib import Path | ||
|
|
||
| SKILL = Path("skills/block-kit/SKILL.md").read_text() | ||
|
|
||
|
|
||
| def test_tooling_is_resolved_before_every_workflow_path() -> None: | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These focused tests intentionally assert durable policy invariants rather than transcript wording: preflight ordering/reuse, canonical calls, semantic same-transport retry, permission non-bypass, and final disclosure. Native agent runs remain supporting behavioral evidence, not unit tests. |
||
| resolve_tooling = SKILL.index("## Resolve Tooling") | ||
| fast_path = SKILL.index("## Fast Path") | ||
| modification_mode = SKILL.index("## Modification Mode") | ||
| validate = SKILL.index("## Step 5: Validate") | ||
| assert resolve_tooling < fast_path < modification_mode < validate | ||
| assert "`slack:slack-cli` skill, **Step 1: Detect the Slack CLI**" in SKILL | ||
| assert SKILL.count("**Step 1: Detect the Slack CLI**") == 1 | ||
| assert "do not propose installation or ask about an alias" in SKILL | ||
| assert "Reuse the `SLACK_CMD` recorded in **Resolve Tooling**" in SKILL | ||
|
|
||
|
|
||
| def test_cli_validation_contract_is_explicit() -> None: | ||
| assert "MUST attempt validation with the Slack CLI" in SKILL | ||
| assert "$SLACK_CMD api blocks.validate --no-auth 'blocks=[...]'" in SKILL | ||
| assert "$SLACK_CMD api blocks.validate --no-auth 'view={...}'" in SKILL | ||
| assert "attempt it directly without running `api --help`" in SKILL | ||
| assert 'validation response with `"ok": false`' in SKILL | ||
| assert "retry with the CLI rather than switching transports" in SKILL | ||
|
|
||
|
|
||
| def test_permission_retry_does_not_bypass_host_policy() -> None: | ||
| assert "retry the identical command" in SKILL | ||
| assert "Never activate a permission bypass" in SKILL | ||
| assert "do not use another transport to evade that boundary" in SKILL | ||
|
|
||
|
|
||
| def test_fallback_and_transport_are_disclosed() -> None: | ||
| assert "Retain the exact fallback reason for Step 6" in SKILL | ||
| assert "Validation transport: Slack CLI (<resolved command>)." in SKILL | ||
| assert "Validation transport: curl (Slack CLI unavailable: <reason>)." in SKILL | ||
| assert "Validation status: not validated" in SKILL | ||
| assert "Do not use\ncurl to hide payload errors" in SKILL | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preflight is deliberately capability-only and runs before every workflow path. Recording
SLACK_CMDonce avoids the original late-discovery failure and prevents validation and preview from repeating detection or drifting into the CLI skill's install/alias branch.