Skip to content

Improve Maestro BPMN wrapper and packaging guidance#840

Merged
gozhang2 merged 8 commits into
mainfrom
review-pr-676-maestro-bpmn-wrapper-guidance
May 19, 2026
Merged

Improve Maestro BPMN wrapper and packaging guidance#840
gozhang2 merged 8 commits into
mainfrom
review-pr-676-maestro-bpmn-wrapper-guidance

Conversation

@gozhang2

Copy link
Copy Markdown
Contributor

Summary

  • Add canonical Maestro BPMN wrapper-shell guidance for namespace, nested uipath:type, sibling uipath:mapping payload movement, parse-safe XML comments, and draft Intsvc shells.
  • Document plain connectionless HTTP recipes and clarify Integration Service vs CLI-owned enrichment boundaries.
  • Clarify local package metadata requirements before pack, including minimal placeholder-safe metadata shape and a validate-file-path eval update.

Validation

  • git diff --check
  • bash skills/uipath-maestro-bpmn/.maintenance/check-all.sh
  • coder-eval run tasks/uipath-maestro-bpmn/authoring/api_workflow_task.yaml tasks/uipath-maestro-bpmn/authoring/business_rule_task.yaml -e experiments/e2e.yaml -j 1 --run-dir runs/2026-05-18_pr-wrapper-guidance-api-business - API passed; business timed out in the combined run.
  • coder-eval run tasks/uipath-maestro-bpmn/authoring/business_rule_task.yaml -e experiments/e2e.yaml -j 1 --run-dir runs/2026-05-18_pr-wrapper-guidance-business-rerun
  • coder-eval run tasks/uipath-maestro-bpmn/connector/integration_service_boundary.yaml -e experiments/integration.yaml -j 1 --run-dir runs/2026-05-18_pr-wrapper-guidance-integration-boundary-rerun
  • coder-eval run tasks/uipath-maestro-bpmn/e2e/author_validate_pack.yaml -e experiments/e2e.yaml -j 1 --run-dir runs/2026-05-18_pr-wrapper-guidance-author-validate-pack-rerun - inconclusive: agent timed out before a useful result.

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @gozhang2's task in 7m 16s —— View job


PR Review: Improve Maestro BPMN wrapper and packaging guidance

  • Read review criteria and project conventions
  • Read full diff and existing PR conversation
  • Read all changed files in full for context
  • Verify relative links resolve
  • Check for cross-skill references, secrets, and conventions
  • Post review

Summary

This PR standardizes all wrapper shells to use nested uipath:type (replacing legacy type="..." shorthand) and moves variable payload inputs/outputs from inside uipath:activity to sibling uipath:mapping version="v1" elements. It adds a new HTTP request recipe for plain connectionless HTTP, documents minimal placeholder-safe metadata shapes for local packaging, and updates the validate test command to target the .bpmn file path instead of the project directory. The refactoring is consistent across 20 files and touches every documented wrapper.

Change-by-Change Review

1. references/shared/wrapper-shells.md (+112/-33)

Severity: OK

Core architectural change. All shells now separate uipath:activity (identity + context) from uipath:mapping (payload I/O). Adds namespace baseline, XML comment parse-safety rule, legacy shorthand warning, and a new Intsvc.ActivityExecution draft shell. The Intsvc.WaitForEvent shell list correctly drops ActivityExecution now that it has its own section. Clean and internally consistent.

2. references/author/references/task-recipes/http-request.md (NEW, +105)

Severity: Medium

Well-structured recipe with clear scope guards, anti-patterns, verification steps, and status language. Two issues:

  • uipath:output inside uipath:activity (line 66) — The XML shape keeps <uipath:output name="response" ... /> inside uipath:activity, contradicting the PR's central pattern of moving payload I/O to sibling uipath:mapping. Every other shell in wrapper-shells.md uses uipath:mapping for outputs. An agent following both files will get conflicting guidance. Fix this →

  • No uipath:mapping usage at all — If the HTTP recipe intentionally keeps output inside uipath:activity because HTTP context fields and response are all activity-intrinsic (not variable payload movement), this should be called out explicitly with a brief note explaining why it differs from the sibling uipath:mapping pattern used by other wrappers. Otherwise agents will apply the wrong pattern or question the recipe.

3. references/shared/local-metadata-regeneration-guide.md (+75/-4)

Severity: OK

Adds a complete "Minimal Local Metadata Shape" section with placeholder-safe JSON for all five required metadata files. The pre-pack check (step 3) is a valuable addition — the previous version did not warn that pack consumes but does not synthesize package-descriptor.json. Step numbering correctly updated. The contentFiles anti-pattern callout is helpful.

4. references/shared/bpmn-xml-contract.md (+32/-2)

Severity: OK

Adds namespace declaration warning (no schemas.uipath.com/workflow/activities), XML comment parse-safety (-- forbidden), canonical nested uipath:type element guidance, and the plain HTTP boundary as a documented exception. Pass 2 description now references wrapper-shells.md instead of listing shell categories inline. All links resolve.

5. references/author/references/plugins/integration-service/impl.md (+43/-59)

Severity: Low

Net reduction of 16 lines. The README template removal is appropriate — it was prescriptive scaffolding that could drift. The blocker checklist (items 1-8) remains. Draft shell changed from bpmn:serviceTask to bpmn:sendTask for Intsvc.ActivityExecution, consistent with wrapper-shells.md and supported-elements.md. Language tightened from "Integration Service" to "connector-backed or dynamically schematized" throughout.

Minor: The em-dash to hyphen normalization (-) in section headers is a style-only change and appears in several files. Consistent, no issue.

6. references/author/references/plugins/multi-instance/impl.md (+21/-0)

Severity: OK

Adds item binding pattern guidance and a generic marker subprocess XML snippet. The iterator[0] pattern and the =currentItem anti-pattern warning are valuable — these prevent a real agent mistake. The XML snippet is a fragment (not a full shell) which is appropriate for a loop characteristic.

7. Plugin impl files: api-workflows/impl.md, business-rules/impl.md, hitl/impl.md, rpa-jobs/impl.md (+36/-8 combined)

Severity: OK

All four files add a canonical shell pointer to wrapper-shells.md, warn against legacy shorthand, and update "Input CDATA" / "Output mappings" to reference uipath:mapping. Changes are uniform and consistent.

8. references/author/references/plugins/integration-service/planning.md (+6/-2)

Severity: OK

Adds HTTP recipe routing ("before deciding whether the node is executable or only draft intent") and updates step 2 to distinguish connector activity, connector HTTP, and plain connectionless HTTP. The draft shell description now explicitly allows uipath:type value="Intsvc.<Variant>" with placeholder strings.

9. references/author/references/supported-elements.md (+18/-3)

Severity: OK

Intsvc.HttpExecution ownership column updated to note the plain HTTP exception with a link to http-request.md. Integration Service boundary section updated with draft shell guidance. Legacy shorthand warning added to UiPath extension coverage intro.

10. references/author/references/planning-arch.md (+4/-0) and planning-impl.md (+9/-6)

Severity: OK

Planning arch adds HTTP recipe routing to the enrichment boundary section. Planning impl updates fill order step 5 to include HTTP recipe, updates shell guidance to use uipath:context inside uipath:activity and uipath:mapping for payloads. Language is precise and consistent.

11. references/author/references/validation.md (+24/-0)

Severity: OK

Adds pre-pack metadata check and "Implementation status summary" section with Executable/Draft/Mock/Blocked classification. The status language matches http-request.md. Good addition — prevents agents from calling a process "fully implemented" when draft nodes remain.

12. references/author/references/task-recipes/README.md (+1/-0)

Severity: OK

Adds the HTTP request row to the recipe index table. Correctly shows bpmn:sendTask wrapper.

13. references/diagnose/references/troubleshooting-guide.md (+6/-0) and references/operate/references/run.md (+4/-0)

Severity: OK

Diagnose adds guidance to inspect variables even on Completed runs (semantic correctness vs control-flow completion). Operate adds business result reporting guidance. Both are concise behavioral additions.

14. references/shared/project-layout.md (+5/-0)

Severity: OK

Documents that pack consumes package-descriptor.json and won't create it. Consistent with the local-metadata-regeneration-guide addition.

15. tests/tasks/uipath-maestro-bpmn/e2e/author_validate_pack.yaml (+3/-3)

Severity: OK

Changes validate command from PurchaseApproval (directory) to PurchaseApproval/PurchaseApproval.bpmn (file path). The initial_prompt, command_pattern, and run_command are all updated consistently. The command_pattern correctly escapes the / and ..

What's Missing

  • HTTP recipe XML shape inconsistency — The uipath:output in http-request.md:66 should either be moved to uipath:mapping (matching the PR's own convention) or the file should include a brief note explaining why HTTP output intentionally stays inside uipath:activity. Without this, the PR introduces conflicting guidance.
  • No HTTP wrapper shell in wrapper-shells.md — The PR adds the HTTP recipe as a separate task-recipes file, but wrapper-shells.md (the canonical shell reference) does not include an Intsvc.HttpExecution connectionless shell. Other wrappers have entries in both places. An agent looking only at wrapper-shells.md during pass 2 won't find the HTTP shape. Consider adding a minimal shell or a redirect note.

Area Ratings

Area Status Notes
Frontmatter OK No frontmatter changes; existing SKILL.md not modified
E2E Tests OK author_validate_pack.yaml updated with corrected validate file path
Skill Body OK No SKILL.md body changes
References & Assets Issue HTTP recipe output placement contradicts sibling uipath:mapping pattern; missing shell in wrapper-shells.md
Repo Hygiene OK CODEOWNERS present, no secrets, no cross-skill refs, all links resolve, changes scoped to one skill

Issues for Manual Review

  • HTTP uipath:output placement — Verify whether the runtime actually expects uipath:output inside uipath:activity for Intsvc.HttpExecution (unlike all other wrappers), or whether it should be in uipath:mapping. If the runtime requires it inside the activity, the recipe is correct and needs an explanatory note; if not, it should be moved.
  • Intsvc.HttpExecution vs Intsvc.UnifiedHttpRequest — The recipe uses Intsvc.HttpExecution; confirm this is the correct service type for the connectionless manual-mode HTTP pattern (vs UnifiedHttpRequest which remains CLI-only).
  • Test task validate path change — The switch from directory to .bpmn file path in the validate command should be confirmed against the current CLI behavior (uip maestro bpmn validate). The PR notes the rerun was "inconclusive: agent timed out."

Conclusion

This is a well-structured documentation refactoring that establishes a clearer, more consistent wrapper pattern across the skill. The sibling uipath:mapping convention, namespace baseline, and legacy shorthand warnings are all valuable additions that will reduce agent mistakes.

Request one change before merge: Resolve the uipath:output placement inconsistency in http-request.md — either move it to uipath:mapping to match the rest of the PR, or add an explicit note explaining why HTTP output lives inside uipath:activity. This is the only place in the PR where the new convention is not followed, and conflicting XML shapes in different reference files will confuse agents.

The remaining items (missing wrapper-shells entry for HTTP, test task verification) are improvements that can land in a follow-up.

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @gozhang2's task in 2m 8s —— View job


Coder-eval task lint (advisory)

1 task YAML changed; verdicts: 0 Critical, 1 High, 0 Medium, 0 Low, 1 OK.

Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.

Evidence of passing run

High — The changed task author_validate_pack.yaml was run but PR body reports "inconclusive: agent timed out before a useful result." This does not constitute a passing-run claim. Please edit the PR description to add a line like:

Ran skill-bpmn-author-validate-pack locally and it passed.

once a successful run is confirmed. (The other coder-eval run invocations in the PR body cover different tasks, not this one.)

Per-task lint

tests/tasks/uipath-maestro-bpmn/e2e/author_validate_pack.yaml — verdict: OK

The change updates the validate command from directory-level (validate PurchaseApproval) to file-level (validate PurchaseApproval/PurchaseApproval.bpmn) across the prompt, command_executed pattern, and run_command — a targeted correctness fix applied consistently in all three locations.

No rubric issues found:

  • Self-report anti-pattern: Not present. Criteria verify real CLI side-effects (run_command for validate/pack, find for .nupkg), not agent-authored summaries.
  • Prompt over-specification: The prompt gives exact CLI commands, but these are the test's goal (validate + pack), not the skill's procedure (BPMN authoring). Node names like "Prepare approval summary" are ground-truth anchors checked by file_contains. No issue.
  • Meaningful coverage: Strong mix — file_contains for BPMN structure, command_executed for agent behavior, and three run_command criteria that actually execute validate, pack, and artifact existence checks against the generated project. Well-designed.
  • Could pass for the wrong reason: No. The run_command criteria run the real CLI against the authored BPMN — an invalid project fails validate, a malformed project fails pack.
  • Near-duplicate: Sibling invoice_exception_triage.yaml also authors a BPMN project but tests structural correctness via a Python script; this task tests the validate+pack lifecycle. Materially distinct operations sharing authoring scaffold — good template reuse per rubric.
  • Validate-only flow tests: Not applicable (tags are uipath-maestro-bpmn, not uipath-maestro-flow).
  • Redundant uip CLI in sandbox: Not present. Sandbox has only python: {}.

Within-PR duplicates

Not applicable — only 1 task YAML changed.

Conclusion

⚠ Task itself passes the rubric (OK), but evidence of a passing run is missing for the changed task (High, advisory). Consider re-running skill-bpmn-author-validate-pack and updating the PR description once it passes.


@gozhang2 gozhang2 merged commit 167dd61 into main May 19, 2026
10 checks passed
@gozhang2 gozhang2 deleted the review-pr-676-maestro-bpmn-wrapper-guidance branch May 19, 2026 05:17
charlesliu9 pushed a commit that referenced this pull request May 19, 2026
* Improve Maestro BPMN wrapper guidance

* Clarify Maestro HTTP modeling boundary

* Reinforce Maestro two-pass authoring boundary

* Centralize Maestro task recipe boundary

* Improve Maestro BPMN dogfood guidance

* Exclude unsupported Maestro BPMN generation

* Improve Maestro BPMN expression validation

* Avoid extra draft handoff project files
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.

3 participants