fix(core): recover html from fenced revision replies

[Sun-sunshine06]

Summary

• recover HTML artifacts when a model returns fenced html instead of <artifact> tags
• tighten revision prompting so inline comments emphasize the selected element and require artifact-tag output
• add regression coverage for fenced revision replies in both generate and apply-comment flows

Validation

• corepack pnpm --filter @open-codesign/core test

[github-actions]

Findings

• [Blocker] Silent artifact-format fallback violates Principle 10 — runModel() now repairs a malformed model response by extracting HTML from collected.text when no <artifact> tag was returned, but it never surfaces that recovery to the caller or throws with context. That masks an upstream contract failure and conflicts with the repo’s “no silent fallbacks” rule, evidence packages/core/src/index.ts:240.
  Suggested fix:

  if (collected.artifacts.length === 0) {
    const fallback = extractFallbackArtifact(collected.text);
    if (fallback.artifact) {
      collected.artifacts.push(fallback.artifact);
      collected.text = [
        'Warning: recovered HTML from a Markdown-fenced response because the model skipped the required <artifact> tag.',
        fallback.message,
      ].filter(Boolean).join('\n\n');
    }
  }

• [Major] Absolute local paths are now sent to the model — replacing the repo basename and attachment filename with designSystem.rootPath / file.path leaks filesystem structure to external providers on every generate/apply-comment request, evidence packages/core/src/index.ts:138, packages/core/src/index.ts:156.
  Suggested fix:

  import { basename } from 'node:path';
  
  const lines = [
    '## Design system to follow',
    `Repository: ${basename(designSystem.rootPath)}`,
    `Summary: ${designSystem.summary}`,
  ];
  
  const lines = [`${index + 1}. ${file.name}`];

Summary

• Review mode: initial
• 2 issues found: the new fenced-HTML recovery is silent even though Principle 10 requires these failures to be user-visible or thrown, and the prompt context now exposes absolute filesystem paths. Regression tests were added for fenced replies, but I did not find coverage that preserves path redaction.

Testing

• Not run (automation): corepack pnpm --filter @open-codesign/core exec vitest run packages/core/src/generate.test.ts failed in this runner because Node v20.20.2 does not satisfy the repo’s >=22 engine requirement, and vitest was not available in the filtered package.

open-codesign Bot

[github-actions]

[Blocker] This silently repairs malformed model output instead of surfacing the contract failure. Principle 10 forbids silent fallbacks, so if we keep this recovery path it needs to be explicit in the returned message (or throw with context).

Suggested fix:

if (collected.artifacts.length === 0) {
  const fallback = extractFallbackArtifact(collected.text);
  if (fallback.artifact) {
    collected.artifacts.push(fallback.artifact);
    collected.text = [
      'Warning: recovered HTML from a Markdown-fenced response because the model skipped the required <artifact> tag.',
      fallback.message,
    ].filter(Boolean).join('\n\n');
  }
}

[github-actions]

[Major] This now includes absolute local paths in prompt context. That leaks filesystem structure to the provider even though the basename/filename was enough before.

Suggested fix:

import { basename } from 'node:path';

const lines = [
  '## Design system to follow',
  `Repository: ${basename(designSystem.rootPath)}`,
  `Summary: ${designSystem.summary}`,
];

const lines = [`${index + 1}. ${file.name}`];

[github-actions]

Findings

• [Blocker] Silent recovery still hides an invalid model response from the caller — runModel() reparses Markdown-fenced/raw HTML when no <artifact> tag was emitted, but it returns that repaired artifact without any warning or thrown error. That violates Principles §10 and makes malformed provider replies indistinguishable from compliant ones, evidence packages/core/src/index.ts:240.
  Suggested fix:

  if (collected.artifacts.length === 0) {
    const fallback = extractFallbackArtifact(collected.text);
    if (fallback.artifact) {
      collected.artifacts.push(fallback.artifact);
      collected.text = [
        'Recovered HTML from a fenced response because the model skipped the required <artifact> tag.',
        fallback.message,
      ].filter(Boolean).join('\n\n');
    }
  }

• [Major] Absolute local paths are still sent to the model prompt — the new Root path: ${designSystem.rootPath} and (${file.path}) strings expose local filesystem structure to the provider, whereas the previous prompt only shared repo/file names. That is a privacy regression directly introduced by this diff, evidence packages/core/src/index.ts:138, packages/core/src/index.ts:156.
  Suggested fix:

  import { basename } from 'node:path';
  
  const lines = [
    '## Design system to follow',
    `Repository: ${basename(designSystem.rootPath)}`,
    `Summary: ${designSystem.summary}`,
  ];
  
  const lines = [`${index + 1}. ${file.name}`];

Summary

• Review mode: follow-up after new commits
• 2 unresolved issues remain at the current head: the fenced-HTML recovery is still silent, and prompt construction still leaks absolute local paths. Test coverage was added for the recovery path, but I did not find coverage that makes the warning user-visible or preserves path redaction.

Testing

• Not run (static review only; this review flow avoids executing PR code)

open-codesign Bot

[github-actions]

[Blocker] This still silently repairs malformed model output instead of surfacing the contract failure. Principles §10 requires the caller/UI to see that recovery happened, otherwise a bad provider response is indistinguishable from a valid <artifact> reply.

Suggested fix:

if (collected.artifacts.length === 0) {
  const fallback = extractFallbackArtifact(collected.text);
  if (fallback.artifact) {
    collected.artifacts.push(fallback.artifact);
    collected.text = [
      'Recovered HTML from a fenced response because the model skipped the required <artifact> tag.',
      fallback.message,
    ].filter(Boolean).join('\n\n');
  }
}

[github-actions]

[Major] This now sends the absolute design-system root path to the provider, and the same leak happens again below with file.path. That is a privacy regression: the prompt only needs repo/file names here, not the user’s local filesystem layout.

Suggested fix:

import { basename } from 'node:path';

const lines = [
  '## Design system to follow',
  `Repository: ${basename(designSystem.rootPath)}`,
  `Summary: ${designSystem.summary}`,
];

const lines = [`${index + 1}. ${file.name}`];