Skip to content

fix: restore portable workflow guidance - #5

Merged
3metaJun merged 3 commits into
mainfrom
fix/restore-portable-skill-fidelity
Sep 11, 2026
Merged

3metaJun merged 3 commits into
mainfrom
fix/restore-portable-skill-fidelity

Conversation

@3metaJun

Copy link
Copy Markdown
Owner

Portable workflow guidance was incomplete or corrupted, so the meta-mode entry point could omit core instructions and several skills retained vendor-specific fragments that do not work across Harness implementations.

This PR restores the complete meta-mode guidance from the upstream source as the baseline, adapts it to Harness-neutral terminology, and removes broken or Cursor-specific wording from why, swarm, and reflect. It also adds skill integrity checks covering required sections, meta-agent linkage, and forbidden replacement artifacts.

Verification:

  • npm test
  • npm run check-package
  • npm run check-upstream -- --source G:\\agents_temp\\mstack-pstack-full-20260911\\pstack --strict
  • git diff --check

All checks pass.


Agent: GPT-6 via Codex

@3metaJun

Copy link
Copy Markdown
Owner Author

Adversarial review (4 parallel reviewers)

Ran a multi-agent adversarial review of this PR from four angles — content fidelity of the restored meta-mode guidance, strength of the new integrity test, portability/vendor-residue sweep, and build/release hygiene. Overall: the restore is faithful to upstream and the verification claims check out, but the new integrity test provides false assurance (it passes while violations of its own forbidden patterns exist in the tree), and the restore dropped a path-resolution contract that 7 playbooks still depend on.

✅ Verified good

  • Restore fidelity: the new skills/meta-mode/SKILL.md body was diffed against the upstream cursor/plugins pstack snapshot at the pinned commit 7366ac1 — only frontmatter plus six legitimate neutral-terminology rewrites differ. All 17 triggers, 24 playbook entries, and 23 principle references resolve; all ~50 cross-referenced targets exist; node scripts/validate.mjs passes over 50 skills.
  • Verification claims reproduce: npm test (75 pass / 0 fail / 1 expected Windows skip), npm run check-package (exit 0, 206 files), git diff --check clean; CI's upstream job is green against the pinned snapshot.
  • Content safety: all added lines scanned for injection/exfiltration/exec patterns — clean. The only URL is a placeholder github.com/<owner>/<repo>/pull/<number> template.

🔴 Major

  1. The forbidden-artifact regex is evaded by backticks — and live violations exist in the tree (scripts/skill-integrity.test.mjs:40). The pattern /worker type:\s*generalPurpose/i cannot match the on-disk form `worker type`: `generalPurpose`, which survives in skills/why/SKILL.md:80,124 (a file this PR edits), skills/how/SKILL.md:23,33,43, skills/interrogate/SKILL.md:42, and skills/no-comments/SKILL.md:18 — while npm test is green. The PR body claims vendor wording was removed "from why", yet its delegation spec survives there. Fix: normalize backticks/whitespace before matching, e.g.

    /worker type`?\s*:\s*`?generalPurpose/i
  2. The integrity scan skips playbooks/ and references/, so the exact artifact class this PR fixes survives one level down: skills/meta-mode/playbooks/worktree-cleanup.md:10 still reads `~/Library/Application Support/the current harness` (`state.vscdb.backup`…) — a mechanical VS Code→"the current harness" replacement that is nonsense on any other harness. references/capability-matrix.md:24-25 and references/bugbot-triage.md also carry Cursor naming unguarded. Fix: walk skills/**/*.md in the scan.

  3. A missing SKILL.md passes vacuously (scripts/skill-integrity.test.mjs:45): the existsSync filter silently drops any skill directory whose SKILL.md vanished — verified empirically: rm skills/tdd/SKILL.md → tests still green. Assert the scanned-file count equals the number of skill directories, and treat a dir without SKILL.md as a failure.

  4. The deleted "Resolve installed paths" contract is still load-bearing (skills/meta-mode/SKILL.md). The PR removed the section defining <mstack-skills> / <meta-mode-tools>, but 7 adapted playbooks still instruct agents to run commands containing those placeholders: playbooks/babysit.md:12, shipping.md:14, orchestrate.md:15,23, multi-phase-plan.md:10, worktree-cleanup.md:5, autopilot-full.md, autopilot-stack.md. Upstream doesn't need the section (its tools live in-tree); mstack's adapted playbooks do. Re-add an adapted resolution section (or point at README's artifact mapping agent-facing).

🟡 Minor

  1. The mode activation/exit gate was dropped entirely — neither upstream's frontmatter gate nor origin/main's body version ("the mode stays active for the current task; do not apply it to a casual turn or after the user opts out") survived, so the restored body reads as unconditional always-on. Two sentences restore the contract.
  2. skills/meta-mode/references/capability-matrix.md is now orphaned — its only inbound reference ("Map capabilities") was removed in this diff, yet it holds exactly the native-vs-fallback delegation table that the new "the active Harness's documented delegation mechanism" phrasing needs a lookup for. Re-link it from the Subagents section, or delete the file.
  3. Residual vendor specifics: swarm/SKILL.md:31 still mandates cloud_base_branch one line below the neutralized text (parameter defined nowhere else in the repo); reflect/SKILL.md:38 still says reviewers return findings "in the Task response body" (Claude Code delegation-tool name); and the Subagents rewrite silently dropped upstream's difficulty-based model-tiering guidance — the claim is terminology adaptation, not content removal.
  4. scripts/check-upstream.mjs exits 0 on a bare run with no --source/MSTACK_PSTACK_SOURCE — a vacuous pass that could masquerade as verification. Making the no-source exit non-zero under --strict would close it.
  5. main is unprotected (branch protection API returns 404), so CI checks — including this new test — are advisory rather than gating.

📦 Release sync (per AGENTS.md)

No version bump in this PR (0.2.0 → 0.2.0; only the test script line in package.json changed), so no Release is required at merge — consistent with repo convention (PRs #2–#4 all shipped under 0.2.0, single v0.2.0 release synced with the npm publish). Note that skills/ and scripts/ ship in the tarball, so after merge main drifts from the published 0.2.0: before the next npm publish, bump to 0.2.1 and create the v0.2.1 GitHub Release in the same motion.


Suggested priority before merge: #1–#3 (small test hardening plus two content edits) and #4 (or a conscious decision to defer with a tracking issue).

@3metaJun

Copy link
Copy Markdown
Owner Author

[gpt-6-codex] RESPONDING ON BEHALF OF 3metajun

Verified and fixed the actionable findings in b36ef68.

  • The integrity check now normalizes fenced terms, scans every Markdown file under skills/, and fails when any skill directory lacks SKILL.md.
  • meta-mode now includes the activation gate, capability map link, and installed-path resolution contract.
  • Vendor-specific delegation wording and the broken Harness state path were removed. Difficulty-based model selection guidance was restored.
  • check-upstream --strict now exits non-zero when no source checkout is supplied, with a regression test.

The branch-protection observation is repository configuration rather than a code defect, so it remains a maintainer setting outside this PR.

@3metaJun
3metaJun merged commit dae672c into main Sep 11, 2026
9 checks passed
@3metaJun
3metaJun deleted the fix/restore-portable-skill-fidelity branch September 11, 2026 04: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.

1 participant