#643 Add update-jira-ticket pre-flight validator and rework recovery protocol

.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

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.

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',
   ]),
 ];

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: '<p><strong><code>x</code></strong></p>',
+      assertResult: assertCompositionViolationFinding,
+    },
+  },
 ];
 
+/** Type guard: narrows `value` to a plain object with unknown property values. */
+function isRecord(value: unknown): value is Record<string, unknown> {
+  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 =

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<string> {
+  const bundlePath = path.join(packageRoot, target.outFile);
+  const invocation: SmokeTestInvocation = target.smokeTest ?? {};
+  const args = invocation.args ?? [];
+
+  return new Promise<string>((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();
+  });
+}

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('<p>Hello, world.</p>');
+    });
+
+    it('accepts every allowlisted element in combination', () => {
+      const html = `
+        <h1>Title</h1>
+        <h2>Subtitle</h2>
+        <p>Paragraph with <strong>bold</strong>, <em>italic</em>, and <a href="https://example.com">a link</a>.</p>
+        <ul><li>One</li><li>Two</li></ul>
+        <ol><li>First</li><li>Second</li></ol>
+        <blockquote><p>Quoted.</p></blockquote>
+        <hr>
+        <p>Inline <code>literal</code> works.</p>
+        <table><thead><tr><th>H</th></tr></thead><tbody><tr><td>D</td></tr></tbody></table>
+        <p>Line one.<br>Line two.</p>
+      `;
+      expectClean(html);
+    });
+
+    it('accepts the three universally-safe named entities', () => {
+      expectClean('<p>A &amp; B, X &lt; Y, P &gt; Q.</p>');
+    });
+
+    it('accepts literal Unicode characters in text', () => {
+      expectClean('<p>An em-dash — here, an ellipsis … there, a nbsp gap.</p>');
+    });
+  });
+
+  describe('composition-code-inline-mark', () => {
+    it('flags <strong><code>', () => {
+      const finding = expectFinding('<p><strong><code>x</code></strong></p>', 'composition-code-inline-mark');
+      expect(finding.snippet).toContain('<code>');
+    });
+
+    it('flags the reverse nesting <code><strong>', () => {
+      expectFinding('<p><code><strong>x</strong></code></p>', 'composition-code-inline-mark');
+    });
+
+    it('flags <em><code>', () => {
+      expectFinding('<p><em><code>x</code></em></p>', 'composition-code-inline-mark');
+    });
+
+    it('flags <code><em>', () => {
+      expectFinding('<p><code><em>x</em></code></p>', 'composition-code-inline-mark');
+    });
+
+    it('flags <a><code>', () => {
+      expectFinding('<p><a href="https://x.test"><code>x</code></a></p>', 'composition-code-inline-mark');
+    });
+
+    it('flags <code><a>', () => {
+      expectFinding('<p><code><a href="https://x.test">x</a></code></p>', 'composition-code-inline-mark');
+    });
+
+    it('does not flag sibling <code> and <strong>', () => {
+      expectClean('<p><strong>Bold</strong> then <code>code</code>.</p>');
+    });
+
+    it('does not flag <code> alone or <strong> alone', () => {
+      expectClean('<p><code>x</code> and <strong>y</strong></p>');
+    });
+  });
+
+  describe('named-entity', () => {
+    it('flags &mdash;', () => {
+      const finding = expectFinding('<p>A&mdash;B</p>', 'named-entity');
+      expect(finding.snippet).toBe('&mdash;');
+    });
+
+    it('flags &nbsp;', () => {
+      expectFinding('<p>A&nbsp;B</p>', 'named-entity');
+    });
+
+    it('flags &hellip;', () => {
+      expectFinding('<p>Wait&hellip;</p>', 'named-entity');
+    });
+
+    it('emits one finding per occurrence', () => {
+      const result = check('<p>&mdash; &mdash; &nbsp;</p>');
+      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 &amp;, &lt;, &gt; in text', () => {
+      expectClean('<p>&amp; and &lt; and &gt;</p>');
+    });
+
+    it('does not flag &quot; or &apos; inside an attribute value', () => {
+      expectClean('<p><a href="https://x.test/?q=&quot;hi&quot;">link</a></p>');
+    });
+  });
+
+  describe('confluence-construct', () => {
+    it('flags <ac:task-list>', () => {
+      const finding = expectFinding('<p>x</p><ac:task-list/>', 'confluence-construct');
+      expect(finding.snippet).toContain('ac:task-list');
+    });
+
+    it('flags <ri:user>', () => {
+      expectFinding('<p>hi <ri:user/></p>', 'confluence-construct');
+    });
+
+    it('flags <ac:structured-macro> with attributes', () => {
+      expectFinding('<p>x</p><ac:structured-macro ac:name="info">y</ac:structured-macro>', 'confluence-construct');
+    });
+
+    it('does not flag tags that merely start with the letters a/r', () => {
+      expectClean('<p><a href="https://x.test">link</a></p>');
+    });
+  });
+
+  describe('pre-multiline', () => {
+    it('flags <pre> containing a newline', () => {
+      expectFinding('<pre>line one\nline two</pre>', 'pre-multiline');
+    });
+
+    it('flags <pre><code> containing a newline (the pre is still the trigger)', () => {
+      expectFinding('<pre><code>line one\nline two</code></pre>', 'pre-multiline');
+    });
+
+    it('does not flag inline <code> containing a newline (no <pre> wrapper)', () => {
+      const result = check('<p><code>line one\nline two</code></p>');
+      if (result.ok) return;
+      expect(result.findings.find((entry) => entry.rule === 'pre-multiline')).toBeUndefined();
+    });
+  });
+
+  describe('disallowed-element', () => {
+    it('flags <div>', () => {
+      const finding = expectFinding('<div>x</div>', 'disallowed-element');
+      expect(finding.snippet).toContain('div');
+    });
+
+    it('flags <span>', () => {
+      expectFinding('<p><span>x</span></p>', 'disallowed-element');
+    });
+
+    it('flags <pre> as disallowed in addition to flagging the multi-line trigger when both apply', () => {
+      const result = check('<pre>line one\nline two</pre>');
+      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('<p><a href="https://x.test" title="X &lt; Y">link</a></p>');
+    });
+
+    it('does not flag literal angle brackets inside quoted attribute values', () => {
+      // Regression guard for the parser's quote-awareness: literal `<Y>` inside a quoted attribute value must not be
+      // tokenized as a tag, so no `disallowed-element` finding for `<Y>` should be produced.
+      expectClean('<p><a href="https://x.test" title="X<Y>Z">link</a></p>');
+    });
+
+    it('does not flag any allowlisted tag', () => {
+      expectClean('<table><thead><tr><th>H</th></tr></thead><tbody><tr><td>D</td></tr></tbody></table>');
+    });
+  });
+
+  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)', () => {
+      // `<!--` is not a recognized tag start, so the surrounding text continues to be scanned and the inner
+      // `<strong><code>` is tokenized as nested tags. Jira HTML in this codebase does not carry comments,
+      // so this false-positive surface is theoretical — but the behavior is documented and worth locking in.
+      expectFinding('<p>before <!-- <strong><code>x</code></strong> --> after</p>', 'composition-code-inline-mark');
+    });
+
+    it('treats sibling tags after an unclosed ancestor as nested under that ancestor', () => {
+      // Unclosed `<code>` followed by `<strong>` registers the `<strong>` as nested under `<code>`,
+      // firing the composition rule. This is fail-loud on imbalanced input, not a bug.
+      expectFinding('<p><code>x<p><strong>y</strong></p>', 'composition-code-inline-mark');
+    });
+  });
+});