Skip to content

Add preflight-learning-path skill for author pre-PR self-review - #462

Merged
tacole02 merged 10 commits into
mainfrom
add-preflight-learning-path-skill
Jul 28, 2026
Merged

Add preflight-learning-path skill for author pre-PR self-review#462
tacole02 merged 10 commits into
mainfrom
add-preflight-learning-path-skill

Conversation

@tacole02

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • Run /preflight-learning-path on a local {slug}-lj/ package with Playwright MCP enabled
  • Confirm Phase 0 stops helpfully if Playwright MCP is missing
  • Confirm Phase 1 chat only lists review-level (post-inline) issues, not nits
  • Confirm Phase 2 Playwright DOM runs after Okta ready, and Block Editor prompt accepts already-tested / walk-me / skip-smoke
  • Confirm Phase 3 readiness + fix / frontend / done offers work as documented
  • Spot-check links to review-learning-path companions (reference-checks, learning-hub-standards, comment-style)

Made with Cursor

Mirrors the five-phase review coach so authors catch post-inline issues before opening a PR, with optional package fixes and a frontend data-testid walkthrough.
@tacole02 tacole02 self-assigned this Jul 20, 2026
@tacole02 tacole02 linked an issue Jul 20, 2026 that may be closed by this pull request
tacole02 added 6 commits July 20, 2026 11:14
Phase 0 now pairs manual Settings steps with an opt-in mcp.json write (or mcp_auth), so authors are not left with instructions-only when the agent can help.
Catch contradicted, unsupported, and overstated product facts against live docs before authors open a PR.
Drop the check primer, show what was checked, and offer fix all / fix N choices with plain-language context.
Keep fake step numbers in a short checklist instead of a separate setup step.
Only offer package fix choices when a guide edit can resolve the item; lead with frontend when live needs an upstream testid.
Add a voice guide, plain-language phase copy, Phase 5 step checkpoints, and clearer fix vs frontend routing in chat.
@bonnywelsford-source

Copy link
Copy Markdown
Contributor

Hey Taylor. I dogfooded the preflight skill from this PR on a brand-new k6 CI/CD path (#477). Overall the checks themselves were useful. The friction was mostly in how the skill talks to the author.

What worked

  • Catching real issues I’d want fixed before review (section bookends / fake step risk, and an overstated claim about CI logs).
  • Playwright DOM check on the interactive milestone was solid once MCP was healthy.
  • The “already-tested” option was great. I’d already smoke-tested Show me / Do it, so I didn’t want a second full walkthrough.

Where I got stuck / confused

  1. Too many phases / “reply yes” moments. It felt like a quiz. I’d rather the skill do everything it can in one go, pause only when it truly needs me (Okta login / smoke choice), then come back with one results summary and fix options.
  2. Reply keywords are easy to misunderstand. Things like “reply yes to refresh readiness” and “if this looks right” made me wonder if I was supposed to verify something I couldn’t see. Clickable choices (or at least much plainer prompts) would help a lot.
  3. Playwright MCP setup. Mine was configured with a broken CDP endpoint (localhost:9222). The skill’s “missing MCP” help didn’t quite cover “configured but broken.” The fix path eventually worked (npx @playwright/mcp@latest), but the diagnostics could be clearer.
  4. “Refresh readiness” jargon. I literally didn’t know what that meant until the agent explained it. Something like “want an updated ready / not-ready summary?” would be better.

Smaller notes

  • Authors will accidentally commit .cursor/lp-preflight-state/ unless gitignore is on main (I added it in my LP PR so I wouldn’t publish reports by mistake). Worth shipping that ignore with the skill.
  • Create/scaffolding didn’t catch the bookend issue. Preflight did. That’s fine if preflight is the intentional second gate. Or create could get a little stricter so we don’t find that so late.

Happy to jump on a quick call if useful. Thanks for building this. The substance is good. I’d just make the author experience less stop-and-type.

Bonny

Pause only for path/MCP blockers, Okta login, smoke choice, and one results menu. Diagnose configured-but-broken Playwright MCP, drop refresh-readiness jargon, and remind authors not to commit lp-preflight-state.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tacole02

Copy link
Copy Markdown
Contributor Author

Thanks for the dogfood notes, Bonny. Landed a hard UX pass in cef6ac5a that addresses your feedback (except create/scaffold bookends, which I’m tracking as follow-up).

What changed

  1. Fewer pauses — Agent batches identify + static + claim-check. Author pauses only for: unclear path, Playwright MCP blocked/broken, Okta login + stack, Block Editor smoke choice (and walk-me if chosen), then one results / fix menu. No more “Phase N complete, reply yes” between agent-only work.
  2. Plainer prompts — Login / smoke / results wording is shorter and more conversational; dropped “refresh readiness” in favor of “updated ready / not-ready summary.”
  3. Playwright MCP diagnostics — Covers missing, needs-auth, toggled-off, errored, and configured-but-broken (e.g. CDP localhost:9222 with nothing listening). Authors can reply fix playwright mcp / add playwright mcp to restore the standard npx @playwright/mcp@latest config.
  4. Gitignore reminder — Command + skill remind that .cursor/lp-preflight-state/ is gitignored and should not be force-added to PRs (already ignored on this branch).

Follow-up (not in this PR)
5b. Stricter create/scaffold bookend enforcement so authors don’t miss intro/summary markdown outside sections. I’ll open that as a separate comment/issue after this lands.

Happy to tweak further if anything still feels quizzy when you re-run /preflight-learning-path.

@tacole02
tacole02 marked this pull request as ready for review July 27, 2026 16:47

@moxious moxious 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.

genuinely useful addition, and the registration side (.gitignore, AGENTS.md, CLAUDE.md, workflows) is clean. I ran it against the repo conventions and the reviewer skill it mirrors, and found four things worth resolving before merge. Two are structural; two are quick.

▎ 1. Preflight isn't actually running the same policy as the reviewer it mirrors (high). The command and skill repeatedly promise the "same checks" as /review-learning-path-pr (preflight-learning-path.md:3, SKILL.md:5-13, AGENTS.md:86, workflows.md:106), but preflight adds an independent claim-check that turns any uncorroborated factual statement into a mandatory pre-PR fix (claim-check.md:47,61-66, reference-checks.md:40-64,72-91). The reviewer's static pass has no equivalent category. So an author can fail preflight on a claim the reviewer never evaluates — the "same checks" promise breaks and there are now two standards that will drift. Either make the claim policy a shared source both skills reference, or, if preflight is intentionally stricter, say so and stop calling it the "same checks."

▎ 2. The resume/state contract can't reliably represent the documented workflow (high). The skill promises a resumable run and initializes a state file (SKILL.md:125,174-180,206), but nothing updates phase/status/checkpoint, and phase is just an unmapped integer 0–5 (reference-checks.md:158-193) — which also conflicts with the "five-phase" description. After a context loss, two agents read phase: 3 differently. Suggest a named-checkpoint enum (identify, login, dom, smoke, results, package_fixes, frontend) with explicit persistence at each gate — or drop the resume promise.

▎ 3. static-only has overlapping allow/deny rules (medium). A new/conversion author with no stack access matches both the "No" and the "Yes, with reason" rows (reference-checks.md:111-115), while SKILL.md:244-252 says reject all new/conversion — so the mandatory Playwright gate can be bypassed exactly where it's required. Make the rows mutually exclusive and state precedence.

▎ 4. Dead anchor on a mandatory static-check link (medium). SKILL.md:220 → #section-intro-markdown-numbered-as-step, but the heading is ...numbered-as-a-step (missing -a-). Same typo pre-exists in review-learning-path/SKILL.md:165, so fixing both closes the loop.

▎ Everything else checked out — Phase-0 MCP handling, the smoke branches and approval gates, referenced files, and external references all resolve; CI is green.

Make claim-check the shared policy for review and preflight, persist named checkpoints for resume, make static-only rows exclusive with precedence, and fix the section-intro markdown anchor.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tacole02

Copy link
Copy Markdown
Contributor Author

Thanks @moxious — addressed all four points in the latest commit:

  1. Claim-check drift — Moved the policy to shared review-learning-path/claim-check.md. Preflight keeps a pointer; review Phase 1 now runs the same pass. Dropped the misleading “same checks” shorthand in favor of “mirrors review (shared static + shared claim-check + Playwright).”

  2. Resume / state — Replaced integer phase 0–5 with a named checkpoint enum (identify | login | dom | smoke | results | package_fixes | frontend) plus status. Each gate now has an explicit Persist line. Legacy numeric phase files map once or start fresh.

  3. static-only overlap — Precedence table: bare reject → new/conversion interactive always reject (even “no stack”) → update-only allow with reason. Same precedence mirrored on the review static-only section.

  4. Dead anchor — Links now use #section-intro-markdown-numbered-as-a-step in both preflight and review (and the in-file TOC link).

Happy to tweak further if anything still feels off.

Resolve .gitignore conflict by keeping main's lp-preflight-state ignore comment.

Co-authored-by: Cursor <cursoragent@cursor.com>

@moxious moxious 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.

LGTM

@tacole02
tacole02 merged commit db08a2d into main Jul 28, 2026
20 checks passed
@tacole02
tacole02 deleted the add-preflight-learning-path-skill branch July 28, 2026 16:46
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.

Create an learning path self-audit skill

3 participants