[docs] docs: unbloat engines.md (507 → 398 lines, 21% reduction)#35263
Conversation
- Condense feature-comparison notes from 8 bullets to one paragraph - Merge GHEC/GHES api-target examples (near-identical config) - Consolidate BYOK examples and overlapping NOTE admonitions - Replace per-engine timeout subsections + duplicate summary table with a single capability table - Drop redundant acceptEdits/bypassPermissions prose subsections (info already in the permission-mode lookup table)
github-actions
Bot
added
automation
doc-unbloat
documentation
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
🧪 Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. PR #35263 is a documentation-only change ('[docs] docs: unbloat engines.md (507 → 398 lines, 21% reduction)'). Test Quality Sentinel skipped. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #35263 does not have the 'implementation' label and has 0 new lines of code in default business logic directories (threshold: 100). |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Streamlines the “Engines” reference documentation by condensing several explanatory sections, examples, and tables into shorter paragraphs and consolidated tables.
Changes:
- Collapsed multiple “Notes” / examples sections into single-paragraph summaries.
- Simplified endpoint and BYOK provider examples by removing alternate variants.
- Replaced/merged multiple timeout and permission-mode explanations into compact tables plus short guidance.
Show a summary per file
| File | Description |
|---|---|
| docs/src/content/docs/reference/engines.md | Refactors engine reference docs for brevity by consolidating notes, examples, and tables. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
docs/src/content/docs/reference/engines.md:143
- The heading says the example covers both GHEC and GHES, but the YAML shown is specifically a tenant-style hostname (
api.acme.ghe.com). To avoid readers copying the wrong value for GHES, consider either (a) using a placeholder likeapi-target: <copilot-endpoint-hostname>plus explicit bullets for GHEC vs GHES, or (b) restoring a minimal second GHES variant.
**Example** — specify a GHEC or GHES Copilot endpoint (use `api.enterprise.githubcopilot.com` for GHES):
```yaml wrap
engine:
id: copilot
api-target: api.acme.ghe.com
- Files reviewed: 1/1 changed files
- Comments generated: 8
| - `engine.agent` references a `.github/agents/` file for custom Copilot agent behavior. See [Copilot Custom Configuration](#copilot-custom-configuration). | ||
| - `engine.bare` disables automatic context loading (memory files, custom instructions). See [Bare Mode](#bare-mode-bare) below. | ||
| - `engine.harness` allows replacing the built-in Copilot harness script. See [Custom Harness Script](#custom-harness-script-harness) below. | ||
| `max-runs` (default `500`) and `max-effective-tokens` (default `25000000`) are top-level frontmatter fields supported by all engines. `max-turns` limits Claude iterations per run; `max-continuations` enables Copilot autopilot mode. Codex `web-search` is opt-in via `tools: web-search:`; other engines use a third-party MCP server — see [Using Web Search](/gh-aw/reference/web-search/). `engine.agent`, `engine.bare`, and `engine.harness` are described below. |
| | Crush | not enforced by gh-aw (engine-managed) | | ||
|
|
||
| See [Tool Timeout Configuration](/gh-aw/reference/tools/#tool-timeout-configuration) for full documentation including `tools.startup-timeout`. | ||
| Defaults: Claude `60s`, Codex `120s`. Other engines (Copilot, Gemini, Crush) are engine-managed and not enforced by gh-aw. See [Tool Timeout Configuration](/gh-aw/reference/tools/#tool-timeout-configuration) for full documentation including `tools.startup-timeout`. |
| | Knob | Copilot | Claude | Codex/Gemini/Crush/OpenCode | Purpose | | ||
| |---|:---:|:---:|:---:|---| | ||
| | `timeout-minutes` | ✅ | ✅ | ✅ | Job-level wall clock | | ||
| | `tools.timeout` | ✅ | ✅ | ✅ | Per tool-call limit (seconds) | |
| |---|:---:|:---:|:---:|---| | ||
| | `timeout-minutes` | ✅ | ✅ | ✅ | Job-level wall clock | | ||
| | `tools.timeout` | ✅ | ✅ | ✅ | Per tool-call limit (seconds) | | ||
| | `tools.startup-timeout` | ✅ | ✅ | ✅ | MCP server startup limit | |
| | `max-continuations` | ✅ | ❌ | ❌ | Autopilot run budget | | ||
|
|
||
| Claude supports `max-turns` to cap the number of AI iterations per run. Set it together with `tools.timeout` to control both breadth (number of turns) and depth (time per tool call): | ||
| Copilot uses `max-continuations` for autopilot runs; Claude uses `max-turns` to cap iterations. Codex, Gemini, Crush, and OpenCode rely solely on `timeout-minutes` and `tools.timeout`. |
| COPILOT_PROVIDER_API_KEY: ${{ secrets.PROVIDER_API_KEY }} # OPTIONAL for local providers | ||
| COPILOT_PROVIDER_TYPE: anthropic # OPTIONAL — default: openai |
| ### Gateway-side enforcement | ||
|
|
||
| The **MCP gateway's `allowed:` filter is the sole effective tool boundary in `bypassPermissions` mode** (and a second layer of enforcement in `acceptEdits` mode). gh-aw compiles the `allowed:` list from each `mcp-servers:` entry into the gateway configuration before the agent starts. The gateway enforces this list server-side, regardless of what the agent requests. | ||
| The MCP gateway's `allowed:` filter is the sole effective tool boundary in `bypassPermissions` mode (and a second layer of enforcement otherwise). Always specify `allowed:` on each `mcp-servers:` entry to restrict which MCP tools are reachable: |
| > [!WARNING] | ||
| > Do not rely on `tools:` or `mcp-servers: allowed:` for security guarantees in `bypassPermissions` mode. The agent can already run arbitrary shell commands when unrestricted bash is granted, so `--allowed-tools` provides no meaningful additional boundary. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Documentation condensation: mostly clean, a few follow-ups needed
The 21% line reduction is well-executed. Most content is preserved accurately. However, the condensation introduced a few contradictions that existing review comments have already flagged; this review adds one new concern.
Issues to address in follow-up
tools.timeout vs prose contradiction (lines 330–337): The prose states Copilot tool timeout is "engine-managed and not enforced by gh-aw", but the new table shows ✅ for Copilot under tools.timeout. These directly contradict each other. The table needs a note or the Copilot cell needs clarification (e.g., "passed through but engine-managed").
Incomplete startup-timeout coverage in prose (line 338): "Codex, Gemini, Crush, and OpenCode rely solely on timeout-minutes and tools.timeout" — the table above that sentence also marks tools.startup-timeout ✅ for those engines. Missing from the prose.
bypassPermissions warning contradicts gateway enforcement prose (line 389): The [!WARNING] says "Do not rely on mcp-servers: allowed:" but the immediately preceding paragraph establishes the gateway allowed: filter as the "sole effective tool boundary" in bypassPermissions mode. This is conflicting security guidance. The old version had this warning positioned after the --allowed-tools bypass explanation, making it clear only client-side enforcement is bypassed. Suggest rewording: "Do not rely on tools: configuration (--allowed-tools is ignored in this mode); gateway allowed: still enforces MCP tool access."
Missing legacy engine.args compatibility note (not previously flagged): The condensation removed the paragraph explaining that old workflows using engine.args: ["--permission-mode", "acceptEdits"] still work. Users upgrading from older configs have no documentation for this migration path.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 811.2K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs — requesting changes on three accuracy issues found during condensation.
📋 Key Themes & Highlights
Issues Found
- GHES network config loss: The merged GHEC/GHES example omits the on-prem GitHub Enterprise hostname from
network.allowed, which GHES users need alongside the Copilot API hostname. - BYOK example accuracy:
COPILOT_PROVIDER_TYPEwas deliberately commented out in the original (it's optional with a default), but is now uncommented — making it look required or implyinganthropicis always needed. - Legacy
engine.argscompatibility dropped: A backward-compat note explaining thatengine.args: ["--permission-mode", ...]still works was removed without replacement, leaving users with existing configs in the dark.
Positive Highlights
- ✅ Excellent use of tables to replace repetitive per-engine prose sections — much more scannable
- ✅ Security-critical WARNING and NOTE admonitions are preserved
- ✅ The permission-mode lookup table is now positioned before the gateway section, which improves reading flow
- ✅ Overall 21% reduction is meaningful and the cuts are well-targeted
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · opus46 3.2M
| @@ -156,19 +148,6 @@ network: | |||
| - api.acme.ghe.com | |||
There was a problem hiding this comment.
[/grill-with-docs] The merged GHEC/GHES example omits the GHES-specific network.allowed entry for the on-prem GitHub hostname.
The original GHES example included github.company.com in network.allowed alongside the Copilot API hostname. GHES workflows typically need both: the Copilot endpoint and the GitHub Enterprise Server hostname. The merged example only shows the Copilot API hostname, which may leave GHES users wondering why their requests are blocked.
💡 Suggested addition
Add a prose note after the example:
For GHES, also add your GitHub Enterprise Server hostname (e.g.
github.company.com) tonetwork.allowed.
Or add it as a comment in the YAML block.
| COPILOT_PROVIDER_BASE_URL: ${{ secrets.PROVIDER_BASE_URL }} # REQUIRED — activates BYOK | ||
| COPILOT_MODEL: claude-sonnet-4 # REQUIRED for most providers | ||
| COPILOT_PROVIDER_API_KEY: ${{ secrets.PROVIDER_API_KEY }} # OPTIONAL for local providers | ||
| COPILOT_PROVIDER_TYPE: anthropic # OPTIONAL — default: openai |
There was a problem hiding this comment.
[/grill-with-docs] COPILOT_PROVIDER_TYPE: anthropic is now uncommented, which may mislead readers into always setting it.
The original example had this line commented out (e.g. # COPILOT_PROVIDER_TYPE: anthropic) to convey that OpenAI-compatible is the default and this field is only needed when switching to a different provider type. The updated example uncomments it, making it look like a required field — or that anthropic is the default.
💡 Suggested fix
Restore the commented form to preserve intent:
COPILOT_PROVIDER_TYPE: anthropic # OPTIONAL — set to "anthropic" or "azure" if needed (default: openai)Or use a placeholder that makes the optional nature clear:
# COPILOT_PROVIDER_TYPE: openai # default; use "anthropic" or "azure" if needed| gh-aw **does not** derive `bypassPermissions` from `bash: "*"`, `bash: [":*"]`, or `bash: null`. To use `bypassPermissions`, set it explicitly with `engine.permission-mode: bypassPermissions`. | ||
|
|
||
| ### `acceptEdits` mode (default) | ||
| `engine.permission-mode` takes precedence over any `--permission-mode` flag supplied through `engine.args`. When unset, the default is `acceptEdits` (or `auto` when `tools.edit: false`). gh-aw **does not** derive `bypassPermissions` implicitly from unrestricted bash — set it explicitly. |
There was a problem hiding this comment.
[/grill-with-docs] The legacy engine.args compatibility note was removed without replacement.
The original had a dedicated subsection explaining that engine.args: ["--permission-mode", "..."] continues to work and that gh-aw strips and subsumes the flag if engine.permission-mode is also set. Users who have existing workflows using the legacy form will no longer find any documentation explaining this behavior, and may be confused about whether their configs are still supported.
💡 Suggested fix
Add a one-sentence note inline, e.g.:
Workflows that previously used
engine.args: ["--permission-mode", "<value>"]continue to work; if both are set,engine.permission-modetakes precedence.
Summary
Documentation-only cleanup that reduces
engines.mdfrom 507 to 398 lines (21% reduction) by consolidating redundant examples, replacing verbose bullet lists with inline prose, and merging duplicate tables.Changes
docs/src/content/docs/reference/engines.mdImpact Assessment
Testing
No testing required — documentation only.