feat(commands): add output format constraints and task budgets#1717
feat(commands): add output format constraints and task budgets#1717rjmurillo wants to merge 14 commits into
Conversation
- Add invocation_limits to pr-review-config.yaml: all_open_max_prs (5), completion_gate_max_retries (3) with escalation actions - Add output_constraints to pr-review-config.yaml: per-PR token cap (4096), summary format and required columns - Update pr-review.md to enforce PR cap, retry limits, output constraints, and structured summary format - Add Completion Criteria and Stop Condition sections to context-hub-setup.md Addresses #1668
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR appears to remove a large set of .agents/ documentation and local tooling configuration files. However, the PR title/description state the goal is to add output format constraints and invocation budgets to PR-review commands/config, which is not reflected in the provided diffs.
Changes:
- Deletes numerous
.agents/analysis/*research/retrospective artifacts. - Removes
.agentssession prompts/docs (SESSION-START-PROMPT.md,SESSION-END-PROMPT.md,.agents/README.md,.agents/AGENTS.md,.agents/HANDOFF.md,.agents/CLAUDE.md). - Removes local/dev tooling config (
.actrc,.PSScriptAnalyzerSettings.psd1).
Reviewed changes
Copilot reviewed 49 out of 4575 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .agents/analysis/156-pr-review-analysis.md | Deletes an analysis artifact (PR review retrospective). |
| .agents/analysis/130-adr037-sync-evidence-review.md | Deletes an analysis artifact (ADR evidence review). |
| .agents/analysis/126-skillbook-deduplication-investigation.md | Deletes an analysis artifact (skillbook dedup investigation). |
| .agents/analysis/123-phase2a-memory-architecture-review.md | Deletes an analysis artifact (architecture review). |
| .agents/analysis/1083-1085-gap-analysis.md | Deletes an analysis artifact (gap analysis). |
| .agents/analysis/085-velocity-bottleneck-analysis.md | Deletes an analysis artifact (process analysis). |
| .agents/analysis/066-adr-032-skill-phase-gates-review.md | Deletes an analysis artifact (ADR review). |
| .agents/analysis/065-local-guardrails-critical-analysis.md | Deletes an analysis artifact (guardrails critique). |
| .agents/analysis/046-adr-046-analyst-evidence-assessment.md | Deletes an analysis artifact (ADR evidence assessment). |
| .agents/analysis/041-adr-codeql-integration-review.md | Deletes an analysis artifact (CodeQL ADR review). |
| .agents/analysis/007-ai-reviewer-bot-consolidation.md | Deletes an analysis artifact (bot consolidation analysis). |
| .agents/analysis/005-semantic-slug-protocol-analysis.md | Deletes an analysis artifact (naming protocol analysis). |
| .agents/analysis/004-markdown-parsing-library-research.md | Deletes an analysis artifact (markdown parsing research). |
| .agents/analysis/003-missing-issues-prs-investigation.md | Deletes an analysis artifact (security investigation). |
| .agents/analysis/003-memory-title-content-alignment-issue-draft.md | Deletes an analysis artifact (issue draft). |
| .agents/analysis/002-pr-checks-skill-extraction.md | Deletes an analysis artifact (skill extraction feasibility). |
| .agents/analysis/002-copilot-cli-limitations-assessment.md | Deletes an analysis artifact (Copilot CLI assessment). |
| .agents/analysis/002-ai-quality-gate-failure-patterns.md | Deletes an analysis artifact (failure patterns doc). |
| .agents/analysis/001-workflow-validation-shift-left-analysis.md | Deletes an analysis artifact (shift-left analysis). |
| .agents/analysis/001-vscode-mcp-configuration-analysis.md | Deletes an analysis artifact (VS Code MCP config analysis). |
| .agents/analysis/001-merge-resolver-session-protocol-gap.md | Deletes an analysis artifact (merge resolver gap analysis). |
| .agents/analysis/001-github-copilot-cli-mcp-config-analysis.md | Deletes an analysis artifact (Copilot CLI MCP format). |
| .agents/analysis/001-gemini-code-assist-config-research.md | Deletes an analysis artifact (Gemini config research). |
| .agents/analysis/001-claude-code-mcp-config-research.md | Deletes an analysis artifact (Claude Code MCP research). |
| .agents/analysis/001-adr-047-plugin-mode-hook-behavior-analysis.md | Deletes an analysis artifact (plugin-mode behavior analysis). |
| .agents/SESSION-START-PROMPT.md | Removes session start instructions/prompt. |
| .agents/SESSION-END-PROMPT.md | Removes session end checklist/prompt. |
| .agents/README.md | Removes .agents/ directory documentation/consumer guide. |
| .agents/HANDOFF.md | Removes read-only project dashboard/handoff. |
| .agents/CLAUDE.md | Removes Claude auto-generated context file. |
| .agents/AGENTS.md | Removes agent artifacts system documentation. |
| .actrc | Removes local act runner configuration. |
| .PSScriptAnalyzerSettings.psd1 | Removes PowerShell analyzer configuration. |
Comments suppressed due to low confidence (5)
.agents/analysis/156-pr-review-analysis.md:1
- The PR title/description indicate adding output format constraints and task budgets to PR review commands/config, but the provided diffs only show deletions of
.agents/**docs and tooling configs. Either the PR description (and likely title) needs to be updated to reflect this cleanup/removal scope, or the intended config/doc changes (e.g.,pr-review-config.yaml,pr-review.md,context-hub-setup.md) are missing from the actual changeset.
.actrc:1 - Removing
.actrcwill change (or break) the repo’s documented local GitHub Actions testing behavior for contributors usingnektos/act(runner image mappings, artifact/cache paths, architecture). If this is intentional, consider replacing it with updated guidance elsewhere (e.g., docs) and/or ensuring any contributor workflows don’t assume.actrcexists; otherwise keep the file to avoid silently degrading local CI reproducibility.
.PSScriptAnalyzerSettings.psd1:1 - Deleting
.PSScriptAnalyzerSettings.psd1can alter PowerShell lint baselines and may break any scripts/CI steps that reference this settings file explicitly (or rely on its rule exclusions likePSAvoidUsingWriteHost). If the repo still uses PSScriptAnalyzer anywhere, either keep this file or update the referencing tooling to use an alternative settings source and document the new lint configuration.
.agents/SESSION-END-PROMPT.md:1 - This deletes the session start/end prompt documents that appear to encode key workflow/protocol steps. If these are being deprecated, there should be a clear replacement location referenced from the remaining canonical docs (so contributors/agents can still find the start/end procedures). Otherwise, removing them risks making the session protocol harder to follow and could increase protocol-check failures.
.agents/HANDOFF.md:1 .agents/HANDOFF.mdis described (in its own content) as a read-only dashboard and a required read at session start. Removing it is a behavioral change for any automation/humans that follow that protocol. If the handoff mechanism has moved elsewhere, consider adding a stub file that points to the new source of truth (or updating the protocol docs in the same PR) to avoid breaking existing workflows.
There was a problem hiding this comment.
Code Review
This pull request deletes the .actrc and .PSScriptAnalyzerSettings.psd1 configuration files. These changes violate the repository style guide by removing mandatory local workflow testing configurations and disabling critical security linting rules designed to prevent command injection vulnerabilities.
I am having trouble creating individual review comments. Click here to see my feedback.
.actrc (1-23)
Deleting the .actrc configuration file violates the mandatory local workflow testing requirement specified in the Repository Style Guide (Line 133). This file is essential for ensuring that local gh act runs use the correct runner images to maintain parity with the production environment.
References
- Local Workflow Testing (MANDATORY): Before pushing any workflow YAML changes, run gh act locally to validate. (link)
.PSScriptAnalyzerSettings.psd1 (1-47)
Removing this file disables the automated enforcement of security patterns required by the Repository Style Guide (Line 26). Specifically, it removes the PSAvoidUsingInvokeExpression check, which is a critical control for preventing Command Injection (CWE-78) as outlined in the security patterns (Line 44).
References
- Security Patterns (BLOCKING): Command Injection Prevention (CWE-78). (link)
- To avoid code injection vulnerabilities, do not use Invoke-Expression to extract variable values from script files in tests. Instead, use safer methods like parsing the script's Abstract Syntax Tree (AST) or using regular expressions to extract the definition as a string.
- To prevent command injection vulnerabilities (CWE-78) in PowerShell, enclose variables passed as arguments to external commands in double quotes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (300)
💤 Files with no reviewable changes (49)
📝 WalkthroughWalkthroughThis PR removes the entire Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
…at-constraints # Conflicts: # .github/workflows/publish.yml # packages/ai-agents-cli/bun.lock # packages/ai-agents-cli/package.json # pyproject.toml
Review Triage RequiredNote Priority: NORMAL - Human approval required before bot responds Review Summary
Next Steps
Powered by PR Maintenance workflow - Add triage:approved label |
1 similar comment
Review Triage RequiredNote Priority: NORMAL - Human approval required before bot responds Review Summary
Next Steps
Powered by PR Maintenance workflow - Add triage:approved label |
Pull request was closed
Summary
Adds missing output format constraints, iteration limits, and completion criteria to agentic commands that previously allowed unbounded operations.
Changes
pr-review-config.yaml
invocation_limitssection:all_open_max_prs: 5,completion_gate_max_retries: 3with escalation actionsoutput_constraintssection: per-PR token cap (4096), summary format and required columnspr-review.md
all-openmode now capped toinvocation_limits.all_open_max_prswith overflow reportingoutput_constraints.per_pr_max_response_tokensoutput_constraints.summary_formatandsummary_required_columnsinvocation_limits.completion_gate_max_retrieswith escalation on exhaustioncontext-hub-setup.md
## Completion Criteriasection with verifiable checklist (Forgetful MCP, Serena, Context7, status table)### Stop Conditiondefining when the command is doneOut of scope
workflow/0-init.mdandworkflow/2-impl.mddo not exist (removed in PR #1611). No action needed.Addresses #1668