Flow service generation using flow service language - #1
Conversation
cpoder
left a comment
There was a problem hiding this comment.
Thanks for this — the FSL path is a genuinely nice addition, and the implementation is clean: it builds, passes clippy and rustfmt, follows the house style (snake_case params, the Err(e) => text_result(...) pattern used everywhere else), and all three tools register correctly (I verified 336 → 339 via tools/list). The package_name / ifc_name / flow_name split correctly avoids the package-prefix trap, and surfacing the "don't repost the node via put_node" warning in the tool description rather than only in a code comment is exactly right.
One blocking issue before merge, plus a few minor follow-ups.
Blocking: the deploy scope loses its package tools
In scopes.rs, the new arm is placed above n if n.starts_with("package_") => &["develop", "deploy"]. Match arms evaluate top-down, so the four package tools listed in the new arm never reach the old one and silently lose the deploy scope. Running the code on this branch:
package_create scopes=["fsl-develop","develop"] deploy_allowed=false <-- regression
package_list scopes=["fsl-develop","develop"] deploy_allowed=false <-- regression
package_reload scopes=["fsl-develop","develop"] deploy_allowed=false <-- regression
package_info scopes=["fsl-develop","develop"] deploy_allowed=false <-- regression
package_delete scopes=["develop","deploy"] deploy_allowed=true
package_enable scopes=["develop","deploy"] deploy_allowed=true
package_compile scopes=["develop","deploy"] deploy_allowed=true
So with WM_SCOPES=deploy you can still delete, enable and compile packages, but no longer create, list, reload or inspect them — an incoherent deploy scope, and nothing warns about it at startup. Splitting the arm fixes it:
"dsl_validate" | "fsl_deploy" | "fsl_extract" | "service_invoke" | "node_list"
| "node_get" | "node_delete" | "folder_create" => &["fsl-develop", "develop"],
"package_create" | "package_list" | "package_reload" | "package_info" => {
&["fsl-develop", "develop", "deploy"]
}Minor (happy to take these as follow-ups)
- Stale tool counts —
README.md:59and thedescriptioninCargo.tomlstill say 336 tools (now 339), and the category table has no FSL row. fsl-developis undocumented — it's missing from the README scope list and from theValid scopes:comment inconfig.rs:41, so it's usable but undiscoverable.- No RAG resource for FSL — this is the one I'd most like to see land eventually. The 5 existing resources are all putNode / flow-tree oriented, so an agent handed
fsl_deployhas no FSL grammar reference and will guess the syntax — the exact failure mode the putNode docs exist to prevent.dsl_validatelets it iterate, which softens the blow, but a short FSL reference resource would be the natural companion to this PR. is_readonly_tool—dsl_validateandfsl_extractare side-effect-free but aren't listed, so they're hidden underWM_SCOPES=readonly, even thoughservice_invokeis listed there.
Only the scope split is blocking; the rest can follow.
|
All the reported issues should now be fixed. |
Follow-up to the merge of main, pushed by the maintainer so the PR can go green on its own. `every_scope_in_use_is_documented` failed on `fsl-develop`: the scope was already added to the README list and the AppConfig::scopes doc comment, but DOCUMENTED_SCOPES did not exist on this branch's base, so there was no way to know about the third place the test requires. The README also still advertised 5 RAG resources. It was already stale at 6 before this PR (fb8a730 added the on-prem provisioning resource without updating the count), and the new FSL language reference makes 7. Verified against a resources/list call on the built binary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLxwTsE7P4RmNskk9LuV9L
cpoder
left a comment
There was a problem hiding this comment.
All the review points are addressed — thanks for the thorough follow-up.
The blocking scope regression is fixed exactly as suggested, and the deploy scope invariant now passes. You also picked up all four minor points, including the one I had flagged as the most valuable follow-up: the FSL language reference resource. That was the missing companion to fsl_deploy — an agent now has a grammar to work from instead of guessing.
Two things I pushed to the branch myself rather than sending you round again, since neither was discoverable from your base:
DOCUMENTED_SCOPES— you correctly addedfsl-developto the README scope list and theAppConfig::scopesdoc comment, but a test landed onmainafter you branched that asserts every scope in use appears in a third place too. It did not exist on your base, so there was no way to know.- RAG resource count — the README said 5. It was already stale at 6 before this PR (fb8a730 added the on-prem provisioning resource without updating the count), and your FSL reference makes 7. Verified against a
resources/listcall on the built binary.
I merged main into the branch to bring those tests in. Full suite on the result: fmt, clippy --all-targets -D warnings, cargo audit all clean, 15/15 tests passing, and CI is green on all four jobs.
Note the branch has two commits from me on top of yours — git pull before you push again.
No description provided.