-
Notifications
You must be signed in to change notification settings - Fork 0
fix(cli): ship every persona-authoring kit in the CLI install tree #327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { readFileSync, readdirSync } from 'node:fs'; | ||
| import test from 'node:test'; | ||
|
|
||
| /** | ||
| * A persona is authored as `persona.ts` and compiled by the CLI, which resolves | ||
| * the file's imports out of the CLI's own install tree — the user's repo | ||
| * usually has no `node_modules` at all when the CLI is installed globally. | ||
| * | ||
| * So every kit a persona.ts can import has to ship inside that tree, or | ||
| * `agentworkforce deploy ./persona.ts` dies with "Could not resolve" on a fresh | ||
| * install. persona-kit was already there; turn-kit and review-kit were not, | ||
| * which is the same failure workforce#325 fixed for persona-kit. | ||
| */ | ||
| function packageJson(dir) { | ||
| return JSON.parse(readFileSync(`packages/${dir}/package.json`, 'utf8')); | ||
| } | ||
|
|
||
| function personaAuthoringKits() { | ||
| const kits = []; | ||
| for (const dir of readdirSync('packages')) { | ||
| let sources; | ||
| try { | ||
| sources = readdirSync(`packages/${dir}/src`, { recursive: true }); | ||
| } catch { | ||
| continue; // not a source package | ||
| } | ||
| const authorsPersonas = sources.some((file) => { | ||
| if (!file.endsWith('.ts') || file.endsWith('.test.ts')) return false; | ||
| const source = readFileSync(`packages/${dir}/src/${file}`, 'utf8'); | ||
| // `function`, `const`, `async function`, and nested files all count — a | ||
| // kit that hides from this check is a kit that breaks on a fresh install. | ||
| return /export (?:async )?(?:function|const) define\w*Persona\b/.test(source); | ||
| }); | ||
| if (authorsPersonas) kits.push(packageJson(dir).name); | ||
| } | ||
| return kits.sort(); | ||
| } | ||
|
|
||
| test('every persona-authoring kit ships in the CLI install tree', () => { | ||
| const kits = personaAuthoringKits(); | ||
| // Guard against the discovery silently finding nothing and passing vacuously. | ||
| assert.ok( | ||
| kits.includes('@agentworkforce/persona-kit'), | ||
| `discovery failed to find the authoring kits (found: ${kits.join(', ') || 'none'})` | ||
| ); | ||
|
|
||
| const cli = packageJson('cli'); | ||
| for (const kit of kits) { | ||
| assert.ok( | ||
| cli.dependencies?.[kit], | ||
| `${kit} exports a define*Persona entry point, so a persona.ts can import it, ` + | ||
| 'but @agentworkforce/cli does not depend on it — it will not be installed ' + | ||
| 'alongside the CLI and the persona will fail to compile on a fresh install' | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| test('authoring kits are published in lockstep with the CLI', () => { | ||
| const publishWorkflow = readFileSync('.github/workflows/publish.yml', 'utf8'); | ||
| const targets = publishWorkflow.match(/echo "packages=([^"]+)"/); | ||
| // Without this the reformatted-workflow case throws an unrelated TypeError | ||
| // and reads as a broken test rather than a broken workflow. | ||
| assert.ok(targets, 'publish workflow must declare its package targets'); | ||
| const published = new Set( | ||
| targets[1].trim().split(/\s+/).map((dir) => packageJson(dir).name) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Compare package versions as well as names, and assert that the CLI target is present. Mapping each target to Prompt for AI agents |
||
| ); | ||
|
|
||
| for (const kit of personaAuthoringKits()) { | ||
| assert.ok(published.has(kit), `${kit} must publish with the CLI to stay version-matched`); | ||
| } | ||
|
Comment on lines
+59
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Assert the actual lockstep invariant. This test only checks that each kit name appears in the workflow target set. It does not verify that 🤖 Prompt for AI Agents |
||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The comment claims a complete guarantee — “a kit that hides from this check is a kit that breaks on a fresh install” — but the regex only matches same-line
export function|const defineXPersona. A kit that declares its persona locally and re-exports it (const defineTurnPersona = …; export { defineTurnPersona };), usesexport default defineTurnPersona, or assigns a named-but-block-scoped definition will not match, so it silently skips the “must ship in the CLI tree” check and breaks fresh installs — the exact failure this test exists to prevent. Consider matching the definition declaration independently of the export keyword (e.g. also test for(?:function|const)\s+define\w*Personain the source) so the check survives these common export idioms.Prompt for AI agents