Skip to content

PR #6: Connect + Onboarding skills - #16

Closed
saengland wants to merge 4 commits into
mainfrom
review/06-skills-connect-onboarding
Closed

PR #6: Connect + Onboarding skills#16
saengland wants to merge 4 commits into
mainfrom
review/06-skills-connect-onboarding

Conversation

@saengland

@saengland saengland commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Adds the connection setup and onboarding skill content under solutions/ess-agent-kit/src/skills/:

  • connect/ ΓÇö integration setup flows for Azure, ServiceNow, and Workday (21 files)
  • onboarding/ ΓÇö guided first-run experience for /setup (6 files)

These are markdown skill files consumed by GitHub Copilot prompts to walk customers through environment connection and credential setup.

SFI focus: any references to internal Microsoft tenants, hardcoded URLs, or sample credentials should be generic placeholders.

Tracker: #10

This branch will be populated via scoped PRs reviewed by Graham, John, and Cavill. Once all PRs are merged here, main2 will replace main.
Adds the connection setup and onboarding skill content under solutions/ess-agent-kit/src/skills/:
- connect/ - integration setup flows for Azure, ServiceNow, and Workday (21 files)
- onboarding/ - guided first-run experience for /setup (6 files)

These are markdown skill files consumed by GitHub Copilot prompts to walk customers through environment connection and credential setup. SFI focus: any references to internal Microsoft tenants, hardcoded URLs, or sample credentials should be generic placeholders.

Reviewer: @CavillMason
Tracker: #10
… contract

PR #15 changed register_oauth_application and register_oidc_provider to accept client_secret_env_var (env var NAME) instead of client_secret (raw value), keeping secrets out of MCP logs and LLM context. Update the four skill docs that call these tools:

- step2-certificate.md, step2-entra.md, step2-graph.md: add a pre-step setting SERVICENOW_OIDC_CLIENT_SECRET_NOT_USED='not-used' (OIDC verification with Entra ID does not actually consume a client secret, but the ServiceNow record requires the field to be non-empty); pass that env var name to register_oidc_provider.

- step2-oauth2.md: add a pre-step writing the freshly-generated CLIENT_SECRET to env var SERVICENOW_OAUTH_CLIENT_SECRET; pass that env var name to register_oauth_application.
rename solutions/ess-agent-kit -> solutions/ess-maker-skills

@johnguy0 John Nguyen (johnguy0) 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.

Big PR (6512 lines, 27 markdown files) but mostly well-structured skill prose. The author has the right instincts on credentials: ${input:...} everywhere, secrets.token_hex(16) for OAuth client secrets, secrets.choice for ISU passwords, env-var indirection for OAuth registration. Multiple-auth ServiceNow flow (basic, certificate, entra, graph, oauth2) and the Workday SAML+ISU dual-credential story are clearly explained.

That said, three findings that need to land before merge.

CRITICAL:

  1. workday/step3.md:212 leaks PR author's username saengland in a hardcoded Windows path. This is exactly what your PR description's SFI focus warns against ("any references to internal Microsoft tenants, hardcoded URLs, or sample credentials should be generic placeholders"). The path C:\Users\saengland\AppData\Local\Microsoft\WindowsApps\pwsh.exe ships in a Microsoft public OSS repo. Replace with pwsh (which resolves via PATH on any system with PowerShell 7), and document the cross-platform alternative.

  2. workday/step1.md:192 instructs the user to weaken their Workday password. The text says "if your password contains &, %, #, try resetting it to one with only letters and numbers temporarily." That's instructing the customer to weaken their actual production credential to work around a credential-encoding bug. The right fix is XML/URL escaping in the SOAP envelope (which the Workday MCP already does at client.py:154), or interactive prompt that handles the special chars correctly. Telling users to weaken passwords is a bad-security pattern that ships into our public docs.

HIGH:

  1. Sensitive temp files are written but never cleaned up. $env:TEMP\ess-*.json is used 8+ times across workday/step2.md, workday/step3.md, azure/app-registration.md to stage Graph PATCH bodies containing Entra app configurations, SAML metadata, OAuth client config, signing certificate requests. Each Out-File writes to the user's TEMP, runs az rest against Graph, and leaves the file on disk indefinitely. Add Remove-Item -Force -ErrorAction SilentlyContinue $env:TEMP\ess-*.json after each use, or write to in-memory pipelines via --body @- if az rest supports stdin.

  2. onboarding/SKILL.md is internally inconsistent. tasks.md has 5 items, but the Fresh Start checklist in SKILL.md shows only 4 (missing "Readiness check"). Section headers ("## Step 1 and Step 2", "## Step 3 or Step 4", "## Step 5") don't match the content (Step 1 routing reads step1.md, Step 2 routing reads step2.md, Step 5 routing reads step3-flightcheck.md). The numbering reads as half-finished from a refactor. Reconcile to a single coherent step model.

MEDIUM:

  1. servicenow/step3-basic.md lacks a dev/test-only warning. The header says "This file is ONLY for Basic authentication" — that's about routing, not security. Add a top-of-file callout: > ⚠️ Basic authentication sends credentials with every request and is not recommended for production. Use Entra or OAuth2 for production deployments.

  2. workday/step2.md:323 password generation has a predictable special-char pattern. The line generates 16 random chars then forces special chars at fixed positions: pw=pw[:4]+'!'+pw[4:8]+'#'+pw[8:12]+'@'+pw[12:]. The pattern XXXX!XXXX#XXXX@XXXX is deterministic, reducing entropy. If Workday's password policy needs special chars and a length floor, generate without the slicing trick: secrets.choice(letters + digits + "!@#$%^&*") for each position, then post-validate that at least one of each class is present, retry if not.

  3. Cross-PR coordination — my/ paths. This PR has 102 references to my/ paths (my/onboarding/, my/connect/, my/agents/, my/config.json). If my/ is renamed to workspace/ + .local/ per my recommendation on PR #2, every one of these needs updating. Specifically: my/connect/{integration}/tasks.md and my/onboarding/tasks.md are agent-managed (LLM updates progress), so they belong in .local/. my/agents/{slug}/ is user-edited, belongs in workspace/.

  4. Cross-PR coordination — scripts/... invocations (8 sites across workday/step3.md, servicenow/step4.md, onboarding/step1b.md, onboarding/step2.md, onboarding/step3-flightcheck.md). All assume cwd is solutions/ess-maker-skills/. Either document this prerequisite at the top of each step file ("Run from the solutions/ess-maker-skills/ directory") or change to absolute paths so they work from any cwd.

PR-level:

  1. PR description still references solutions/ess-agent-kit/ instead of solutions/ess-maker-skills/. Same stale-rename as PRs #2-#5.

Inline below for the spots that anchor to specific lines.

Use the PowerApps Admin API (run in terminal, do not show to user):

```
C:\Users\saengland\AppData\Local\Microsoft\WindowsApps\pwsh.exe -ExecutionPolicy Bypass -NoProfile -Command "Import-Module Microsoft.PowerApps.Administration.PowerShell -Force -WarningAction SilentlyContinue; Add-PowerAppsAccount; Get-AdminFlow -EnvironmentName '{ENV_ID}' | Where-Object { `$_.DisplayName -match 'workday|WD_|Workday' } | Select-Object DisplayName, @{n='State';e={`$_.Internal.properties.state}} | Format-Table"

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.

CRITICAL — leaks PR author's username (saengland) in a hardcoded path. Direct violation of the PR description's stated SFI focus.

The path C:\Users\saengland\AppData\Local\Microsoft\WindowsApps\pwsh.exe is the author's personal pwsh install path on Windows. Two issues:

  1. Ships an internal Microsoft username in a public Microsoft OSS repo.
  2. The hardcoded user-specific path won't work for any other user — pwsh isn't installed at the same path for everyone, and it doesn't exist at all on macOS/Linux.

Fix:

pwsh -ExecutionPolicy Bypass -NoProfile -Command "Import-Module Microsoft.PowerApps.Administration.PowerShell -Force -WarningAction SilentlyContinue; Add-PowerAppsAccount; Get-AdminFlow -EnvironmentName '{ENV_ID}' | Where-Object { `$_.DisplayName -match 'workday|WD_|Workday' } | Select-Object DisplayName, @{n='State';e={`$_.Internal.properties.state}} | Format-Table"

If the PowerShell module isn't installed, the existing fallback path (Dataverse MCP workflow table query at line 220) handles that. Keep both.

to confirm.
- **Special characters in password?** Characters like `&`, `%`, `#` can
cause issues. If your password contains these, try resetting it to one
with only letters and numbers temporarily.

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.

CRITICAL — instructs the user to weaken their Workday password.

This line tells the customer: "If your password contains [special chars], try resetting it to one with only letters and numbers temporarily." That's instructing them to weaken a production credential to work around a credential-encoding bug.

The Workday MCP already XML-escapes credentials in the SOAP envelope (client.py:154 in PR #14). So this guidance is also wrong on the merits — the special-char issue should be handled in the credential prompt or escaping layer, not by asking the user to set a less secure password.

Fix: remove this bullet entirely. Replace with: "If you're hitting persistent auth errors, capture the exact error code and contact your Workday admin — don't change your password to work around the issue."


```powershell
$body = @{displayName="Workday (ESS Copilot)"} | ConvertTo-Json
$body | Out-File "$env:TEMP\ess-wd-template.json" -Encoding utf8

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.

HIGH — sensitive temp files written but never cleaned up.

This line writes Entra app template config to $env:TEMP\ess-wd-template.json. Same pattern at lines 168 (SSO config), 183 (SAML metadata), 200 (signing certificate request) in this file, plus several in workday/step3.md and azure/app-registration.md (8+ total).

Each file contains sensitive Graph PATCH bodies (app config, SAML cert generation requests). They're written, used by az rest, then left on disk indefinitely. On Windows, $env:TEMP is a per-user dir that survives reboots. Anyone with local read access to that user account can read them.

Add cleanup after each az rest invocation:

Remove-Item -Force -ErrorAction SilentlyContinue "$env:TEMP\ess-wd-template.json"

Or factor into a helper that wraps the write→call→cleanup pattern. Better: check if az rest accepts --body @- for stdin input and pipe the body without ever writing to disk.

|---|------|--------|
| 1 | Dataverse configured | ⬜ |
| 2 | Agent discovered | ⬜ |
| 3 | Agent extracted | ⬜ |

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.

HIGH — Fresh Start checklist drops the 5th task.

tasks.md defines 5 tasks: Dataverse configured, Agent discovered, Agent extracted, MCP server started, Readiness check. The Fresh Start message here only lists 4 (missing Readiness check). User sees "this takes about 5 minutes" then a 4-row table, but the actual flow is 5 steps culminating in flightcheck.

Fix: add the 5th row, mark it as optional in the visual:

| 5 | Readiness check (optional) | ⬜ |


---

## Step 3 or Step 4

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.

HIGH — section headers are inconsistent / leftover from a refactor.

The headers below this point read "## Step 1 and Step 2", "## Step 3 or Step 4", "## Step 5". The content under each is single-step routing (read step1.md / read step2.md / read step3-flightcheck.md). The header naming is misleading — "Step 3 or Step 4" implies a branch where there is none.

Align with the actual file structure (step1.md → step1b.md → step2.md → step3-flightcheck.md):

## Step 1: Connect to Dataverse
Read `src/skills/onboarding/step1.md` and follow it.

## Step 2: Discover and select agent
Read `src/skills/onboarding/step1b.md` and follow it.

## Step 3: Extract agent and start MCP
Read `src/skills/onboarding/step2.md` and follow it.

## Step 4: Readiness check
Read `src/skills/onboarding/step3-flightcheck.md` and follow it.

(Or rename the underlying files so step1b becomes step2, step2 becomes step3, step3-flightcheck becomes step4 — same intent, cleaner numbering.)

@@ -0,0 +1,135 @@
# ServiceNow Step 3: Install Extension Pack (Basic Auth)

**This file is ONLY for Basic authentication. Do not use for OAuth2 or Entra.**

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.

MEDIUM — add a production-use warning.

This line says "This file is ONLY for Basic authentication. Do not use for OAuth2 or Entra." That's about file routing, not about the security implications of the choice.

Add a callout above this line:

> ⚠️ Basic authentication sends credentials with every request and is not recommended for production. Choose Entra or OAuth2 for production deployments. Basic auth is intended for dev/test environments only.

Matches the bar set in the kit's main README which lists Basic as "Dev/test only."

(do not show this command or its output to the user):

```
python -c "import secrets,string; chars=string.ascii_letters+string.digits+'!@#$'; pw=''.join(secrets.choice(chars) for _ in range(16)); pw=pw[:4]+'!'+pw[4:8]+'#'+pw[8:12]+'@'+pw[12:]; print(pw); pw2=''.join(secrets.choice(chars) for _ in range(16)); pw2=pw2[:4]+'!'+pw2[4:8]+'#'+pw2[8:12]+'@'+pw2[12:]; print(pw2)"

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.

MEDIUM — predictable special-char pattern in generated password.

The one-liner generates 16 random chars then injects fixed special chars at positions 4, 9, 14: pw=pw[:4]+'!'+pw[4:8]+'#'+pw[8:12]+'@'+pw[12:]. The resulting pattern is [A-Za-z0-9]{4}![A-Za-z0-9]{4}#[A-Za-z0-9]{4}@[A-Za-z0-9]{4} — special chars and their positions are deterministic.

If Workday password policy requires character-class diversity, do it without sacrificing entropy:

import secrets, string
charset = string.ascii_letters + string.digits + "!@#$%^&*"
while True:
    pw = ''.join(secrets.choice(charset) for _ in range(20))
    if (any(c.isupper() for c in pw) and any(c.islower() for c in pw)
        and any(c.isdigit() for c in pw) and any(c in "!@#$%^&*" for c in pw)):
        break
print(pw)

Length bumped to 20 since you no longer have fixed special-char positions.


## Routing

Each integration has its own folder with its own tasks.md and step files:

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.

Cross-PR coordination — my/ paths.

This PR references my/connect/{integration}/tasks.md, my/connect/{integration}/config.json, etc. across 102 sites in the skills tree. If my/workspace/ + .local/ lands per PR #2 review, these all change.

Recommended mapping based on what the user actually does with these:

  • my/connect/{integration}/tasks.md.local/connect/{integration}/tasks.md (agent-managed checklist, not user-authored)
  • my/connect/{integration}/config.json.local/connect/{integration}/config.json (kit bookkeeping)
  • my/onboarding/tasks.md.local/onboarding/tasks.md
  • my/agents/{slug}/workspace/agents/{slug}/ (user edits topic YAMLs and workflow JSON here)
  • my/tests/{date}/workspace/tests/{date}/ (user reviews and uploads CSVs)
  • my/flightcheck/history/workspace/flightcheck/history/ (user opens HTML reports)

Not blocking on this PR — the rename has to land first on PR #2, then this PR rebases. Just flagging for the inevitable coordination.

Run this single command in the terminal (substitute all values):

```
python scripts/fetch_and_setup.py --url "{ENV_URL}" --bot-id "{BOT_ID}" --name "{BOT_NAME}" --schema "{SCHEMA_NAME}" {--managed if IS_MANAGED is true}

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.

MEDIUM — python scripts/... paths assume cwd is the solution root.

Same pattern at workday/step3.md:54, 289, 309, 322, servicenow/step4.md:34, onboarding/step1b.md:22, 66, onboarding/step3-flightcheck.md:57. All bare python scripts/... invocations.

After PR #2's layout, the actual path is solutions/ess-maker-skills/scripts/.... The bare path works only if cwd is solutions/ess-maker-skills/. The kit doesn't currently enforce or document this.

Two options:

  1. Add a top-of-file note in each step that uses python scripts/...: > Run from the kit's solution directory (solutions/ess-maker-skills/).
  2. Change to repo-rooted paths: python solutions/ess-maker-skills/scripts/setup.py ... (works from repo root).

Option 2 is more robust against the user accidentally being in the wrong directory.

@saengland

Copy link
Copy Markdown
Collaborator Author

This PR was auto-closed when main was rolled back during the tooling-incident recovery earlier today (the diff base disappeared, so GitHub closed it). Replacement PR with the same branch and code is now open as #34 against main2 for proper review.

saengland added a commit that referenced this pull request Jun 8, 2026
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.
saengland pushed a commit that referenced this pull request Jun 9, 2026
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.
saengland added a commit that referenced this pull request Jun 9, 2026
* Fix: stale my/ paths in skill docs (bulk migration)

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.

* Fix: pva_client makedirs still created dead my/ dir

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>

---------

Co-authored-by: Sam England <40703683+saengland@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nkemms pushed a commit that referenced this pull request Aug 5, 2026
…rom doc comments

Scrub references that only make sense inside the originating workspace, so the
docstrings and comments read cleanly in this standalone public kit:

- Drop the internal probe filename, port-ledger doc, and "switch-back" path from
  the cdp_driver / drive_surface attribution; keep a neutral provenance note that
  the DOM logic is adapted from an internal Microsoft ESS bot-test harness.
- Drop the internal "ADK gap #16 / #17" issue numbers, the "G23 cascade/trap"
  finding id, and "ported from capture_turn_reply" — keep the technical rationale
  each was attached to (card+follow-up bubble shape, omitted data-node-id
  limitation, scope-vs-handler cascade).
- Reword debug_plant's "fragility of the ad-hoc technique" to "manual debug-node
  insertion".

Comment/docstring only; no code or test behavior changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 071e24ea-6c07-4acf-88ad-df159f3de578
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.

2 participants