diff --git a/.gitignore b/.gitignore index bdff0377..af795a57 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ docs/plans/ # Bundled skill helpers (generated by the agents build) packages/agents/content/skills/kb-add/kb-add.mjs packages/agents/content/skills/kb-retrieve/kb-retrieve.mjs +packages/agents/content/skills/update-jira-ticket/update-jira-ticket.mjs # Credentials *.pem diff --git a/packages/agents/.prettierignore b/packages/agents/.prettierignore index 9c4fd0d6..5da019d8 100644 --- a/packages/agents/.prettierignore +++ b/packages/agents/.prettierignore @@ -1,9 +1,10 @@ coverage/ dist/ -# Generated esbuild bundles of the kb-add and kb-retrieve helpers, not authored source. +# Generated esbuild bundles of skill helpers, not authored source. content/skills/kb-add/kb-add.mjs content/skills/kb-retrieve/kb-retrieve.mjs +content/skills/update-jira-ticket/update-jira-ticket.mjs # Test fixtures that are intentionally syntactically malformed YAML. Prettier cannot # parse them, and reformatting would erase the defect they exist to test. diff --git a/packages/agents/content/skills/update-jira-ticket/SKILL.md b/packages/agents/content/skills/update-jira-ticket/SKILL.md index e39f86db..db8d0713 100644 --- a/packages/agents/content/skills/update-jira-ticket/SKILL.md +++ b/packages/agents/content/skills/update-jira-ticket/SKILL.md @@ -1,20 +1,73 @@ --- name: update-jira-ticket -description: 'Use whenever updating a Jira issue description or comment via the update_jira_issue MCP tool. Prevents the recurring INVALID_INPUT failure class by constraining HTML to a narrow allowlist and forbidding named entities, Confluence macros, and file-path mode.' +description: 'Use whenever updating a Jira issue description or comment via the update_jira_issue MCP tool. Runs a pre-flight checker against the HTML payload to catch known triggers of INVALID_INPUT (composition rules, named entities, Confluence macros, multi-line
, disallowed elements) before any MCP round-trip.'
 user-invocable: true
 ---
 
 # Update Jira ticket
 
-Use whenever calling `update_jira_issue` (or `create_jira_issue`) with `description_html` or `comment_html`. The MCP tool advertises a permissive HTML surface, but the payload is converted to Atlassian Document Format (ADF) before persistence and frequently rejects valid-looking HTML with an opaque `INVALID_INPUT` error. This skill prescribes the one path that avoids the known triggers.
+Use whenever calling `update_jira_issue` (or `create_jira_issue`) with `description_html` or `comment_html`. The MCP tool advertises a permissive HTML surface, but the payload is converted to Atlassian Document Format (ADF) before persistence and frequently rejects valid-looking HTML with an opaque `INVALID_INPUT` error. This skill prescribes the one path that avoids the known triggers, backed by a deterministic pre-flight checker.
 
 ## The one correct path
 
 1. **Source content as Markdown.** Prefer a local Markdown artefact when one exists. Otherwise, compose in Markdown first — never author HTML directly.
 2. **Convert Markdown to HTML using only the allowlist below.** Anything outside the allowlist must be omitted or rewritten.
-3. **Pass the HTML inline** to `description_html` or `comment_html`.
-4. **Never pass a file path** to `description_html` / `comment_html`. File-path mode is forbidden — it has been observed to fail with `INVALID_INPUT`.
-5. **Never include `version_message`** as an argument. It is not a parameter of `update_jira_issue` or `create_jira_issue` — including it triggers a validation failure and a wasted retry.
+3. **Run the pre-flight checker against the rendered HTML.** Fix everything it flags, then re-run until it returns `ok: true`. See [Pre-flight checker](#pre-flight-checker) for the contract.
+4. **Pass the HTML inline** to `description_html` or `comment_html`.
+5. **Never pass a file path** to `description_html` / `comment_html`. File-path mode is forbidden — it has been observed to fail with `INVALID_INPUT`.
+6. **Never include `version_message`** as an argument. It is not a parameter of `update_jira_issue` or `create_jira_issue` — including it triggers a validation failure and a wasted retry.
+
+## Pre-flight checker
+
+A bundled helper at `{platform_home_dir}/skills/update-jira-ticket/update-jira-ticket.mjs` validates the rendered HTML against every known failure class. The agent invokes it before every `update_jira_issue` / `create_jira_issue` call.
+
+### Invocation
+
+Pipe the HTML on stdin; the helper writes a JSON result to stdout and exits 0 in both the pass and fail cases (only invocation errors exit non-zero).
+
+```bash
+cat <<'EOF' | node "$(dirname "$SKILL_PATH")/update-jira-ticket.mjs"
+

Your rendered HTML payload here.

+EOF +``` + +Or, when the skill directory is known: + +```bash +cat <<'EOF' | node {platform_home_dir}/skills/update-jira-ticket/update-jira-ticket.mjs +

Your rendered HTML payload here.

+EOF +``` + +### Output + +`ok: true` means the payload passes every rule: + +```json +{ "ok": true } +``` + +`ok: false` carries a `findings` array. Each finding identifies the rule, the offending source snippet, the 1-based line (when derivable), and a suggested fix: + +```json +{ + "ok": false, + "findings": [ + { + "rule": "composition-code-inline-mark", + "snippet": "...", + "line": 12, + "fix": "Move the outside , or drop the inline mark." + } + ] +} +``` + +The rule classes are: `composition-code-inline-mark`, `named-entity`, `confluence-construct`, `pre-multiline`, `disallowed-element`. The same payload may emit multiple findings; fix them all before the next MCP attempt. + +### Acting on findings + +For each finding, apply the suggested fix to the source. Do not invoke `update_jira_issue` / `create_jira_issue` until the checker returns `ok: true`. Findings are not optional — every rule corresponds to a documented `INVALID_INPUT` trigger. ## Allowed elements @@ -22,24 +75,15 @@ Exhaustive list. Nothing else. `h1`, `h2`, `h3`, `h4`, `h5`, `h6`, `p`, `ul`, `ol`, `li`, `strong`, `em`, `code`, `a`, `blockquote`, `hr`, `br`, `table`, `thead`, `tbody`, `tr`, `th`, `td` -**Always strip `` and `` constructs unconditionally.** These are Confluence storage-format extensions: `` for Confluence elements like task lists and structured macros (e.g., ``, ``); `` for resource identifiers (e.g., ``, ``, ``). They have no Jira analogue, and including them produces `INVALID_INPUT`. If you have been working with Confluence content in the same session, audit the payload before sending. +**Always strip `` and `` constructs unconditionally.** These are Confluence storage-format extensions: `` for Confluence elements like task lists and structured macros (e.g., ``, ``); `` for resource identifiers (e.g., ``, ``, ``). They have no Jira analogue, and including them produces `INVALID_INPUT`. If you have been working with Confluence content in the same session, audit the payload before sending — the checker will catch any that slip through. -## Composition rules +## Composition rules (reference) -These constraints govern how individually-allowed elements may be combined. Both elements may be valid on their own; the combination is rejected. +The pre-flight checker enforces these; this section explains why they exist. ### `` combined with other inline marks -Do not apply inline styling to `` content. The following nesting patterns will be rejected, in **either** direction: - -- `X` and `X` -- `X` and `X` -- `X` and `X` -- the same for ``, ``, ``, `` - -The rule is symmetric — flipping the nesting order is not a workaround. - -**Why:** ADF represents inline styling as marks on text nodes, and the `code` mark is mutually exclusive with `strong`, `em`, `link`, `strike`, `underline`, `subsup`. Beyond the schema constraint, applying styling to monospace code has no defensible rendering — code is meant to display literal characters. +`` may not nest with ``, ``, ``, ``, ``, ``, or `` in either direction. ADF represents inline styling as marks on text nodes, and the `code` mark is mutually exclusive with the other inline marks. Applying styling to monospace code has no defensible rendering anyway — code is meant to display literal characters. **Workaround:** Move the `` outside the styling wrapper so the two apply to different text runs, or drop the styling entirely. @@ -56,65 +100,106 @@ The rule is symmetric — flipping the nesting order is not a workaround. ### Multi-line code samples -Do not wrap multi-line content in `
`. The `
` element is omitted from the allowlist entirely — multi-line `
` blocks combining embedded newlines with quoted strings or apostrophes have been observed to trigger `INVALID_INPUT`.
-
-**Why:** ADF's `codeBlock` node accepts plain text, but the converter mishandles the combination of newlines and quote characters inside the `pre` block. Inline `` in `

` survives the same characters, so the `

` wrapper is the differentiator.
+`
` is omitted from the allowlist entirely, and the checker also flags multi-line `
` separately. ADF's `codeBlock` node accepts plain text, but the converter mishandles the combination of newlines and quote characters inside the `pre` block. Inline `` in `

` survives the same characters, so the `

` wrapper is the differentiator.
 
 **Workaround:** Render multi-line code as either multiple `

...

` paragraphs (one per logical line) or a single `

` with `
` separators between lines and inline `` wrapping the code on each line. Single-line code is unchanged — continue to use inline `` inside `

` or `

  • ` as usual. ## Character handling -Use **literal Unicode** in HTML. Do not use named HTML entities outside the three universally-safe ones. +Use **literal Unicode** in HTML. Do not use named HTML entities outside the three universally-safe ones. The checker flags any named entity other than `&`, `<`, `>` in text content. -| Don't write | Write instead | -| ----------- | -------------------------- | -| `—` | `—` (U+2014) | -| `–` | `–` (U+2013) | -| `…` | `…` (U+2026) | -| ` ` | regular space, or `\u00a0` | -| `©` | `©` (U+00A9) | -| `’` | `'` (U+2019) | +| Don't write | Write instead | +| ----------- | ------------------------ | +| `—` | `—` (U+2014) | +| `–` | `–` (U+2013) | +| `…` | `…` (U+2026) | +| ` ` | regular space, or U+00A0 | +| `©` | `©` (U+00A9) | +| `’` | `'` (U+2019) | -Only `&`, `<`, `>` are valid in payload text. `"` and `'` are valid only inside attribute values where they're needed to avoid clashing with the attribute's quote style. +`"` and `'` are valid only inside attribute values where they're needed to avoid clashing with the attribute's quote style. The checker only scans text content for named entities, so legitimate attribute-value uses are not flagged. ## Recovery protocol (backstop) -Use only if `INVALID_INPUT` still fires after following the rules above. +Use only if `INVALID_INPUT` still fires after the pre-flight checker returned `ok: true`. A clean checker result followed by an MCP rejection means the payload triggered an unknown failure class that the checker does not yet catch. + +### 1. Surface the failure to the user + +Do not create a probe ticket silently. Present the situation to the user and let them choose how to proceed. Use the [recommendation-gradient format](../_data/recommendation-gradient.md): + +> Jira rejected this payload and the pre-flight checker found no known issues. This is likely a new failure class. How should I proceed? +> +> 1. ■■□ Probe and bisect: +> ➕ pinpoints the exact failing fragment for a fix or a future checker rule; +> ➖ creates a real ticket tagged `mcp-probe` that needs eventual cleanup. +> 2. ■□□ Show the payload for manual submission: +> ➕ no probe ticket created; you can edit and submit via the Jira UI; +> ➖ no diagnostic captured for future hardening. +> 3. ■□□ Skip ticket creation: +> ➕ no further side effects; +> ➖ the failure class remains unidentified. -1. **Probe.** Send `

    ok

    ` as the entire payload. If this also fails, the problem is call shape, permissions, or the issue itself — not the payload. Stop and report. -2. **Bisect.** If `

    ok

    ` succeeds, the failure is in the payload's content. Bisect the payload (split in half, test each half, recurse) to isolate the smallest fragment that still triggers `INVALID_INPUT`. -3. **Cap retries.** Do not exceed 4 retry attempts beyond the original failure. If the bisection has not converged by then, surface the smallest failing fragment to the user and stop. -4. **Record the failure.** Append a single JSON object (one line, no trailing comma) to `~/ai-artifacts/skill-failures/update-jira-ticket.jsonl`. Create the directory and file if absent. +### 2. If the user picks option 1 (probe and bisect) - Required fields: +a. **Probe.** Create a ticket with `

    ok

    ` as the entire payload. The create call must include the tagging contract below ([Probe-ticket tagging contract](#probe-ticket-tagging-contract)). If the probe also fails, the problem is call shape, permissions, or the issue itself — not the payload. Stop and report. +b. **Bisect.** If the probe succeeds, the failure is in the payload's content. Bisect the payload (split in half, test each half, recurse) to isolate the smallest fragment that still triggers `INVALID_INPUT`. +c. **Cap retries.** Do not exceed 4 retry attempts beyond the original failure. If the bisection has not converged by then, surface the smallest failing fragment to the user and stop. + +### 3. Record the failure + +Regardless of which option the user picked, append a single JSON object (one line, no trailing comma) to `~/ai-artifacts/skill-failures/update-jira-ticket.jsonl`. Create the directory and file if absent. + +Required fields: + +```json +{ + "timestamp": "2026-04-28T03:15:32Z", + "skill": "update-jira-ticket", + "project_slug": "codeassembly", + "failing_fragment": "...", + "notes": "matched known trigger: " +} +``` - ```json - { - "timestamp": "2026-04-28T03:15:32Z", - "skill": "update-jira-ticket", - "project_slug": "codeassembly", - "failing_fragment": "...", - "notes": "matched known trigger: " - } - ``` +- `timestamp`: ISO 8601 UTC. +- `skill`: Literal string `update-jira-ticket`. +- `project_slug`: Basename of the repo root (or whatever convention the agent already uses for artefact paths in this session). +- `failing_fragment`: The smallest payload fragment that reproduced `INVALID_INPUT`. When the user chose option 2 or 3, record the full rejected payload. +- `notes`: Free-form. Name the suspected trigger class if recognisable, otherwise leave empty. + +### Probe-ticket tagging contract + +When (and only when) a probe ticket is created in step 2a, it **must** carry all three markers: + +- **Label:** include `mcp-probe` in the `labels` argument of the create call. +- **Title:** `mcp-probe: {YYYY-MM-DD HH:MM} bisection probe`. Use UTC. +- **Description:** prefix the description with `Auto-created by recovery protocol on {YYYY-MM-DD}; safe to delete.` followed by a blank line and then the probe payload. + +A probe ticket that lacks any of these markers will not be picked up by the cleanup query below and risks polluting the user's backlog indefinitely. + +## Probe-ticket cleanup + +Probe tickets created via the recovery protocol are designed to be swept by a single JQL query: + +``` +project = AND labels = mcp-probe AND created < -1d +``` - - `timestamp`: ISO 8601 UTC. - - `skill`: Literal string `update-jira-ticket`. - - `project_slug`: Basename of the repo root (or whatever convention the agent already uses for artefact paths in this session). - - `failing_fragment`: The smallest payload fragment that reproduced `INVALID_INPUT`. - - `notes`: Free-form. Name the suspected trigger class if recognisable, otherwise leave empty. +Run this query periodically and bulk-transition any matches to a closed/deleted state. Probe tickets created before this skill version went live will not carry the `mcp-probe` label and must be cleaned up by hand. ## Escalation criterion -If recorded failures concentrate in **known trigger classes** (named entities, ``, file-path mode) at frequency that costs real iterations, file a follow-up to add a deterministic sanitiser script to this skill — see [#467](https://github.com/williamthorsen/codeassembly/issues/467) for the prior decision and [#468](https://github.com/williamthorsen/codeassembly/issues/468) for the generic-logging follow-up. +If recorded failures concentrate in a **new trigger class** that the checker does not currently cover, file a follow-up to add a rule for it. The rule list in `rules.ts` is the canonical inventory of what the checker catches; extending it is the right unit of escalation. -If failures distribute across **unknown classes** (no clear pattern), the recovery protocol remains the right tool. A sanitiser would not help, since it can only enforce known rules. +If recorded failures distribute across truly **unknown classes** (no clear pattern), the recovery protocol remains the right tool. See [#467](https://github.com/williamthorsen/codeassembly/issues/467) for the prior decision context and [#468](https://github.com/williamthorsen/codeassembly/issues/468) for the generic-logging follow-up. ## Antipatterns +- Skipping the pre-flight check before invoking `update_jira_issue` / `create_jira_issue`. +- Creating a probe ticket without the `mcp-probe` label, the deterministic title, and the description prefix. - Hand-authoring HTML containing constructs outside the allowlist. -- Combining `` with other inline marks on the same text run — see [Composition rules](#composition-rules). -- Using named HTML entities other than `&`, `<`, `>`. +- Combining `` with other inline marks on the same text run. +- Using named HTML entities other than `&`, `<`, `>` in text content. - Passing a file path to `description_html` / `comment_html`. - Retrying past the 4-retry cap. -- Skipping the failure record after a recovery — this removes the evidence needed to decide whether to escalate. +- Skipping the failure record after a recovery — this removes the evidence needed to extend the checker. diff --git a/packages/agents/eslint.config.js b/packages/agents/eslint.config.js index e32ffcc8..86f422f6 100644 --- a/packages/agents/eslint.config.js +++ b/packages/agents/eslint.config.js @@ -4,10 +4,11 @@ import baseConfig from '../../eslint.config.js'; export default [ ...baseConfig, - // `content/skills/kb-{add,retrieve}/kb-*.mjs` are generated esbuild bundles, not authored source. + // Generated esbuild bundles, not authored source. globalIgnores([ 'content/skills/_platforms/**', 'content/skills/kb-add/kb-add.mjs', 'content/skills/kb-retrieve/kb-retrieve.mjs', + 'content/skills/update-jira-ticket/update-jira-ticket.mjs', ]), ]; diff --git a/packages/agents/scripts/bundle-skill-helpers.ts b/packages/agents/scripts/bundle-skill-helpers.ts index 630d105e..5d0a5b1d 100644 --- a/packages/agents/scripts/bundle-skill-helpers.ts +++ b/packages/agents/scripts/bundle-skill-helpers.ts @@ -8,8 +8,9 @@ * The bundle is written into `content/skills/`, so a subsequent `copy-content.ts` carries it into `dist/content/` * and the dev and built layouts both ship the helper. * - * The bundle list is a plain array of `{ entry, outFile }` pairs; - * the sibling kb-add and kb-curate skills extend it by appending an entry. + * The bundle list is a plain array of `BundleTarget` entries; new skills register themselves by appending one. + * Each entry may carry an optional `smokeTest` clause that pipes a specific payload and asserts on the result; + * absent that, the smoke test runs the bundle with no args and empty stdin. */ import path from 'node:path'; import process from 'node:process'; @@ -26,6 +27,18 @@ export interface BundleTarget { entry: string; /** Path to the bundled output, relative to the package root. */ outFile: string; + /** Optional per-bundle smoke-test invocation. When absent, the bundle is run with no args and empty stdin. */ + smokeTest?: SmokeTestInvocation; +} + +/** How the smoke test should invoke a bundle. Stdin is piped only when `stdin` is provided. */ +export interface SmokeTestInvocation { + /** Argv to pass to the bundled `.mjs`. Defaults to no args. */ + args?: readonly string[]; + /** UTF-8 body to pipe on stdin. Defaults to leaving stdin closed (EOF immediately). */ + stdin?: string; + /** Optional structural assertion run against the parsed stdout JSON. Throw to signal failure. */ + assertResult?: (result: unknown) => void; } /** Every skill helper bundle; the smoke test reuses this list to exercise each built `.mjs`. */ @@ -38,8 +51,39 @@ export const targets: BundleTarget[] = [ entry: 'src/kb-retrieve/cli.ts', outFile: 'content/skills/kb-retrieve/kb-retrieve.mjs', }, + { + entry: 'src/update-jira-ticket/cli.ts', + outFile: 'content/skills/update-jira-ticket/update-jira-ticket.mjs', + smokeTest: { + stdin: '

    x

    ', + assertResult: assertCompositionViolationFinding, + }, + }, ]; +/** Type guard: narrows `value` to a plain object with unknown property values. */ +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +/** Assert the parsed smoke-test result reports a composition-code-inline-mark finding. */ +function assertCompositionViolationFinding(result: unknown): void { + if (!isRecord(result)) { + throw new TypeError('expected object result'); + } + if (result.ok !== false) { + throw new Error(`expected ok: false, got ${JSON.stringify(result.ok)}`); + } + const findings = result.findings; + if (!Array.isArray(findings) || findings.length === 0) { + throw new Error('expected non-empty findings array'); + } + const rules = findings.map((entry: unknown) => (isRecord(entry) ? entry.rule : undefined)); + if (!rules.includes('composition-code-inline-mark')) { + throw new Error(`expected composition-code-inline-mark finding; got rules: ${JSON.stringify(rules)}`); + } +} + // A CommonJS dependency (`yaml`) reaches Node built-ins via bare `require('process')` calls. // esbuild's ESM output otherwise has no `require`, so this banner restores a real one via `createRequire`. const requireShim = diff --git a/packages/agents/scripts/smoke-test-skill-helpers.ts b/packages/agents/scripts/smoke-test-skill-helpers.ts index 4cdfeae6..68973cbb 100644 --- a/packages/agents/scripts/smoke-test-skill-helpers.ts +++ b/packages/agents/scripts/smoke-test-skill-helpers.ts @@ -1,30 +1,34 @@ /** * Post-build smoke test: Build every skill helper bundle and run each `.mjs` under `node`, asserting it exits 0 and - * prints valid JSON to stdout. + * prints valid JSON to stdout. Targets may provide a `smokeTest` clause to pipe a specific payload and assert on the + * parsed result; targets without one are exercised with no args and empty stdin (the deterministic, side-effect-free + * baseline). * * Unit tests run the TypeScript source through vitest and never exercise the bundled artifact. The bundle carries a * `createRequire` banner, the `format: 'esm'` option, and the `conditions: ['source']` resolution setting; a * regression to any of them would crash the installed helper at load time, undetected by the unit suite. * This test runs the built bundle exactly as an installed skill would. */ -import { execFile } from 'node:child_process'; +import { spawn } from 'node:child_process'; import path from 'node:path'; import process from 'node:process'; -import { promisify } from 'node:util'; -import { bundleSkillHelpers, packageRoot, targets } from './bundle-skill-helpers.ts'; - -const execFileAsync = promisify(execFile); +import { + bundleSkillHelpers, + type BundleTarget, + packageRoot, + type SmokeTestInvocation, + targets, +} from './bundle-skill-helpers.ts'; await bundleSkillHelpers(); let failed = false; for (const target of targets) { - const bundlePath = path.join(packageRoot, target.outFile); try { - // An empty argv yields the `no query provided` diagnostic — a deterministic, side-effect-free run. - const { stdout } = await execFileAsync(process.execPath, [bundlePath]); - JSON.parse(stdout); + const stdout = await runBundle(target); + const parsed: unknown = JSON.parse(stdout); + target.smokeTest?.assertResult?.(parsed); console.info(`Smoke test passed: ${target.outFile} exits 0 with valid JSON.`); } catch (error) { failed = true; @@ -36,3 +40,33 @@ for (const target of targets) { if (failed) { process.exitCode = 1; } + +/** Run the built bundle for `target` under node, returning its stdout. Throws on non-zero exit. */ +async function runBundle(target: BundleTarget): Promise { + const bundlePath = path.join(packageRoot, target.outFile); + const invocation: SmokeTestInvocation = target.smokeTest ?? {}; + const args = invocation.args ?? []; + + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [bundlePath, ...args]); + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + child.stdout.on('data', (chunk: Buffer) => stdoutChunks.push(chunk)); + child.stderr.on('data', (chunk: Buffer) => stderrChunks.push(chunk)); + child.on('error', reject); + child.on('close', (code) => { + const stdout = Buffer.concat(stdoutChunks).toString('utf8'); + const stderr = Buffer.concat(stderrChunks).toString('utf8'); + if (code !== 0) { + reject(new Error(`exited with code ${code}; stderr: ${stderr.trim()}`)); + return; + } + resolve(stdout); + }); + + if (invocation.stdin !== undefined) { + child.stdin.write(invocation.stdin); + } + child.stdin.end(); + }); +} diff --git a/packages/agents/src/update-jira-ticket/__tests__/check.test.ts b/packages/agents/src/update-jira-ticket/__tests__/check.test.ts new file mode 100644 index 00000000..f0e99048 --- /dev/null +++ b/packages/agents/src/update-jira-ticket/__tests__/check.test.ts @@ -0,0 +1,210 @@ +import { describe, expect, it } from 'vitest'; + +import { check } from '../check.ts'; +import type { Finding, RuleId } from '../types.ts'; + +/** Convenience: assert that `check(html)` returns `ok: false` and includes a finding whose `rule` is `ruleId`. */ +function expectFinding(html: string, ruleId: RuleId): Finding { + const result = check(html); + if (result.ok) { + throw new Error(`Expected ok: false with rule ${ruleId}, got ok: true for: ${html}`); + } + const finding = result.findings.find((entry) => entry.rule === ruleId); + if (!finding) { + const seen = result.findings.map((entry) => entry.rule).join(', '); + throw new Error(`Expected finding with rule ${ruleId}; got: [${seen}]`); + } + return finding; +} + +/** Convenience: assert that `check(html)` returns `ok: true`. Fails with the seen findings on mismatch. */ +function expectClean(html: string): void { + const result = check(html); + if (!result.ok) { + const seen = result.findings.map((entry) => `${entry.rule}: ${entry.snippet}`).join('; '); + throw new Error(`Expected ok: true; got findings: [${seen}]`); + } +} + +describe(check, () => { + describe('clean payloads', () => { + it('accepts a simple paragraph', () => { + expectClean('

    Hello, world.

    '); + }); + + it('accepts every allowlisted element in combination', () => { + const html = ` +

    Title

    +

    Subtitle

    +

    Paragraph with bold, italic, and a link.

    +
    • One
    • Two
    +
    1. First
    2. Second
    +

    Quoted.

    +
    +

    Inline literal works.

    +
    H
    D
    +

    Line one.
    Line two.

    + `; + expectClean(html); + }); + + it('accepts the three universally-safe named entities', () => { + expectClean('

    A & B, X < Y, P > Q.

    '); + }); + + it('accepts literal Unicode characters in text', () => { + expectClean('

    An em-dash — here, an ellipsis … there, a nbsp gap.

    '); + }); + }); + + describe('composition-code-inline-mark', () => { + it('flags ', () => { + const finding = expectFinding('

    x

    ', 'composition-code-inline-mark'); + expect(finding.snippet).toContain(''); + }); + + it('flags the reverse nesting ', () => { + expectFinding('

    x

    ', 'composition-code-inline-mark'); + }); + + it('flags ', () => { + expectFinding('

    x

    ', 'composition-code-inline-mark'); + }); + + it('flags ', () => { + expectFinding('

    x

    ', 'composition-code-inline-mark'); + }); + + it('flags
    ', () => { + expectFinding('

    x

    ', 'composition-code-inline-mark'); + }); + + it('flags ', () => { + expectFinding('

    x

    ', 'composition-code-inline-mark'); + }); + + it('does not flag sibling and ', () => { + expectClean('

    Bold then code.

    '); + }); + + it('does not flag alone or alone', () => { + expectClean('

    x and y

    '); + }); + }); + + describe('named-entity', () => { + it('flags —', () => { + const finding = expectFinding('

    A—B

    ', 'named-entity'); + expect(finding.snippet).toBe('—'); + }); + + it('flags  ', () => { + expectFinding('

    A B

    ', 'named-entity'); + }); + + it('flags …', () => { + expectFinding('

    Wait…

    ', 'named-entity'); + }); + + it('emits one finding per occurrence', () => { + const result = check('

    — —  

    '); + if (result.ok) throw new Error('expected findings'); + const entityFindings = result.findings.filter((entry) => entry.rule === 'named-entity'); + expect(entityFindings).toHaveLength(3); + }); + + it('does not flag &, <, > in text', () => { + expectClean('

    & and < and >

    '); + }); + + it('does not flag " or ' inside an attribute value', () => { + expectClean('

    link

    '); + }); + }); + + describe('confluence-construct', () => { + it('flags ', () => { + const finding = expectFinding('

    x

    ', 'confluence-construct'); + expect(finding.snippet).toContain('ac:task-list'); + }); + + it('flags ', () => { + expectFinding('

    hi

    ', 'confluence-construct'); + }); + + it('flags with attributes', () => { + expectFinding('

    x

    y', 'confluence-construct'); + }); + + it('does not flag tags that merely start with the letters a/r', () => { + expectClean('

    link

    '); + }); + }); + + describe('pre-multiline', () => { + it('flags
     containing a newline', () => {
    +      expectFinding('
    line one\nline two
    ', 'pre-multiline'); + }); + + it('flags
     containing a newline (the pre is still the trigger)', () => {
    +      expectFinding('
    line one\nline two
    ', 'pre-multiline'); + }); + + it('does not flag inline containing a newline (no
     wrapper)', () => {
    +      const result = check('

    line one\nline two

    '); + if (result.ok) return; + expect(result.findings.find((entry) => entry.rule === 'pre-multiline')).toBeUndefined(); + }); + }); + + describe('disallowed-element', () => { + it('flags
    ', () => { + const finding = expectFinding('
    x
    ', 'disallowed-element'); + expect(finding.snippet).toContain('div'); + }); + + it('flags ', () => { + expectFinding('

    x

    ', 'disallowed-element'); + }); + + it('flags
     as disallowed in addition to flagging the multi-line trigger when both apply', () => {
    +      const result = check('
    line one\nline two
    '); + if (result.ok) throw new Error('expected findings'); + const rules = result.findings.map((entry) => entry.rule); + expect(rules).toContain('disallowed-element'); + expect(rules).toContain('pre-multiline'); + }); + + it('does not flag tag-like substrings inside attribute values (entity form)', () => { + expectClean('

    link

    '); + }); + + it('does not flag literal angle brackets inside quoted attribute values', () => { + // Regression guard for the parser's quote-awareness: literal `` inside a quoted attribute value must not be + // tokenized as a tag, so no `disallowed-element` finding for `` should be produced. + expectClean('

    link

    '); + }); + + it('does not flag any allowlisted tag', () => { + expectClean('
    H
    D
    '); + }); + }); + + describe('documented parser limitations', () => { + // These tests lock in the tokenizer's intentional behavior at known edges; see the parser.ts header. + // If a future change "fixes" any of these, the test will fail and force a conscious doctrine change. + + it('tokenizes tag-shaped content inside HTML comments as real tags (no comment handling)', () => { + // ` after

    ', 'composition-code-inline-mark'); + }); + + it('treats sibling tags after an unclosed ancestor as nested under that ancestor', () => { + // Unclosed `` followed by `` registers the `` as nested under ``, + // firing the composition rule. This is fail-loud on imbalanced input, not a bug. + expectFinding('

    x

    y

    ', 'composition-code-inline-mark'); + }); + }); +}); diff --git a/packages/agents/src/update-jira-ticket/check.ts b/packages/agents/src/update-jira-ticket/check.ts new file mode 100644 index 00000000..902fb012 --- /dev/null +++ b/packages/agents/src/update-jira-ticket/check.ts @@ -0,0 +1,14 @@ +// Pure orchestrator. Tokenizes the input once, runs every rule against the token stream, and assembles the +// discriminated-union payload. No I/O — `cli.ts` is responsible for reading stdin and writing stdout. + +import { tokenize } from './parser.ts'; +import { ALL_RULES } from './rules.ts'; +import type { CheckResult } from './types.ts'; + +/** Validate `html` against every rule. Returns `{ ok: true }` for a clean payload or `{ ok: false, findings }`. */ +export function check(html: string): CheckResult { + const tokens = tokenize(html); + const findings = ALL_RULES.flatMap((rule) => rule(tokens, html)); + if (findings.length === 0) return { ok: true }; + return { ok: false, findings }; +} diff --git a/packages/agents/src/update-jira-ticket/cli.ts b/packages/agents/src/update-jira-ticket/cli.ts new file mode 100644 index 00000000..e0a26d1b --- /dev/null +++ b/packages/agents/src/update-jira-ticket/cli.ts @@ -0,0 +1,64 @@ +/* eslint n/no-process-exit: off */ +/* eslint unicorn/no-process-exit: off */ +// CLI entry point for the update-jira-ticket pre-flight checker. +// +// Reads the HTML payload from stdin, runs `check()`, and writes the discriminated-union result to stdout as +// pretty-printed JSON. Exit 0 for both `ok: true` and `ok: false` (recoverable findings are not system errors); +// exit 1 only when the invocation itself is wrong (unreadable stdin). + +import { realpathSync } from 'node:fs'; +import process from 'node:process'; +import type { Readable } from 'node:stream'; +import { fileURLToPath } from 'node:url'; + +import { check } from './check.ts'; + +/** Read every chunk of `stream` and concatenate into a single UTF-8 string. */ +async function readAll(stream: Readable): Promise { + const chunks: Buffer[] = []; + for await (const chunk of stream) { + if (!Buffer.isBuffer(chunk)) { + throw new TypeError('readAll: expected Buffer chunks (stream must be in binary mode)'); + } + chunks.push(chunk); + } + return Buffer.concat(chunks).toString('utf8'); +} + +/** Top-level entry: read stdin, run the check, emit JSON. */ +async function main(): Promise { + try { + const html = await readAll(process.stdin); + const result = check(html); + process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`update-jira-ticket: ${message}\n`); + process.exit(1); + } +} + +// Run as a CLI when invoked directly; stay importable for tests. +if (isEntryPoint()) { + await main(); +} + +/** + * Returns true when this module is the process entry point. Both sides are resolved through `realpathSync`, so a + * symlinked invocation path (e.g. a `--link` install of the agents skill bundle) still matches. On a `realpathSync` + * failure (broken symlink, permission denied) the function emits a warning to stderr and returns `false`, matching + * the pattern in `src/kb-add/cli.ts` so silent skips do not hide environment problems. + */ +function isEntryPoint(): boolean { + const entry = process.argv[1]; + if (entry === undefined) { + return false; + } + try { + return realpathSync(fileURLToPath(import.meta.url)) === realpathSync(entry); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`update-jira-ticket: warning: could not determine entry point: ${message}\n`); + return false; + } +} diff --git a/packages/agents/src/update-jira-ticket/parser.ts b/packages/agents/src/update-jira-ticket/parser.ts new file mode 100644 index 00000000..7bd58947 --- /dev/null +++ b/packages/agents/src/update-jira-ticket/parser.ts @@ -0,0 +1,263 @@ +// Minimal HTML tokenizer for the pre-flight checker. +// +// Not a conformant HTML5 parser. Just enough to: +// - recognize tag boundaries, including self-closing and namespaced tags like `ac:task-list`, +// - separate text content from attribute values (so named-entity scans skip `"` inside `href="..."`), +// - track source offsets so rules can derive 1-based line numbers. +// +// Malformed input (unbalanced quotes, runaway `<`) is tolerated — the tokenizer prefers progress over +// strictness, since the goal is finding known-bad patterns, not validating that the HTML is well-formed. +// +// Known limitations (intentional; documented here so future contributors don't quietly "fix" them): +// - HTML comments (``), CDATA sections (``), and `` declarations are +// not recognized; their `` followed by a +// sibling `` will register the `` as nested under the `` and fire a composition +// finding. This is fail-loud by design: an imbalanced payload almost certainly indicates a generation +// bug, and surfacing it as a finding is preferable to silently auto-balancing. + +/** A single open-tag token. Self-closing variants like `
    ` or `
    ` set `selfClosing: true`. */ +export interface OpenTagToken { + type: 'open-tag'; + name: string; + /** Original tag name as written (preserves case for snippet display). */ + rawName: string; + attrs: Attribute[]; + selfClosing: boolean; + offset: number; + /** Verbatim source slice covering `<...>` including delimiters. */ + raw: string; +} + +/** A single close-tag token: ``. */ +export interface CloseTagToken { + type: 'close-tag'; + name: string; + rawName: string; + offset: number; + raw: string; +} + +/** Text content between tags. May contain entity references; the tokenizer does not decode them. */ +export interface TextToken { + type: 'text'; + value: string; + offset: number; +} + +export type Token = OpenTagToken | CloseTagToken | TextToken; + +/** A parsed attribute. `value` is `null` for valueless attributes (rare in well-formed HTML). */ +export interface Attribute { + name: string; + value: string | null; +} + +/** Self-closing element names per HTML — used to treat `
    ` and `
    ` as self-closing without `/`. */ +const VOID_ELEMENTS = new Set(['br', 'hr', 'img', 'input', 'meta', 'link', 'area', 'base', 'col', 'embed', 'source']); + +/** Tokenize `html` into a flat stream. Always returns; never throws on malformed input. */ +export function tokenize(html: string): Token[] { + const tokens: Token[] = []; + const length = html.length; + let index = 0; + let textStart = 0; + + while (index < length) { + const char = html[index]; + const nextChar = html[index + 1] ?? ''; + if (char === '<' && isTagStartChar(nextChar)) { + if (index > textStart) { + tokens.push({ type: 'text', value: html.slice(textStart, index), offset: textStart }); + } + const parsed = parseTag(html, index); + tokens.push(parsed.token); + index = parsed.nextIndex; + textStart = index; + } else { + index += 1; + } + } + + if (textStart < length) { + tokens.push({ type: 'text', value: html.slice(textStart, length), offset: textStart }); + } + + return tokens; +} + +/** A tag-start char is `/` (close tag) or an ASCII letter (open tag). */ +function isTagStartChar(char: string): boolean { + return char === '/' || (char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z'); +} + +/** Parse a single tag starting at `html[start]` (which is `<`). Returns the token and the index just past `>`. */ +function parseTag(html: string, start: number): { token: Token; nextIndex: number } { + const isClose = html[start + 1] === '/'; + const nameStart = isClose ? start + 2 : start + 1; + const nameEnd = readWhile(html, nameStart, isNameChar); + const rawName = html.slice(nameStart, nameEnd); + const name = rawName.toLowerCase(); + + if (isClose) { + const closeEnd = readUntilChar(html, nameEnd, '>'); + const end = Math.min(closeEnd + 1, html.length); + return { + token: { type: 'close-tag', name, rawName, offset: start, raw: html.slice(start, end) }, + nextIndex: end, + }; + } + + const { attrs, selfClosing, end: bodyEnd } = parseAttributes(html, nameEnd); + const end = bodyEnd < html.length ? bodyEnd + 1 : html.length; + const effectivelySelfClosing = selfClosing || VOID_ELEMENTS.has(name); + return { + token: { + type: 'open-tag', + name, + rawName, + attrs, + selfClosing: effectivelySelfClosing, + offset: start, + raw: html.slice(start, end), + }, + nextIndex: end, + }; +} + +/** Parse the attribute section of an open tag. Returns parsed attrs, the self-closing flag, and the offset of `>`. */ +function parseAttributes(html: string, startIndex: number): { attrs: Attribute[]; selfClosing: boolean; end: number } { + const length = html.length; + const attrs: Attribute[] = []; + let selfClosing = false; + let index = startIndex; + + while (index < length) { + index = skipWhitespace(html, index); + const char = html[index]; + if (char === undefined || char === '>') break; + if (char === '/') { + selfClosing = true; + index += 1; + continue; + } + + const attrNameStart = index; + index = readWhile(html, index, isAttrNameChar); + if (index === attrNameStart) { + // Defensive: unrecognized char inside a tag — skip one to make progress. + index += 1; + continue; + } + const attrName = html.slice(attrNameStart, index).toLowerCase(); + const { value, next } = parseAttributeValue(html, index); + attrs.push({ name: attrName, value }); + index = next; + } + + return { attrs, selfClosing, end: index }; +} + +/** Read an optional `=value` clause; returns the value (or `null` for valueless attrs) and the next index. */ +function parseAttributeValue(html: string, startIndex: number): { value: string | null; next: number } { + let index = skipWhitespace(html, startIndex); + if (html[index] !== '=') return { value: null, next: index }; + + index = skipWhitespace(html, index + 1); + const quote = html[index]; + if (quote === '"' || quote === "'") { + const valueStart = index + 1; + const valueEnd = readUntilChar(html, valueStart, quote); + const next = valueEnd < html.length ? valueEnd + 1 : html.length; + return { value: html.slice(valueStart, valueEnd), next }; + } + + const valueStart = index; + const valueEnd = readWhile(html, valueStart, (char) => !isWhitespace(char) && char !== '>' && char !== '/'); + return { value: html.slice(valueStart, valueEnd), next: valueEnd }; +} + +/** Advance `index` while the predicate holds for the current char. Returns the index of the first char that fails. */ +function readWhile(html: string, startIndex: number, predicate: (char: string) => boolean): number { + const length = html.length; + let index = startIndex; + while (index < length) { + const char = html[index]; + if (char === undefined || !predicate(char)) break; + index += 1; + } + return index; +} + +/** Advance until the target char is found. Returns its index, or `html.length` if not found. */ +function readUntilChar(html: string, startIndex: number, target: string): number { + const length = html.length; + let index = startIndex; + while (index < length && html[index] !== target) index += 1; + return index; +} + +/** Skip whitespace starting at `startIndex`; returns the index of the first non-whitespace char. */ +function skipWhitespace(html: string, startIndex: number): number { + return readWhile(html, startIndex, isWhitespace); +} + +/** Tag-name chars: ASCII letters, digits, `:` (namespaces), `-` (custom elements). */ +function isNameChar(char: string): boolean { + return ( + (char >= 'a' && char <= 'z') || + (char >= 'A' && char <= 'Z') || + (char >= '0' && char <= '9') || + char === ':' || + char === '-' + ); +} + +/** Attribute names are more permissive than tag names — letters, digits, `-`, `_`, `:`. */ +function isAttrNameChar(char: string): boolean { + return ( + (char >= 'a' && char <= 'z') || + (char >= 'A' && char <= 'Z') || + (char >= '0' && char <= '9') || + char === '-' || + char === '_' || + char === ':' + ); +} + +function isWhitespace(char: string): boolean { + return char === ' ' || char === '\t' || char === '\n' || char === '\r' || char === '\f'; +} + +/** Derive a 1-based line number from a byte offset into the original source. */ +export function lineOf(source: string, offset: number): number { + let line = 1; + for (let i = 0; i < offset && i < source.length; i += 1) { + if (source[i] === '\n') line += 1; + } + return line; +} + +/** Visitor signature for {@link walkTokens}. Receives the current open-tag token and its parent stack. */ +export type Visitor = (token: OpenTagToken, parents: readonly OpenTagToken[]) => void; + +/** Walk `tokens` and call `visit` for each open tag, exposing the chain of currently-open ancestors. */ +export function walkTokens(tokens: readonly Token[], visit: Visitor): void { + const stack: OpenTagToken[] = []; + for (const token of tokens) { + if (token.type === 'open-tag') { + visit(token, stack); + if (!token.selfClosing) stack.push(token); + } else if (token.type === 'close-tag') { + // Pop the matching open tag if present; tolerate mismatches by popping the innermost match. + for (let i = stack.length - 1; i >= 0; i -= 1) { + if (stack[i]?.name === token.name) { + stack.length = i; + break; + } + } + } + } +} diff --git a/packages/agents/src/update-jira-ticket/rules.ts b/packages/agents/src/update-jira-ticket/rules.ts new file mode 100644 index 00000000..1d7d44c5 --- /dev/null +++ b/packages/agents/src/update-jira-ticket/rules.ts @@ -0,0 +1,179 @@ +// One exported function per rule class. Each takes the tokenized payload (and the original source for line +// derivation) and returns zero or more findings. The rule IDs are stable across versions — the skill body and +// tests both reference them by string. + +import { lineOf, type Token, walkTokens } from './parser.ts'; +import type { Finding } from './types.ts'; + +/** Allowlisted element names. Mirrors the SKILL.md allowlist exactly; this is the canonical source of truth. */ +export const ALLOWED_ELEMENTS = new Set([ + 'h1', + 'h2', + 'h3', + 'h4', + 'h5', + 'h6', + 'p', + 'ul', + 'ol', + 'li', + 'strong', + 'em', + 'code', + 'a', + 'blockquote', + 'hr', + 'br', + 'table', + 'thead', + 'tbody', + 'tr', + 'th', + 'td', +]); + +/** Inline-mark elements that may not nest with `` in either direction. */ +const INLINE_MARK_ELEMENTS = new Set(['strong', 'em', 'a', 'strike', 'u', 'sub', 'sup']); + +/** Universally-safe HTML entities. Anything else in text content is flagged by {@link namedEntityRule}. */ +const SAFE_TEXT_ENTITIES = new Set(['amp', 'lt', 'gt']); + +/** + * Flag any `` whose direct or transitive open ancestor is an inline-mark element, and vice versa. + * Both nesting directions are caught because each tag is visited as it opens and its parent stack is + * inspected. + */ +export function compositionCodeInlineMarkRule(tokens: readonly Token[], source: string): Finding[] { + const findings: Finding[] = []; + walkTokens(tokens, (token, parents) => { + if (token.name === 'code') { + const offender = parents.find((parent) => INLINE_MARK_ELEMENTS.has(parent.name)); + if (offender) { + findings.push({ + rule: 'composition-code-inline-mark', + snippet: `${offender.raw}...${token.raw}`, + line: lineOf(source, offender.offset), + fix: `Move the outside <${offender.name}>, or drop the inline mark.`, + }); + } + } else if (INLINE_MARK_ELEMENTS.has(token.name)) { + const offender = parents.find((parent) => parent.name === 'code'); + if (offender) { + findings.push({ + rule: 'composition-code-inline-mark', + snippet: `${offender.raw}...${token.raw}`, + line: lineOf(source, offender.offset), + fix: `Move <${token.name}> outside the , or drop the inline mark.`, + }); + } + } + }); + return findings; +} + +/** + * Flag named HTML entities other than `&`, `<`, `>` that appear in text content. + * Attribute values are not scanned, because that's where `"` and `'` legitimately live — + * the tokenizer separates text from attribute values so this rule never sees them. + */ +export function namedEntityRule(tokens: readonly Token[], source: string): Finding[] { + const findings: Finding[] = []; + const pattern = /&([a-zA-Z][a-zA-Z0-9]*);/g; + for (const token of tokens) { + if (token.type !== 'text') continue; + for (const match of token.value.matchAll(pattern)) { + const entityName = match[1]; + if (entityName === undefined || SAFE_TEXT_ENTITIES.has(entityName)) continue; + const offset = token.offset + match.index; + findings.push({ + rule: 'named-entity', + snippet: match[0], + line: lineOf(source, offset), + fix: `Replace with literal Unicode (e.g., — for —, … for …, a regular space for  ).`, + }); + } + } + return findings; +} + +/** Flag any `` or `` element (Confluence storage-format constructs). */ +export function confluenceConstructRule(tokens: readonly Token[], source: string): Finding[] { + const findings: Finding[] = []; + for (const token of tokens) { + if (token.type !== 'open-tag') continue; + if (token.name.startsWith('ac:') || token.name.startsWith('ri:')) { + findings.push({ + rule: 'confluence-construct', + snippet: token.raw, + line: lineOf(source, token.offset), + fix: `Remove the <${token.rawName}> construct. Confluence storage-format elements have no Jira analogue.`, + }); + } + } + return findings; +} + +/** + * Flag any `
    ` element whose contents (until its matching close tag) contain a newline. Inline ``
    + * with the same text is not flagged, because the trigger is the `
    ` wrapper, not the newline alone.
    + */
    +export function preMultilineRule(tokens: readonly Token[], source: string): Finding[] {
    +  const findings: Finding[] = [];
    +  for (const [i, token] of tokens.entries()) {
    +    if (token.type !== 'open-tag' || token.name !== 'pre') continue;
    +    if (hasNewlineInPre(tokens, i)) {
    +      findings.push({
    +        rule: 'pre-multiline',
    +        snippet: token.raw,
    +        line: lineOf(source, token.offset),
    +        fix: 'Replace the multi-line 
     with per-line 

    ...

    , or a single

    using
    separators.', + }); + } + } + return findings; +} + +/** Scan from the `

    ` open tag at `tokens[startIndex]` until its matching close, returning true on a newline. */
    +function hasNewlineInPre(tokens: readonly Token[], startIndex: number): boolean {
    +  let depth = 1;
    +  for (let j = startIndex + 1; j < tokens.length; j += 1) {
    +    const inner = tokens[j];
    +    if (inner === undefined) break;
    +    if (inner.type === 'open-tag' && inner.name === 'pre' && !inner.selfClosing) {
    +      depth += 1;
    +    } else if (inner.type === 'close-tag' && inner.name === 'pre') {
    +      depth -= 1;
    +      if (depth === 0) return false;
    +    } else if (inner.type === 'text' && inner.value.includes('\n')) {
    +      return true;
    +    }
    +  }
    +  return false;
    +}
    +
    +/** Flag any open tag whose name is not in {@link ALLOWED_ELEMENTS} and not a ``/`` construct. */
    +export function disallowedElementRule(tokens: readonly Token[], source: string): Finding[] {
    +  const findings: Finding[] = [];
    +  for (const token of tokens) {
    +    if (token.type !== 'open-tag') continue;
    +    if (ALLOWED_ELEMENTS.has(token.name)) continue;
    +    // Confluence constructs are handled by their own rule; don't double-report under disallowed-element.
    +    if (token.name.startsWith('ac:') || token.name.startsWith('ri:')) continue;
    +    findings.push({
    +      rule: 'disallowed-element',
    +      snippet: token.raw,
    +      line: lineOf(source, token.offset),
    +      fix: `Remove or replace <${token.rawName}>. The allowlist is documented in update-jira-ticket/SKILL.md.`,
    +    });
    +  }
    +  return findings;
    +}
    +
    +/** All rule functions in deterministic order. The order shapes the output ordering when multiple rules fire. */
    +export const ALL_RULES: ReadonlyArray<(tokens: readonly Token[], source: string) => Finding[]> = [
    +  compositionCodeInlineMarkRule,
    +  namedEntityRule,
    +  confluenceConstructRule,
    +  preMultilineRule,
    +  disallowedElementRule,
    +];
    diff --git a/packages/agents/src/update-jira-ticket/types.ts b/packages/agents/src/update-jira-ticket/types.ts
    new file mode 100644
    index 00000000..73462e21
    --- /dev/null
    +++ b/packages/agents/src/update-jira-ticket/types.ts
    @@ -0,0 +1,24 @@
    +// Shapes for the update-jira-ticket pre-flight checker.
    +//
    +// The checker's stdout payload is a discriminated union on `ok`. Clean payloads return `{ ok: true }`;
    +// payloads with detectable problems return `{ ok: false, findings: [...] }`. Both exit 0 — only invocation
    +// errors (unreadable stdin, unknown flag) exit non-zero with a stderr message.
    +
    +/** Stable identifier for a rule class. The skill body, tests, and stdout payload all reference these. */
    +export type RuleId =
    +  | 'composition-code-inline-mark'
    +  | 'named-entity'
    +  | 'confluence-construct'
    +  | 'pre-multiline'
    +  | 'disallowed-element';
    +
    +/** A single rule violation. `line` is best-effort (1-based, from the offset of the offending construct). */
    +export interface Finding {
    +  rule: RuleId;
    +  snippet: string;
    +  line?: number;
    +  fix: string;
    +}
    +
    +/** Payload emitted to stdout. Clean payloads have no `findings` field. */
    +export type CheckResult = { ok: true } | { ok: false; findings: Finding[] };