Fix: stale my/ paths in skill docs (bulk migration)#68
Conversation
nehaoss
left a comment
There was a problem hiding this comment.
Bulk path migration from \my/\ to .local/\ (and \workspace/\ for some paths). Clean mechanical change across 40 files.
Comments:
-
Scope: 159 additions / 159 deletions across 40 files — pure rename, no logic changes. Good that it's isolated from feature work.
-
Consistency check needed: With 40 files touched, verify no references to the old \my/\ path remain. Run \grep -r 'my/' --include='.md' --include='.py' --include='*.json'\ in the solutions folder to confirm completeness.
-
Breaking change for existing users: Users with existing \my/config.json, \my/connect/, \my/.token_cache.bin\ etc. will lose their state after pulling this change. Consider adding a migration note in the PR description (or a one-time migration script) so existing kit users know to move their local files.
-
Prompt file updates: The \connect.prompt.md\ and \setup.prompt.md\ changes point to the new paths — good that these are included.
d2a2189 to
837bb91
Compare
Fix: stale my/ paths in skill docs (bulk migration) Migrates 159 stale `my/` path references across 40 files to the new convention established in PR #2 and reinforced by John in PRs #16, #17, (user-edited content) and `.local/` (kit bookkeeping). The Python scripts (`auth.py`, `setup.py`, `checkpoint.py`, `flightcheck/cli.py`, every flightcheck check) were migrated correctly during the SFI cutover. The skill docs that the LLM reads at task time were not. This produced a class of silent failure where the LLM was told to write to `my/connect/servicenow/config.json` while `auth.py` read from `.local/connect/servicenow/config.json` -- exact same defect John flagged on PR #54, scaled to 30+ more files. Mapping applied (mechanical, ordered to avoid prefix collisions): | Old path | New path | Reason | |-----------------------------------|-------------------------------------|-----------------------| | my/agents/ | workspace/agents/ | User-edited | | my/onboarding/ | workspace/onboarding/ | User-visible state | | my/flightcheck/ | workspace/flightcheck/ | User-visible reports | | my/tests/ | workspace/tests/ | User-visible test sets| | my/connect/ | .local/connect/ | Kit bookkeeping | | my/config.json | .local/config.json | Kit bookkeeping | | my/.token_cache.bin | .local/.token_cache.bin | Kit bookkeeping | | my/.azure-login-attempts.json | .local/.azure-login-attempts.json | Kit bookkeeping | Files touched (39 docs + 1 runtime): - `.github/prompts/{connect,setup}.prompt.md` - `scripts/flightcheck/AGENTS.md` -- carries over from PR #61 follow-up - `scripts/flightcheck/pva_client.py:55` -- the one remaining runtime fix (was `os.path.join("my", ".token_cache.bin")`); aligns with `auth.py:127`, `graph_client.py:78`, `pp_admin_client.py:71` - `src/reference/ess-docs/flightcheck/{permissions-required,remediation-guide}.md` - `src/skills/cleanup/SKILL.md` - `src/skills/connect/{SKILL.md,step1.md}` - `src/skills/connect/azure/{app-registration,login}.md` - `src/skills/connect/servicenow/{step1,step2-certificate,step2-entra,step2-graph,step2-oauth2,step3-basic,step3-certificate,step3-entra,step3-graph,step3-oauth2,step4}.md` - `src/skills/connect/workday/{step1,step2,step3}.md` - `src/skills/evaluations/{create,delete,update}/SKILL.md` - `src/skills/flightcheck/SKILL.md` - `src/skills/onboarding/{SKILL.md,step1b,step2,step3-flightcheck}.md` - `src/skills/topics/{create,delete,update}/SKILL.md` - `src/skills/troubleshoot/SKILL.md` - `src/skills/workflows/{create,delete,update}/SKILL.md` Plus a small prose tweak in `src/skills/connect/workday/step2.md` lines 415/419: the bulk migration left two standalone ``my/`` mentions referring to the working directory as a concept; updated those to ``.local/`` since the surrounding paragraph is about the kit bookkeeping config location, not user content. Not changed (intentional): - `scripts/auth.py:44` -- historical comment ("Renamed from `my/` -> `.local/` in PR #2") referencing the migration itself; accurate as-is. - `scripts/flightcheck/checks/local_files.py:342` -- same kind of historical comment. - `src/examples/ess-samples/` -- vendored upstream sample content, not kit code. Diff: 159 insertions(+), 159 deletions(-) (one-for-one substitutions). CI: ruff + compileall pass locally.
The token cache writes to .local/.token_cache.bin but makedirs still created the dead my/ dir (straggler from the my/ -> .local migration, same defect class as the rest of this PR). Aligns the dir creation with cache_path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
837bb91 to
e989aab
Compare
PR #68 (now merged) moved kit bookkeeping from my/ to .local/. This PR added new install-path detection lines that still referenced my/connect/workday/config.json. Point them at .local/ so the runbook is consistent with merged main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #68 (now merged) moved kit bookkeeping from my/ to .local/. This PR added new install-path detection lines that still referenced my/connect/workday/config.json. Point them at .local/ so the runbook is consistent with merged main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(connect): support Workday simplified 2-connection install The /connect workday skill assumed the legacy 3-connection install (ISU accounts, security groups, RaaS report). Microsoft now ships a simplified install needing only the OAuthUser connection (ff0df) plus Dataverse, using the REST /workers/me endpoint for user context. step1 now classifies the install path (simplified vs legacy; fresh installs default to simplified) and derives the Workday REST base URL. step2 gates the ISU/security-group/auth-policy/API-client/domain-permission/RaaS tasks to the legacy path; Entra SSO stays mandatory on both. step3 adds a 2-connection install table with the REST base URL field, path-aware verification, and routes the user-context topic redirect to the V2 (REST) topic on simplified installs. SKILL.md, copilot-instructions.md, and README.md updated to describe both paths. Flightcheck and doc vendoring intentionally out of scope. Refs #93 * fix(connect): address review on Workday install-path detection Handle partial legacy installs and older extension pack versions surfaced in PR #111 review: - step1.md 1.7a: classify the 2-connection case (only d6081 and/or 0786a present) as a partial legacy install rather than falling through to simplified. Persist partialInstall flag in config so step3 can branch on it. - step3.md 3.4: read partialInstall and tell the model to finish the missing legacy connection(s) instead of starting a fresh simplified install on top. - step3.md 3.4 simplified: add a one-line escape hatch for older Workday extension pack versions that don't expose the 'Workday REST base URL' field on the OAuthUser connection. User types 'legacy' to switch paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: simplified Workday path needs the API client The simplified (2-connection) install routes admin setup through Task 1 only and lists 'API client' as not-needed, but step3's ff0df OAuthUser connection requires {oauthClientId}, which is captured only in Task 4. As written, simplified leaves oauthClientId unresolved. The ff0df connection signs in via a customer-registered Workday API client (Assertion Verification = Use Configured IdPs, backed by the Task 1 Entra IdP), per the canonical workday.md integration doc and the WORKDAY_OAUTH_CLIENT_ID runtime env var. So the API client is required on both paths. - Route simplified through Task 1 AND Task 4 (skip only 2, 3, 5, 6), with explicit installPath branches at every Task 1 exit - Stop listing 'API client' as not-needed for simplified in step2/SKILL - Add oauthClientId to the step2 config-read list and tasks.md checklist - Clarify Dataverse verification in step3 (platform connector, not %workday%, so confirm from the install green-check not the query) - Name connection counts by the 3 Workday SOAP refs (Dataverse common to both paths) in step1 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Migrate new Workday config refs to .local/ to match merged #68 PR #68 (now merged) moved kit bookkeeping from my/ to .local/. This PR added new install-path detection lines that still referenced my/connect/workday/config.json. Point them at .local/ so the runbook is consistent with merged main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(workday): clarify partial-install repair and redirect checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: saengland <40703683+saengland@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Migrates 159 stale
my/path references across 40 files to the new convention established in PR #2 and reinforced by John in PRs #16, #17, #18, #19, #54, #61, #62, #63:my/is dead, replaced byworkspace/(user-edited content) and.local/(kit bookkeeping).The Python scripts (
auth.py,setup.py,checkpoint.py,flightcheck/cli.py, every flightcheck check) were migrated correctly during the SFI cutover. The skill docs that the LLM reads at task time were not. This produces a class of silent failure: the LLM is told to write tomy/connect/servicenow/config.jsonwhileauth.pyreads from.local/connect/servicenow/config.json-- the same defect John flagged on PR #54, scaled to 30+ more files.Mapping applied (mechanical, ordered to avoid prefix collisions)
my/agents/workspace/agents/my/onboarding/workspace/onboarding/my/flightcheck/workspace/flightcheck/my/tests/workspace/tests/my/connect/.local/connect/my/config.json.local/config.jsonmy/.token_cache.bin.local/.token_cache.binmy/.azure-login-attempts.json.local/.azure-login-attempts.jsonFiles touched (39 docs + 1 runtime)
.github/prompts/{connect,setup}.prompt.mdscripts/flightcheck/AGENTS.md-- carries over from PR Add flightcheck developer guide (AGENTS.md) [reposted from #58] #61 follow-upscripts/flightcheck/pva_client.py:55-- runtime fix; aligns withauth.py:127,graph_client.py:78,pp_admin_client.py:71src/reference/ess-docs/flightcheck/{permissions-required,remediation-guide}.mdsrc/skills/cleanup/SKILL.mdsrc/skills/connect/{SKILL.md,step1.md}src/skills/connect/azure/{app-registration,login}.mdsrc/skills/connect/servicenow/-- 11 filessrc/skills/connect/workday/{step1,step2,step3}.mdsrc/skills/evaluations/{create,delete,update}/SKILL.mdsrc/skills/flightcheck/SKILL.mdsrc/skills/onboarding/{SKILL.md,step1b,step2,step3-flightcheck}.mdsrc/skills/topics/{create,delete,update}/SKILL.mdsrc/skills/troubleshoot/SKILL.mdsrc/skills/workflows/{create,delete,update}/SKILL.mdNot changed (intentional)
scripts/auth.py:44-- historical comment referencing the migration itself; accurate as-isscripts/flightcheck/checks/local_files.py:342-- samesrc/examples/ess-samples/-- vendored upstream sample contentType of change
Testing
ruff check scripts/ src/mcp/-- passes locallypython -m compileall -q scripts/ src/mcp/-- passes locally-line and corresponding+line differ only in the path mapping aboveChecklist