Skip to content

Emit WFWorkflow dict in run() codegen#193

Open
khoivinh wants to merge 1 commit into
electrikmilk:mainfrom
khoivinh:fix-run-workflow-dict
Open

Emit WFWorkflow dict in run() codegen#193
khoivinh wants to merge 1 commit into
electrikmilk:mainfrom
khoivinh:fix-run-workflow-dict

Conversation

@khoivinh

Copy link
Copy Markdown

Summary

The runworkflow action requires a structured WFWorkflow dict to bind the target shortcut. Without it the action silently halts every subsequent action in the workflow.

Cherri previously emitted only WFInput + WFWorkflowName from the DSL-defined run action. This PR moves run to a Go-side definition with an appendParamsFunc that injects the WFWorkflow dict at compile time, sourcing workflowName from the first argument.

Changes

  • actions_std.go: New run entry with appendParamsFunc emitting {workflowIdentifier, workflowName, isSelf}
  • actions/shortcuts.cherri: Remove the now-redundant DSL definition for run
  • tests/zz-run-workflow.cherri: New compile-only test input
  • cherri_test.go: New TestRunWorkflowEmitsWFWorkflowDict asserting the dict appears in the output plist with the expected shape

Test plan

  • go test -run TestRunWorkflowEmitsWFWorkflowDict passes
  • go test -run TestCherriNoSign passes (no regressions across 50+ test files)

🤖 Generated with Claude Code

The runworkflow action requires a structured WFWorkflow dict
(`workflowIdentifier`, `workflowName`, `isSelf`) to bind the target
shortcut. Without it the action silently halts every subsequent action.

Cherri previously emitted only `WFInput` + `WFWorkflowName` from the
DSL-defined `run` action. Moves `run` to a Go-side definition with
an `appendParamsFunc` that injects the WFWorkflow dict at compile
time, sourcing `workflowName` from the first argument.

Adds a compile-only regression test asserting the dict appears in
the output plist with the expected shape.

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

@electrikmilk electrikmilk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this! This is a pretty sore spot and makes me want to focus on improving our Shortcuts actions.

Just a few changes, otherwise this looks good.

Comment thread cherri_test.go
resetParser()
}

func TestRunWorkflowEmitsWFWorkflowDict(t *testing.T) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this test and only keep the Cherri one; it matters more that the raw action compiles correctly.

Comment thread actions_std.go
},
},
// The runtime requires a WFWorkflow dict to bind the target shortcut;
// without it the action silently halts every subsequent action.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't include this elsewhere when adding params, please remove the "what" comment per CONTRIBUTING.md:

Only put comments in code to explain why something does something, not what it is doing. Your code should be readable enough that it is obvious to anyone reading it.

You may have written this using Claude before the more recent updates to the project, where CLAUDE.md or AGENTS.md were added. In the future, make sure they read them, as they explain the quirks of the project and require them to read the CONTRIBUTING document.

Comment thread actions_std.go
Comment on lines +1154 to +1161
var name = getArgValue(args[0]).(string)
return map[string]any{
"WFWorkflow": map[string]any{
"workflowIdentifier": uuid.New().String(),
"isSelf": false,
"workflowName": name,
},
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to account for the fact that Shortcuts allows variables for this parameter.

Suggested change
var name = getArgValue(args[0]).(string)
return map[string]any{
"WFWorkflow": map[string]any{
"workflowIdentifier": uuid.New().String(),
"isSelf": false,
"workflowName": name,
},
}
if args[0].valueType == Variable {
return map[string]any{
"WFWorkflow": argumentValue(args, 0),
}
}
var name = getArgValue(args[0]).(string)
return map[string]any{
"WFWorkflow": map[string]any{
"workflowIdentifier": uuid.New().String(),
"isSelf": false,
"workflowName": name,
},
}

Comment thread actions_std.go
return
},
},
"run": {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please move this action to be next to the runSelf() action for convenience sake?

Comment thread actions_std.go
},
// The runtime requires a WFWorkflow dict to bind the target shortcut;
// without it the action silently halts every subsequent action.
appendParamsFunc: func(args []actionArgument) map[string]any {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please additionally apply this fix to runSelf(). That should probably be unified into a function that returns parameters that they both use similarly to other actions in this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants