feat(uipath-governance): merge gov-aops-policy + gov-access-policy into single uber skill#601
feat(uipath-governance): merge gov-aops-policy + gov-access-policy into single uber skill#601jianjunwang2 wants to merge 1 commit intomainfrom
Conversation
jianjunwang2
requested review from
bansal-anushree and
sriramva-uipath
as code owners
PR Review: feat(uipath-governance) — merge gov-aops-policy + gov-access-policy into single uber skill
|
|
Claude finished @jianjunwang2's task in 3m 49s —— View job Coder-eval task lint (advisory)7 task YAMLs changed (all renames with skill-name rewrites from Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.
Evidence of passing run❌ High — PR body does not claim the changed tasks have been run and passed. The "Validation" section covers description and link validation only; the "Test plan" checkboxes are unchecked TODO items, not passing-run claims. Please edit the PR description to add a line like: Per-task lint
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ea7cb1b8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude finished @jianjunwang2's task in 3m 31s —— View job PR Review: feat(uipath-governance) — merge gov-aops-policy + gov-access-policy
SummaryReplaces two sibling governance skills ( Change-by-Change Review1.
|
| Area | Status | Notes |
|---|---|---|
| Frontmatter | OK | Valid YAML, name matches folder, description 549 chars, brand-leading, → redirect present |
| E2E Tests | Medium | 7 smoke tasks present and correctly rewritten; no e2e tasks (pre-existing debt) |
| Skill Body | OK | Canonical section order, 6 critical rules, disambiguation workflow, anti-patterns |
| References & Assets | OK | All 24 files reachable from SKILL.md, all relative links resolve, no orphaned files, no cross-skill refs |
| Repo Hygiene | OK | CODEOWNERS updated, no secrets, no binaries, changes scoped to governance skill |
Issues for Manual Review
- Disambiguation accuracy. The strong-signal tables in
disambiguation-guide.mdand the phrasing-to-branch mappings are domain-specific. I verified structural correctness (links, formatting, completeness) but cannot confirm whether the signal heuristics correctly classify real-world governance prompts. The test plan's manual check items (canonical disambiguation prompt, unambiguous AOps/Access prompts) should cover this. - Workflow Step 1 vs Critical Rule feat(EvalsBreakdown): break down eval skill #1 ordering (detailed above). Low severity, but worth aligning for agent clarity.
- CLI command correctness. All
uip gov aops-policyanduip gov access-policycommands are inherited from the prior skills. I verified syntax consistency but cannot confirm they match the current CLI API surface.
Conclusion
Clean, well-structured merge. The new uber skill solves the real non-deterministic activation problem with a clear classify-then-route pattern. SKILL.md is lean (91 lines), description is tight (549 chars), all links resolve, CODEOWNERS is updated, and no cross-skill references or secrets are present. The only gap is the missing e2e test coverage, which is pre-existing debt from the prior skills.
Recommendation: approve. One Low suggestion to align Critical Rule #1 wording with Workflow Step 1 ordering, and a Medium note to add e2e test coverage in a follow-up.
uipreliga
left a comment
uipreliga
left a comment
There was a problem hiding this comment.
Can you remove the part 'use skill X' from tests - that's too easy and it results in not much being tested.
…eport tests, classify-before-CLI Three review-comment fixes: 1. Test tasks no longer instruct the agent to "Use the uipath-governance skill" (uipreliga, CHANGES_REQUESTED). The skill must self-activate from prompt context — naming it explicitly defeats the activation test. 2. Replace `report.json` self-narration pattern with actual CLI output capture to `output.txt` (uipreliga + lint-task advisory). Previously tasks asked the agent to write a self-narrated summary of the commands it claimed to have run; that was effectively self-grading. Now each task instructs the agent to redirect stdout+stderr of every `uip` invocation to ./output.txt via `> output.txt 2>&1` and `>> output.txt 2>&1`. `command_executed` criteria (which inspect the framework's record of actual Bash tool calls — objective evidence) remain as the primary signal; `file_exists ./output.txt` confirms the agent ran *something*. Dropped `file_contains` includes/excludes and `json_check` assertions that read the agent's own narrative. 3. SKILL.md Workflow steps reordered so classification is Step 1 and `uip` verify/login is Step 2 (Codex P2). Critical Rule #1 says "First action is to classify intent" — running `which uip` before classification contradicted that, especially when the request would redirect to a sibling skill anyway. The login check now happens only after classification routes to a governance branch. Added an explicit "Do not run any CLI command before classification is settled" qualifier to Step 1. Validation: 354 markdown links resolve, hooks/validate-skill-descriptions.sh exit 0, 8 files changed (+56/-309 lines — net reduction comes from deleting the per-task self-narration block). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
jianjunwang2
force-pushed
the
feat/uipath-governance-merge
branch
from
9ea7cb1 to
1232988
Compare
|
Claude finished @jianjunwang2's task in 3m 25s —— View job Coder-eval task lint (advisory)7 task YAMLs changed (all renames with content rewrites from Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.
Evidence of passing run❌ High — PR body does not claim the changed tasks have been run and passed. The "Validation" section covers description and link validation only; the "Test plan" checkboxes are unchecked TODO items. Please edit the PR description to add a line like: Per-task lint
|
…to single uber skill
Replaces the two sibling governance skills (uipath-gov-aops-policy,
uipath-gov-access-policy) with a single user-shaped uipath-governance
skill. The two old skills front-loaded the same verbs (block, restrict,
deny) and produced non-deterministic activation on prompts that fit
either layer (e.g. "Block ChatGPT for finance using Studio"). The merge
puts classification at the top — the skill classifies into AOps product
policy vs Access ToolUsePolicy before authoring, and asks one numbered
disambiguation question only when both branches plausibly fit.
Structure follows progressive disclosure:
- skills/uipath-governance/SKILL.md (~95 lines) — routing core: identity,
when-to-use, critical rules, workflow, disambiguation question
template, reference navigation, anti-patterns. Classification is
Workflow Step 1; uip verify/login is Step 2 (only after a governance
branch is chosen).
- references/disambiguation-guide.md — strong signals per branch,
ambiguous-phrase tables, canonical worked example.
- references/aops-policy/ and references/access-policy/ — mechanic
libraries inheriting the previously-shipped behavior, each owning
its own Critical Rules / Quick Start / Completion Output.
Filenames follow <topic>-overview-guide.md per skill-structure.md.
Test tasks moved to tests/tasks/uipath-governance/{aops,access}-policy/.
Tasks no longer instruct the agent to "Use the uipath-governance skill"
(skill must self-activate from prompt context). Tasks no longer ask
the agent to write a self-narrated report.json; instead each task
redirects actual stdout+stderr of every uip invocation to ./output.txt.
Self-grading file_contains/json_check criteria removed; objective
command_executed criteria (which inspect the framework's record of
real Bash invocations) plus a single file_exists for output.txt remain.
CODEOWNERS collapses the two prior governance entries into one
/skills/uipath-governance/ owner set.
User-facing terminology: the disambiguation question's option for
Branch B reads "Govern resource/tool use" (was "Govern tool-use") to
align with how the access-policy mechanic describes its own scope as
governing tool-use / resource-use. The branch-B identity table row
and the description follow the same phrasing.
Description: 552 chars, brand-leading, products enumerated, →redirect
to uipath-platform; hooks/validate-skill-descriptions.sh passes.
354 internal markdown links resolve.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
jianjunwang2
force-pushed
the
feat/uipath-governance-merge
branch
from
1232988 to
adf39ef
Compare
|
Claude finished @jianjunwang2's task in 2m 49s —— View job Coder-eval task lint (advisory)7 task YAMLs changed (all renames with content rewrites from Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.
Evidence of passing run❌ High — PR body does not claim the changed tasks have been run and passed. The "Validation" section covers description and link validation only; the "Test plan" checkboxes are unchecked TODO items. Please edit the PR description to add a line like: Per-task lint
|
3 participants
Summary
Replaces two sibling governance skills (
uipath-gov-aops-policy,uipath-gov-access-policy) with a single user-shapeduipath-governanceskill. Both old skills front-loaded the same verbs (block,restrict,deny) and produced non-deterministic activation on prompts that fit either layer — the canonical example is "Block ChatGPT for finance using Studio," which both skills would silently authoring on, with no way for the user to tell which interpretation was applied.The merge puts disambiguation at the top of one skill instead of across two siblings: classify into AOps product policy vs Access ToolUsePolicy first, ask one numbered question only when both branches plausibly fit, then route to the matching mechanic.
New folder structure
Progressive disclosure layout:
SKILL.mdis the only file loaded on activation. It holds routing logic and the disambiguation question template — nothing mechanic-specific.disambiguation-guide.mdis loaded at Workflow Step 2 to classify the request.aops-policy/andaccess-policy/subfolders are mechanic libraries. Each has its own*-overview-guide.mdas the branch entry point; the branch's other guides (commands, manage, deploy, etc.) are loaded on demand from there.compliance-pack.mdcross-cutting flow +assets/{packs,scripts,templates}/will land in a follow-up PR.What changed (file-level)
skills/uipath-governance/SKILL.md(91 lines) — routing core only: identity, when-to-use, 6 critical rules, 3-step workflow, disambiguation question template, reference navigation, anti-patterns.references/disambiguation-guide.md(67 lines) — strong signals per branch, ambiguous-phrase tables, canonical worked example.references/aops-policy/—aops-policy-overview-guide.md(absorbs old AOps SKILL.md content) plus existing manage / deploy / deployed / commands / configure / recipes guides.references/access-policy/—access-policy-overview-guide.md(absorbs old Access SKILL.md content) plus existing planning / impl / policy-manage / commands / sample / resource-lookup / plugins guides.<topic>-overview-guide.mdmatches<topic>-guide.mdper.claude/rules/skill-structure.md.tests/tasks/uipath-governance/{aops,access}-policy/with skill-name references rewritten in YAML./skills/uipath-governance/(and matching/tests/tasks/uipath-governance/) entry; same owners (@UiPath/AuthZ @sriramva-uipath @bansal-anushree @jianjunwang2).Description tightening
Old descriptions used
[PREVIEW]prefix and verboseTriggers:clauses. New description (543 chars):UiPath governance via uip gov) ✓Agent→Agent/Maestro/Flow/RPA/API/Case) ✓→redirect touipath-platform✓[PREVIEW]prefix; preview status moved to body callout ✓Validation
hooks/validate-skill-descriptions.shexit 0 (description 543 chars, under 1024 cap)Test plan
block,restrict,enforce,allow only,who can,compliance).Govern the productandGovern tool-use. Reply with1and confirm routing toaops-policy/aops-policy-overview-guide.md.2and confirm routing toaccess-policy/access-policy-overview-guide.md.tests/tasks/uipath-governance/aops-policy/list_policies_smoke.yaml(or any other relocated test) — confirm the new skill nameuipath-governanceactivates correctly.🤖 Generated with Claude Code