add /provision skill#77
Conversation
Adds the `/provision` command that automates end-to-end Power Platform environment provisioning with ESS base + Workday ISV extension. ## What it does - Creates or binds a PP environment (PAC CLI with capacity pre-checks) - Installs ESS base + Workday ISV via `pac application install` - Creates Workday SOAP + Dataverse connections (Connectivity API) - Binds connection references to connections (Dataverse PATCH) - Enables flows and outputs manual CPS wiring instructions - Configures User Context Setup topic redirect - Validates readiness via FlightCheck
|
@is-goutham please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
nehaoss
left a comment
There was a problem hiding this comment.
Solid work overall. Shell injection defenses (regex allowlists), OData injection guards, HTTPS validation on token-bearing requests, and token cache permission handling are all correctly implemented. Found two issues worth addressing.
| if any(p.lower() in name.lower() for p in patterns): | ||
| matched.append(f) | ||
| if prefix: | ||
| if any(name.startswith(p) for p in patterns): |
There was a problem hiding this comment.
Case-sensitive prefix matching for Workday flows
The prefix=True path uses name.startswith(p) which is case-sensitive, but the prefix=False path uses case-insensitive matching (p.lower() in name.lower()). The patterns are ('ESS HR Workday', 'ESS IT Workday', 'WorkdayRESTExecution').
If the Power Platform API ever returns a display name with different casing (e.g., 'ESS HR WORKDAY...'), the prefix match silently fails and those flows are not detected, causing flightcheck to incorrectly report 'No Workday flows found' (WD-001 -> NOT_CONFIGURED).
Suggestion: Normalize both sides: if any(name.lower().startswith(p.lower()) for p in patterns):
| resp = _SESSION.get(url, headers=self.headers, params=params, timeout=60) | ||
| if resp.status_code in (401, 403): | ||
| return items | ||
| return {"_error": "insufficient_permissions", "_status": resp.status_code} |
There was a problem hiding this comment.
_get_all silently discards accumulated data on mid-pagination auth failure
The previous implementation returned the partially-accumulated items list on 401/403 during pagination. The new implementation returns {'_error': ...} instead, discarding any successfully-fetched pages.
If a token expires mid-pagination (e.g., page 1 of get_flows succeeds with 250 flows, page 2 returns 401), all data from page 1 is silently dropped. For environments with hundreds of flows, this could cause confusing false failures.
Suggestion: Return the error dict only when items is empty (first-page failure = real permission problem). When items is non-empty, either return the partial list with a warning, or include the partial data in the error dict: {'_error': 'insufficient_permissions', '_partial': items}.
nehaoss
left a comment
There was a problem hiding this comment.
Large feature PR (3448 additions, 24 files) adding a /provision skill. High-level observations:
-
Scope: This adds end-to-end environment provisioning - creating Power Platform environments, connections, flow bindings, and user-context topic wiring. That's a significant new capability. The 4-step structure (step1-step4 + tasks.md) follows the established pattern from /connect.
-
New scripts: 6 new Python scripts (create_env.py, create_connection.py, bind_connection_refs.py, wire_flow_bindings.py, update_user_context_topic.py, whoami.py, pp_helpers.py). These should have corresponding unit tests - I don't see test files in this PR.
-
pp_helpers.py (286 lines): This appears to be a shared utility module for Power Platform API operations. Good separation of concerns. Check for overlap with existing pp_admin_client.py in flightcheck - ensure no duplicate functions per the repo's code quality rules.
-
Auth changes (auth.py): The diff shows 13 additions / 12 deletions - verify the refactoring doesn't break existing scripts that import from auth.py.
-
FlightCheck integration: Changes to cli.py (75+/26-), environment.py, external_systems.py, workday.py, pp_admin_client.py, and runner.py suggest the provision skill also wires into FlightCheck. Ensure these changes don't regress existing check behavior.
-
Missing tests: A PR of this size adding new API-calling scripts should include unit tests with mocked responses. Consider adding tests before merge.
Adds the
/provisioncommand that automates end-to-end Power Platformenvironment provisioning with ESS base + Workday ISV extension.
What it does
pac application installDescription
Introduces 7 new Python scripts and 6 skill docs that together automate provisioning a Power Platform environment with the ESS agent + Workday ISV. The skill follows the same AI-as-orchestrator pattern used by
/connectand/onboarding, markdown step files guide the agent, Python scripts handle the API calls. Flow runtime binding requires a manual Copilot Studio step (PVA API needs legacy permissions not available to custom Entra apps).Design is ISV-extensible for ServiceNow/SAP in future PRs.
Related issue
Refs #
Type of change
functionality to not work as expected)
Testing
End-to-end tested against Preprod ring.
Final run verified:
env creation, ESS + Workday ISV install, connection creation (Workday SOAP + Dataverse), connection ref binding, flow enablement, topic redirect patch and FlightCheck validation, all passing. Manual CPS wiring confirmed functional post-script.
Checklist