Store sandbox.agent.runtime in aw_info.json and add --runtime filter to logs/audit - #51465
Conversation
…to logs/audit + MCP tools Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. This PR only updates workflow lock files (.lock.yml artifacts). Test Quality Sentinel analysis skipped.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check.
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
|
✅ Ponytail Reviewer completed successfully! pr-diff.patch is empty (0 bytes), so there are no changed lines to review for over-engineering. Stopping per instructions.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — requesting changes on a validation gap and a test coverage gap.
📋 Key Themes & Highlights
Key Themes
- Validation asymmetry:
logs --runtimevalidates the value eagerly;audit --runtimedoes not, silently skipping all runs on a typo. - Code duplication:
shouldSkipAuditRunre-implements the runtime-matching logic frommatchRuntimeFilterinstead of calling it. - Test coverage gaps:
shouldSkipAuditRun's new branch has no unit tests;TestApplyRunFilters_Runtimeis missing theempty agent_runtimeedge case.
Positive Highlights
- ✅ Clean end-to-end threading of the new field: compiler →
generate_aw_info.cjs→aw_info.json→ Go struct → filter flag. - ✅
matchRuntimeFilterfollows the same contract asmatchEngineFilter— consistent pattern. - ✅ Tests for the new
generate_aw_info.cjsbehaviour and theTestMatchRuntimeFilterunit are well structured and cover all expected states. - ✅ MCP tool surfaces kept in sync with CLI flags.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 65.1 AIC · ⌖ 7.85 AIC · ⊞ 7.1K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/cli/audit.go:156
[/codebase-design] audit silently accepts any --runtime value (no input validation), but logs calls validateLogsRuntime and returns an error on unknown values. gh aw audit --runtime typo will silently skip all runs rather than failing fast.
<details>
<summary>💡 Suggested fix</summary>
Add validation in getAuditCommandOptions, after reading opts.runtimeFilter:
if err := validateLogsRuntime(opts.runtimeFilter); err != nil {
return auditCommandOptions{}, err
}…
pkg/cli/audit.go:582
[/codebase-design] The runtime-filter logic in shouldSkipAuditRun is a copy of matchRuntimeFilter in logs_orchestrator_filters.go — it directly calls parseAwInfo and branches on AgentRuntime instead of reusing the shared helper. If the skip message format or the fallback logic changes later, both sites need updating.
<details>
<summary>💡 Suggested refactor</summary>
Reuse matchRuntimeFilter here (or move shouldSkipAuditRun's runtime block into a shared package-level helper)…
pkg/cli/audit.go:563
[/tdd] shouldSkipAuditRun has no unit tests for the new runtimeFilter branch. The function is a pure function that takes strings — it is easy to test in isolation, and the logs side already has TestApplyRunFilters_Runtime as a model.
<details>
<summary>💡 Suggested tests</summary>
Add a TestShouldSkipAuditRun_Runtime table-driven test covering:
- matching runtime → not skipped
- non-matching runtime → skipped + correct message
- empty
aw_info.json→ skipped (unknown runtime)
-…
pkg/cli/logs_orchestrator_filters_test.go:443
[/tdd] TestApplyRunFilters_Runtime is missing the empty agent_runtime case (where aw_info.json exists but agent_runtime is ""). logs_engine_filter_test.go covers this for engine but not for runtime in the integration-level filter test.
<details>
<summary>💡 Suggested test case to add</summary>
{
name: "empty agent_runtime is skipped",
awInfo: `{"agent_runtime":""}`,
filterRuntime: "gvisor",
wantSkip: true,
},This ensures the fil…
…ntime filter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (300 new lines across business logic paths) but does not have a linked Architecture Decision Record (ADR). 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
|
@copilot Quick triage nudge for this PR. Please address the current maintainer-facing review feedback below, refresh the branch if GitHub can update it cleanly, run the Open review context (newest first):
Branch refresh was requested. Run: https://github.com/github/gh-aw/actions/runs/31278943751
|
|
@copilot Quick triage nudge for this PR. Please address the current maintainer-facing review feedback, refresh the branch if GitHub can update it cleanly, run the Open items (newest first):
Branch refresh was requested.
|
…ze ADR Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
PR Triage
|
|
@copilot Quick triage nudge for this PR. Please refresh the branch if GitHub can update it cleanly, re-check the current maintainer-facing state, run the Open review context (newest first):
Branch refresh was requested. Run: https://github.com/github/gh-aw/actions/runs/31287357931
|
PR Triage
New audit/filter feature, CI green, but blocked mergeable_state and dismissed review - needs human attention.
|
sandbox.agent.runtime(e.g.gvisor,docker-sbx) was not recorded anywhere post-compile, making it impossible to filter or audit runs by sandbox runtime after the fact.aw_info.json generation
GH_AW_INFO_AGENT_RUNTIMEfromsandbox.agent.runtimein thegenerate_aw_infostep.generate_aw_info.cjsreads this env var and writesagent_runtimeintoaw_info.json.AwInfoGo struct gains a matchingAgentRuntimefield for CLI-side parsing.--runtimefilter forlogsandauditlogs: new--runtimeflag, filtering wired throughrunFilterOpts/applyRunFilters(mirrors the existing--enginefilter).audit: new--runtimeflag;shouldSkipAuditRunnow also checksagent_runtimefromaw_info.json.MCP tools
logsandauditMCP tools gain aruntimeparameter, forwarded as--runtime <value>to the underlying CLI invocation, keeping MCP and CLI surfaces in sync.All 284 compiled
.lock.ymlworkflow files are regenerated to include the newGH_AW_INFO_AGENT_RUNTIMEenv var.Run: https://github.com/github/gh-aw/actions/runs/31287357931> Generated by 👨🍳 PR Sous Chef · gpt54 · 20.7 AIC · ⌖ 6.68 AIC · ⊞ 8.5K · ◷