From 37a3c1bee4b68fb9d24f4b887bb2d81d0e9f3700 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Tue, 15 Sep 2026 05:33:17 +0200 Subject: [PATCH 1/2] codex-operator-summary-ux: streamline operator summaries --- build/api/index.js | 2 +- build/cli/index.js | 49 +++-- build/github_action/index.js | 195 ++++++++++++++---- docs/development/architecture.mdx | 15 +- docs/features.mdx | 7 +- docs/how-to-use.mdx | 5 +- docs/issues/notifications-and-auto-close.mdx | 6 +- docs/pull-requests/ai-description.mdx | 6 +- docs/pull-requests/capabilities.mdx | 2 +- specs/CATALOG.md | 4 +- specs/catalog.json | 1 + .../pull-request-lifecycle-and-enrichment.md | 12 +- specs/repository-locale-and-localization.md | 6 +- ...tic-github-publication-and-notification.md | 17 +- .../github_action_completion.test.ts | 5 +- .../__tests__/action_summary_policy.test.ts | 43 +++- .../__tests__/agent_response_schemas.test.ts | 3 + ...request_description_content_policy.test.ts | 14 ++ .../action_summary_message_catalog.ts | 118 ++++++++++- .../policies/action_summary_policy.ts | 45 ++-- .../policies/agent_response_schemas.ts | 4 +- ...pull_request_description_content_policy.ts | 44 ++-- ..._pull_request_description_use_case.test.ts | 13 ++ .../github_publication_boundaries.test.ts | 10 + .../update_pull_request_description.test.ts | 3 +- .../update_pull_request_description.ts | 2 +- 26 files changed, 507 insertions(+), 124 deletions(-) diff --git a/build/api/index.js b/build/api/index.js index 94c99cd35..420ad67f7 100644 --- a/build/api/index.js +++ b/build/api/index.js @@ -6153,7 +6153,7 @@ Write every human-readable sentence in {{targetLocale}}. Preserve code identifie 3. Use the issue description below for context and intent. 4. Provide \`overview\` as one to three sentences that state the outcome and why it matters. 5. Provide \`whatChangedHeading\` as the plain-text {{targetLocale}} equivalent of "What changed" and \`changes\` as two to six short, outcome-oriented items. Do not inventory files, use-case names, internal categories, or every implementation step. -6. Provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only commands, automated checks, or manual scenarios supported by available evidence. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no execution evidence is available, say concisely in {{targetLocale}} that validation was not run or was not available. +6. When execution or manual-verification evidence is available, provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only the supported commands, automated checks, or manual scenarios. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no verification evidence is available, set both fields to \`null\`; do not add a “not run” placeholder. 7. Set \`reviewNotesHeading\` and \`reviewNotes\` to \`null\` unless reviewers need material migration, security, performance, compatibility, rollout, manual-verification, risk, or follow-up context. Otherwise use the localized plain-text heading and one to four concise items. {{relatedIssueInstruction}} 8. Keep the description practical and normally under 4,000 characters. It must never exceed 12,000 characters. Do not use emoji, horizontal separators, generic checklists, empty headings, repeated statements, placeholder text, or unsupported "no impact" claims. 9. Return one JSON object with exactly \`outputLocale\`, \`overview\`, \`whatChangedHeading\`, \`changes\`, \`validationHeading\`, \`validation\`, \`reviewNotesHeading\`, \`reviewNotes\`, and \`closesLinkedIssue\`. Every content field is plain text except Markdown links, code spans, refs, and commands inside content values. The application renders the Markdown structure; do not include headings, bullet prefixes, a preamble, meta-commentary, or code fence in the values. diff --git a/build/cli/index.js b/build/cli/index.js index e62e7f527..5dfd64bfb 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -41457,9 +41457,9 @@ exports.PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA = { maxItems: 6, items: { type: 'string', minLength: 1, maxLength: 1000 }, }, - validationHeading: { type: 'string', minLength: 1, maxLength: 100 }, + validationHeading: { type: ['string', 'null'], minLength: 1, maxLength: 100 }, validation: { - type: 'array', + type: ['array', 'null'], minItems: 1, maxItems: 8, items: { type: 'string', minLength: 1, maxLength: 1000 }, @@ -45372,9 +45372,9 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN const rawContent = [ parsed.overview, parsed.whatChangedHeading, - parsed.validationHeading, + ...(parsed.validationHeading ? [parsed.validationHeading] : []), ...parsed.changes, - ...parsed.validation, + ...(parsed.validation ?? []), ...(parsed.reviewNotesHeading ? [parsed.reviewNotesHeading] : []), ...(parsed.reviewNotes ?? []), ]; @@ -45383,9 +45383,11 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN } const overview = sanitizeBlock(parsed.overview); const whatChangedHeading = sanitizeInline(parsed.whatChangedHeading); - const validationHeading = sanitizeInline(parsed.validationHeading); + const validationHeading = parsed.validationHeading === null + ? null + : sanitizeInline(parsed.validationHeading); const changes = parsed.changes.map(sanitizeInline); - const validation = parsed.validation.map(sanitizeInline); + const validation = parsed.validation?.map(sanitizeInline) ?? null; const reviewNotesHeading = parsed.reviewNotesHeading === null ? null : sanitizeInline(parsed.reviewNotesHeading); @@ -45393,9 +45395,9 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN const allContent = [ overview, whatChangedHeading, - validationHeading, + ...(validationHeading ? [validationHeading] : []), ...changes, - ...validation, + ...(validation ?? []), ...(reviewNotesHeading ? [reviewNotesHeading] : []), ...(reviewNotes ?? []), ]; @@ -45406,15 +45408,17 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN return { kind: 'invalid', reason: 'sentence-count' }; } if (hasDuplicates(changes, targetLocale) - || hasDuplicates(validation, targetLocale) + || (validation !== null && hasDuplicates(validation, targetLocale)) || (reviewNotes && hasDuplicates(reviewNotes, targetLocale))) { return { kind: 'invalid', reason: 'duplicate-item' }; } const sections = [ overview, `## ${whatChangedHeading}\n\n${renderList(changes)}`, - `## ${validationHeading}\n\n${renderList(validation)}`, ]; + if (validationHeading && validation) { + sections.push(`## ${validationHeading}\n\n${renderList(validation)}`); + } if (reviewNotesHeading && reviewNotes) { sections.push(`## ${reviewNotesHeading}\n\n${renderList(reviewNotes)}`); } @@ -45433,18 +45437,31 @@ function parseContent(payload) { return undefined; } const changes = stringArray(payload.changes, 2, 6); - const validation = stringArray(payload.validation, 1, 8); if (typeof payload.overview !== 'string' || payload.overview.length > 1500 || typeof payload.whatChangedHeading !== 'string' || payload.whatChangedHeading.length > 100 - || typeof payload.validationHeading !== 'string' - || payload.validationHeading.length > 100 || !changes - || !validation || typeof payload.closesLinkedIssue !== 'boolean') { return undefined; } + let validation; + let validationHeading; + if (payload.validation === null) { + if (payload.validationHeading !== null) + return undefined; + validation = null; + validationHeading = null; + } + else { + const parsedValidation = stringArray(payload.validation, 1, 8); + if (!parsedValidation + || typeof payload.validationHeading !== 'string' + || payload.validationHeading.length > 100) + return undefined; + validation = parsedValidation; + validationHeading = payload.validationHeading; + } let reviewNotes; let reviewNotesHeading; if (payload.reviewNotes === null) { @@ -45466,7 +45483,7 @@ function parseContent(payload) { overview: payload.overview, whatChangedHeading: payload.whatChangedHeading, changes, - validationHeading: payload.validationHeading, + validationHeading, validation, reviewNotesHeading, reviewNotes, @@ -79482,7 +79499,7 @@ Write every human-readable sentence in {{targetLocale}}. Preserve code identifie 3. Use the issue description below for context and intent. 4. Provide \`overview\` as one to three sentences that state the outcome and why it matters. 5. Provide \`whatChangedHeading\` as the plain-text {{targetLocale}} equivalent of "What changed" and \`changes\` as two to six short, outcome-oriented items. Do not inventory files, use-case names, internal categories, or every implementation step. -6. Provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only commands, automated checks, or manual scenarios supported by available evidence. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no execution evidence is available, say concisely in {{targetLocale}} that validation was not run or was not available. +6. When execution or manual-verification evidence is available, provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only the supported commands, automated checks, or manual scenarios. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no verification evidence is available, set both fields to \`null\`; do not add a “not run” placeholder. 7. Set \`reviewNotesHeading\` and \`reviewNotes\` to \`null\` unless reviewers need material migration, security, performance, compatibility, rollout, manual-verification, risk, or follow-up context. Otherwise use the localized plain-text heading and one to four concise items. {{relatedIssueInstruction}} 8. Keep the description practical and normally under 4,000 characters. It must never exceed 12,000 characters. Do not use emoji, horizontal separators, generic checklists, empty headings, repeated statements, placeholder text, or unsupported "no impact" claims. 9. Return one JSON object with exactly \`outputLocale\`, \`overview\`, \`whatChangedHeading\`, \`changes\`, \`validationHeading\`, \`validation\`, \`reviewNotesHeading\`, \`reviewNotes\`, and \`closesLinkedIssue\`. Every content field is plain text except Markdown links, code spans, refs, and commands inside content values. The application renders the Markdown structure; do not include headings, bullet prefixes, a preamble, meta-commentary, or code fence in the values. diff --git a/build/github_action/index.js b/build/github_action/index.js index 4a37961ef..5115377a6 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -41144,7 +41144,8 @@ const SIMPLE_MESSAGE_KEYS = Object.freeze([ 'pullRequestLocale', 'catalogResolution', 'descriptors', 'reason', 'failure', 'findings', 'partial', 'superseded', 'skipped', 'dryRun', 'success', 'invalid', 'none', 'noResult', 'unnamedResult', 'impact', 'cause', 'action', - 'retainedState', 'reference', + 'retainedState', 'reference', 'retryable', 'yes', 'no', + 'resultSucceeded', 'resultFailed', 'resultSkipped', ]); const TEMPLATE_MESSAGE_IDS = Object.freeze([ 'summary.target.pullRequest', @@ -41156,10 +41157,15 @@ const FINDING_STATE_KEYS = Object.freeze([ 'open', 'reopened', 'fixed', 'obsolete', 'dismissed', 'verification-required', 'unknown', ]); +const ERROR_KIND_KEYS = Object.freeze([ + 'configuration', 'authorization', 'provider', 'agent', 'validation', 'workflow', 'unknown', +]); +const ERROR_FIELD_KEYS = Object.freeze(['impact', 'action', 'retainedState']); exports.ACTION_SUMMARY_MESSAGE_IDS = Object.freeze([ ...SIMPLE_MESSAGE_KEYS.map(key => `summary.${key}`), ...TEMPLATE_MESSAGE_IDS, ...FINDING_STATE_KEYS.map(key => `summary.findingState.${key}`), + ...ERROR_KIND_KEYS.flatMap(kind => ERROR_FIELD_KEYS.map(field => `summary.error.${kind}.${field}`)), ]); const ENGLISH_SIMPLE = Object.freeze({ heading: 'Copilot execution', @@ -41194,10 +41200,16 @@ const ENGLISH_SIMPLE = Object.freeze({ noResult: 'No application result was produced.', unnamedResult: 'Unnamed result', impact: 'Impact', - cause: 'Cause', + cause: 'Error code', action: 'Action', retainedState: 'Retained state', reference: 'Reference', + retryable: 'Retryable', + yes: 'Yes', + no: 'No', + resultSucceeded: 'Succeeded', + resultFailed: 'Failed', + resultSkipped: 'Skipped', }); const SPANISH_SIMPLE = Object.freeze({ heading: 'Ejecución de Copilot', @@ -41232,10 +41244,16 @@ const SPANISH_SIMPLE = Object.freeze({ noResult: 'No se ha producido ningún resultado de aplicación.', unnamedResult: 'Resultado sin nombre', impact: 'Impacto', - cause: 'Causa', + cause: 'Código de error', action: 'Acción', retainedState: 'Estado conservado', reference: 'Referencia', + retryable: 'Reintentable', + yes: 'Sí', + no: 'No', + resultSucceeded: 'Completado', + resultFailed: 'Fallido', + resultSkipped: 'Omitido', }); const ENGLISH_TEMPLATES = Object.freeze({ 'summary.target.pullRequest': 'PR #{number}', @@ -41267,24 +41285,102 @@ const SPANISH_FINDING_STATES = Object.freeze({ 'verification-required': 'requieren verificación', unknown: 'desconocidos', }); -function catalogMessages(simple, templates, findingStates) { +const ENGLISH_ERRORS = Object.freeze({ + configuration: Object.freeze({ + impact: 'The operation could not use the configured values.', + action: 'Correct the configuration or choose a supported capability before retrying.', + retainedState: 'No new state or external effect was created.', + }), + authorization: Object.freeze({ + impact: 'The operation could not authenticate or access a required resource.', + action: 'Correct the credential or grant the documented permission before retrying.', + retainedState: 'No new state or external effect was created.', + }), + provider: Object.freeze({ + impact: 'A provider operation did not complete.', + action: 'Inspect the error code and retry only when the provider state or availability has changed.', + retainedState: 'Existing state and completed external effects remain in place.', + }), + agent: Object.freeze({ + impact: 'The configured agent did not produce usable product content.', + action: 'Inspect the sanitized agent status and retry with a compatible provider or model.', + retainedState: 'Existing state remains in place; rejected content was not published.', + }), + validation: Object.freeze({ + impact: 'The supplied input was rejected before the operation could continue.', + action: 'Correct the input and retry.', + retainedState: 'No new state or external effect was created.', + }), + workflow: Object.freeze({ + impact: 'The workflow could not complete the requested operation.', + action: 'Inspect the current state and retry only if the operation is still required.', + retainedState: 'Existing state and confirmed completed effects remain in place.', + }), + unknown: Object.freeze({ + impact: 'An unexpected failure was handled safely.', + action: 'Use the reference to investigate before retrying.', + retainedState: 'Existing state and confirmed completed effects remain in place.', + }), +}); +const SPANISH_ERRORS = Object.freeze({ + configuration: Object.freeze({ + impact: 'La operación no pudo usar los valores configurados.', + action: 'Corrige la configuración o elige una capacidad compatible antes de reintentarlo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + authorization: Object.freeze({ + impact: 'La operación no pudo autenticarse o acceder a un recurso necesario.', + action: 'Corrige la credencial o concede el permiso documentado antes de reintentarlo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + provider: Object.freeze({ + impact: 'Una operación del proveedor no se completó.', + action: 'Revisa el código de error y reinténtalo solo cuando haya cambiado el estado o la disponibilidad del proveedor.', + retainedState: 'El estado existente y los efectos externos completados se mantienen.', + }), + agent: Object.freeze({ + impact: 'El agente configurado no produjo contenido de producto utilizable.', + action: 'Revisa el estado saneado del agente y reinténtalo con un proveedor o modelo compatible.', + retainedState: 'El estado existente se mantiene y el contenido rechazado no se publicó.', + }), + validation: Object.freeze({ + impact: 'La entrada suministrada se rechazó antes de continuar la operación.', + action: 'Corrige la entrada y reinténtalo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + workflow: Object.freeze({ + impact: 'El workflow no pudo completar la operación solicitada.', + action: 'Revisa el estado actual y reinténtalo solo si la operación sigue siendo necesaria.', + retainedState: 'El estado existente y los efectos completados y confirmados se mantienen.', + }), + unknown: Object.freeze({ + impact: 'Un fallo inesperado se gestionó de forma segura.', + action: 'Usa la referencia para investigar antes de reintentarlo.', + retainedState: 'El estado existente y los efectos completados y confirmados se mantienen.', + }), +}); +function catalogMessages(simple, templates, findingStates, errors) { return Object.freeze({ ...Object.fromEntries(SIMPLE_MESSAGE_KEYS.map(key => [`summary.${key}`, simple[key]])), ...templates, ...Object.fromEntries(FINDING_STATE_KEYS.map(key => [`summary.findingState.${key}`, findingStates[key]])), + ...Object.fromEntries(ERROR_KIND_KEYS.flatMap(kind => ERROR_FIELD_KEYS.map(field => [ + `summary.error.${kind}.${field}`, + errors[kind][field], + ]))), }); } exports.ENGLISH_ACTION_SUMMARY_DEFINITION = Object.freeze({ version: message_catalog_1.MESSAGE_CATALOG_VERSION, locale: 'en-US', compatibleBaseLanguage: 'en', - messages: catalogMessages(ENGLISH_SIMPLE, ENGLISH_TEMPLATES, ENGLISH_FINDING_STATES), + messages: catalogMessages(ENGLISH_SIMPLE, ENGLISH_TEMPLATES, ENGLISH_FINDING_STATES, ENGLISH_ERRORS), }); exports.SPANISH_ACTION_SUMMARY_DEFINITION = Object.freeze({ version: message_catalog_1.MESSAGE_CATALOG_VERSION, locale: 'es-ES', compatibleBaseLanguage: 'es', - messages: catalogMessages(SPANISH_SIMPLE, SPANISH_TEMPLATES, SPANISH_FINDING_STATES), + messages: catalogMessages(SPANISH_SIMPLE, SPANISH_TEMPLATES, SPANISH_FINDING_STATES, SPANISH_ERRORS), }); exports.ACTION_SUMMARY_CATALOG_DEFINITIONS = Object.freeze([ exports.ENGLISH_ACTION_SUMMARY_DEFINITION, @@ -41310,7 +41406,6 @@ exports.buildActionSummary = buildActionSummary; exports.actionSummaryLocalizationLabels = actionSummaryLocalizationLabels; exports.renderLocalizationSummarySection = renderLocalizationSummarySection; const github_comment_publication_policy_1 = __nccwpck_require__(72712); -const application_error_presentation_policy_1 = __nccwpck_require__(95067); const bugbot_telemetry_projection_policy_1 = __nccwpck_require__(43244); const bugbot_result_finding_state_projection_policy_1 = __nccwpck_require__(98117); const review_state_1 = __nccwpck_require__(79200); @@ -41328,7 +41423,7 @@ const ENGLISH_LOCALIZATION_SUMMARY_LABELS = Object.freeze({ }); /** Builds one bounded, publication-safe, repository-locale GitHub Actions Job Summary. */ function buildActionSummary(context, catalog = (0, action_summary_message_catalog_1.resolveStaticActionSummaryCatalog)(context.locale?.repository ?? 'en-US')) { - const failures = context.results.filter(result => !result.success && result.executed); + const failures = context.results.filter(result => result.errors.length > 0 || (!result.success && result.executed)); const findingStateProjection = (0, bugbot_result_finding_state_projection_policy_1.projectBugbotResultFindingStates)(context.results); const findingStates = findingStateProjection.status === 'valid' ? findingStateProjection.counts : undefined; const telemetryProjection = (0, bugbot_telemetry_projection_policy_1.projectBugbotResultTelemetry)(context.results); @@ -41341,6 +41436,7 @@ function buildActionSummary(context, catalog = (0, action_summary_message_catalo hasActionableFindings, failOnUnresolvedFindings: context.failOnUnresolvedFindings === true, bugbotTelemetry, + allResultsSkipped: context.results.length > 0 && context.results.every(result => !result.executed), }, catalog); const target = resolveActionSummaryTarget(context, catalog); const lifecycle = context.lifecycleState ? `\`${(0, github_comment_publication_policy_1.sanitizeAgentMarkdown)(context.lifecycleState, 100)}\`` : '—'; @@ -41426,6 +41522,8 @@ function resolveActionSummaryStatus(input, catalog) { return `❌ ${catalogText(catalog, 'summary.failure')}`; if (input.hasActionableFindings) return `⚠️ ${catalogText(catalog, 'summary.findings')}`; + if (input.allResultsSkipped) + return `⏭️ ${catalogText(catalog, 'summary.skipped')}`; switch (input.bugbotTelemetry?.outcome) { case 'partial': return `⚠️ ${catalogText(catalog, 'summary.partial')}`; case 'superseded': return `⏭️ ${catalogText(catalog, 'summary.superseded')}`; @@ -41467,24 +41565,34 @@ function renderResults(results, catalog) { if (results.length === 0) return `_${catalogText(catalog, 'summary.noResult')}_`; return results.map(result => { - const icon = result.success ? '✅' : '❌'; - const details = result.steps - .filter(step => step.trim()) - .map(step => ` - ${(0, github_comment_publication_policy_1.sanitizeAgentMarkdown)(step, 1000)}`); + const failed = result.errors.length > 0 || (!result.success && result.executed); + const outcome = failed + ? { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') } + : !result.executed + ? { icon: '⏭️', label: catalogText(catalog, 'summary.resultSkipped') } + : result.success + ? { icon: '✅', label: catalogText(catalog, 'summary.resultSucceeded') } + : { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') }; const errors = result.errors .flatMap((error) => { - const view = (0, application_error_presentation_policy_1.buildApplicationErrorPresentation)(error); return [ - ` - **${catalogText(catalog, 'summary.impact')}:** ${(0, github_comment_publication_policy_1.sanitizePublishedError)(view.impact)}`, - ` - **${catalogText(catalog, 'summary.cause')} (\`${view.code}\`):** ${(0, github_comment_publication_policy_1.sanitizePublishedError)(view.cause)}`, - ` - **${catalogText(catalog, 'summary.action')}:** ${(0, github_comment_publication_policy_1.sanitizePublishedError)(view.action)}`, - ` - **${catalogText(catalog, 'summary.retainedState')}:** ${(0, github_comment_publication_policy_1.sanitizePublishedError)(view.retainedState)}`, - ` - **${catalogText(catalog, 'summary.reference')}:** \`${view.reference}\``, + ` - **${catalogText(catalog, 'summary.impact')}:** ${errorCatalogText(catalog, error.kind, 'impact')}`, + ` - **${catalogText(catalog, 'summary.cause')}:** \`${error.code}\``, + ` - **${catalogText(catalog, 'summary.action')}:** ${errorCatalogText(catalog, error.kind, 'action')}`, + ` - **${catalogText(catalog, 'summary.retainedState')}:** ${errorCatalogText(catalog, error.kind, 'retainedState')}`, + ` - **${catalogText(catalog, 'summary.retryable')}:** ${catalogText(catalog, error.retryable ? 'summary.yes' : 'summary.no')}`, + ` - **${catalogText(catalog, 'summary.reference')}:** \`${error.correlationId}\``, ]; }); - return [`- ${icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}**`, ...details, ...errors].join('\n'); + return [ + `- ${outcome.icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}** — ${outcome.label}`, + ...errors, + ].join('\n'); }).join('\n'); } +function errorCatalogText(catalog, kind, field) { + return catalogText(catalog, `summary.error.${kind}.${field}`); +} function catalogText(catalog, id, variables = {}) { return escapeMarkdownText(catalog.message(id, variables)); } @@ -42281,9 +42389,9 @@ exports.PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA = { maxItems: 6, items: { type: 'string', minLength: 1, maxLength: 1000 }, }, - validationHeading: { type: 'string', minLength: 1, maxLength: 100 }, + validationHeading: { type: ['string', 'null'], minLength: 1, maxLength: 100 }, validation: { - type: 'array', + type: ['array', 'null'], minItems: 1, maxItems: 8, items: { type: 'string', minLength: 1, maxLength: 1000 }, @@ -46588,9 +46696,9 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN const rawContent = [ parsed.overview, parsed.whatChangedHeading, - parsed.validationHeading, + ...(parsed.validationHeading ? [parsed.validationHeading] : []), ...parsed.changes, - ...parsed.validation, + ...(parsed.validation ?? []), ...(parsed.reviewNotesHeading ? [parsed.reviewNotesHeading] : []), ...(parsed.reviewNotes ?? []), ]; @@ -46599,9 +46707,11 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN } const overview = sanitizeBlock(parsed.overview); const whatChangedHeading = sanitizeInline(parsed.whatChangedHeading); - const validationHeading = sanitizeInline(parsed.validationHeading); + const validationHeading = parsed.validationHeading === null + ? null + : sanitizeInline(parsed.validationHeading); const changes = parsed.changes.map(sanitizeInline); - const validation = parsed.validation.map(sanitizeInline); + const validation = parsed.validation?.map(sanitizeInline) ?? null; const reviewNotesHeading = parsed.reviewNotesHeading === null ? null : sanitizeInline(parsed.reviewNotesHeading); @@ -46609,9 +46719,9 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN const allContent = [ overview, whatChangedHeading, - validationHeading, + ...(validationHeading ? [validationHeading] : []), ...changes, - ...validation, + ...(validation ?? []), ...(reviewNotesHeading ? [reviewNotesHeading] : []), ...(reviewNotes ?? []), ]; @@ -46622,15 +46732,17 @@ function renderPullRequestDescriptionContent(payload, targetLocale, linkedIssueN return { kind: 'invalid', reason: 'sentence-count' }; } if (hasDuplicates(changes, targetLocale) - || hasDuplicates(validation, targetLocale) + || (validation !== null && hasDuplicates(validation, targetLocale)) || (reviewNotes && hasDuplicates(reviewNotes, targetLocale))) { return { kind: 'invalid', reason: 'duplicate-item' }; } const sections = [ overview, `## ${whatChangedHeading}\n\n${renderList(changes)}`, - `## ${validationHeading}\n\n${renderList(validation)}`, ]; + if (validationHeading && validation) { + sections.push(`## ${validationHeading}\n\n${renderList(validation)}`); + } if (reviewNotesHeading && reviewNotes) { sections.push(`## ${reviewNotesHeading}\n\n${renderList(reviewNotes)}`); } @@ -46649,18 +46761,31 @@ function parseContent(payload) { return undefined; } const changes = stringArray(payload.changes, 2, 6); - const validation = stringArray(payload.validation, 1, 8); if (typeof payload.overview !== 'string' || payload.overview.length > 1500 || typeof payload.whatChangedHeading !== 'string' || payload.whatChangedHeading.length > 100 - || typeof payload.validationHeading !== 'string' - || payload.validationHeading.length > 100 || !changes - || !validation || typeof payload.closesLinkedIssue !== 'boolean') { return undefined; } + let validation; + let validationHeading; + if (payload.validation === null) { + if (payload.validationHeading !== null) + return undefined; + validation = null; + validationHeading = null; + } + else { + const parsedValidation = stringArray(payload.validation, 1, 8); + if (!parsedValidation + || typeof payload.validationHeading !== 'string' + || payload.validationHeading.length > 100) + return undefined; + validation = parsedValidation; + validationHeading = payload.validationHeading; + } let reviewNotes; let reviewNotesHeading; if (payload.reviewNotes === null) { @@ -46682,7 +46807,7 @@ function parseContent(payload) { overview: payload.overview, whatChangedHeading: payload.whatChangedHeading, changes, - validationHeading: payload.validationHeading, + validationHeading, validation, reviewNotesHeading, reviewNotes, @@ -76821,7 +76946,7 @@ Write every human-readable sentence in {{targetLocale}}. Preserve code identifie 3. Use the issue description below for context and intent. 4. Provide \`overview\` as one to three sentences that state the outcome and why it matters. 5. Provide \`whatChangedHeading\` as the plain-text {{targetLocale}} equivalent of "What changed" and \`changes\` as two to six short, outcome-oriented items. Do not inventory files, use-case names, internal categories, or every implementation step. -6. Provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only commands, automated checks, or manual scenarios supported by available evidence. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no execution evidence is available, say concisely in {{targetLocale}} that validation was not run or was not available. +6. When execution or manual-verification evidence is available, provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only the supported commands, automated checks, or manual scenarios. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no verification evidence is available, set both fields to \`null\`; do not add a “not run” placeholder. 7. Set \`reviewNotesHeading\` and \`reviewNotes\` to \`null\` unless reviewers need material migration, security, performance, compatibility, rollout, manual-verification, risk, or follow-up context. Otherwise use the localized plain-text heading and one to four concise items. {{relatedIssueInstruction}} 8. Keep the description practical and normally under 4,000 characters. It must never exceed 12,000 characters. Do not use emoji, horizontal separators, generic checklists, empty headings, repeated statements, placeholder text, or unsupported "no impact" claims. 9. Return one JSON object with exactly \`outputLocale\`, \`overview\`, \`whatChangedHeading\`, \`changes\`, \`validationHeading\`, \`validation\`, \`reviewNotesHeading\`, \`reviewNotes\`, and \`closesLinkedIssue\`. Every content field is plain text except Markdown links, code spans, refs, and commands inside content values. The application renders the Markdown structure; do not include headings, bullet prefixes, a preamble, meta-commentary, or code fence in the values. diff --git a/docs/development/architecture.mdx b/docs/development/architecture.mdx index 7c0bebea3..ee0694263 100644 --- a/docs/development/architecture.mdx +++ b/docs/development/architecture.mdx @@ -142,9 +142,11 @@ less likely to couple unrelated capabilities. Application failures use the closed `ApplicationError` contract: one of 20 semantic codes, a derived category and retry decision, impact, recommended action, retained-state summary, and request correlation UUID. A raw cause is ECMAScript-private, -never serialized or logged, and cannot enter `Result.errors`. CLI output, -GitHub comments, Job Summaries, and Check failures project the same semantic -view. +never serialized or logged, and cannot enter `Result.errors`. The generic Job +Summary projects the stable kind/code/retry/reference through its repository- +locale catalog and does not replay producer error prose. CLI and early boundary +failures retain the safe default English error view until repository-aware +presentation is available. The main-run setup path follows that rule end to end. Its action boundary copies and freezes a `SetupExecutionContext`, setup returns a discriminated result, and @@ -365,5 +367,8 @@ The generic GitHub Actions Job Summary follows the same boundary. One complete typed catalog is resolved for `repository-locale`; bundled English and Spanish, arbitrary dynamic locales, and atomic English fallback share the repository localization resolver. The summary renders localization evidence once, keeps -machine event/state/error values stable, and retains sanitized internal result -detail without publishing it to an issue or pull-request conversation. +machine event/state/error values stable, and projects each result as a compact +localized outcome. Internal `Result.steps` and arbitrary error messages remain +in logs or machine evidence; they are never replayed into the summary. Failures +show category-localized impact, action, and retained-state guidance beside the +stable code, retry decision, and correlation reference. diff --git a/docs/features.mdx b/docs/features.mdx index e5b3104d2..e5fbff293 100644 --- a/docs/features.mdx +++ b/docs/features.mdx @@ -38,7 +38,7 @@ When the workflow runs on `issues` (opened, edited, labeled, unlabeled, etc.): | **Issue type** | Sets the GitHub issue type (Task, Bug, Feature, Documentation, etc.) from labels. | | **Emoji titles** | Optionally adds emojis to issue titles based on labels (`emoji-labeled-title`). | | **Size labels** | Assigns size labels (XS–XXL) and checks size thresholds (lines, files, commits) for prioritization. | -| **Planning guidance** | When planning is requested, maintains one bounded plan card with the outcome and next action. Internal execution steps and Git-Flow reminders stay in the Job Summary. | +| **Planning guidance** | When planning is requested, maintains one bounded plan card with the outcome and next action. The Job Summary keeps only compact operator state; internal execution narration remains machine/log evidence. | | **Lifecycle labels** | Maintains an exclusive durable `state:*` phase, an optional `state:ai-processing` activity marker, and an optional human-waiting label. Activity can coexist with the durable phase and is removed when the agent run finishes. | ### 2. Pull request events (`on: pull_request`) @@ -52,7 +52,7 @@ or `closed`): | **Project linking** | Adds the PR to the configured GitHub Projects and moves it to the configured column. | | **Reviewers** | Assigns up to `desired-reviewers-count` reviewers. | | **Priority & size** | Applies priority and size checks (labels and thresholds). | -| **AI PR description** | When `ai-pull-request-description-mode` is `replace`, `append`, or `preserve`, the selected agent CLI writes a concise outcome, material changes, and validation from the optional issue context and branch diff. `disabled` turns the feature off. See [Pull Requests → AI-generated PR description](/pull-requests/ai-description). | +| **AI PR description** | When `ai-pull-request-description-mode` is `replace`, `append`, or `preserve`, the selected agent CLI writes a concise outcome and material changes from the optional issue context and branch diff. Validation appears only when supported by evidence; `disabled` turns the feature off. See [Pull Requests → AI-generated PR description](/pull-requests/ai-description). | | **Conversation UX** | Publishes only purpose-specific replies and durable status cards. It does not add a generic action recap or decorative image. | | **Bugbot review** | Reviews the full PR on open/reopen and the new commit range on synchronize; publishes historical review snapshots and stable line findings, then reconciles native threads, review status blocks, one current PR card, and the Check from a provider-verified projection. | @@ -148,7 +148,8 @@ mentions receive at most one correlated reply. Routine labels, assignees, project moves, title normalization, PR linkage, description refreshes, pushes, reopens, merges, and closes do not create a -generic roll-up. Detailed steps and errors remain in the Job Summary and logs. +generic roll-up. The Job Summary keeps compact result states and safe error +recovery evidence; internal step narration and arbitrary error text remain in logs. Hidden markers provide stable identity and are trusted only when the comment is authored by the configured bot. Decorative image inputs default to `false` and are deprecated. diff --git a/docs/how-to-use.mdx b/docs/how-to-use.mdx index 1bc37dfcc..0a1b8ff7e 100644 --- a/docs/how-to-use.mdx +++ b/docs/how-to-use.mdx @@ -420,8 +420,9 @@ The **labels** in each template must match the label names configured in the act Copied to `.github/pull_request_template.md`. Used as the default body for new PRs and as guidance for AI-generated descriptions. The shipped template asks -for a short outcome, material changes, and validation; reviewer notes are -conditional. You can add repository-specific instructions, but the agent omits +for a short outcome, material changes, and supported validation evidence; +validation and reviewer notes are conditional. You can add repository-specific +instructions, but the agent omits empty boilerplate, generic checklists, and unsupported claims rather than reproducing every heading. No Copilot logic depends on specific headings; only the deploy/release/hotfix flows depend on **workflow filenames** and **label diff --git a/docs/issues/notifications-and-auto-close.mdx b/docs/issues/notifications-and-auto-close.mdx index 25e85924d..9005f5bda 100644 --- a/docs/issues/notifications-and-auto-close.mdx +++ b/docs/issues/notifications-and-auto-close.mdx @@ -16,10 +16,12 @@ current progress, an action, a direct answer, or a terminal policy explanation. When the push workflow runs for a branch linked to an issue, Copilot updates one owned progress card only when the progress projection changes. It does not post a commit list, image, reopen notice, debug report, or generic “Actions” summary. -The workflow Job Summary remains the place for bounded operational detail. Its +The workflow Job Summary remains the place for bounded operational state. Its headings and explanatory labels use `repository-locale` (English by default), while event names, lifecycle values, error codes, and other machine facts remain -stable. It contains one localization section showing the effective issue and PR +stable. It reports compact per-result outcomes and category-localized recovery +guidance without replaying internal `Result.steps` or arbitrary provider text. +It contains one localization section showing the effective issue and PR locales plus exact, base-language, dynamic, or English-fallback catalog evidence. - **Where:** the issue associated with the branch (for example, diff --git a/docs/pull-requests/ai-description.mdx b/docs/pull-requests/ai-description.mdx index bc601bb12..933331d80 100644 --- a/docs/pull-requests/ai-description.mdx +++ b/docs/pull-requests/ai-description.mdx @@ -13,7 +13,7 @@ description: How the configured agent writes a concise pull request description - Reads the repository's **pull request template** as content guidance and constraints. - Computes the merge-base **diff** between base and head (`git diff base...head`) to understand what changed. - Uses the **issue description** (when the PR branch is linked to an issue) as context; otherwise it infers intent from the PR metadata and diff. -3. The agent returns bounded semantic fields for one short outcome paragraph, two to six changes, validation evidence, and optional review notes. The action validates and sanitizes those fields, then renders the fixed Markdown hierarchy; the model does not control arbitrary sections or closing-reference syntax. +3. The agent returns bounded semantic fields for one short outcome paragraph, two to six changes, optional verified validation evidence, and optional review notes. The action validates and sanitizes those fields, then renders the fixed Markdown hierarchy; the model does not control arbitrary sections or closing-reference syntax. 4. The request carries the effective PR locale (`pull-requests-locale`, otherwise `repository-locale`, otherwise `en-US`). The response must echo that exact canonical tag in `outputLocale`; a mismatch stops before the body changes. 5. The action writes the validated result to the PR body according to the configured ownership mode. @@ -28,7 +28,9 @@ The AI reads your repository's **pull request template** for team-specific guida The generated body is normally under 4,000 characters and can never exceed 12,000. It excludes emoji, separators, empty headings, generic checklists, file-by-file inventories, repeated statements, and unsupported claims that a -test passed or a change has no impact. A template can add useful constraints, +test passed or a change has no impact. When no verification evidence is +available, both validation fields are `null` and the entire section is omitted; +Copilot does not add a “not run” placeholder. A template can add useful constraints, but it cannot force the agent to reproduce empty boilerplate. diff --git a/docs/pull-requests/capabilities.mdx b/docs/pull-requests/capabilities.mdx index 8c58151c3..8901c095c 100644 --- a/docs/pull-requests/capabilities.mdx +++ b/docs/pull-requests/capabilities.mdx @@ -69,7 +69,7 @@ Thresholds are defined in [Configuration](/configuration) (e.g. `size-m-threshol - Reads your repo’s **pull request template** (`.github/pull_request_template.md`). - Uses the optional **issue description** and the merge-base **branch diff** (`base...head`) as context. -- Writes a concise outcome, material changes, and validation evidence. Reviewer notes appear only when they carry useful risk, migration, rollout, or manual-verification context. +- Writes a concise outcome and material changes. Validation appears only with supporting evidence; reviewer notes appear only when they carry useful risk, migration, rollout, or manual-verification context. - Omits empty template sections, generic checklists, implementation inventories, and self-closing issue references. See [AI PR description](/pull-requests/ai-description) for details, optional issue context, and how to enable it in your workflow. diff --git a/specs/CATALOG.md b/specs/CATALOG.md index e9ba30613..3cf9302fb 100644 --- a/specs/CATALOG.md +++ b/specs/CATALOG.md @@ -21,7 +21,7 @@ debt or convert unknown historic intent into a design decision. | `comment-automation` | Implemented | Admit only explicit commands or exact mentions, then route them while protecting repository mutations | [Comment automation and authorization](./comment-automation-and-authorization.md) | 52 paths · 2026-09-15 | | `bugbot-analysis-and-autofix` | Implemented | Select one canonical PR, analyze bounded evidence, publish stable findings, and apply authorized verified fixes | [Bugbot analysis, finding publication, and autofix](./bugbot-analysis-publication-and-autofix.md) + 1 companion | 63 paths · 2026-09-13 | | `branch-synchronization` | As-built baseline | Observe parent drift and safely merge a parent branch into a linked working branch | [Branch synchronization and conflict recovery](./branch-synchronization-and-conflict-recovery.md) | 15 paths · 2026-09-11 | -| `pull-request-lifecycle` | Implemented | Enrich linked and unlinked pull requests with safe issue linkage, projects, metadata, reviewers, concise descriptions, and distinct workflow evidence | [Pull request lifecycle and enrichment](./pull-request-lifecycle-and-enrichment.md) | 37 paths · 2026-09-15 | +| `pull-request-lifecycle` | Implemented | Enrich linked and unlinked pull requests with safe issue linkage, projects, metadata, reviewers, concise descriptions, and distinct workflow evidence | [Pull request lifecycle and enrichment](./pull-request-lifecycle-and-enrichment.md) | 38 paths · 2026-09-15 | | `agent-runtime` | Implemented | Resolve, provision, authenticate, authorize, and execute only the agent roles reachable by a run | [Agent runtime, provider, model, and role routing](./agent-runtime-provider-and-model-routing.md) + 1 companion | 51 paths · 2026-09-12 | | `cli-and-single-actions` | As-built baseline | Expose bounded local commands and workflow-dispatched operations through the shared application core | [CLI and single-action execution](./cli-and-single-action-execution.md) | 29 paths · 2026-09-15 | @@ -156,7 +156,7 @@ debt or convert unknown historic intent into a design decision. - Workflows: [`.github/workflows/copilot_pull_request.yml`](../.github/workflows/copilot_pull_request.yml) · [`.github/workflows/copilot_pull_request_merge_queue.yml`](../.github/workflows/copilot_pull_request_merge_queue.yml) · [`setup/workflows/copilot_pull_request.yml`](../setup/workflows/copilot_pull_request.yml) · [`setup/workflows/copilot_pull_request_merge_queue.yml`](../setup/workflows/copilot_pull_request_merge_queue.yml) - Entrypoints: [`src/application/usecases/pull_request_use_case.ts`](../src/application/usecases/pull_request_use_case.ts) · [`src/actions/common_action.ts`](../src/actions/common_action.ts) - Core code: [`src/application/usecases/execution/execution_issue_number_policy.ts`](../src/application/usecases/execution/execution_issue_number_policy.ts) · [`src/application/usecases/execution/setup_execution_workflow.ts`](../src/application/usecases/execution/setup_execution_workflow.ts) · [`src/application/usecases/pull_request_workflow_context.ts`](../src/application/usecases/pull_request_workflow_context.ts) · [`src/application/usecases/pull_request_workflow.ts`](../src/application/usecases/pull_request_workflow.ts) · [`src/application/usecases/pull_request_workflow_steps.ts`](../src/application/usecases/pull_request_workflow_steps.ts) · [`src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts`](../src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts) · [`src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts`](../src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts) · [`src/prompts/update_pull_request_description.ts`](../src/prompts/update_pull_request_description.ts) · [`src/application/policies/agent_response_schemas.ts`](../src/application/policies/agent_response_schemas.ts) · [`src/application/policies/pull_request_description_content_policy.ts`](../src/application/policies/pull_request_description_content_policy.ts) · [`src/infrastructure/composition/lifecycle_capability_port_binding.ts`](../src/infrastructure/composition/lifecycle_capability_port_binding.ts) · [`src/domain/pull_request_description.ts`](../src/domain/pull_request_description.ts) · [`src/data/repository/pull_request/pull_request_lifecycle_repository.ts`](../src/data/repository/pull_request/pull_request_lifecycle_repository.ts) -- Tests: [`src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts`](../src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts) · [`src/data/model/__tests__/execution.test.ts`](../src/data/model/__tests__/execution.test.ts) · [`src/actions/__tests__/common_action.test.ts`](../src/actions/__tests__/common_action.test.ts) · [`src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts`](../src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts) · [`src/application/usecases/__tests__/pull_request_use_case.test.ts`](../src/application/usecases/__tests__/pull_request_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts) · [`src/application/policies/__tests__/pull_request_description_content_policy.test.ts`](../src/application/policies/__tests__/pull_request_description_content_policy.test.ts) · [`src/prompts/__tests__/update_pull_request_description.test.ts`](../src/prompts/__tests__/update_pull_request_description.test.ts) · [`src/tooling/__tests__/validate_workflow_contract.test.ts`](../src/tooling/__tests__/validate_workflow_contract.test.ts) · [`src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts`](../src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts) · [`src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts`](../src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts) +- Tests: [`src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts`](../src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts) · [`src/data/model/__tests__/execution.test.ts`](../src/data/model/__tests__/execution.test.ts) · [`src/actions/__tests__/common_action.test.ts`](../src/actions/__tests__/common_action.test.ts) · [`src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts`](../src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts) · [`src/application/usecases/__tests__/pull_request_use_case.test.ts`](../src/application/usecases/__tests__/pull_request_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts) · [`src/application/policies/__tests__/pull_request_description_content_policy.test.ts`](../src/application/policies/__tests__/pull_request_description_content_policy.test.ts) · [`src/application/policies/__tests__/agent_response_schemas.test.ts`](../src/application/policies/__tests__/agent_response_schemas.test.ts) · [`src/prompts/__tests__/update_pull_request_description.test.ts`](../src/prompts/__tests__/update_pull_request_description.test.ts) · [`src/tooling/__tests__/validate_workflow_contract.test.ts`](../src/tooling/__tests__/validate_workflow_contract.test.ts) · [`src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts`](../src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts) · [`src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts`](../src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts) - User documentation: [`docs/pull-requests/capabilities.mdx`](../docs/pull-requests/capabilities.mdx) · [`docs/pull-requests/ai-description.mdx`](../docs/pull-requests/ai-description.mdx) · [`docs/pull-requests/configuration.mdx`](../docs/pull-requests/configuration.mdx) · [`docs/features.mdx`](../docs/features.mdx) · [`docs/bugbot/how-it-works.mdx`](../docs/bugbot/how-it-works.mdx) · [`docs/bugbot/configuration.mdx`](../docs/bugbot/configuration.mdx) ### `agent-runtime` — Agent runtime, provider, model, and role routing diff --git a/specs/catalog.json b/specs/catalog.json index 2555f6bcf..252ae8239 100644 --- a/specs/catalog.json +++ b/specs/catalog.json @@ -933,6 +933,7 @@ "src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts", "src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts", "src/application/policies/__tests__/pull_request_description_content_policy.test.ts", + "src/application/policies/__tests__/agent_response_schemas.test.ts", "src/prompts/__tests__/update_pull_request_description.test.ts", "src/tooling/__tests__/validate_workflow_contract.test.ts", "src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts", diff --git a/specs/pull-request-lifecycle-and-enrichment.md b/specs/pull-request-lifecycle-and-enrichment.md index 4bf73efa2..2630df1ac 100644 --- a/specs/pull-request-lifecycle-and-enrichment.md +++ b/specs/pull-request-lifecycle-and-enrichment.md @@ -15,7 +15,7 @@ On an opened PR, Copilot enriches the PR even when no issue is linked. When the branch identifies a separate issue, Copilot also links and synchronizes that issue; it never falls back to treating the PR number as the issue number. Generated descriptions prioritize reviewer decisions: a short outcome, -material changes, verified validation evidence, and conditional review notes. +material changes, verified validation evidence when available, and conditional review notes. Synchronize events refresh enabled generated content and review; metadata-only edits are ignored so Copilot's own body update cannot retrigger the pipeline. A merged PR closes only a distinct linked issue. Human-authored body content is @@ -254,7 +254,10 @@ by Copilot updating its own description. Metadata-only PR edits no longer normalize titles automatically; human edits are preserved. ``` -The opening outcome and the first two sections are required. `Review notes` and a +The opening outcome and `What changed` are required. `Validation` appears only +when at least one command, check, or manual scenario has supporting evidence; +the whole section is omitted instead of displaying a “not run” placeholder. +`Review notes` and a distinct `Closes #…` reference appear only when supported. The normal target is 4,000 characters and the schema rejects more than 12,000. Empty template sections, emoji, separators, generic checklists, file/use-case inventories, @@ -386,8 +389,9 @@ body mutation that produces no follow-up PR workflow. reference, both, or neither remain, and replay never layers another marker. 12. A generated body is rendered from a strict structured response, starts with a one-to-three sentence outcome, has two to six - material-change bullets and evidence-based validation, stays within 12,000 - characters, and omits empty/generic sections. + material-change bullets, includes validation only when evidence exists, + stays within 12,000 characters, and omits empty/generic sections and + “not run” placeholders. 13. Copilot's own PR body update creates zero follow-up PR workflow runs. 14. Actions distinguish PR analysis, review-state observation, and merge-queue admission without requiring log inspection; review state uses a distinct diff --git a/specs/repository-locale-and-localization.md b/specs/repository-locale-and-localization.md index 12a1fe38c..232ab4619 100644 --- a/specs/repository-locale-and-localization.md +++ b/specs/repository-locale-and-localization.md @@ -962,8 +962,10 @@ keeps raw provider and credential-health diagnostics out of UI. Setup itself remains one authoritative English artifact while it creates the repository profile. The generic Actions Job Summary now resolves one complete catalog in the repository locale, localizes its headings and explanatory labels, preserves -machine values, and emits localization evidence once instead of duplicating it -inside and below the main table. Lifecycle and the remaining public surfaces are +machine values, projects compact localized result states and error recovery by +stable error category, omits internal step narration and arbitrary error text, +and emits localization evidence once instead of duplicating it inside and below +the main table. Lifecycle and the remaining public surfaces are not claimed complete by this evidence. The addressed-Think follow-up removes its feature-owned comment mutation and diff --git a/specs/semantic-github-publication-and-notification.md b/specs/semantic-github-publication-and-notification.md index 9158c2fe7..5ef6e3b08 100644 --- a/specs/semantic-github-publication-and-notification.md +++ b/specs/semantic-github-publication-and-notification.md @@ -546,7 +546,9 @@ names may follow repository conventions: 4. one coordinator for status-card reconciliation and transition deduplication; 5. feature renderers for plan, progress, branch sync, Bugbot, release, access, inactivity, command replies, and failures; -6. a Job Summary projection that includes all former step/debug evidence; and +6. a Job Summary projection that includes compact semantic outcomes, stable + machine evidence, and localized recovery guidance while leaving step/debug + narration in masked logs; and 7. a temporary compatibility adapter that rejects attempts to publish generic `Result.steps` and records the attempted source in tests/logs. @@ -915,8 +917,9 @@ generic output is left intact. 1. Land contracts, characterization tests, metrics, and conversation string/call inventories with no behavior change. -2. Route routine lifecycle and completion to `none`; move all former step/debug - evidence to Job Summary. +2. Route routine lifecycle and completion to `none`; move useful semantic + evidence to the Job Summary and keep internal step/debug narration in masked + logs or machine results. 3. Migrate plan, progress, commit/reopen, closure, inactivity, and access-policy publication. 4. Adopt the shared marker/localization envelope in branch sync, Bugbot, and @@ -936,8 +939,9 @@ machine markers, and bounded English-default/localized renderers. The executable mutation inventory, locale-branch ratchet, and pseudo-plural ratchet protect these boundaries. Subsequent deployment and setup-doctor slices cover their feature-owned views, and the generic Job Summary slice resolves repository- -locale copy atomically, retains sanitized operator steps, and renders one -localization evidence section instead of two. Other capability rows and the +locale copy atomically, renders compact result states and safe category-localized +error recovery, excludes internal steps and arbitrary error messages, and renders +one localization evidence section instead of two. Other capability rows and the global numeric budget remain open and are not claimed complete by this milestone. The initial-issue slice now enforces the §6.2 matrix at the route boundary. A @@ -1133,7 +1137,8 @@ removed. 4. Implement owned-card query/mutation adapters, reconciliation, stale guards, transition deduplication, and provider/race tests. 5. Migrate routine lifecycle and common completion; make generic `Result.steps` - publication impossible and move its evidence to Job Summary. + publication impossible and project only stable, useful operator evidence in + the Job Summary. 6. Migrate issue onboarding/help, plans, progress, push/reopen, issue close, inactivity, access-policy, and single actions with end-to-end tests. 7. Adopt shared contracts in branch sync, Bugbot, and release while retaining diff --git a/src/actions/__tests__/github_action_completion.test.ts b/src/actions/__tests__/github_action_completion.test.ts index 854aa48d2..a8213413b 100644 --- a/src/actions/__tests__/github_action_completion.test.ts +++ b/src/actions/__tests__/github_action_completion.test.ts @@ -311,6 +311,8 @@ describe('finishGithubAction', () => { expect(summary).toContain('## Detalles del resultado'); expect(summary).toContain('| Locale del repositorio | `es-ES` |'); expect(summary.match(/## Localización/gu)).toHaveLength(1); + expect(summary).toContain('✅ **MetadataUseCase** — Completado'); + expect(summary).not.toContain('Updated labels.'); expect(summary).not.toContain('## Localization'); }); @@ -518,7 +520,8 @@ describe('finishGithubAction', () => { ); expect(mockPublishInvoke).toHaveBeenCalledWith(expect.objectContaining({ locale: 'en-US' })); - expect(mockSummaryPublish).toHaveBeenCalledWith(expect.stringContaining('Title normalization failed.')); + expect(mockSummaryPublish).toHaveBeenCalledWith(expect.stringContaining('`provider.unavailable`')); + expect(mockSummaryPublish).toHaveBeenCalledWith(expect.not.stringContaining('Title normalization failed.')); expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('Title normalization failed.')); }); diff --git a/src/application/policies/__tests__/action_summary_policy.test.ts b/src/application/policies/__tests__/action_summary_policy.test.ts index ffac0b099..202d92784 100644 --- a/src/application/policies/__tests__/action_summary_policy.test.ts +++ b/src/application/policies/__tests__/action_summary_policy.test.ts @@ -89,7 +89,7 @@ describe('action summary policy', () => { expect(summary).not.toContain('
Repository
'); }); - it('renders bounded result details and lifecycle metadata', () => { + it('renders compact result states and keeps internal step narration out of the summary', () => { const summary = buildActionSummary({ owner: 'owner', repository: 'repo', @@ -97,12 +97,18 @@ describe('action summary policy', () => { issueNumber: 7, pullRequestNumber: -1, lifecycleState: 'planned', - results: [new Result({ id: 'Plan', success: true, executed: true, steps: ['## Ready', 'safe | text'] })], + results: [ + new Result({ id: 'Plan', success: true, executed: true, steps: ['## Ready', 'safe | text'] }), + new Result({ id: 'OptionalStep', success: false, executed: false, steps: ['Skipped because of internal policy.'] }), + ], }); expect(summary).toContain('# Copilot execution'); expect(summary).toContain('`planned`'); - expect(summary).toContain('safe | text'); + expect(summary).toContain('✅ **Plan** — Succeeded'); + expect(summary).toContain('⏭️ **OptionalStep** — Skipped'); + expect(summary).not.toContain('safe | text'); + expect(summary).not.toContain('Skipped because of internal policy.'); expect(summary).not.toContain('{{'); }); @@ -142,6 +148,11 @@ describe('action summary policy', () => { expect(summary).toContain('## Detalles del resultado'); expect(summary).toContain('**Resultado sin nombre**'); expect(summary).toContain('**Impacto:**'); + expect(summary).toContain('El workflow no pudo completar la operación solicitada.'); + expect(summary).toContain('**Código de error:** `workflow.failed`'); + expect(summary).toContain('**Reintentable:** Sí'); + expect(summary).not.toContain('Workflow failed.'); + expect(summary).not.toContain('The workflow could not complete'); expect(summary).toContain('## Localización'); expect(summary).not.toContain('## Localization'); }); @@ -157,10 +168,36 @@ describe('action summary policy', () => { }); expect(summary).toContain('❌ Failure'); + expect(summary).toContain('**Error code:** `workflow.failed`'); + expect(summary).toContain('**Retryable:** Yes'); expect(summary).not.toContain('secret-value'); expect(summary).not.toContain('at hidden'); }); + it('distinguishes an intentional all-skipped run from success and treats any semantic error as failure', () => { + const base = { + owner: 'owner', repository: 'repo', eventName: 'issues', issueNumber: 7, pullRequestNumber: -1, + }; + const skipped = buildActionSummary({ + ...base, + results: [new Result({ id: 'Optional', success: false, executed: false })], + }); + const rejected = buildActionSummary({ + ...base, + results: [new Result({ + id: 'Rejected', success: false, executed: false, + errors: [new ApplicationError('authorization.denied', 'Internal rejection detail.')], + })], + }); + + expect(skipped).toContain('| Status | ⏭️ Skipped |'); + expect(skipped).toContain('⏭️ **Optional** — Skipped'); + expect(rejected).toContain('| Status | ❌ Failure |'); + expect(rejected).toContain('❌ **Rejected** — Failed'); + expect(rejected).toContain('**Error code:** `authorization.denied`'); + expect(rejected).not.toContain('Internal rejection detail.'); + }); + it('reports active findings as a warning unless fail-on-unresolved is enabled', () => { const summary = buildActionSummary({ owner: 'owner', diff --git a/src/application/policies/__tests__/agent_response_schemas.test.ts b/src/application/policies/__tests__/agent_response_schemas.test.ts index b885e4368..e02dbb2cf 100644 --- a/src/application/policies/__tests__/agent_response_schemas.test.ts +++ b/src/application/policies/__tests__/agent_response_schemas.test.ts @@ -43,9 +43,12 @@ describe('production agent response schemas', () => { maxItems: 6, }); expect(PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA.properties.validation).toMatchObject({ + type: ['array', 'null'], minItems: 1, maxItems: 8, }); + expect(PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA.properties.validationHeading) + .toMatchObject({ type: ['string', 'null'] }); expect(PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA.required).toContain('closesLinkedIssue'); }); }); diff --git a/src/application/policies/__tests__/pull_request_description_content_policy.test.ts b/src/application/policies/__tests__/pull_request_description_content_policy.test.ts index ba06eaed2..9aa7e4b72 100644 --- a/src/application/policies/__tests__/pull_request_description_content_policy.test.ts +++ b/src/application/policies/__tests__/pull_request_description_content_policy.test.ts @@ -44,6 +44,18 @@ describe('pull request description content policy', () => { expect(result.kind === 'valid' && result.markdown).toContain('Closes #42'); }); + it('omits validation entirely when no verified evidence is available', () => { + const result = renderPullRequestDescriptionContent(content({ + validationHeading: null, + validation: null, + }), 'en-US'); + + expect(result).toMatchObject({ kind: 'valid' }); + expect(result.kind === 'valid' && result.markdown).not.toContain('Validation'); + expect(result.kind === 'valid' && result.markdown).not.toContain('not run'); + expect(result.kind === 'valid' && result.markdown).toContain('## What changed'); + }); + it.each([ ['missing fields', { validation: undefined }, 'shape'], ['additional fields', { untrusted: 'value' }, 'shape'], @@ -51,6 +63,8 @@ describe('pull request description content policy', () => { ['blank content', { changes: [' ', 'Material change.'] }, 'unsafe-markdown'], ['notes without heading', { reviewNotes: ['Risk.'], reviewNotesHeading: null }, 'shape'], ['heading without notes', { reviewNotes: null, reviewNotesHeading: 'Review notes' }, 'shape'], + ['validation without heading', { validation: ['`pnpm test`'], validationHeading: null }, 'shape'], + ['validation heading without evidence', { validation: null, validationHeading: 'Validation' }, 'shape'], ['closing without issue', { closesLinkedIssue: true }, 'shape'], ['more than three overview sentences', { overview: 'One. Two. Three. Four.' }, 'sentence-count'], ['more than three Japanese overview sentences', { overview: '一つです。二つです。三つです。四つです。' }, 'sentence-count'], diff --git a/src/application/policies/action_summary_message_catalog.ts b/src/application/policies/action_summary_message_catalog.ts index dfca7872f..adc9d3670 100644 --- a/src/application/policies/action_summary_message_catalog.ts +++ b/src/application/policies/action_summary_message_catalog.ts @@ -1,4 +1,5 @@ import type { AgentConfiguration } from '../../domain/agent'; +import type { ApplicationErrorKind } from '../../data/model/application_error'; import { MESSAGE_CATALOG_VERSION, type CatalogMessage, @@ -18,7 +19,8 @@ const SIMPLE_MESSAGE_KEYS = Object.freeze([ 'pullRequestLocale', 'catalogResolution', 'descriptors', 'reason', 'failure', 'findings', 'partial', 'superseded', 'skipped', 'dryRun', 'success', 'invalid', 'none', 'noResult', 'unnamedResult', 'impact', 'cause', 'action', - 'retainedState', 'reference', + 'retainedState', 'reference', 'retryable', 'yes', 'no', + 'resultSucceeded', 'resultFailed', 'resultSkipped', ] as const); const TEMPLATE_MESSAGE_IDS = Object.freeze([ @@ -33,19 +35,28 @@ const FINDING_STATE_KEYS = Object.freeze([ 'verification-required', 'unknown', ] as const); +const ERROR_KIND_KEYS: readonly ApplicationErrorKind[] = Object.freeze([ + 'configuration', 'authorization', 'provider', 'agent', 'validation', 'workflow', 'unknown', +]); + +const ERROR_FIELD_KEYS = Object.freeze(['impact', 'action', 'retainedState'] as const); + type SimpleMessageKey = typeof SIMPLE_MESSAGE_KEYS[number]; type SimpleMessageId = `summary.${SimpleMessageKey}`; type TemplateMessageId = typeof TEMPLATE_MESSAGE_IDS[number]; export type ActionSummaryFindingState = typeof FINDING_STATE_KEYS[number]; type FindingStateMessageId = `summary.findingState.${ActionSummaryFindingState}`; +type ErrorField = typeof ERROR_FIELD_KEYS[number]; +type ErrorMessageId = `summary.error.${ApplicationErrorKind}.${ErrorField}`; -export type ActionSummaryMessageId = SimpleMessageId | TemplateMessageId | FindingStateMessageId; +export type ActionSummaryMessageId = SimpleMessageId | TemplateMessageId | FindingStateMessageId | ErrorMessageId; export type ActionSummaryMessageCatalog = ResolvedMessageCatalogView; export const ACTION_SUMMARY_MESSAGE_IDS: readonly ActionSummaryMessageId[] = Object.freeze([ ...SIMPLE_MESSAGE_KEYS.map(key => `summary.${key}` as const), ...TEMPLATE_MESSAGE_IDS, ...FINDING_STATE_KEYS.map(key => `summary.findingState.${key}` as const), + ...ERROR_KIND_KEYS.flatMap(kind => ERROR_FIELD_KEYS.map(field => `summary.error.${kind}.${field}` as const)), ]); const ENGLISH_SIMPLE: Readonly> = Object.freeze({ @@ -81,10 +92,16 @@ const ENGLISH_SIMPLE: Readonly> = Object.freeze noResult: 'No application result was produced.', unnamedResult: 'Unnamed result', impact: 'Impact', - cause: 'Cause', + cause: 'Error code', action: 'Action', retainedState: 'Retained state', reference: 'Reference', + retryable: 'Retryable', + yes: 'Yes', + no: 'No', + resultSucceeded: 'Succeeded', + resultFailed: 'Failed', + resultSkipped: 'Skipped', }); const SPANISH_SIMPLE: Readonly> = Object.freeze({ @@ -120,10 +137,16 @@ const SPANISH_SIMPLE: Readonly> = Object.freeze noResult: 'No se ha producido ningún resultado de aplicación.', unnamedResult: 'Resultado sin nombre', impact: 'Impacto', - cause: 'Causa', + cause: 'Código de error', action: 'Acción', retainedState: 'Estado conservado', reference: 'Referencia', + retryable: 'Reintentable', + yes: 'Sí', + no: 'No', + resultSucceeded: 'Completado', + resultFailed: 'Fallido', + resultSkipped: 'Omitido', }); const ENGLISH_TEMPLATES: Readonly> = Object.freeze({ @@ -160,15 +183,98 @@ const SPANISH_FINDING_STATES: Readonly unknown: 'desconocidos', }); +type ErrorMessages = Readonly>>>; + +const ENGLISH_ERRORS: ErrorMessages = Object.freeze({ + configuration: Object.freeze({ + impact: 'The operation could not use the configured values.', + action: 'Correct the configuration or choose a supported capability before retrying.', + retainedState: 'No new state or external effect was created.', + }), + authorization: Object.freeze({ + impact: 'The operation could not authenticate or access a required resource.', + action: 'Correct the credential or grant the documented permission before retrying.', + retainedState: 'No new state or external effect was created.', + }), + provider: Object.freeze({ + impact: 'A provider operation did not complete.', + action: 'Inspect the error code and retry only when the provider state or availability has changed.', + retainedState: 'Existing state and completed external effects remain in place.', + }), + agent: Object.freeze({ + impact: 'The configured agent did not produce usable product content.', + action: 'Inspect the sanitized agent status and retry with a compatible provider or model.', + retainedState: 'Existing state remains in place; rejected content was not published.', + }), + validation: Object.freeze({ + impact: 'The supplied input was rejected before the operation could continue.', + action: 'Correct the input and retry.', + retainedState: 'No new state or external effect was created.', + }), + workflow: Object.freeze({ + impact: 'The workflow could not complete the requested operation.', + action: 'Inspect the current state and retry only if the operation is still required.', + retainedState: 'Existing state and confirmed completed effects remain in place.', + }), + unknown: Object.freeze({ + impact: 'An unexpected failure was handled safely.', + action: 'Use the reference to investigate before retrying.', + retainedState: 'Existing state and confirmed completed effects remain in place.', + }), +}); + +const SPANISH_ERRORS: ErrorMessages = Object.freeze({ + configuration: Object.freeze({ + impact: 'La operación no pudo usar los valores configurados.', + action: 'Corrige la configuración o elige una capacidad compatible antes de reintentarlo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + authorization: Object.freeze({ + impact: 'La operación no pudo autenticarse o acceder a un recurso necesario.', + action: 'Corrige la credencial o concede el permiso documentado antes de reintentarlo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + provider: Object.freeze({ + impact: 'Una operación del proveedor no se completó.', + action: 'Revisa el código de error y reinténtalo solo cuando haya cambiado el estado o la disponibilidad del proveedor.', + retainedState: 'El estado existente y los efectos externos completados se mantienen.', + }), + agent: Object.freeze({ + impact: 'El agente configurado no produjo contenido de producto utilizable.', + action: 'Revisa el estado saneado del agente y reinténtalo con un proveedor o modelo compatible.', + retainedState: 'El estado existente se mantiene y el contenido rechazado no se publicó.', + }), + validation: Object.freeze({ + impact: 'La entrada suministrada se rechazó antes de continuar la operación.', + action: 'Corrige la entrada y reinténtalo.', + retainedState: 'No se creó ningún estado ni efecto externo nuevo.', + }), + workflow: Object.freeze({ + impact: 'El workflow no pudo completar la operación solicitada.', + action: 'Revisa el estado actual y reinténtalo solo si la operación sigue siendo necesaria.', + retainedState: 'El estado existente y los efectos completados y confirmados se mantienen.', + }), + unknown: Object.freeze({ + impact: 'Un fallo inesperado se gestionó de forma segura.', + action: 'Usa la referencia para investigar antes de reintentarlo.', + retainedState: 'El estado existente y los efectos completados y confirmados se mantienen.', + }), +}); + function catalogMessages( simple: Readonly>, templates: Readonly>, findingStates: Readonly>, + errors: ErrorMessages, ): Readonly> { return Object.freeze({ ...Object.fromEntries(SIMPLE_MESSAGE_KEYS.map(key => [`summary.${key}`, simple[key]])), ...templates, ...Object.fromEntries(FINDING_STATE_KEYS.map(key => [`summary.findingState.${key}`, findingStates[key]])), + ...Object.fromEntries(ERROR_KIND_KEYS.flatMap(kind => ERROR_FIELD_KEYS.map(field => [ + `summary.error.${kind}.${field}`, + errors[kind][field], + ]))), }) as Readonly>; } @@ -176,14 +282,14 @@ export const ENGLISH_ACTION_SUMMARY_DEFINITION: MessageCatalogDefinition = Object.freeze({ version: MESSAGE_CATALOG_VERSION, locale: 'es-ES', compatibleBaseLanguage: 'es', - messages: catalogMessages(SPANISH_SIMPLE, SPANISH_TEMPLATES, SPANISH_FINDING_STATES), + messages: catalogMessages(SPANISH_SIMPLE, SPANISH_TEMPLATES, SPANISH_FINDING_STATES, SPANISH_ERRORS), }); export const ACTION_SUMMARY_CATALOG_DEFINITIONS = Object.freeze([ diff --git a/src/application/policies/action_summary_policy.ts b/src/application/policies/action_summary_policy.ts index 1e2f73a90..4f96519aa 100644 --- a/src/application/policies/action_summary_policy.ts +++ b/src/application/policies/action_summary_policy.ts @@ -1,6 +1,5 @@ import type { Result } from '../../data/model/result'; -import { sanitizeAgentMarkdown, sanitizePublishedError } from './github_comment_publication_policy'; -import { buildApplicationErrorPresentation } from './application_error_presentation_policy'; +import { sanitizeAgentMarkdown } from './github_comment_publication_policy'; import { projectBugbotResultTelemetry, type BugbotResultTelemetryProjection, @@ -61,7 +60,7 @@ export function buildActionSummary( context: ActionSummaryContext, catalog: ActionSummaryMessageCatalog = resolveStaticActionSummaryCatalog(context.locale?.repository ?? 'en-US'), ): string { - const failures = context.results.filter(result => !result.success && result.executed); + const failures = context.results.filter(result => result.errors.length > 0 || (!result.success && result.executed)); const findingStateProjection = projectBugbotResultFindingStates(context.results); const findingStates = findingStateProjection.status === 'valid' ? findingStateProjection.counts : undefined; const telemetryProjection = projectBugbotResultTelemetry(context.results); @@ -74,6 +73,7 @@ export function buildActionSummary( hasActionableFindings, failOnUnresolvedFindings: context.failOnUnresolvedFindings === true, bugbotTelemetry, + allResultsSkipped: context.results.length > 0 && context.results.every(result => !result.executed), }, catalog); const target = resolveActionSummaryTarget(context, catalog); const lifecycle = context.lifecycleState ? `\`${sanitizeAgentMarkdown(context.lifecycleState, 100)}\`` : '—'; @@ -176,6 +176,7 @@ interface ActionSummaryStatusInput { readonly hasActionableFindings: boolean; readonly failOnUnresolvedFindings: boolean; readonly bugbotTelemetry?: BugbotTelemetryProjection; + readonly allResultsSkipped: boolean; } function resolveActionSummaryStatus(input: ActionSummaryStatusInput, catalog: ActionSummaryMessageCatalog): string { @@ -183,6 +184,7 @@ function resolveActionSummaryStatus(input: ActionSummaryStatusInput, catalog: Ac if (input.bugbotTelemetry?.outcome === 'failed') return `❌ ${catalogText(catalog, 'summary.failure')}`; if (input.hasActionableFindings && input.failOnUnresolvedFindings) return `❌ ${catalogText(catalog, 'summary.failure')}`; if (input.hasActionableFindings) return `⚠️ ${catalogText(catalog, 'summary.findings')}`; + if (input.allResultsSkipped) return `⏭️ ${catalogText(catalog, 'summary.skipped')}`; switch (input.bugbotTelemetry?.outcome) { case 'partial': return `⚠️ ${catalogText(catalog, 'summary.partial')}`; case 'superseded': return `⏭️ ${catalogText(catalog, 'summary.superseded')}`; @@ -228,25 +230,40 @@ function formatFindingStates( function renderResults(results: readonly Result[], catalog: ActionSummaryMessageCatalog): string { if (results.length === 0) return `_${catalogText(catalog, 'summary.noResult')}_`; return results.map(result => { - const icon = result.success ? '✅' : '❌'; - const details = result.steps - .filter(step => step.trim()) - .map(step => ` - ${sanitizeAgentMarkdown(step, 1_000)}`); + const failed = result.errors.length > 0 || (!result.success && result.executed); + const outcome = failed + ? { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') } + : !result.executed + ? { icon: '⏭️', label: catalogText(catalog, 'summary.resultSkipped') } + : result.success + ? { icon: '✅', label: catalogText(catalog, 'summary.resultSucceeded') } + : { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') }; const errors = result.errors .flatMap((error) => { - const view = buildApplicationErrorPresentation(error); return [ - ` - **${catalogText(catalog, 'summary.impact')}:** ${sanitizePublishedError(view.impact)}`, - ` - **${catalogText(catalog, 'summary.cause')} (\`${view.code}\`):** ${sanitizePublishedError(view.cause)}`, - ` - **${catalogText(catalog, 'summary.action')}:** ${sanitizePublishedError(view.action)}`, - ` - **${catalogText(catalog, 'summary.retainedState')}:** ${sanitizePublishedError(view.retainedState)}`, - ` - **${catalogText(catalog, 'summary.reference')}:** \`${view.reference}\``, + ` - **${catalogText(catalog, 'summary.impact')}:** ${errorCatalogText(catalog, error.kind, 'impact')}`, + ` - **${catalogText(catalog, 'summary.cause')}:** \`${error.code}\``, + ` - **${catalogText(catalog, 'summary.action')}:** ${errorCatalogText(catalog, error.kind, 'action')}`, + ` - **${catalogText(catalog, 'summary.retainedState')}:** ${errorCatalogText(catalog, error.kind, 'retainedState')}`, + ` - **${catalogText(catalog, 'summary.retryable')}:** ${catalogText(catalog, error.retryable ? 'summary.yes' : 'summary.no')}`, + ` - **${catalogText(catalog, 'summary.reference')}:** \`${error.correlationId}\``, ]; }); - return [`- ${icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}**`, ...details, ...errors].join('\n'); + return [ + `- ${outcome.icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}** — ${outcome.label}`, + ...errors, + ].join('\n'); }).join('\n'); } +function errorCatalogText( + catalog: ActionSummaryMessageCatalog, + kind: Result['errors'][number]['kind'], + field: 'impact' | 'action' | 'retainedState', +): string { + return catalogText(catalog, `summary.error.${kind}.${field}`); +} + function catalogText( catalog: ActionSummaryMessageCatalog, id: Parameters[0], diff --git a/src/application/policies/agent_response_schemas.ts b/src/application/policies/agent_response_schemas.ts index bd3c4ee4d..8c5e50a97 100644 --- a/src/application/policies/agent_response_schemas.ts +++ b/src/application/policies/agent_response_schemas.ts @@ -90,9 +90,9 @@ export const PULL_REQUEST_DESCRIPTION_RESPONSE_SCHEMA = { maxItems: 6, items: { type: 'string', minLength: 1, maxLength: 1_000 }, }, - validationHeading: { type: 'string', minLength: 1, maxLength: 100 }, + validationHeading: { type: ['string', 'null'], minLength: 1, maxLength: 100 }, validation: { - type: 'array', + type: ['array', 'null'], minItems: 1, maxItems: 8, items: { type: 'string', minLength: 1, maxLength: 1_000 }, diff --git a/src/application/policies/pull_request_description_content_policy.ts b/src/application/policies/pull_request_description_content_policy.ts index 65c036451..49c905a8d 100644 --- a/src/application/policies/pull_request_description_content_policy.ts +++ b/src/application/policies/pull_request_description_content_policy.ts @@ -19,8 +19,8 @@ export type PullRequestDescriptionContent = { readonly overview: string; readonly whatChangedHeading: string; readonly changes: readonly string[]; - readonly validationHeading: string; - readonly validation: readonly string[]; + readonly validationHeading: string | null; + readonly validation: readonly string[] | null; readonly reviewNotesHeading: string | null; readonly reviewNotes: readonly string[] | null; readonly closesLinkedIssue: boolean; @@ -52,9 +52,9 @@ export function renderPullRequestDescriptionContent( const rawContent = [ parsed.overview, parsed.whatChangedHeading, - parsed.validationHeading, + ...(parsed.validationHeading ? [parsed.validationHeading] : []), ...parsed.changes, - ...parsed.validation, + ...(parsed.validation ?? []), ...(parsed.reviewNotesHeading ? [parsed.reviewNotesHeading] : []), ...(parsed.reviewNotes ?? []), ]; @@ -64,9 +64,11 @@ export function renderPullRequestDescriptionContent( const overview = sanitizeBlock(parsed.overview); const whatChangedHeading = sanitizeInline(parsed.whatChangedHeading); - const validationHeading = sanitizeInline(parsed.validationHeading); + const validationHeading = parsed.validationHeading === null + ? null + : sanitizeInline(parsed.validationHeading); const changes = parsed.changes.map(sanitizeInline); - const validation = parsed.validation.map(sanitizeInline); + const validation = parsed.validation?.map(sanitizeInline) ?? null; const reviewNotesHeading = parsed.reviewNotesHeading === null ? null : sanitizeInline(parsed.reviewNotesHeading); @@ -75,9 +77,9 @@ export function renderPullRequestDescriptionContent( const allContent = [ overview, whatChangedHeading, - validationHeading, + ...(validationHeading ? [validationHeading] : []), ...changes, - ...validation, + ...(validation ?? []), ...(reviewNotesHeading ? [reviewNotesHeading] : []), ...(reviewNotes ?? []), ]; @@ -88,7 +90,7 @@ export function renderPullRequestDescriptionContent( return { kind: 'invalid', reason: 'sentence-count' }; } if (hasDuplicates(changes, targetLocale) - || hasDuplicates(validation, targetLocale) + || (validation !== null && hasDuplicates(validation, targetLocale)) || (reviewNotes && hasDuplicates(reviewNotes, targetLocale))) { return { kind: 'invalid', reason: 'duplicate-item' }; } @@ -96,8 +98,10 @@ export function renderPullRequestDescriptionContent( const sections = [ overview, `## ${whatChangedHeading}\n\n${renderList(changes)}`, - `## ${validationHeading}\n\n${renderList(validation)}`, ]; + if (validationHeading && validation) { + sections.push(`## ${validationHeading}\n\n${renderList(validation)}`); + } if (reviewNotesHeading && reviewNotes) { sections.push(`## ${reviewNotesHeading}\n\n${renderList(reviewNotes)}`); } @@ -118,18 +122,28 @@ function parseContent(payload: Readonly>): PullRequestDe return undefined; } const changes = stringArray(payload.changes, 2, 6); - const validation = stringArray(payload.validation, 1, 8); if (typeof payload.overview !== 'string' || payload.overview.length > 1_500 || typeof payload.whatChangedHeading !== 'string' || payload.whatChangedHeading.length > 100 - || typeof payload.validationHeading !== 'string' - || payload.validationHeading.length > 100 || !changes - || !validation || typeof payload.closesLinkedIssue !== 'boolean') { return undefined; } + let validation: readonly string[] | null; + let validationHeading: string | null; + if (payload.validation === null) { + if (payload.validationHeading !== null) return undefined; + validation = null; + validationHeading = null; + } else { + const parsedValidation = stringArray(payload.validation, 1, 8); + if (!parsedValidation + || typeof payload.validationHeading !== 'string' + || payload.validationHeading.length > 100) return undefined; + validation = parsedValidation; + validationHeading = payload.validationHeading; + } let reviewNotes: readonly string[] | null; let reviewNotesHeading: string | null; if (payload.reviewNotes === null) { @@ -148,7 +162,7 @@ function parseContent(payload: Readonly>): PullRequestDe overview: payload.overview, whatChangedHeading: payload.whatChangedHeading, changes, - validationHeading: payload.validationHeading, + validationHeading, validation, reviewNotesHeading, reviewNotes, diff --git a/src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts b/src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts index f80479dca..932b96743 100644 --- a/src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts +++ b/src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts @@ -188,6 +188,19 @@ describe('UpdatePullRequestDescriptionUseCase', () => { expect(mockUpdateDescription).toHaveBeenCalled(); }); + it('publishes no validation section when the agent has no verified evidence', async () => { + mockAskAgent.mockResolvedValue(descriptionContent('PR does X.', { + validationHeading: null, + validation: null, + })); + + const results = await useCase.invoke(request()); + + expect(results[0]).toMatchObject({ success: true, executed: true }); + expect(mockUpdateDescription).toHaveBeenCalledWith(10, expect.not.stringContaining('Validation')); + expect(mockUpdateDescription).toHaveBeenCalledWith(10, expect.not.stringContaining('not run')); + }); + it('does not publish blank agent output', async () => { mockAskAgent.mockResolvedValue(''); const results = await useCase.invoke(request()); diff --git a/src/architecture/__tests__/github_publication_boundaries.test.ts b/src/architecture/__tests__/github_publication_boundaries.test.ts index 1ce8010d6..4b43103e2 100644 --- a/src/architecture/__tests__/github_publication_boundaries.test.ts +++ b/src/architecture/__tests__/github_publication_boundaries.test.ts @@ -116,6 +116,16 @@ describe('GitHub conversation publication boundaries', () => { expect(presentation).not.toMatch(/operations|Automatic Actions|Feature Actions/u); }); + it('keeps the generic Job Summary independent from internal Result steps', () => { + const presentation = readFileSync( + join(root, 'src/application/policies/action_summary_policy.ts'), + 'utf8', + ); + + expect(presentation).not.toMatch(/result\.steps/u); + expect(presentation).not.toContain('buildApplicationErrorPresentation'); + }); + it('keeps Bugbot public presentation free of pseudo-plural copy', () => { const files = [ 'src/application/policies/bugbot_message_catalog.ts', diff --git a/src/prompts/__tests__/update_pull_request_description.test.ts b/src/prompts/__tests__/update_pull_request_description.test.ts index e3cfc0c9c..8b080a67e 100644 --- a/src/prompts/__tests__/update_pull_request_description.test.ts +++ b/src/prompts/__tests__/update_pull_request_description.test.ts @@ -20,7 +20,8 @@ describe('getUpdatePullRequestDescriptionPrompt', () => { expect(prompt).toContain('git diff main...feature/123'); expect(prompt).toContain('`whatChangedHeading`'); expect(prompt).toContain('never infer that result from the presence of test files or commands'); - expect(prompt).toContain('validation was not run or was not available'); + expect(prompt).toContain('set both fields to `null`'); + expect(prompt).toContain('do not add a “not run” placeholder'); expect(prompt).toContain('`validationHeading`'); expect(prompt).toContain('application renders the Markdown structure'); expect(prompt).toContain('normally under 4,000 characters'); diff --git a/src/prompts/update_pull_request_description.ts b/src/prompts/update_pull_request_description.ts index 1de72373b..dfcd712c2 100644 --- a/src/prompts/update_pull_request_description.ts +++ b/src/prompts/update_pull_request_description.ts @@ -19,7 +19,7 @@ Write every human-readable sentence in {{targetLocale}}. Preserve code identifie 3. Use the issue description below for context and intent. 4. Provide \`overview\` as one to three sentences that state the outcome and why it matters. 5. Provide \`whatChangedHeading\` as the plain-text {{targetLocale}} equivalent of "What changed" and \`changes\` as two to six short, outcome-oriented items. Do not inventory files, use-case names, internal categories, or every implementation step. -6. Provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only commands, automated checks, or manual scenarios supported by available evidence. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no execution evidence is available, say concisely in {{targetLocale}} that validation was not run or was not available. +6. When execution or manual-verification evidence is available, provide \`validationHeading\` as the plain-text {{targetLocale}} equivalent of "Validation" and \`validation\` with only the supported commands, automated checks, or manual scenarios. Never claim a check passed unless the evidence says it did, and never infer that result from the presence of test files or commands. When no verification evidence is available, set both fields to \`null\`; do not add a “not run” placeholder. 7. Set \`reviewNotesHeading\` and \`reviewNotes\` to \`null\` unless reviewers need material migration, security, performance, compatibility, rollout, manual-verification, risk, or follow-up context. Otherwise use the localized plain-text heading and one to four concise items. {{relatedIssueInstruction}} 8. Keep the description practical and normally under 4,000 characters. It must never exceed 12,000 characters. Do not use emoji, horizontal separators, generic checklists, empty headings, repeated statements, placeholder text, or unsupported "no impact" claims. 9. Return one JSON object with exactly \`outputLocale\`, \`overview\`, \`whatChangedHeading\`, \`changes\`, \`validationHeading\`, \`validation\`, \`reviewNotesHeading\`, \`reviewNotes\`, and \`closesLinkedIssue\`. Every content field is plain text except Markdown links, code spans, refs, and commands inside content values. The application renders the Markdown structure; do not include headings, bullet prefixes, a preamble, meta-commentary, or code fence in the values. From 91ff6d6c1540fba8b0dca618aa6fab9b14c50177 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Tue, 15 Sep 2026 05:45:54 +0200 Subject: [PATCH 2/2] codex-operator-summary-ux: aggregate routine action results --- build/github_action/index.js | 60 ++++++++++--------- docs/development/architecture.mdx | 5 +- docs/features.mdx | 5 +- docs/issues/notifications-and-auto-close.mdx | 4 +- specs/repository-locale-and-localization.md | 5 +- ...tic-github-publication-and-notification.md | 9 +-- .../github_action_completion.test.ts | 10 +++- .../__tests__/action_summary_policy.test.ts | 22 +++---- .../action_summary_message_catalog.ts | 10 +--- .../policies/action_summary_policy.ts | 48 +++++++++------ .../github_publication_boundaries.test.ts | 1 + 11 files changed, 102 insertions(+), 77 deletions(-) diff --git a/build/github_action/index.js b/build/github_action/index.js index 5115377a6..7e2beb651 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -41143,7 +41143,7 @@ const SIMPLE_MESSAGE_KEYS = Object.freeze([ 'resultDetails', 'localization', 'repositoryLocale', 'issueLocale', 'pullRequestLocale', 'catalogResolution', 'descriptors', 'reason', 'failure', 'findings', 'partial', 'superseded', 'skipped', 'dryRun', 'success', 'invalid', - 'none', 'noResult', 'unnamedResult', 'impact', 'cause', 'action', + 'none', 'impact', 'cause', 'action', 'retainedState', 'reference', 'retryable', 'yes', 'no', 'resultSucceeded', 'resultFailed', 'resultSkipped', ]); @@ -41180,7 +41180,7 @@ const ENGLISH_SIMPLE = Object.freeze({ results: 'Results', findingStates: 'Finding states', bugbotReview: 'Bugbot review', - resultDetails: 'Result details', + resultDetails: 'Failure details', localization: 'Localization', repositoryLocale: 'Repository locale', issueLocale: 'Issue locale', @@ -41197,8 +41197,6 @@ const ENGLISH_SIMPLE = Object.freeze({ success: 'Success', invalid: 'invalid', none: 'none', - noResult: 'No application result was produced.', - unnamedResult: 'Unnamed result', impact: 'Impact', cause: 'Error code', action: 'Action', @@ -41224,7 +41222,7 @@ const SPANISH_SIMPLE = Object.freeze({ results: 'Resultados', findingStates: 'Estados de los hallazgos', bugbotReview: 'Revisión de Bugbot', - resultDetails: 'Detalles del resultado', + resultDetails: 'Detalles del fallo', localization: 'Localización', repositoryLocale: 'Locale del repositorio', issueLocale: 'Locale de la issue', @@ -41241,8 +41239,6 @@ const SPANISH_SIMPLE = Object.freeze({ success: 'Correcto', invalid: 'no válido', none: 'ninguno', - noResult: 'No se ha producido ningún resultado de aplicación.', - unnamedResult: 'Resultado sin nombre', impact: 'Impacto', cause: 'Código de error', action: 'Acción', @@ -41423,7 +41419,7 @@ const ENGLISH_LOCALIZATION_SUMMARY_LABELS = Object.freeze({ }); /** Builds one bounded, publication-safe, repository-locale GitHub Actions Job Summary. */ function buildActionSummary(context, catalog = (0, action_summary_message_catalog_1.resolveStaticActionSummaryCatalog)(context.locale?.repository ?? 'en-US')) { - const failures = context.results.filter(result => result.errors.length > 0 || (!result.success && result.executed)); + const failures = context.results.filter(resultFailed); const findingStateProjection = (0, bugbot_result_finding_state_projection_policy_1.projectBugbotResultFindingStates)(context.results); const findingStates = findingStateProjection.status === 'valid' ? findingStateProjection.counts : undefined; const telemetryProjection = (0, bugbot_telemetry_projection_policy_1.projectBugbotResultTelemetry)(context.results); @@ -41446,7 +41442,7 @@ function buildActionSummary(context, catalog = (0, action_summary_message_catalo `| ${catalogText(catalog, 'summary.target')} | ${escapeTable(target)} |`, `| ${catalogText(catalog, 'summary.lifecycle')} | ${lifecycle} |`, `| ${catalogText(catalog, 'summary.descriptionPolicy')} | ${escapeTable(context.pullRequestDescriptionMode ?? '—')} |`, - `| ${catalogText(catalog, 'summary.results')} | ${context.results.length} |`, + `| ${catalogText(catalog, 'summary.results')} | ${formatResultCounts(context.results, catalog)} |`, `| ${catalogText(catalog, 'summary.findingStates')} | ${formatFindingStates(findingStateProjection, catalog)} |`, `| ${catalogText(catalog, 'summary.bugbotReview')} | ${formatBugbotTelemetry(telemetryProjection, catalog)} |`, ]; @@ -41459,12 +41455,13 @@ function buildActionSummary(context, catalog = (0, action_summary_message_catalo `| ${catalogText(catalog, 'summary.property')} | ${catalogText(catalog, 'summary.value')} |`, '| --- | --- |', ...rows, - '', - `## ${catalogText(catalog, 'summary.resultDetails')}`, - '', - renderResults(context.results, catalog), - '', - localization, + ...(failures.length > 0 ? [ + '', + `## ${catalogText(catalog, 'summary.resultDetails')}`, + '', + renderFailures(failures, catalog), + ] : []), + ...(localization ? ['', localization] : []), ].join('\n'); } function actionSummaryLocalizationLabels(catalog) { @@ -41561,18 +41558,27 @@ function formatFindingStates(projection, catalog) { .map(([state, value]) => `${catalogText(catalog, `summary.findingState.${state}`)}=${value}`) .join(', ') || catalogText(catalog, 'summary.none'); } -function renderResults(results, catalog) { - if (results.length === 0) - return `_${catalogText(catalog, 'summary.noResult')}_`; +function formatResultCounts(results, catalog) { + const counts = results.reduce((current, result) => { + if (resultFailed(result)) + current.failed += 1; + else if (!result.executed) + current.skipped += 1; + else + current.succeeded += 1; + return current; + }, { succeeded: 0, failed: 0, skipped: 0 }); + return [ + `${catalogText(catalog, 'summary.resultSucceeded')}: ${counts.succeeded}`, + `${catalogText(catalog, 'summary.resultFailed')}: ${counts.failed}`, + `${catalogText(catalog, 'summary.resultSkipped')}: ${counts.skipped}`, + ].join(' · '); +} +function resultFailed(result) { + return result.errors.length > 0 || (!result.success && result.executed); +} +function renderFailures(results, catalog) { return results.map(result => { - const failed = result.errors.length > 0 || (!result.success && result.executed); - const outcome = failed - ? { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') } - : !result.executed - ? { icon: '⏭️', label: catalogText(catalog, 'summary.resultSkipped') } - : result.success - ? { icon: '✅', label: catalogText(catalog, 'summary.resultSucceeded') } - : { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') }; const errors = result.errors .flatMap((error) => { return [ @@ -41585,7 +41591,7 @@ function renderResults(results, catalog) { ]; }); return [ - `- ${outcome.icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}** — ${outcome.label}`, + `- ❌ **${catalogText(catalog, 'summary.resultFailed')}**`, ...errors, ].join('\n'); }).join('\n'); diff --git a/docs/development/architecture.mdx b/docs/development/architecture.mdx index ee0694263..5b2537e93 100644 --- a/docs/development/architecture.mdx +++ b/docs/development/architecture.mdx @@ -367,8 +367,9 @@ The generic GitHub Actions Job Summary follows the same boundary. One complete typed catalog is resolved for `repository-locale`; bundled English and Spanish, arbitrary dynamic locales, and atomic English fallback share the repository localization resolver. The summary renders localization evidence once, keeps -machine event/state/error values stable, and projects each result as a compact -localized outcome. Internal `Result.steps` and arbitrary error messages remain +machine event/state/error values stable, and projects routine results as compact +localized aggregate counts. Only failed results receive an expanded diagnostic +section. Internal result names, `Result.steps`, and arbitrary error messages remain in logs or machine evidence; they are never replayed into the summary. Failures show category-localized impact, action, and retained-state guidance beside the stable code, retry decision, and correlation reference. diff --git a/docs/features.mdx b/docs/features.mdx index e5fbff293..2b9745089 100644 --- a/docs/features.mdx +++ b/docs/features.mdx @@ -148,8 +148,9 @@ mentions receive at most one correlated reply. Routine labels, assignees, project moves, title normalization, PR linkage, description refreshes, pushes, reopens, merges, and closes do not create a -generic roll-up. The Job Summary keeps compact result states and safe error -recovery evidence; internal step narration and arbitrary error text remain in logs. +generic roll-up. The Job Summary keeps aggregate result counts and expands only +safe error recovery evidence; internal result names, step narration, and arbitrary +error text remain in logs. Hidden markers provide stable identity and are trusted only when the comment is authored by the configured bot. Decorative image inputs default to `false` and are deprecated. diff --git a/docs/issues/notifications-and-auto-close.mdx b/docs/issues/notifications-and-auto-close.mdx index 9005f5bda..622cb3da3 100644 --- a/docs/issues/notifications-and-auto-close.mdx +++ b/docs/issues/notifications-and-auto-close.mdx @@ -20,7 +20,9 @@ The workflow Job Summary remains the place for bounded operational state. Its headings and explanatory labels use `repository-locale` (English by default), while event names, lifecycle values, error codes, and other machine facts remain stable. It reports compact per-result outcomes and category-localized recovery -guidance without replaying internal `Result.steps` or arbitrary provider text. +guidance for failed results without replaying internal result names, +`Result.steps`, or arbitrary provider text. Routine successes and skips are +reported only as aggregate counts. It contains one localization section showing the effective issue and PR locales plus exact, base-language, dynamic, or English-fallback catalog evidence. diff --git a/specs/repository-locale-and-localization.md b/specs/repository-locale-and-localization.md index 232ab4619..f28ccc70d 100644 --- a/specs/repository-locale-and-localization.md +++ b/specs/repository-locale-and-localization.md @@ -962,8 +962,9 @@ keeps raw provider and credential-health diagnostics out of UI. Setup itself remains one authoritative English artifact while it creates the repository profile. The generic Actions Job Summary now resolves one complete catalog in the repository locale, localizes its headings and explanatory labels, preserves -machine values, projects compact localized result states and error recovery by -stable error category, omits internal step narration and arbitrary error text, +machine values, projects compact localized aggregate result counts and expands +only error recovery by stable error category, omits internal result names, step +narration, and arbitrary error text, and emits localization evidence once instead of duplicating it inside and below the main table. Lifecycle and the remaining public surfaces are not claimed complete by this evidence. diff --git a/specs/semantic-github-publication-and-notification.md b/specs/semantic-github-publication-and-notification.md index 5ef6e3b08..bda1978ba 100644 --- a/specs/semantic-github-publication-and-notification.md +++ b/specs/semantic-github-publication-and-notification.md @@ -547,8 +547,8 @@ names may follow repository conventions: 5. feature renderers for plan, progress, branch sync, Bugbot, release, access, inactivity, command replies, and failures; 6. a Job Summary projection that includes compact semantic outcomes, stable - machine evidence, and localized recovery guidance while leaving step/debug - narration in masked logs; and + machine evidence, and localized recovery guidance while leaving internal + result names and step/debug narration in masked logs; and 7. a temporary compatibility adapter that rejects attempts to publish generic `Result.steps` and records the attempted source in tests/logs. @@ -939,8 +939,9 @@ machine markers, and bounded English-default/localized renderers. The executable mutation inventory, locale-branch ratchet, and pseudo-plural ratchet protect these boundaries. Subsequent deployment and setup-doctor slices cover their feature-owned views, and the generic Job Summary slice resolves repository- -locale copy atomically, renders compact result states and safe category-localized -error recovery, excludes internal steps and arbitrary error messages, and renders +locale copy atomically, renders aggregate result counts and expands only safe +category-localized error recovery, excludes internal result names, steps, and +arbitrary error messages, and renders one localization evidence section instead of two. Other capability rows and the global numeric budget remain open and are not claimed complete by this milestone. diff --git a/src/actions/__tests__/github_action_completion.test.ts b/src/actions/__tests__/github_action_completion.test.ts index a8213413b..95949834d 100644 --- a/src/actions/__tests__/github_action_completion.test.ts +++ b/src/actions/__tests__/github_action_completion.test.ts @@ -308,10 +308,11 @@ describe('finishGithubAction', () => { const summary = mockSummaryPublish.mock.calls[0][0] as string; expect(summary).toContain('# Ejecución de Copilot'); expect(summary).toContain('| Estado | ✅ Correcto |'); - expect(summary).toContain('## Detalles del resultado'); + expect(summary).toContain('| Resultados | Completado: 1 · Fallido: 0 · Omitido: 0 |'); + expect(summary).not.toContain('## Detalles del fallo'); expect(summary).toContain('| Locale del repositorio | `es-ES` |'); expect(summary.match(/## Localización/gu)).toHaveLength(1); - expect(summary).toContain('✅ **MetadataUseCase** — Completado'); + expect(summary).not.toContain('MetadataUseCase'); expect(summary).not.toContain('Updated labels.'); expect(summary).not.toContain('## Localization'); }); @@ -491,7 +492,10 @@ describe('finishGithubAction', () => { { publish: mockSummaryPublish }, ); - expect(mockSummaryPublish).toHaveBeenCalledWith(expect.stringContaining('UpdateTitleUseCase')); + const summary = mockSummaryPublish.mock.calls[0][0] as string; + expect(summary).toContain('| Results | Succeeded: 1 · Failed: 0 · Skipped: 0 |'); + expect(summary).not.toContain('UpdateTitleUseCase'); + expect(summary).not.toContain('Title normalized'); expect(mockPublishInvoke).toHaveBeenCalledWith(expect.objectContaining({ locale: 'en-US' })); expect(mockEvidencePublish).not.toHaveBeenCalled(); }); diff --git a/src/application/policies/__tests__/action_summary_policy.test.ts b/src/application/policies/__tests__/action_summary_policy.test.ts index 202d92784..76c0bd799 100644 --- a/src/application/policies/__tests__/action_summary_policy.test.ts +++ b/src/application/policies/__tests__/action_summary_policy.test.ts @@ -43,7 +43,6 @@ describe('action summary policy', () => { 'summary.heading': 'Summary\n# Forged heading', 'summary.repository': '[Forged link](https://example.com)', 'summary.status': 'Status | forged cell', - 'summary.noResult': '
forged block
', }; const catalog: ActionSummaryMessageCatalog = { locale: 'fr-FR', @@ -60,11 +59,9 @@ describe('action summary policy', () => { expect(summary).toContain('# Summary # Forged heading'); expect(summary).toContain('\\[Forged link\\](https:\u200b//example.com)'); expect(summary).toContain('| Status \\| forged cell |'); - expect(summary).toContain('_\\forged block\\_'); expect(summary).not.toContain('\n# Forged heading'); expect(summary).not.toContain('[Forged link](https://example.com)'); expect(summary).not.toContain('https://example.com'); - expect(summary).not.toContain('
forged block
'); }); it('sanitizes labels supplied to a specialized localization section', () => { @@ -89,7 +86,7 @@ describe('action summary policy', () => { expect(summary).not.toContain('
Repository
'); }); - it('renders compact result states and keeps internal step narration out of the summary', () => { + it('aggregates routine result states without exposing internal result names or step narration', () => { const summary = buildActionSummary({ owner: 'owner', repository: 'repo', @@ -105,8 +102,10 @@ describe('action summary policy', () => { expect(summary).toContain('# Copilot execution'); expect(summary).toContain('`planned`'); - expect(summary).toContain('✅ **Plan** — Succeeded'); - expect(summary).toContain('⏭️ **OptionalStep** — Skipped'); + expect(summary).toContain('| Results | Succeeded: 1 · Failed: 0 · Skipped: 1 |'); + expect(summary).not.toContain('## Failure details'); + expect(summary).not.toContain('Plan'); + expect(summary).not.toContain('OptionalStep'); expect(summary).not.toContain('safe | text'); expect(summary).not.toContain('Skipped because of internal policy.'); expect(summary).not.toContain('{{'); @@ -145,8 +144,8 @@ describe('action summary policy', () => { expect(summary).toContain('Repositorio: [owner/repo]'); expect(summary).toContain('| Estado | ❌ Fallo |'); expect(summary).toContain('| Destino | Issue n.º 7 |'); - expect(summary).toContain('## Detalles del resultado'); - expect(summary).toContain('**Resultado sin nombre**'); + expect(summary).toContain('## Detalles del fallo'); + expect(summary).toContain('- ❌ **Fallido**'); expect(summary).toContain('**Impacto:**'); expect(summary).toContain('El workflow no pudo completar la operación solicitada.'); expect(summary).toContain('**Código de error:** `workflow.failed`'); @@ -191,9 +190,12 @@ describe('action summary policy', () => { }); expect(skipped).toContain('| Status | ⏭️ Skipped |'); - expect(skipped).toContain('⏭️ **Optional** — Skipped'); + expect(skipped).toContain('| Results | Succeeded: 0 · Failed: 0 · Skipped: 1 |'); + expect(skipped).not.toContain('Optional'); expect(rejected).toContain('| Status | ❌ Failure |'); - expect(rejected).toContain('❌ **Rejected** — Failed'); + expect(rejected).toContain('| Results | Succeeded: 0 · Failed: 1 · Skipped: 0 |'); + expect(rejected).toContain('- ❌ **Failed**'); + expect(rejected).not.toContain('Rejected'); expect(rejected).toContain('**Error code:** `authorization.denied`'); expect(rejected).not.toContain('Internal rejection detail.'); }); diff --git a/src/application/policies/action_summary_message_catalog.ts b/src/application/policies/action_summary_message_catalog.ts index adc9d3670..552aef54a 100644 --- a/src/application/policies/action_summary_message_catalog.ts +++ b/src/application/policies/action_summary_message_catalog.ts @@ -18,7 +18,7 @@ const SIMPLE_MESSAGE_KEYS = Object.freeze([ 'resultDetails', 'localization', 'repositoryLocale', 'issueLocale', 'pullRequestLocale', 'catalogResolution', 'descriptors', 'reason', 'failure', 'findings', 'partial', 'superseded', 'skipped', 'dryRun', 'success', 'invalid', - 'none', 'noResult', 'unnamedResult', 'impact', 'cause', 'action', + 'none', 'impact', 'cause', 'action', 'retainedState', 'reference', 'retryable', 'yes', 'no', 'resultSucceeded', 'resultFailed', 'resultSkipped', ] as const); @@ -72,7 +72,7 @@ const ENGLISH_SIMPLE: Readonly> = Object.freeze results: 'Results', findingStates: 'Finding states', bugbotReview: 'Bugbot review', - resultDetails: 'Result details', + resultDetails: 'Failure details', localization: 'Localization', repositoryLocale: 'Repository locale', issueLocale: 'Issue locale', @@ -89,8 +89,6 @@ const ENGLISH_SIMPLE: Readonly> = Object.freeze success: 'Success', invalid: 'invalid', none: 'none', - noResult: 'No application result was produced.', - unnamedResult: 'Unnamed result', impact: 'Impact', cause: 'Error code', action: 'Action', @@ -117,7 +115,7 @@ const SPANISH_SIMPLE: Readonly> = Object.freeze results: 'Resultados', findingStates: 'Estados de los hallazgos', bugbotReview: 'Revisión de Bugbot', - resultDetails: 'Detalles del resultado', + resultDetails: 'Detalles del fallo', localization: 'Localización', repositoryLocale: 'Locale del repositorio', issueLocale: 'Locale de la issue', @@ -134,8 +132,6 @@ const SPANISH_SIMPLE: Readonly> = Object.freeze success: 'Correcto', invalid: 'no válido', none: 'ninguno', - noResult: 'No se ha producido ningún resultado de aplicación.', - unnamedResult: 'Resultado sin nombre', impact: 'Impacto', cause: 'Código de error', action: 'Acción', diff --git a/src/application/policies/action_summary_policy.ts b/src/application/policies/action_summary_policy.ts index 4f96519aa..1a8716ca6 100644 --- a/src/application/policies/action_summary_policy.ts +++ b/src/application/policies/action_summary_policy.ts @@ -60,7 +60,7 @@ export function buildActionSummary( context: ActionSummaryContext, catalog: ActionSummaryMessageCatalog = resolveStaticActionSummaryCatalog(context.locale?.repository ?? 'en-US'), ): string { - const failures = context.results.filter(result => result.errors.length > 0 || (!result.success && result.executed)); + const failures = context.results.filter(resultFailed); const findingStateProjection = projectBugbotResultFindingStates(context.results); const findingStates = findingStateProjection.status === 'valid' ? findingStateProjection.counts : undefined; const telemetryProjection = projectBugbotResultTelemetry(context.results); @@ -83,7 +83,7 @@ export function buildActionSummary( `| ${catalogText(catalog, 'summary.target')} | ${escapeTable(target)} |`, `| ${catalogText(catalog, 'summary.lifecycle')} | ${lifecycle} |`, `| ${catalogText(catalog, 'summary.descriptionPolicy')} | ${escapeTable(context.pullRequestDescriptionMode ?? '—')} |`, - `| ${catalogText(catalog, 'summary.results')} | ${context.results.length} |`, + `| ${catalogText(catalog, 'summary.results')} | ${formatResultCounts(context.results, catalog)} |`, `| ${catalogText(catalog, 'summary.findingStates')} | ${formatFindingStates(findingStateProjection, catalog)} |`, `| ${catalogText(catalog, 'summary.bugbotReview')} | ${formatBugbotTelemetry(telemetryProjection, catalog)} |`, ]; @@ -101,12 +101,13 @@ export function buildActionSummary( `| ${catalogText(catalog, 'summary.property')} | ${catalogText(catalog, 'summary.value')} |`, '| --- | --- |', ...rows, - '', - `## ${catalogText(catalog, 'summary.resultDetails')}`, - '', - renderResults(context.results, catalog), - '', - localization, + ...(failures.length > 0 ? [ + '', + `## ${catalogText(catalog, 'summary.resultDetails')}`, + '', + renderFailures(failures, catalog), + ] : []), + ...(localization ? ['', localization] : []), ].join('\n'); } @@ -227,17 +228,26 @@ function formatFindingStates( .join(', ') || catalogText(catalog, 'summary.none'); } -function renderResults(results: readonly Result[], catalog: ActionSummaryMessageCatalog): string { - if (results.length === 0) return `_${catalogText(catalog, 'summary.noResult')}_`; +function formatResultCounts(results: readonly Result[], catalog: ActionSummaryMessageCatalog): string { + const counts = results.reduce((current, result) => { + if (resultFailed(result)) current.failed += 1; + else if (!result.executed) current.skipped += 1; + else current.succeeded += 1; + return current; + }, { succeeded: 0, failed: 0, skipped: 0 }); + return [ + `${catalogText(catalog, 'summary.resultSucceeded')}: ${counts.succeeded}`, + `${catalogText(catalog, 'summary.resultFailed')}: ${counts.failed}`, + `${catalogText(catalog, 'summary.resultSkipped')}: ${counts.skipped}`, + ].join(' · '); +} + +function resultFailed(result: Result): boolean { + return result.errors.length > 0 || (!result.success && result.executed); +} + +function renderFailures(results: readonly Result[], catalog: ActionSummaryMessageCatalog): string { return results.map(result => { - const failed = result.errors.length > 0 || (!result.success && result.executed); - const outcome = failed - ? { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') } - : !result.executed - ? { icon: '⏭️', label: catalogText(catalog, 'summary.resultSkipped') } - : result.success - ? { icon: '✅', label: catalogText(catalog, 'summary.resultSucceeded') } - : { icon: '❌', label: catalogText(catalog, 'summary.resultFailed') }; const errors = result.errors .flatMap((error) => { return [ @@ -250,7 +260,7 @@ function renderResults(results: readonly Result[], catalog: ActionSummaryMessage ]; }); return [ - `- ${outcome.icon} **${escapeTable(result.id || catalogText(catalog, 'summary.unnamedResult'))}** — ${outcome.label}`, + `- ❌ **${catalogText(catalog, 'summary.resultFailed')}**`, ...errors, ].join('\n'); }).join('\n'); diff --git a/src/architecture/__tests__/github_publication_boundaries.test.ts b/src/architecture/__tests__/github_publication_boundaries.test.ts index 4b43103e2..33e075f87 100644 --- a/src/architecture/__tests__/github_publication_boundaries.test.ts +++ b/src/architecture/__tests__/github_publication_boundaries.test.ts @@ -123,6 +123,7 @@ describe('GitHub conversation publication boundaries', () => { ); expect(presentation).not.toMatch(/result\.steps/u); + expect(presentation).not.toMatch(/result\.id/u); expect(presentation).not.toContain('buildApplicationErrorPresentation'); });