Skip to content

Set frontmatter defaults and add shared import/expression support for max limits#31484

Merged
pelikhan merged 8 commits into
mainfrom
copilot/update-front-matter-defaults
May 11, 2026
Merged

Set frontmatter defaults and add shared import/expression support for max limits#31484
pelikhan merged 8 commits into
mainfrom
copilot/update-front-matter-defaults

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 11, 2026

✨ Enhancement

What does this improve?
This PR raises default frontmatter limits (max-runs=100, max-effective-tokens=25M) and makes those fields work consistently with shared imports and GitHub Actions expressions.

Why is this valuable?
Shared agentic workflows can define these limits once for consumers, and workflows can now use ${{ ... }} expressions for both fields without schema validation failures.

Implementation approach:

  • Updated defaults and ensured AWF apiProxy.maxRuns / apiProxy.maxEffectiveTokens are emitted with default values when omitted.
  • Added import merge support for top-level max-runs and max-effective-tokens from shared workflows (with consumer override preserved).
  • Updated schema validation for both fields to accept positive numeric values and GitHub Actions expression strings.
  • Added regression tests for:
    • rejecting max-effective-tokens: "0"
    • accepting expression values for both fields
    • importing shared defaults and propagating them into generated apiProxy config.

Copilot AI and others added 3 commits May 11, 2026 11:11
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Set frontmatter defaults for max-runs and max-effective-tokens Set frontmatter defaults: max-runs=100 and max-effective-tokens=25M May 11, 2026
Copilot AI requested a review from pelikhan May 11, 2026 11:21
@pelikhan pelikhan marked this pull request as ready for review May 11, 2026 11:22
Copilot AI review requested due to automatic review settings May 11, 2026 11:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates gh-aw’s default workflow frontmatter limits so that omitted max-runs and max-effective-tokens values resolve to higher-capacity defaults and are explicitly emitted into the generated AWF apiProxy config.

Changes:

  • Updated Go runtime defaults (DefaultMaxRuns=100, DefaultMaxEffectiveTokens=25,000,000) and ensured apiProxy.maxRuns/maxEffectiveTokens are always populated.
  • Updated workflow schema + editor autocomplete metadata to document/reflect the new defaults.
  • Updated WASM golden fixtures and unit tests to match the newly emitted default values.
Show a summary per file
File Description
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden Golden fixture updated to include emitted apiProxy.maxRuns=100 and maxEffectiveTokens=25000000.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden Golden fixture updated to include emitted default maxRuns and updated token budget.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden Golden fixture updated to include emitted default maxRuns and updated token budget.
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden Golden fixture updated to include emitted default maxRuns and updated token budget.
pkg/workflow/engine.go GetMaxRuns() now falls back to the default rather than returning 0 when unset.
pkg/workflow/awf_config.go AWF config generation now defaults maxRuns to DefaultMaxRuns instead of omitting it.
pkg/workflow/awf_config_test.go Tests updated to expect emitted default maxRuns when not configured.
pkg/parser/schemas/main_workflow_schema.json Added schema default metadata and updated descriptions for the new default values.
pkg/constants/constants.go Bumped default max effective tokens to 25M and introduced DefaultMaxRuns=100.
docs/src/content/docs/reference/engines.md Documentation updated to state the new omission defaults.
docs/public/editor/autocomplete-data.json Autocomplete descriptions updated to reflect omission defaults.
.github/workflows/issue-monster.lock.yml Comment-only change in a workflow lockfile snippet.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 12/12 changed files
  • Comments generated: 1

Comment on lines +3692 to +3696
@@ -3693,7 +3693,8 @@
"description": "Maximum effective-token (ET) budget as a numeric string."
}
],
"description": "Explicit ET budget control for firewall cost enforcement. Defaults to 10000000 when omitted."
"default": 25000000,
@github-actions github-actions Bot mentioned this pull request May 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

Excellent test quality

Metric Value
New/modified tests analyzed 2
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (100%)
Duplicate test clusters 0
Test inflation detected Yes (3 lines added in test vs 1 in production = 3:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
TestBuildAWFConfigJSON/"defaults" — added maxRuns assertion pkg/workflow/awf_config_test.go:50 ✅ Design None
TestBuildAWFConfigJSON/"default max-runs is emitted when not configured" pkg/workflow/awf_config_test.go:139–158 ✅ Design None

Analysis

Both changes are straightforward behavioral contract updates reflecting the new max-runs=100 and max-effective-tokens=25M defaults:

  1. Added assertion (line 50): Verifies that BuildAWFConfigJSON emits "maxRuns":100 in the apiProxy section when using default config. Directly tests observable JSON output — a clear behavioral contract. Assertion message present ✅.

  2. Renamed + updated sub-test (line 139–158): Previously asserted "maxRuns" was absent when not configured; now asserts it equals the default value (constants.DefaultMaxRuns). This change correctly captures the behavioral shift introduced by the PR. Error path covered via require.NoError ✅. Assertion message present ✅.

Build tag: //go:build !integration present on line 1 ✅
No mock libraries: No gomock, testify/mock, .EXPECT(), or .On() usage ✅
Assertion messages: All assertions include descriptive messages ✅

Test Inflation Note

Test file: +3 additions / Production file (awf_config.go): +1 addition → 3:1 ratio, which technically triggers the inflation flag. In practice, the extra lines are a renamed sub-test and a more precise assertion — both adding value rather than padding. The 10-point penalty is applied per rubric (80 vs 90), but this is not a quality concern.


Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Both modified tests verify observable JSON output and correctly reflect the new default values introduced by this PR.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 2 tests — unit (//go:build !integration)

References: §25667079717

🧪 Test quality analysis by Test Quality Sentinel · ● 5.7M ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 5.5M

const DefaultMaxEffectiveTokens int64 = 25000000

// DefaultMaxRuns is the default AWF invocation cap enforced by the AWF API proxy.
const DefaultMaxRuns = 100
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] DefaultMaxRuns is an untyped constant, while DefaultMaxEffectiveTokens is explicitly typed as int64. Since GetMaxRuns() returns int and the struct field is MaxRuns int, the constant should carry an explicit type for consistency and to catch accidental misuse:

const DefaultMaxRuns int = 100

Untyped integer constants quietly satisfy any numeric type, which can hide narrowing bugs at call sites.

@@ -154,7 +155,7 @@ func TestBuildAWFConfigJSON(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The renamed test covers the path where WorkflowData is nil — great. There's now an additional observable behavior introduced in this PR: GetMaxRuns() falls back to DefaultMaxRuns when EngineConfig.MaxRuns is zero (i.e. explicitly set to 0 or left uninitialized). A direct unit test on GetMaxRuns() for this path would lock in the new contract:

func TestGetMaxRuns_ZeroValueFallsBackToDefault(t *testing.T) {
    e := &EngineConfig{MaxRuns: 0}
    assert.Equal(t, constants.DefaultMaxRuns, e.GetMaxRuns(),
        "zero MaxRuns should fall back to DefaultMaxRuns")
}

Without this, the fallback inside GetMaxRuns() is only tested indirectly through BuildAWFConfigJSON.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Copilot AI and others added 2 commits May 11, 2026 11:39
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit 5aa1d64.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 11, 2026

@copilot review all comments

Done. I reviewed and addressed the actionable review feedback.

Fixed in 31f1384:

  • Tightened max-effective-tokens string schema pattern to ^[1-9][0-9]*$ so "0" is rejected (matches integer minimum: 1 semantics).
  • Added a parser schema regression test ensuring max-effective-tokens: "0" fails validation.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot make sure those fields can be imported by shared agentic workflows and support github actions expressions (${{...)

Copilot AI and others added 2 commits May 11, 2026 11:49
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 11, 2026

@copilot make sure those fields can be imported by shared agentic workflows and support github actions expressions (${{...)

Addressed in 192edb6.

  • max-runs and max-effective-tokens are now importable from shared workflows (first-wins import merge, with consumer/top-level override preserved).
  • Added import regression coverage verifying imported defaults flow into generated apiProxy.maxRuns and apiProxy.maxEffectiveTokens.
  • Updated main workflow schema so both fields also accept GitHub Actions expression strings (${{ ... }}) in addition to positive numeric values.

Copilot AI changed the title Set frontmatter defaults: max-runs=100 and max-effective-tokens=25M Set frontmatter defaults and add shared import/expression support for max limits May 11, 2026
@pelikhan pelikhan merged commit 5e55315 into main May 11, 2026
@pelikhan pelikhan deleted the copilot/update-front-matter-defaults branch May 11, 2026 12:03
github-actions Bot added a commit that referenced this pull request May 12, 2026
…default

Adds a glossary entry for the top-level `max-effective-tokens` frontmatter
field introduced for AWF apiProxy enforcement, mirroring the existing
`max-runs` pattern. Updates the `max-runs` entry to mention the default
value of 100, set by PR #31484.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants