Skip to content

docs(uipath-maestro-flow): decompile carries an unbundled version's definition with .definitions([...]) - #3955

Merged
tmatup merged 2 commits into
mainfrom
docs/1128-carried-definitions
Oct 21, 2026
Merged

tmatup merged 2 commits into
mainfrom
docs/1128-carried-definitions

Conversation

@tmatup

@tmatup tmatup commented Oct 14, 2026

Copy link
Copy Markdown
Member

Do not merge before UiPath/builder-sdk#1167 merges. This doc describes that PR's behaviour.

Summary

Short version: UiPath/builder-sdk#1167 lets compile resolve a pinned { version } that the SDK does not bundle, using the definition the .flow file itself carries. Decompile copies that definition into a const, passes it to the new flow(...).definitions([...]) builder method, and keeps the pin. A product-written node such as quick-form@1.6 now compiles unedited and keeps its version even when the author edits it. This PR documents that in references/brownfield.md, on top of #3900 (which documented the interim omit-and-warn behaviour from builder-sdk#1159).

Example: uip maestro flow node add writes uipath.human-in-the-loop.quick-form@1.6, and the SDK bundles 1.0 and 1.1.

#3900 (interim): .step("review1", hitl({...}))                        -> compiles; an EDITED review1 moves to 1.1
this PR:         const quickFormV1_6Definition = { ... };
                 .definitions([quickFormV1_6Definition])
                 .step("review1", hitl({...}), { version: "1.6" })   -> compiles; an edited review1 stays at 1.6

What changed

  • skills/uipath-maestro-flow/references/brownfield.md, decompile warnings list, the unbundled-pin bullet:
    • The omit-and-warn path now applies only when the file holds no definition for the pair. It also names the merge warning (node "<id>" moved <type>@<old> -> <new>) an edited node produces there.
    • A new paragraph covers the normal case (the file carries the definition): no warning, the const + .definitions([...]) shape, and the pin kept through edits.
    • It tells the agent to leave the const and the .definitions([...]) call alone (without them compile refuses the pin), and to validate the merged file, because an edited node's inputs still come from the SDK's own factory.
  • The text follows website/reference/brownfield.md on the SDK branch.
  • The change is in a reference file, not SKILL.md, so the SKILL.md section byte budgets do not change.

Tests

  • pytest tests/tasks/uipath-maestro-flow/_shared: 1416 passed (includes the SKILL.md size-budget test).
  • node scripts/check-skill-links.mjs: all links resolve.
  • python3 scripts/check-skill-verbs.py skills/uipath-maestro-flow: 0 blocking findings.

🤖 Generated with Claude Code
Co-Authored-By: Claude

…efinition with .definitions([...])

builder-sdk#1167 lets compile resolve a pinned { version } the SDK does not
bundle against the definition the .flow file carries. Decompile copies that
definition into a const passed to flow(...).definitions([...]) and keeps the
pin, so an edited node keeps the file's version. The omit-and-warn path
documented in #3900 now applies only when the file holds no definition for
the pair.

🤖 Generated with Claude Code
Co-Authored-By: [Claude](mailto:noreply@anthropic.com)
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @tmatup's task in 1m 54s —— View job


PR Review: docs(uipath-maestro-flow) — carried definitions

  • Gather context (review criteria, full diff, existing comments)
  • Read full changed file for surrounding context
  • Run skill-specific checks (links, cross-refs, secrets)
  • Post review findings

Summary

This PR updates skills/uipath-maestro-flow/references/brownfield.md to document the new .definitions([...]) builder method (from UiPath/builder-sdk#1167). It splits the existing unbundled-version bullet into two sub-cases: (a) file holds no definition → omit-and-warn (the interim path from #3900), and (b) file holds the definition → const + .definitions([...]) preserves the pin. The change is well-scoped to a single reference file.

Change-by-Change Review

1. skills/uipath-maestro-flow/references/brownfield.md (+29/−12)

Severity: OK

The restructuring is clean and correct:

  • Lines 149–157 — The omit-and-warn case is now properly scoped to "and the file holds no definition for it." It also names the merge warning format (node "<id>" moved <type>@<old> -> <new>), which is actionable for agents diagnosing unexpected output. The removed sentence about hand-written pins being "refused by compile" is moved to the new paragraph where it fits better.

  • Lines 159–178 — The new "normal case" paragraph is prescriptive and follows the repo's terse documentation style. The code example (const quickFormV1_6Definition = { ... }; .definitions([...])) shows exactly the shape decompile produces. The final instruction to validate the merged file is a good safety net.

  • Line 153 — The merge warning format node "<id>" moved <type>@<old> -> <new> uses appropriate <PLACEHOLDER> style for the variable parts.

  • No new links introduced; no broken internal references.

  • No secrets, tokens, or personal paths.

  • No cross-skill dependencies.

  • Consistent with surrounding prose style and the token-optimization rules (prescriptive, no filler, actionable).

What's Missing

Nothing identified. The change is a documentation addition to an existing reference file, scoped to exactly the new SDK behavior it describes. The existing brownfield test tasks (e.g., hitl/quality_04_brownfield_insert.yaml) cover the brownfield editing workflow; a test exercising the specific .definitions([...]) round-trip would require the upstream SDK change (builder-sdk#1167) to be available in @uipath/cli, so deferring that is reasonable.

Area Ratings

Area Status Notes
Frontmatter OK Not changed; existing frontmatter unaffected
E2E Tests OK Reference-only change; existing brownfield tasks still valid
Skill Body OK SKILL.md unchanged
References & Assets OK Clean restructure, accurate content, no broken links
Repo Hygiene OK Single-file change, no secrets, no cross-skill deps

Issues for Manual Review

  • SDK dependency gate: The PR description states "Do not merge before UiPath/builder-sdk#1167 merges." Reviewers should verify that dependency is merged before this lands, since the documented .definitions([...]) method won't exist in the SDK without it.
  • Accuracy of the code example: The const quickFormV1_6Definition = { nodeType: "...", version: "1.6", ... } shape and the .definitions([quickFormV1_6Definition]) API should be verified against the actual builder-sdk#1167 implementation by someone with access to that branch.

Conclusion

Clean, well-scoped documentation change. No issues found. The restructuring improves clarity by separating the two cases (definition present vs. absent) and the new content is prescriptive, actionable, and consistent with repo conventions. Approve once the upstream SDK dependency (builder-sdk#1167) is confirmed merged.

@rockymadden rockymadden left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: 🟢 Approved. Text matches builder-sdk#1167 source; only minors remain.

What

  • Narrows the omit-and-warn bullet in references/brownfield.md to files with no carried definition, and names merge's moved warning.
  • Adds the normal case: decompile hoists the carried definition into a const, passes it to .definitions([...]), keeps the pin, and an edited node stays at the file's version.
  • Documents builder-sdk#1167, on top of #3900.

Overall findings

  • minor · Merge gate is prose only. builder-sdk#1167 is still open, and .definitions([...]) plus the node "<id>" moved ... merge warning exist only on its branch (main has neither). Merged early, the skill tells agents the carried case is silent while the shipped SDK still omits the pin and warns. Fix: mark this PR draft until #1167 merges and ships in the SDK the skill installs.

Checked

  • ✅ decompile.ts carriedPairs + .definitions([...]) emission, const naming quickFormV1_6Definition.
  • ✅ merge.ts warning text node "<id>" moved <type>@<old> -> <new>.
  • ✅ Refusal text names .definitions([...]); NODE_VERSION_UNREGISTERED skipped for carried pins.
  • ✅ Matches website/reference/brownfield.md on the SDK branch.
  • ✅ No flavor drift: skill-flavors/studioweb/uipath-maestro-flow/.canonical pins classic, and there is no brownfield.md override.

tl;dr

Accurate against the SDK branch. Hold the merge until builder-sdk#1167 lands. One minor on what to do when validate fails.

Comment thread skills/uipath-maestro-flow/references/brownfield.md Outdated
…s differ

Replace the closing "validate the merged file as usual" with the action:
decompile warns when the carried definition's inputs differ from the
factory's version, and the fix is to edit that node's inputs in the
merged .flow. rawNode is not a substitute yet: a raw node cannot route a
human task's outcome exits in the released SDK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TLSHyUCcjom37GWkboYUVF
@tmatup
tmatup merged commit 656054b into main Oct 21, 2026
47 of 48 checks passed
@tmatup
tmatup deleted the docs/1128-carried-definitions branch October 21, 2026 18:35
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