diff --git a/actions/setup/js/add_reaction_and_edit_comment.cjs b/actions/setup/js/add_reaction_and_edit_comment.cjs index 92dd2f9e11e..450941699a7 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.cjs @@ -13,6 +13,7 @@ const { addReaction, addDiscussionReaction } = require("./add_reaction.cjs"); /** * Event type descriptions for comment messages + * @type {Record} */ const EVENT_TYPE_DESCRIPTIONS = { issues: "issue", @@ -23,6 +24,119 @@ const EVENT_TYPE_DESCRIPTIONS = { discussion_comment: "discussion comment", }; +/** Valid GitHub reaction types */ +const VALID_REACTIONS = Object.freeze(["+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"]); + +/** + * Resolve the reaction and comment API endpoints for a given event. + * Returns null (after calling core.setFailed) when the event or payload is invalid. + * @param {string} eventName - The GitHub event name + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {Record} payload - The event payload + * @returns {Promise<{reactionEndpoint: string, commentUpdateEndpoint: string} | null>} + */ +async function resolveEventEndpoints(eventName, owner, repo, payload) { + switch (eventName) { + case "issues": { + const issueNumber = payload?.issue?.number; + if (!issueNumber) { + core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`); + return null; + } + return { + reactionEndpoint: `/repos/${owner}/${repo}/issues/${issueNumber}/reactions`, + commentUpdateEndpoint: `/repos/${owner}/${repo}/issues/${issueNumber}/comments`, + }; + } + + case "issue_comment": { + const commentId = payload?.comment?.id; + const issueNumber = payload?.issue?.number; + if (!commentId) { + core.setFailed(`${ERR_VALIDATION}: Comment ID not found in event payload`); + return null; + } + if (!issueNumber) { + core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`); + return null; + } + return { + reactionEndpoint: `/repos/${owner}/${repo}/issues/comments/${commentId}/reactions`, + // Create new comment on the issue itself, not on the comment + commentUpdateEndpoint: `/repos/${owner}/${repo}/issues/${issueNumber}/comments`, + }; + } + + case "pull_request": { + const prNumber = payload?.pull_request?.number; + if (!prNumber) { + core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`); + return null; + } + // PRs are "issues" for the reactions endpoint + return { + reactionEndpoint: `/repos/${owner}/${repo}/issues/${prNumber}/reactions`, + commentUpdateEndpoint: `/repos/${owner}/${repo}/issues/${prNumber}/comments`, + }; + } + + case "pull_request_review_comment": { + const reviewCommentId = payload?.comment?.id; + const prNumber = payload?.pull_request?.number; + if (!reviewCommentId) { + core.setFailed(`${ERR_VALIDATION}: Review comment ID not found in event payload`); + return null; + } + if (!prNumber) { + core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`); + return null; + } + return { + reactionEndpoint: `/repos/${owner}/${repo}/pulls/comments/${reviewCommentId}/reactions`, + // Create new comment on the PR itself (using issues endpoint since PRs are issues) + commentUpdateEndpoint: `/repos/${owner}/${repo}/issues/${prNumber}/comments`, + }; + } + + case "discussion": { + const discussionNumber = payload?.discussion?.number; + if (!discussionNumber) { + core.setFailed(`${ERR_NOT_FOUND}: Discussion number not found in event payload`); + return null; + } + // Discussions use GraphQL API - get the node ID + const discussion = await getDiscussionId(owner, repo, discussionNumber); + return { + reactionEndpoint: discussion.id, // Store node ID for GraphQL + commentUpdateEndpoint: `discussion:${discussionNumber}`, // Special format to indicate discussion + }; + } + + case "discussion_comment": { + const discussionNumber = payload?.discussion?.number; + const commentId = payload?.comment?.id; + if (!discussionNumber || !commentId) { + core.setFailed(`${ERR_NOT_FOUND}: Discussion or comment information not found in event payload`); + return null; + } + const commentNodeId = payload?.comment?.node_id; + if (!commentNodeId) { + core.setFailed(`${ERR_NOT_FOUND}: Discussion comment node ID not found in event payload`); + return null; + } + return { + reactionEndpoint: commentNodeId, // Store node ID for GraphQL + commentUpdateEndpoint: `discussion_comment:${discussionNumber}:${commentId}`, // Special format + }; + } + + default: + core.setFailed(`${ERR_VALIDATION}: Unsupported event type: ${eventName}`); + return null; + } +} + async function main() { const reaction = process.env.GH_AW_REACTION || "eyes"; const command = process.env.GH_AW_COMMAND; // Only present for command workflows @@ -34,113 +148,20 @@ async function main() { core.info(`Run ID: ${context.runId}`); core.info(`Run URL: ${runUrl}`); - // Validate reaction type - const validReactions = ["+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"]; - if (!validReactions.includes(reaction)) { - core.setFailed(`${ERR_VALIDATION}: Invalid reaction type: ${reaction}. Valid reactions are: ${validReactions.join(", ")}`); + if (!VALID_REACTIONS.includes(reaction)) { + core.setFailed(`${ERR_VALIDATION}: Invalid reaction type: ${reaction}. Valid reactions are: ${VALID_REACTIONS.join(", ")}`); return; } - let reactionEndpoint; - let commentUpdateEndpoint; const eventName = invocationContext.eventName; - const owner = invocationContext.eventRepo.owner; - const repo = invocationContext.eventRepo.repo; + const { owner, repo } = invocationContext.eventRepo; const payload = invocationContext.eventPayload; try { - switch (eventName) { - case "issues": { - const issueNumber = payload?.issue?.number; - if (!issueNumber) { - core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`); - return; - } - reactionEndpoint = `/repos/${owner}/${repo}/issues/${issueNumber}/reactions`; - commentUpdateEndpoint = `/repos/${owner}/${repo}/issues/${issueNumber}/comments`; - break; - } - - case "issue_comment": { - const commentId = payload?.comment?.id; - const issueNumberForComment = payload?.issue?.number; - if (!commentId) { - core.setFailed(`${ERR_VALIDATION}: Comment ID not found in event payload`); - return; - } - if (!issueNumberForComment) { - core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`); - return; - } - reactionEndpoint = `/repos/${owner}/${repo}/issues/comments/${commentId}/reactions`; - // Create new comment on the issue itself, not on the comment - commentUpdateEndpoint = `/repos/${owner}/${repo}/issues/${issueNumberForComment}/comments`; - break; - } + const endpoints = await resolveEventEndpoints(eventName, owner, repo, payload); + if (!endpoints) return; - case "pull_request": { - const prNumber = payload?.pull_request?.number; - if (!prNumber) { - core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`); - return; - } - // PRs are "issues" for the reactions endpoint - reactionEndpoint = `/repos/${owner}/${repo}/issues/${prNumber}/reactions`; - commentUpdateEndpoint = `/repos/${owner}/${repo}/issues/${prNumber}/comments`; - break; - } - - case "pull_request_review_comment": { - const reviewCommentId = payload?.comment?.id; - const prNumberForReviewComment = payload?.pull_request?.number; - if (!reviewCommentId) { - core.setFailed(`${ERR_VALIDATION}: Review comment ID not found in event payload`); - return; - } - if (!prNumberForReviewComment) { - core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`); - return; - } - reactionEndpoint = `/repos/${owner}/${repo}/pulls/comments/${reviewCommentId}/reactions`; - // Create new comment on the PR itself (using issues endpoint since PRs are issues) - commentUpdateEndpoint = `/repos/${owner}/${repo}/issues/${prNumberForReviewComment}/comments`; - break; - } - - case "discussion": { - const discussionNumber = payload?.discussion?.number; - if (!discussionNumber) { - core.setFailed(`${ERR_NOT_FOUND}: Discussion number not found in event payload`); - return; - } - // Discussions use GraphQL API - get the node ID - const discussion = await getDiscussionId(owner, repo, discussionNumber); - reactionEndpoint = discussion.id; // Store node ID for GraphQL - commentUpdateEndpoint = `discussion:${discussionNumber}`; // Special format to indicate discussion - break; - } - - case "discussion_comment": { - const discussionCommentNumber = payload?.discussion?.number; - const discussionCommentId = payload?.comment?.id; - if (!discussionCommentNumber || !discussionCommentId) { - core.setFailed(`${ERR_NOT_FOUND}: Discussion or comment information not found in event payload`); - return; - } - const commentNodeId = payload?.comment?.node_id; - if (!commentNodeId) { - core.setFailed(`${ERR_NOT_FOUND}: Discussion comment node ID not found in event payload`); - return; - } - reactionEndpoint = commentNodeId; // Store node ID for GraphQL - commentUpdateEndpoint = `discussion_comment:${discussionCommentNumber}:${discussionCommentId}`; // Special format - break; - } - - default: - core.setFailed(`${ERR_VALIDATION}: Unsupported event type: ${eventName}`); - return; - } + const { reactionEndpoint, commentUpdateEndpoint } = endpoints; core.info(`Reaction API endpoint: ${reactionEndpoint}`); @@ -151,10 +172,8 @@ async function main() { await addReaction(reactionEndpoint, reaction); } - if (commentUpdateEndpoint) { - core.info(`Comment endpoint: ${commentUpdateEndpoint}`); - await addCommentWithWorkflowLink(commentUpdateEndpoint, runUrl, eventName, invocationContext); - } + core.info(`Comment endpoint: ${commentUpdateEndpoint}`); + await addCommentWithWorkflowLink(commentUpdateEndpoint, runUrl, eventName, invocationContext); } catch (error) { if (isLockedError(error)) { core.info(`Cannot add reaction: resource is locked (this is expected and not an error)`); @@ -319,4 +338,4 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName, invocatio } } -module.exports = { main, addCommentWithWorkflowLink, addReaction, addDiscussionReaction }; +module.exports = { main, addCommentWithWorkflowLink, resolveEventEndpoints, VALID_REACTIONS, addReaction, addDiscussionReaction }; diff --git a/actions/setup/js/add_reaction_and_edit_comment.test.cjs b/actions/setup/js/add_reaction_and_edit_comment.test.cjs index c7691f1c600..d19c6f85697 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.test.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.test.cjs @@ -38,8 +38,8 @@ global.context = mockContext; // Helper to import the module fresh (bust module cache) async function loadModule() { - const { main, addCommentWithWorkflowLink, addReaction, addDiscussionReaction } = await import("./add_reaction_and_edit_comment.cjs?" + Date.now()); - return { main, addCommentWithWorkflowLink, addReaction, addDiscussionReaction }; + const { main, addCommentWithWorkflowLink, addReaction, addDiscussionReaction, resolveEventEndpoints, VALID_REACTIONS } = await import("./add_reaction_and_edit_comment.cjs?" + Date.now()); + return { main, addCommentWithWorkflowLink, addReaction, addDiscussionReaction, resolveEventEndpoints, VALID_REACTIONS }; } describe("add_reaction_and_edit_comment.cjs", () => { @@ -609,4 +609,88 @@ describe("add_reaction_and_edit_comment.cjs", () => { expect(mockCore.setOutput).toHaveBeenCalledWith("reaction-id", ""); }); }); + + describe("VALID_REACTIONS", () => { + it("should export the list of valid reaction types", async () => { + const { VALID_REACTIONS } = await loadModule(); + expect(VALID_REACTIONS).toEqual(["+1", "-1", "laugh", "confused", "heart", "hooray", "rocket", "eyes"]); + }); + }); + + describe("resolveEventEndpoints()", () => { + it("should resolve endpoints for issues event", async () => { + const { resolveEventEndpoints } = await loadModule(); + const payload = { issue: { number: 42 } }; + const result = await resolveEventEndpoints("issues", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "/repos/owner/repo/issues/42/reactions", + commentUpdateEndpoint: "/repos/owner/repo/issues/42/comments", + }); + }); + + it("should return null and call setFailed when issue number is missing", async () => { + const { resolveEventEndpoints } = await loadModule(); + const result = await resolveEventEndpoints("issues", "owner", "repo", {}); + expect(result).toBeNull(); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining(ERR_NOT_FOUND)); + }); + + it("should resolve endpoints for pull_request event", async () => { + const { resolveEventEndpoints } = await loadModule(); + const payload = { pull_request: { number: 7 } }; + const result = await resolveEventEndpoints("pull_request", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "/repos/owner/repo/issues/7/reactions", + commentUpdateEndpoint: "/repos/owner/repo/issues/7/comments", + }); + }); + + it("should resolve endpoints for issue_comment event", async () => { + const { resolveEventEndpoints } = await loadModule(); + const payload = { comment: { id: 55 }, issue: { number: 10 } }; + const result = await resolveEventEndpoints("issue_comment", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "/repos/owner/repo/issues/comments/55/reactions", + commentUpdateEndpoint: "/repos/owner/repo/issues/10/comments", + }); + }); + + it("should resolve endpoints for pull_request_review_comment event", async () => { + const { resolveEventEndpoints } = await loadModule(); + const payload = { comment: { id: 99 }, pull_request: { number: 3 } }; + const result = await resolveEventEndpoints("pull_request_review_comment", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "/repos/owner/repo/pulls/comments/99/reactions", + commentUpdateEndpoint: "/repos/owner/repo/issues/3/comments", + }); + }); + + it("should resolve endpoints for discussion event using GraphQL node ID", async () => { + mockGithub.graphql.mockResolvedValueOnce({ repository: { discussion: { id: "D_node123", url: "https://github.com/testowner/testrepo/discussions/5" } } }); + const { resolveEventEndpoints } = await loadModule(); + const payload = { discussion: { number: 5 } }; + const result = await resolveEventEndpoints("discussion", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "D_node123", + commentUpdateEndpoint: "discussion:5", + }); + }); + + it("should resolve endpoints for discussion_comment event", async () => { + const { resolveEventEndpoints } = await loadModule(); + const payload = { discussion: { number: 5 }, comment: { id: 88, node_id: "DC_node88" } }; + const result = await resolveEventEndpoints("discussion_comment", "owner", "repo", payload); + expect(result).toEqual({ + reactionEndpoint: "DC_node88", + commentUpdateEndpoint: "discussion_comment:5:88", + }); + }); + + it("should return null and call setFailed for unknown event type", async () => { + const { resolveEventEndpoints } = await loadModule(); + const result = await resolveEventEndpoints("push", "owner", "repo", {}); + expect(result).toBeNull(); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining(ERR_VALIDATION)); + }); + }); }); diff --git a/docs/src/content/docs/examples/scheduled.md b/docs/src/content/docs/examples/scheduled.md index 16b528a1aa3..761c648adae 100644 --- a/docs/src/content/docs/examples/scheduled.md +++ b/docs/src/content/docs/examples/scheduled.md @@ -1,11 +1,11 @@ --- title: Scheduled Workflows -description: Workflows that run automatically on a schedule using cron expressions - daily reports, weekly research, and continuous improvement patterns +description: Workflows that run automatically on a schedule using fuzzy schedules - daily reports, weekly research, and continuous improvement patterns sidebar: order: 1 --- -Scheduled workflows run automatically at specified times using cron expressions. They're perfect for recurring tasks like daily status updates, weekly research reports, continuous code improvements, and automated maintenance. +Scheduled workflows run automatically at specified times using fuzzy schedule expressions. They're perfect for recurring tasks like daily status updates, weekly research reports, continuous code improvements, and automated maintenance. ## When to Use Scheduled Workflows diff --git a/docs/src/content/docs/patterns/batch-ops.md b/docs/src/content/docs/patterns/batch-ops.md index 84983210a65..805bb4cc6a0 100644 --- a/docs/src/content/docs/patterns/batch-ops.md +++ b/docs/src/content/docs/patterns/batch-ops.md @@ -25,8 +25,7 @@ Split work into fixed-size pages using `GITHUB_RUN_NUMBER`. Each run processes o ```aw wrap --- on: - schedule: - - cron: "0 2 * * 1-5" # Weekdays at 2 AM + schedule: daily on weekdays workflow_dispatch: tools: diff --git a/docs/src/content/docs/patterns/central-repo-ops.mdx b/docs/src/content/docs/patterns/central-repo-ops.mdx index bcd7c80b5a9..ebe78860ae4 100644 --- a/docs/src/content/docs/patterns/central-repo-ops.mdx +++ b/docs/src/content/docs/patterns/central-repo-ops.mdx @@ -24,8 +24,7 @@ Navigate to your central repository and create a workflow file `.github/workflow ```aw wrap --- on: - schedule: - - cron: '0 9 * * 1' + schedule: weekly on monday tools: github: diff --git a/docs/src/content/docs/patterns/daily-ops.md b/docs/src/content/docs/patterns/daily-ops.md index 76016d33339..ffbffc6edbe 100644 --- a/docs/src/content/docs/patterns/daily-ops.md +++ b/docs/src/content/docs/patterns/daily-ops.md @@ -16,8 +16,7 @@ Workflows run on weekday schedules (avoiding weekends) with `workflow_dispatch` ```aw wrap --- on: - schedule: - - cron: "0 2 * * 1-5" # Weekdays only (no short syntax available) + schedule: daily on weekdays workflow_dispatch: --- ``` diff --git a/docs/src/content/docs/patterns/project-ops.mdx b/docs/src/content/docs/patterns/project-ops.mdx index dc4f3d9f8ea..0ba28302769 100644 --- a/docs/src/content/docs/patterns/project-ops.mdx +++ b/docs/src/content/docs/patterns/project-ops.mdx @@ -82,8 +82,7 @@ Let's start with a simple agentic workflow that reviews project board state and ```aw wrap --- on: - schedule: - - cron: "0 14 * * 1" + schedule: weekly on monday permissions: contents: read actions: read diff --git a/docs/src/content/docs/patterns/research-plan-assign-ops.md b/docs/src/content/docs/patterns/research-plan-assign-ops.md index 32db16149b0..dba54e71713 100644 --- a/docs/src/content/docs/patterns/research-plan-assign-ops.md +++ b/docs/src/content/docs/patterns/research-plan-assign-ops.md @@ -25,8 +25,7 @@ The [`go-fan`](https://github.com/github/gh-aw/blob/main/.github/workflows/go-fa --- name: Go Fan on: - schedule: - - cron: "0 7 * * 1-5" + schedule: daily on weekdays workflow_dispatch: engine: claude safe-outputs: diff --git a/docs/src/content/docs/patterns/workqueue-ops.md b/docs/src/content/docs/patterns/workqueue-ops.md index 6006dfe5aad..4dcd0776680 100644 --- a/docs/src/content/docs/patterns/workqueue-ops.md +++ b/docs/src/content/docs/patterns/workqueue-ops.md @@ -52,8 +52,7 @@ Create one sub-issue per work item. The agent queries open sub-issues of a paren ```aw wrap --- on: - schedule: - - cron: "0 * * * *" # Every hour + schedule: hourly workflow_dispatch: tools: @@ -90,8 +89,7 @@ Store queue state as a JSON file in [cache-memory](/gh-aw/reference/cache-memory ```aw wrap --- on: - schedule: - - cron: "0 6 * * 1-5" # Weekdays at 6 AM + schedule: daily on weekdays workflow_dispatch: tools: @@ -138,8 +136,7 @@ Use a GitHub Discussion to track pending work items. Unresolved replies represen ```aw wrap --- on: - schedule: - - cron: "0 8 * * *" # Daily at 8 AM + schedule: daily workflow_dispatch: tools: diff --git a/docs/src/content/docs/reference/cross-repository.md b/docs/src/content/docs/reference/cross-repository.md index b2ff92a5a00..273a287a3a9 100644 --- a/docs/src/content/docs/reference/cross-repository.md +++ b/docs/src/content/docs/reference/cross-repository.md @@ -256,8 +256,7 @@ A scheduled workflow that automatically pushes changes to open pull-request bran ```aw wrap --- on: - schedule: - - cron: "0 * * * *" + schedule: hourly checkout: - repository: org/target-repo diff --git a/docs/src/content/docs/reference/effective-tokens-specification.md b/docs/src/content/docs/reference/effective-tokens-specification.md index fde89238f78..4027eb2102b 100644 --- a/docs/src/content/docs/reference/effective-tokens-specification.md +++ b/docs/src/content/docs/reference/effective-tokens-specification.md @@ -40,8 +40,9 @@ This document is governed by the GitHub Agentic Workflows project specifications 10. [Compliance Testing](#10-compliance-testing) 11. [Appendices](#appendices) 12. [Model Multiplier Registry](#model-multiplier-registry) -13. [References](#references) -14. [Change Log](#change-log) +13. [Sync Notes](#sync-notes) +14. [References](#references) +15. [Change Log](#change-log) --- @@ -473,12 +474,31 @@ This file is embedded at compile time into the `gh-aw` binary using a Go `//go:e **R-REG-006**: Custom multipliers supplied by the caller (e.g., via API or configuration) MUST be merged with registry multipliers. Custom values take precedence and MUST be disclosed in any report that uses them. +**R-REG-007**: The registry MUST NOT contain placeholder values such as `TBD`, `null`, or empty strings for any model multiplier entry. Each declared model key MUST map to a numeric multiplier value. + +**R-REG-008**: When adding support for a new model, maintainers MUST register the model in `pkg/cli/data/model_multipliers.json` with a concrete numeric multiplier before release. If calibration is incomplete, the model MUST be omitted from the registry and the implementation fallback behavior in R-REG-005 applies. + ### Registry Versioning The `version` field in `model_multipliers.json` corresponds to the registry schema version, not the gh-aw binary version. Implementations SHOULD include the registry version in all ET summary reports to enable historical reconstruction. --- +## Sync Notes + +The Effective Tokens registry is maintained in `pkg/cli/data/model_multipliers.json` and loaded by `pkg/cli/effective_tokens.go`. + +To keep specification and implementation synchronized: + +1. Update this specification's registry requirements when adding, removing, or re-scaling model multipliers. +2. Update `pkg/cli/data/model_multipliers.json` in the same change. +3. Verify loading and fallback behavior in `pkg/cli/effective_tokens_test.go` (`TestModelMultipliersJSONEmbedded`, `TestResolveEffectiveWeightsDefault`, and inventory checks). +4. Run `make build` so the embedded registry is rebuilt into the `gh-aw` binary. + +Conforming releases SHOULD include a test assertion for newly added model multipliers to ensure implementation-registry parity. + +--- + ## References ### Normative References diff --git a/docs/src/content/docs/reference/experiments-specification.md b/docs/src/content/docs/reference/experiments-specification.md index 36483d4b612..eda7770ca27 100644 --- a/docs/src/content/docs/reference/experiments-specification.md +++ b/docs/src/content/docs/reference/experiments-specification.md @@ -691,6 +691,13 @@ implemented within a single workflow file. Engine-switching experiments **MUST** compiled workflow files (one per variant), which can then be compared via their respective GitHub Actions run metrics. +**R-MULTI-005**: When two or more experiments are simultaneously active in the same analysis +window, reporting tools **MUST** detect and bound interaction risk by preserving the full +assignment vector per run and evaluating whether each observed combination cell has sufficient +sample coverage. If interaction effects cannot be bounded (for example, sparse cells below +`min_samples`), the report **MUST** emit an explicit interaction-risk status and **MUST NOT** +recommend PROMOTE for affected variants. + ### 12.1 Conflict Resolution Norms A **conflict** occurs when two or more simultaneously active experiments would assign @@ -1115,10 +1122,26 @@ approximate minimum runs per variant are: 5. **State branch growth**: The experiments git branch grows monotonically. Operators **MAY** prune old commits from the experiments branch without affecting the current state. +### Sync Follow-ups (May 2026 Expert Review) + +This appendix itemizes corrective follow-ups referenced in the abstract. + +- **FR-001 (implemented via R-SELECT-006)**: Weighted selection increments invocation counters after each selection. +- **FR-002 (implemented via R-STAT-001/R-STAT-002)**: Reporting uses `state.runs` assignment records instead of count-delta inference. +- **FR-003 (implemented via R-STAT-011/R-STAT-012)**: Reporting workflows that write issues/discussions declare explicit write permissions. +- **FR-004 (implemented via R-MULTI-005)**: Concurrent-experiment interaction effects are explicitly detected and bounded before promotion decisions. +- **TODO(experiments, owner: @gh-aw-maintainers, target: v1.1.0)**: Add factorial-interaction analysis helpers to reporting workflows for K₁×K₂ cell significance output. +- **TODO(experiments, owner: @gh-aw-maintainers, target: v1.1.0)**: Add compiler diagnostics for sparse interaction cells when >1 experiment is active and weighted traffic is configured. + --- ## Change Log +### Version 1.0.1 (Draft) — 2026-05-07 + +- **Added**: R-MULTI-005 requiring interaction-risk detection/bounding for simultaneous experiments. +- **Added**: Sync Follow-ups appendix with itemized May 2026 expert-review corrective items and owned TODOs. + ### Version 1.0.0 (Draft) — 2026-05-03 - **Initial publication** consolidating ADR-29534, ADR-29618, ADR-29628, ADR-29985, and ADR-29996. diff --git a/docs/src/content/docs/reference/faq.md b/docs/src/content/docs/reference/faq.md index 2b289b21f47..168b8b559d0 100644 --- a/docs/src/content/docs/reference/faq.md +++ b/docs/src/content/docs/reference/faq.md @@ -146,7 +146,13 @@ This enables reusable tool configurations, network settings, and permissions acr ### Can I run workflows on a schedule? -Yes, use cron expressions in the `on:` trigger: +Yes, use fuzzy schedule expressions in the `on:` trigger (recommended): + +```yaml wrap +on: weekly on monday # Automatically scattered to avoid load spikes +``` + +Or use standard cron syntax for fixed times: ```yaml wrap on: @@ -154,7 +160,7 @@ on: - cron: "0 9 * * MON" # Every Monday at 9am UTC ``` -See [Schedule Syntax](/gh-aw/reference/schedule-syntax/) for cron expression reference. +See [Schedule Syntax](/gh-aw/reference/schedule-syntax/) for all supported formats. ### Can I run workflows conditionally? diff --git a/docs/src/content/docs/reference/frontmatter-hash-specification.md b/docs/src/content/docs/reference/frontmatter-hash-specification.md index 4f04674b8f7..80034b3d313 100644 --- a/docs/src/content/docs/reference/frontmatter-hash-specification.md +++ b/docs/src/content/docs/reference/frontmatter-hash-specification.md @@ -1,10 +1,21 @@ --- title: Frontmatter Hash Specification description: Specification for computing deterministic hashes of agentic workflow frontmatter +version: 1.0.0 +status: Draft +publication_date: 2026-05-07 --- # Frontmatter Hash Specification +**Version**: 1.0.0 +**Status**: Draft +**Publication Date**: 2026-05-07 +**Latest Version**: [frontmatter-hash-specification](/gh-aw/reference/frontmatter-hash-specification/) +**Editor**: GitHub Agentic Workflows Team + +--- + This document specifies the algorithm for computing a deterministic hash of agentic workflow frontmatter, including contributions from imported workflows. ## Purpose @@ -14,6 +25,17 @@ The frontmatter hash provides: 2. **Reproducibility**: Ensure identical configurations produce identical hashes across languages (Go and JavaScript) 3. **Change detection**: Verify that workflow configuration has not changed between compilation and execution +## Conformance + +### Conformance Classes + +- **Basic Conformance**: An implementation MUST compute a deterministic SHA-256 hash from canonicalized frontmatter input and MUST produce the same output for identical input. +- **Full Conformance**: An implementation MUST satisfy Basic Conformance and MUST implement cross-language consistency checks between Go and JavaScript implementations. + +### Requirements Notation + +The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this document are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + ## Hash Algorithm ### 1. Input Collection @@ -262,6 +284,12 @@ The Go and JavaScript implementations must produce byte-for-byte identical canon **Mitigation**: Maintain a shared test-vector file (at minimum: empty frontmatter, single-field workflow, multi-level imports, all field types). Run cross-language hash tests in CI. Any change to the serialization algorithm in either language MUST be accompanied by updated test vectors verified against both implementations. +### S-6: Maximum Frontmatter Input Size + +Very large frontmatter payloads can cause excessive memory use and hash-computation latency during compilation and runtime verification. This can degrade CI reliability and increase stale-lock false positives due to timeout or resource pressure. + +**Mitigation**: Implementations SHOULD enforce a maximum cumulative frontmatter input size and MUST fail deterministically with a descriptive error when the limit is exceeded. A limit of 1 MiB for the combined normalized frontmatter input is RECOMMENDED unless repository-specific requirements justify a higher bound. + --- ## Security Considerations diff --git a/docs/src/content/docs/reference/fuzzy-schedule-specification.md b/docs/src/content/docs/reference/fuzzy-schedule-specification.md index 8e59fedc000..bd7ce974a18 100644 --- a/docs/src/content/docs/reference/fuzzy-schedule-specification.md +++ b/docs/src/content/docs/reference/fuzzy-schedule-specification.md @@ -524,6 +524,15 @@ The scattering algorithm MUST provide: 3. **Stability**: Scattered times remain constant across recompilations 4. **Uniqueness**: Different workflow identifiers produce different scattered times +The scattering algorithm uses the following formal input entities: + +| Entity | Type | Constraints | Description | +|---|---|---|---| +| `workflow_identifier` | string | MUST be non-empty; SHOULD use `owner/repo/path/to/workflow.md` format | Canonical identifier hashed for deterministic scatter selection | +| `schedule_string` | string | MUST match a supported fuzzy placeholder form (`FUZZY:*`) | Parsed schedule expression that determines algorithm branch | +| `seed` | unsigned 32-bit integer | MUST be derived deterministically from `workflow_identifier` using the configured hash function | Hash-derived seed used for modulo operations | +| `window_minutes` | integer | MUST be positive; MUST NOT exceed 1440 | Candidate-minute search window for around/between scattering | + ### 6.2 Hash Function Requirements #### 6.2.1 Hash Algorithm Selection diff --git a/docs/src/content/docs/reference/mcp-scripts-specification.md b/docs/src/content/docs/reference/mcp-scripts-specification.md index bc2ed21c4b4..37678cfe985 100644 --- a/docs/src/content/docs/reference/mcp-scripts-specification.md +++ b/docs/src/content/docs/reference/mcp-scripts-specification.md @@ -186,7 +186,7 @@ mcp-scripts: // JavaScript implementation env: SECRET_NAME: "${{ secrets.SECRET_NAME }}" - timeout: 60 + timeout: 30 ``` **JSON Schema**: [mcp-scripts-config.schema.json](/gh-aw/schemas/mcp-scripts-config.schema.json) @@ -209,7 +209,7 @@ Each tool configuration MAY contain: | `py` | string | Conditional* | Python script implementation | | `go` | string | Conditional* | Go code implementation | | `env` | object | No | Environment variables (typically secrets) | -| `timeout` | integer | No | Execution timeout in seconds (default: 60, applies to run/py/go only) | +| `timeout` | integer | No | Execution timeout in seconds (default: 30, applies to run/py/go only) | | `dependencies` | array[string] | No | Package dependencies to install in execution environment (runtime-specific) | *Exactly ONE of `script`, `run`, `py`, or `go` MUST be provided per tool. @@ -417,6 +417,14 @@ For JavaScript tools: - Thrown errors indicate failure - Async functions are awaited +### 5.6 Runtime Timeout Requirements + +Each runtime handler (`script`, `run`, `py`, and `go`) **MUST** enforce a configurable execution timeout and **MUST** terminate tool execution when the timeout is reached. + +Implementations **SHOULD** default this timeout to 30 seconds or less unless the workflow author explicitly configures a different value. + +When a timeout occurs, the server **MUST** return a JSON-RPC execution error (`-32603`) that explicitly identifies timeout termination. + --- ## 6. Language Support diff --git a/docs/src/content/docs/reference/playwright.md b/docs/src/content/docs/reference/playwright.md index c99c4b4f6db..f79195ac512 100644 --- a/docs/src/content/docs/reference/playwright.md +++ b/docs/src/content/docs/reference/playwright.md @@ -139,8 +139,7 @@ Playwright includes three browser engines: **Chromium** (Chrome/Edge, most commo ```aw wrap --- on: - schedule: - - cron: "0 9 * * *" # Daily at 9 AM + schedule: daily tools: playwright: diff --git a/docs/src/content/docs/reference/triggers.md b/docs/src/content/docs/reference/triggers.md index 6c1e7ba3bdb..fd85b18a9b6 100644 --- a/docs/src/content/docs/reference/triggers.md +++ b/docs/src/content/docs/reference/triggers.md @@ -563,8 +563,7 @@ on: imports: - .github/workflows/shared/shared-ops.md on: - schedule: - - cron: "*/30 * * * *" + schedule: every 30 minutes skip-if-no-match: query: "org:myorg label:agent-fix is:issue is:open" scope: none @@ -620,8 +619,7 @@ By default the query is scoped to the current repository. Use `scope: none` to d ```yaml wrap on: - schedule: - - cron: "*/15 * * * *" + schedule: every 15 minutes skip-if-match: query: "org:myorg label:ops:in-progress is:issue is:open" scope: none @@ -662,8 +660,7 @@ The same `scope: none` field available on `skip-if-match` works identically here ```yaml wrap on: - schedule: - - cron: "*/15 * * * *" + schedule: every 15 minutes skip-if-no-match: query: "org:myorg label:agent-fix -label:ops:agentic is:issue is:open" scope: none diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index 43f6d3fff38..d90c13e4b21 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -145,11 +145,6 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor return nil, fmt.Errorf("%s: %w", cleanPath, err) } - // Validate Pi engine requirements (gh-proxy + cli-proxy). - if err := c.validatePiEngineRequirements(workflowData); err != nil { - return nil, fmt.Errorf("%s: %w", cleanPath, err) - } - // Validate GitHub tool configuration if err := validateGitHubToolConfig(workflowData.ParsedTools, workflowData.Name); err != nil { return nil, fmt.Errorf("%s: %w", cleanPath, err) diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 1527d0229fa..31ffb6d1f04 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -365,41 +365,6 @@ func (c *Compiler) validateSingleEngineSpecification(mainEngineSetting string, i return "", fmt.Errorf("invalid engine configuration in included file, missing or invalid 'id' field. Expected string, object with 'id' field, or inline definition with 'runtime.id'.\n\nExample (string):\nengine: copilot\n\nExample (object with id):\nengine:\n id: copilot\n model: gpt-4\n\nExample (inline runtime definition):\nengine:\n runtime:\n id: codex\n\nSee: %s", constants.DocsEnginesURL) } -// validatePiEngineRequirements validates that workflows using the Pi engine have -// the required configuration: gh-proxy mode and CLI proxy must both be enabled. -// -// Pi's API integration requires: -// - tools.github.mode: gh-proxy — the pre-authenticated gh CLI must be available -// - tools.cli-proxy: true — MCP servers must be mounted as CLI tools on PATH -// -// This requirement cannot be relaxed; the Pi engine communicates with GitHub via -// the gh CLI rather than the GitHub MCP protocol. -func (c *Compiler) validatePiEngineRequirements(workflowData *WorkflowData) error { - if workflowData.EngineConfig == nil || workflowData.EngineConfig.ID != string(constants.PiEngine) { - return nil - } - - engineValidationLog.Print("Validating Pi engine requirements: gh-proxy and cli-proxy") - - ghProxyEnabled := isGitHubCLIModeEnabled(workflowData) - cliProxyEnabled := workflowData.ParsedTools != nil && workflowData.ParsedTools.CLIProxy - - if !ghProxyEnabled && !cliProxyEnabled { - return fmt.Errorf("the Pi engine requires gh-proxy and CLI proxy to be enabled.\n\nAdd the following to your workflow frontmatter:\n\n tools:\n github:\n mode: gh-proxy\n cli-proxy: true\n\nSee: %s", constants.DocsEnginesURL) - } - - if !ghProxyEnabled { - return fmt.Errorf("the Pi engine requires gh-proxy to be enabled.\n\nAdd the following to your workflow frontmatter:\n\n tools:\n github:\n mode: gh-proxy\n\nSee: %s", constants.DocsEnginesURL) - } - - if !cliProxyEnabled { - return fmt.Errorf("the Pi engine requires CLI proxy to be enabled.\n\nAdd the following to your workflow frontmatter:\n\n tools:\n cli-proxy: true\n\nSee: %s", constants.DocsEnginesURL) - } - - engineValidationLog.Print("Pi engine requirements satisfied: gh-proxy and cli-proxy are enabled") - return nil -} - // EngineHasValidateSecretStep checks if the engine provides a validate-secret step. // This is used to determine whether the secret_verification_result job output should be added. // diff --git a/pkg/workflow/pi_validation_test.go b/pkg/workflow/pi_validation_test.go deleted file mode 100644 index e59a8994fdf..00000000000 --- a/pkg/workflow/pi_validation_test.go +++ /dev/null @@ -1,84 +0,0 @@ -//go:build !integration - -package workflow - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidatePiEngineRequirements_NoPiEngine(t *testing.T) { - c := NewCompiler() - workflowData := &WorkflowData{ - EngineConfig: &EngineConfig{ID: "copilot"}, - } - err := c.validatePiEngineRequirements(workflowData) - assert.NoError(t, err, "Non-Pi engine should not trigger Pi validation") -} - -func TestValidatePiEngineRequirements_NilEngineConfig(t *testing.T) { - c := NewCompiler() - workflowData := &WorkflowData{AI: "copilot"} - err := c.validatePiEngineRequirements(workflowData) - assert.NoError(t, err, "Nil EngineConfig should not trigger Pi validation") -} - -func TestValidatePiEngineRequirements_BothMissing(t *testing.T) { - c := NewCompiler() - workflowData := &WorkflowData{ - EngineConfig: &EngineConfig{ID: "pi"}, - Tools: map[string]any{}, - ParsedTools: NewTools(map[string]any{}), - } - err := c.validatePiEngineRequirements(workflowData) - require.Error(t, err, "Pi engine without gh-proxy and cli-proxy should fail") - assert.Contains(t, err.Error(), "gh-proxy") - assert.Contains(t, err.Error(), "CLI proxy") -} - -func TestValidatePiEngineRequirements_OnlyGhProxy(t *testing.T) { - c := NewCompiler() - workflowData := &WorkflowData{ - EngineConfig: &EngineConfig{ID: "pi"}, - Tools: map[string]any{ - "github": map[string]any{"mode": "gh-proxy"}, - }, - ParsedTools: NewTools(map[string]any{ - "github": map[string]any{"mode": "gh-proxy"}, - "cli-proxy": false, - }), - } - err := c.validatePiEngineRequirements(workflowData) - require.Error(t, err, "Pi engine without cli-proxy should fail") - assert.Contains(t, err.Error(), "CLI proxy") - assert.NotContains(t, err.Error(), "gh-proxy required") -} - -func TestValidatePiEngineRequirements_OnlyCliProxy(t *testing.T) { - c := NewCompiler() - workflowData := &WorkflowData{ - EngineConfig: &EngineConfig{ID: "pi"}, - Tools: map[string]any{}, - ParsedTools: NewTools(map[string]any{"cli-proxy": true}), - } - err := c.validatePiEngineRequirements(workflowData) - require.Error(t, err, "Pi engine without gh-proxy should fail") - assert.Contains(t, err.Error(), "gh-proxy") -} - -func TestValidatePiEngineRequirements_BothEnabled(t *testing.T) { - c := NewCompiler() - toolsRaw := map[string]any{ - "github": map[string]any{"mode": "gh-proxy"}, - "cli-proxy": true, - } - workflowData := &WorkflowData{ - EngineConfig: &EngineConfig{ID: "pi"}, - Tools: toolsRaw, - ParsedTools: NewTools(toolsRaw), - } - err := c.validatePiEngineRequirements(workflowData) - assert.NoError(t, err, "Pi engine with both gh-proxy and cli-proxy should be valid") -}