Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions actions/setup/js/comment_memory_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const COMMENT_MEMORY_EXTENSION = ".md";
const MAX_MEMORY_ID_LENGTH = 128;
const COMMENT_MEMORY_MAX_SCAN_PAGES = 50;
const COMMENT_MEMORY_MAX_SCAN_EMPTY_PAGES = 5;
const COMMENT_MEMORY_MAX_FILE_BYTES = 16 * 1024;
const COMMENT_MEMORY_MAX_TOTAL_BYTES = 48 * 1024;
const COMMENT_MEMORY_PROMPT_START_MARKER = "<!-- gh-aw-comment-memory-prompt:start -->";
const COMMENT_MEMORY_PROMPT_END_MARKER = "<!-- gh-aw-comment-memory-prompt:end -->";
const COMMENT_MEMORY_CODE_FENCE = "``````";
Expand Down Expand Up @@ -147,6 +149,8 @@ module.exports = {
COMMENT_MEMORY_EXTENSION,
COMMENT_MEMORY_MAX_SCAN_PAGES,
COMMENT_MEMORY_MAX_SCAN_EMPTY_PAGES,
COMMENT_MEMORY_MAX_FILE_BYTES,
COMMENT_MEMORY_MAX_TOTAL_BYTES,
COMMENT_MEMORY_PROMPT_START_MARKER,
COMMENT_MEMORY_PROMPT_END_MARKER,
COMMENT_MEMORY_CODE_FENCE,
Expand Down
15 changes: 15 additions & 0 deletions actions/setup/js/setup_comment_memory_files.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const {
COMMENT_MEMORY_DIR,
COMMENT_MEMORY_MAX_SCAN_PAGES,
COMMENT_MEMORY_MAX_SCAN_EMPTY_PAGES,
COMMENT_MEMORY_MAX_FILE_BYTES,
COMMENT_MEMORY_MAX_TOTAL_BYTES,
COMMENT_MEMORY_PROMPT_START_MARKER,
COMMENT_MEMORY_PROMPT_END_MARKER,
extractCommentMemoryEntries,
Expand Down Expand Up @@ -144,6 +146,19 @@ async function collectCommentMemoryFiles(githubClient, commentMemoryConfig) {
} catch (err) {
throw new Error(`Failed to create directory ${COMMENT_MEMORY_DIR}: ${String(err)}`, { cause: err });
}
let totalBytes = 0;
for (const [memoryId, content] of memoryMap.entries()) {
const writtenContent = `${content}\n`;
const contentBytes = Buffer.byteLength(writtenContent, "utf8");
if (contentBytes > COMMENT_MEMORY_MAX_FILE_BYTES) {
throw new Error(`${ERR_VALIDATION}: comment_memory file '${memoryId}.md' is ${contentBytes} bytes, exceeding max ${COMMENT_MEMORY_MAX_FILE_BYTES} bytes`);
}
totalBytes += contentBytes;
if (totalBytes > COMMENT_MEMORY_MAX_TOTAL_BYTES) {
throw new Error(`${ERR_VALIDATION}: comment_memory total size ${totalBytes} bytes exceeds max ${COMMENT_MEMORY_MAX_TOTAL_BYTES} bytes`);
}
}

const writtenFiles = [];
for (const [memoryId, content] of memoryMap.entries()) {
const filePath = path.join(COMMENT_MEMORY_DIR, `${memoryId}.md`);
Expand Down
72 changes: 72 additions & 0 deletions actions/setup/js/setup_comment_memory_files.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,76 @@ describe("setup_comment_memory_files", () => {
);
expect(global.core.warning).not.toHaveBeenCalledWith(expect.stringContaining("E004"));
});

it("warns and skips writing when a memory entry exceeds per-file size cap", async () => {
fs.writeFileSync(CONFIG_PATH, JSON.stringify({ "comment-memory": { target: "triggering" } }));
const oversizedBody = `${"a".repeat(16 * 1024 + 1)}`;
global.github = {
rest: {
issues: {
listComments: vi.fn().mockResolvedValue({
data: [{ body: `\`\`\`\`\`\`gh-aw-comment-memory:default\n${oversizedBody}\n\`\`\`\`\`\`\n` }],
}),
},
},
};

const module = await import("./setup_comment_memory_files.cjs");
await module.main();

expect(fs.existsSync(path.join(COMMENT_MEMORY_DIR, "default.md"))).toBe(false);
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("exceeding max 16384 bytes"));
});

it("counts the trailing newline when enforcing the per-file size cap", async () => {
// Content of exactly 16384 bytes passes the old check (16384 > 16384 is false)
// but the written file would be 16385 bytes (content + \n). The fix measures the
// written string, so 16385 > 16384 triggers the cap correctly.
fs.writeFileSync(CONFIG_PATH, JSON.stringify({ "comment-memory": { target: "triggering" } }));
const atCapBody = "a".repeat(16 * 1024); // 16384 bytes — exactly at the cap before \n
global.github = {
rest: {
issues: {
listComments: vi.fn().mockResolvedValue({
data: [{ body: `\`\`\`\`\`\`gh-aw-comment-memory:default\n${atCapBody}\n\`\`\`\`\`\`\n` }],
}),
},
},
};

const module = await import("./setup_comment_memory_files.cjs");
await module.main();

expect(fs.existsSync(path.join(COMMENT_MEMORY_DIR, "default.md"))).toBe(false);
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("exceeding max 16384 bytes"));
});

it("warns and skips writing when total memory size exceeds cap", async () => {
fs.writeFileSync(CONFIG_PATH, JSON.stringify({ "comment-memory": { target: "triggering" } }));
const chunk = "a".repeat(12 * 1024 + 1);
global.github = {
rest: {
issues: {
listComments: vi.fn().mockResolvedValue({
data: [
{
body:
`\`\`\`\`\`\`gh-aw-comment-memory:one\n${chunk}\n\`\`\`\`\`\`\n` +
`\`\`\`\`\`\`gh-aw-comment-memory:two\n${chunk}\n\`\`\`\`\`\`\n` +
`\`\`\`\`\`\`gh-aw-comment-memory:three\n${chunk}\n\`\`\`\`\`\`\n` +
`\`\`\`\`\`\`gh-aw-comment-memory:four\n${chunk}\n\`\`\`\`\`\`\n`,
},
],
}),
},
},
};

const module = await import("./setup_comment_memory_files.cjs");
await module.main();

expect(fs.existsSync(path.join(COMMENT_MEMORY_DIR, "one.md"))).toBe(false);
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("total size"));
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("exceeds max 49152 bytes"));
});
});
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# ADR-27325: Organize pkg/workflow Helpers by Semantic Responsibility

**Date**: 2026-04-20
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan, Copilot

---
Expand Down Expand Up @@ -46,6 +46,10 @@ A dedicated sub-package would enforce a hard import boundary and make the API ta
- No public API surface changes; `GetCopilotAPITarget`, `GetGeminiAPITarget`, and `DefaultGeminiAPITarget` retain their signatures and are re-exported from the new file.
- The refactor is behavior-preserving: empty-string filtering and type coercion semantics are maintained across all moved and renamed helpers.

### Norms

- CI **MUST** enforce this file-organization boundary with `TestAWFHelpersDoesNotContainEngineAPITargetHelpers` in `pkg/workflow/engine_api_targets_file_organization_test.go`.

---

## Part 2 — Normative Specification (RFC 2119)
Expand All @@ -71,4 +75,4 @@ An implementation is considered conformant with this ADR if it satisfies all **M

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24667638374) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
*Accepted after implementation and conformance validation.*
5 changes: 3 additions & 2 deletions docs/adr/27327-parser-utility-test-strategy.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# ADR-27327: Test Strategy for Parser Package Utility Functions

**Date**: 2026-04-20
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan, Copilot

---
Expand Down Expand Up @@ -41,6 +41,7 @@ Parser utilities could be tested exclusively at the workflow-loading integration
#### Neutral
- The renamed function `TestIsNotFoundError_RemoteNested` in `import_remote_nested_test.go` disambiguates the two test functions but changes the test name string reported in CI output and `go test -v` listings.
- No production code is modified by this decision; all changes are confined to `_test.go` files.
- Conformance is implemented in `pkg/parser/frontmatter_utils_test.go` and `pkg/parser/import_remote_nested_test.go`.

---

Expand Down Expand Up @@ -71,4 +72,4 @@ An implementation is considered conformant with this ADR if it satisfies all **M

---

*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*
*Accepted after implementation and conformance validation.*
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# ADR-27387: Filter Non-Frontmatter Markdown Files During compile-all Discovery

**Date**: 2026-04-20
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan, Copilot

---
Expand Down Expand Up @@ -44,6 +44,7 @@ Workflow Markdown files could be required to follow a specific naming pattern (e
#### Neutral
- The first-line check is intentionally strict (`bytes.Equal(firstLine, []byte("---"))`); any leading whitespace or UTF-8 BOM before `---` will cause the file to be skipped. This is consistent with how YAML frontmatter is defined in the gh-aw spec.
- Both compile pipelines now share a single filtering function, reducing future drift risk.
- Error propagation behavior is covered by `TestCompileAllWorkflowFiles/compile all propagates frontmatter filter I/O errors` in `pkg/cli/commands_file_watching_test.go`.

---

Expand Down Expand Up @@ -75,4 +76,4 @@ An implementation is considered conformant with this ADR if it satisfies all **M

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24679557357) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
*Accepted after implementation and conformance validation.*
19 changes: 17 additions & 2 deletions docs/adr/27476-extend-safe-outputs-dependencies-via-needs-field.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# ADR-27476: Extend `safe_outputs` Job Dependencies via `safe-outputs.needs` Field

**Date**: 2026-04-21
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan

---
Expand Down Expand Up @@ -41,6 +41,16 @@ The compiler could parse expression strings inside `safe-outputs` config values
- A new validation function (`validateSafeOutputsNeeds`) is added to the compiler pipeline, following the same structure as the existing `validateSafeJobNeeds` validator.
- The schema change (`main_workflow_schema.json`) affects all schema-validation tooling and IDE integrations that consume the schema.

### Safeguards

- Dependency cycles introduced through `safe-outputs.needs` are prevented by `JobManager.ValidateDependencies`, which validates the fully assembled job graph (including all `safe_outputs` edges) as a required compile-time validation gate in `pkg/workflow/compiler_yaml.go`.
- Negative validation coverage for reserved and unknown `safe-outputs.needs` targets is required and implemented in `pkg/workflow/safe_jobs_needs_validation_test.go`.

### Norms

- Changes to `safe-outputs.needs` semantics **MUST** update `pkg/parser/schemas/main_workflow_schema.json` and preserve `uniqueItems: true`.
- Any future extension of this field **MUST** include a schema version update and corresponding IDE/schema-consumer sync.

---

## Part 2 — Normative Specification (RFC 2119)
Expand All @@ -65,6 +75,11 @@ The compiler could parse expression strings inside `safe-outputs` config values
2. A `safe-outputs.needs` entry **MUST NOT** reference a job name that is not declared in the workflow's top-level `jobs:` map.
3. When a violation of rules 1 or 2 is detected, the compiler **MUST** emit a compile-time error and **MUST NOT** produce a compiled workflow artifact.

### Safeguards and Schema Sync

1. `JobManager.ValidateDependencies` **MUST** run after `safe-outputs.needs` merge/validation so that the fully assembled job graph is checked for cycles before a compiled workflow artifact is emitted.
2. Any schema change to `safe-outputs.needs` semantics **MUST** update `pkg/parser/schemas/main_workflow_schema.json` and related IDE/schema-consumer documentation in the same release.

### Import Merge Behavior

1. When a workflow imports another workflow, the importer's `safe-outputs.needs` **MUST** be merged with the imported workflow's `safe-outputs.needs` as a deduplicated union.
Expand All @@ -76,4 +91,4 @@ An implementation is considered conformant with this ADR if it satisfies all **M

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24701927396) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
*Accepted after implementation and conformance validation.*
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# ADR-27479: Comment Memory — File-Based Agent Memory with GitHub Comment Persistence

**Date**: 2026-04-21
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan, copilot-swe-agent

---
Expand Down Expand Up @@ -49,6 +49,16 @@ Memory could be stored in a dedicated GitHub issue (a "memory issue") or externa
- Existing safe-output handler manager gains automatic file-based sync logic that runs unconditionally after each agent turn
- The W3C-style safe outputs specification is updated to formally document the `comment_memory` type and end-to-end data flow

### Safeguards

- Pre-agent comment-memory materialization enforces hard size caps: each memory file **MUST** be ≤16 KiB and the total materialized directory **MUST** be ≤48 KiB.
- If either limit is exceeded, setup **MUST** fail safely (warning + skip persistence setup) rather than materializing oversized memory into prompts.

### Norms

- The size-cap constants are source-of-truth values in `actions/setup/js/comment_memory_helpers.cjs` and **MUST** be kept aligned with the safe-outputs specification.
- Any change to these caps **MUST** update both implementation tests (`setup_comment_memory_files.test.cjs`) and the W3C-style safe-outputs spec.

---

## Part 2 — Normative Specification (RFC 2119)
Expand All @@ -62,6 +72,12 @@ Memory could be stored in a dedicated GitHub issue (a "memory issue") or externa
3. Memory files **MUST** be writable by the agent process at runtime.
4. Memory files **MUST NOT** include the XML marker tags or footer content — only the user-editable body of the managed block **SHALL** be materialized.

### Memory Size Limits

1. Each materialized memory file **MUST NOT** exceed 16 KiB.
2. The total size of all materialized memory files under `/tmp/gh-aw/comment-memory/` **MUST NOT** exceed 48 KiB.
3. When either size limit is exceeded, setup **MUST** fail safely and **MUST NOT** materialize oversized memory into the agent prompt context.

### Agent Interaction with Memory

1. Agents **MUST NOT** call a `comment_memory` safe-output tool to persist memory; memory persistence **SHALL** occur exclusively via the automatic file-sync mechanism.
Expand Down Expand Up @@ -94,4 +110,4 @@ An implementation is considered conformant with this ADR if it satisfies all **M

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24706352108) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
*Accepted after implementation and conformance validation.*
1 change: 1 addition & 0 deletions docs/src/content/docs/reference/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,7 @@ safe-outputs:
```

Use the single `safe-outputs.needs` field for all explicit custom dependencies.
The field is schema-defined in `pkg/parser/schemas/main_workflow_schema.json`; update local editor schema integrations when upgrading gh-aw schema versions.

Validation rules:

Expand Down
2 changes: 2 additions & 0 deletions docs/src/content/docs/specs/safe-outputs-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,7 @@ for (const op of issueOps) {
When `tools.comment-memory` is enabled, implementations MUST support this additional data-flow path:

1. **GitHub comment → local files (pre-agent setup)**: A setup step reads the managed comment body from the target issue or pull request, extracts content between `<gh-aw-comment-memory id="...">` and `</gh-aw-comment-memory>`, and writes one file per memory entry under `/tmp/gh-aw/comment-memory/<memory_id>.md`.
- Setup MUST enforce memory-size safeguards: each materialized file MUST be ≤16 KiB and the aggregate materialized directory MUST be ≤48 KiB. Exceeding either limit MUST emit a warning and skip writing all memory files, leaving the comment-memory directory empty.
2. **Local files → agent**: The prompt MUST include instructions that memory files are edited directly in `/tmp/gh-aw/comment-memory/`.
3. **Agent → artifact**: The unified agent artifact MUST include `/tmp/gh-aw/comment-memory/` when comment memory is enabled.
4. **Artifact → threat detection**: Threat-detection prompt setup MUST include discovered comment-memory files in analysis context.
Expand Down Expand Up @@ -2514,6 +2515,7 @@ This section provides complete definitions for all remaining safe output types.

- `memory_id` MUST be validated as `[A-Za-z0-9_-]+` with path traversal patterns rejected.
- Managed comment scan MUST be bounded by a maximum page limit.
- Materialized comment-memory files MUST enforce a per-file cap of 16 KiB and an aggregate cap of 48 KiB during setup.
- Body content MUST undergo sanitization and comment size/mention/link limit validation before upsert.
- Cross-repository targets MUST be validated against `allowed-repos`.
- Only content within managed marker tags is treated as editable memory; footer/provenance text MUST NOT be imported into editable files. For example, in `<gh-aw-comment-memory id="default">MEMORY</gh-aw-comment-memory>\n\n<!-- provenance footer -->`, only `MEMORY` is editable/imported.
Expand Down
26 changes: 26 additions & 0 deletions pkg/cli/commands_file_watching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,32 @@ func TestCompileAllWorkflowFiles(t *testing.T) {
assert.NoFileExists(t, docsLockFile, "Should not emit lock file for documentation markdown without frontmatter")
})

t.Run("compile all propagates frontmatter filter I/O errors", func(t *testing.T) {
tempDir := testutil.TempDir(t, "test-*")
workflowsDir := filepath.Join(tempDir, ".github/workflows")
err := os.MkdirAll(workflowsDir, 0o755)
require.NoError(t, err)

validWorkflow := filepath.Join(workflowsDir, "valid.md")
validContent := "---\non: push\nengine: claude\n---\n# Valid Workflow\n\nContent"
err = os.WriteFile(validWorkflow, []byte(validContent), 0o644)
require.NoError(t, err)

// Create a directory named broken.md — os.Open succeeds but a subsequent Read
// call returns an EISDIR error ("is a directory"), exercising the
// frontmatter-filter I/O propagation path without relying on process privileges
// or file-permission portability.
brokenWorkflow := filepath.Join(workflowsDir, "broken.md")
err = os.MkdirAll(brokenWorkflow, 0o755)
require.NoError(t, err)

compiler := workflow.NewCompiler()
_, err = compileAllWorkflowFiles(context.Background(), compiler, workflowsDir, false)
require.Error(t, err)
require.ErrorContains(t, err, "failed to filter markdown files")
require.ErrorContains(t, err, "failed to read workflow file")
})

t.Run("compile all handles glob error", func(t *testing.T) {
// Use a malformed glob pattern that will cause filepath.Glob to error
invalidDir := "/tmp/gh-aw/[invalid"
Expand Down
34 changes: 34 additions & 0 deletions pkg/workflow/engine_api_targets_file_organization_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//go:build !integration

package workflow

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
)

func TestAWFHelpersDoesNotContainEngineAPITargetHelpers(t *testing.T) {
content, err := os.ReadFile(filepath.Join("awf_helpers.go"))
require.NoError(t, err)

forbiddenFunctionSignatures := []string{
"func extractAPITargetHost(",
"func extractAPIBasePath(",
"func extractAPITargetAuthHeader(",
"func GetCopilotAPITarget(",
"func extractLiteralEngineEnvHost(",
"func GetCopilotAllowlistTargets(",
"func GetAntigravityAPITarget(",
"func GetGeminiAPITarget(",
"func getEngineAPIHosts(",
"const DefaultAntigravityAPITarget",
"const DefaultGeminiAPITarget",
}

for _, signature := range forbiddenFunctionSignatures {
require.NotContains(t, string(content), signature, "awf_helpers.go must not define %s", signature)
}
}
Loading