run biome check --linter-enabled=false on CI to check unsorted imports too#892
run biome check --linter-enabled=false on CI to check unsorted imports too#892robert-inkeep merged 6 commits intomainfrom
biome check --linter-enabled=false on CI to check unsorted imports too#892Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @dimaMachina's task —— View job Code Review CompleteTodo List
Overall AssessmentThis PR introduces a focused import organization check to the CI pipeline by adding Detailed Analysis✅ What Works Well1. Minimal, Surgical Changes: The PR makes only the necessary changes - adding the script and CI step without touching unrelated code. 2. Consistent with Existing Patterns: The approach follows existing biome integration patterns already established in the codebase. 3. Strategic Use of 🔍 Areas for Consideration1. Missing Context & Documentation
2. Potential Redundancy with Existing Checks
3. CI Pipeline Positioning
4. Error Handling & Feedback
🏗️ System Architecture Implications1. Tooling Strategy: This continues the pattern of using Biome as the primary formatting/linting tool, which is consistent with the existing architecture. 2. CI Performance: Adding a sequential step rather than parallelizing with Turborepo may impact CI times, though the impact should be minimal for this check. 3. Developer Experience: The 📝 Specific Recommendations1. Add PR Description (Critical) 2. Consider CI Integration Options # Option A: Add to existing parallel checks in turbo.json
# Option B: Add conditional logic to CI summary
# Option C: Document why sequential execution is preferred3. Add Inline Documentation - name: Biome Check (Import Organization)
run: pnpm biome:check
# Explain: Checks import organization without running linter rules
# This complements format:check by catching import sorting issues🎯 Questions for Clarification
🔒 Security & Best Practices
VerdictThis is a well-executed, minimal change that addresses a specific need. The main gap is missing context - once the PR description explains the "why" and "what problems this solves," this would be ready to merge. The change follows good engineering practices: small scope, consistent patterns, and provides local reproducibility. Consider the CI integration suggestions for optimal developer experience. |
|
@claude format:check to me sounds like it shouldn’t ever write |
|
Claude finished @robert-inkeep's task —— View job Clarification on
|
| Script | Modifies Files? | Purpose |
|---|---|---|
format |
✅ Yes | Fixes formatting AND import organization |
format:check |
❌ No | Only checks for issues without fixing |
What This PR Does
The PR adds import organization checking to both scripts:
formatnow also fixes import organization (with--write)format:checknow also checks import organization (without--write)
The --linter-enabled=false flag ensures only the "assist" features (like organizeImports) run, without running the full linter rules.
Conclusion
The naming is appropriate - format:check remains read-only as it should be. Both biome format and biome check --linter-enabled=false without the --write flag only report issues without modifying any files.
biome check --linter-enabled=false on CI to check unsorted imports for examplebiome check --linter-enabled=false on CI to check unsorted imports too
Fixes from Claude Code review: 1. Add merge validation for workflow_dispatch (Major) - Prevents analyzing unmerged PRs via manual trigger - Validates PR is merged before proceeding 2. Use unique HEREDOC delimiters (Major) - Prevents collision if PR body/comments contain "EOF" - Uses unique suffixes like __BODY_DELIM_7f3a9b2c__ 3. Pin claude-code-action to SHA (Major) - Aligns with claude-code-review.yml for consistency - Tracks issue #892 for AJV validation bug 4. Add concurrency control (Minor) - Prevents race conditions on concurrent runs - Groups by PR number, doesn't cancel in-progress 5. Add shell error handling (Minor) - set -eo pipefail in all shell blocks - Fail fast on command errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* feat: Add closed-pr-review-auto-improver agent Automated system that analyzes human reviewer feedback after PRs are merged to identify generalizable improvements for the pr-review-* subagent system. - Workflow triggers on merged PRs, extracts human/bot comments - Agent applies 4-criteria generalizability test - Creates draft PRs with improvements to pr-review-*.md files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Add context-gathering phase for deeper comment analysis - Include diffHunk in GraphQL query (shows code each comment is on) - Add Phase 2 "Deep-Dive on Promising Comments" with explicit guidance: - Read the full file to understand broader context - Grep for schemas/types/patterns mentioned in comments - Understand the anti-pattern before judging generalizability - Update Tool Policy to emphasize context gathering - Renumber phases (now 6 phases total) The agent now actively investigates each comment rather than judging based on comment text alone. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Apply write-agent best practices Based on write-agent skill guidance: 1. Add near-miss example (questions/discussions ≠ reviewer feedback) 2. Strengthen Role & Mission - describe what "excellence looks like" 3. Failure modes now use contrastive examples (❌ vs ✅) 4. Phase 2 now checklist format with stop condition 5. Example shows completed checklist, not just steps Key insight: "Stop here if you can't articulate a clear principle" prevents vague improvements from polluting reviewers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Add git time-travel for progressive context gathering - Phase 2 now uses git rev-list + git show to see code at comment time - Progressive gathering: diffHunk → full file → PR diff → other files - GraphQL query now includes createdAt for all comment types - Added git rev-list and git show to allowedTools This ensures the agent sees what the human reviewer saw, not the final merged state which may have fixes applied. Co-Authored-By: Claude <noreply@anthropic.com> * feat: Add explicit stop conditions for context gathering Two exit paths at each level: - EXIT A: Not generalizable (repo-specific, one-off bug, style preference) - EXIT B: Pattern found (can articulate anti-pattern + universal principle) Includes decision flow diagram and two contrasting examples showing early exit (repo-specific DateUtils) vs pattern discovery (type/schema DRY). Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Strengthen agent per write-agent best practices - Role & Mission: Add "what the best human analyst would do" section - Failure modes: Add "Asserting when uncertain" with contrastive example - Generalizability: Add confidence calibration guidance - Add explicit conservative default: "when torn, choose lower confidence" Per write-agent skill review: personality should describe best human behavior, failure modes should include asserting when uncertain (relevant for classification tasks). Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Add Schema-Type Derivation Discipline learnings Pattern extracted from PR #1737 human reviewer feedback (amikofalvy): - Types should derive from Zod schemas using z.infer<typeof schema> - Use Pick/Omit/Partial instead of manually redefining type subsets - Extract shared enum/union schemas instead of inline string literals Changes: - pr-review-types.md: New anti-pattern + analysis step 6 with detection patterns - pr-review-consistency.md: Extended "Reuse" section to cover types This demonstrates the closed-pr-review-auto-improver output — these are the exact changes the agent proposed when run against PR #1737. Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Expand type derivation sources Extended "Schema-Type Derivation Discipline" to cover full spectrum: - Zod/validation schemas (z.infer) - Database schemas (Prisma, Drizzle generated types) - Internal packages (@inkeep/*, shared types) - External packages/SDKs (OpenAI, Vercel AI SDK) - Function signatures (Parameters<>, ReturnType<>) - Existing domain types (Pick, Omit, Partial) Added table format for clarity and comprehensive detection patterns. Co-Authored-By: Claude <noreply@anthropic.com> * pr-review: Add advanced type derivation patterns from codebase research Expanded type derivation guidance based on actual patterns found in agents repo: - Awaited<ReturnType<>> for async function returns - keyof typeof for constants-derived types - interface extends and intersection (&) for composition - Discriminated unions with type guards - satisfies operator for type-safe constants - Re-exports for API surface boundaries - Type duplication detection signals Patterns sourced from agents-api codebase analysis including: - env.ts, middleware/*, types/app.ts, domains/run/* Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * pr-review: Add Zod schema composition patterns Added guidance for Zod schema extension/derivation patterns based on codebase research (packages/agents-core/src/validation/schemas.ts): - .extend() for adding/overriding fields - .pick()/.omit() for field subsetting - .partial() for Insert → Update schema derivation - .extend().refine() for cross-field validation - Anti-patterns: parallel schemas, duplicated fields Examples from codebase: - SubAgentInsertSchema.extend({ id: ResourceIdSchema }) - SubAgentUpdateSchema = SubAgentInsertSchema.partial() - StopWhenSchema.pick({ transferCountIs: true }) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * pr-review: Rationalize scope between types and consistency agents Clear separation of concerns: - pr-review-types: Illegal states, invariants, unsafe narrowing - pr-review-consistency: DRY, schema reuse, convention conformance Moved to consistency: - Zod schema composition patterns (.extend, .pick, .partial) - Type derivation detection signals - satisfies operator, re-exports conventions Kept in types (type safety focus): - Discriminated unions vs optional fields (prevents illegal states) - Type guards vs unsafe `as` assertions - Detection of union types without discriminants Added cross-reference note in types agent pointing to consistency for derivation/DRY concerns. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * closed-pr-review-auto-improver: Add exit states, skills integration, and Phase 5.5 - Add skills: pr-review-subagents-available, pr-review-subagents-guidelines, find-similar-patterns - Add proper exit states at Phase 1, 2, and 4 (embedded in workflow, not separate section) - Add Phase 5 step 2: "Find examples of the pattern" with judgment guidance - Add Phase 5.5: Full file review & integration planning (scope fit, duplication check) - Update output contract with detailed JSON structure and exit examples - Add reviewer tagging to close the feedback loop Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * closed-pr-review-auto-improver: Add "keep agents standalone" guidance Agents should be self-contained without cross-references to other agents. This prevents coupling and ensures agents work correctly when read in isolation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Recover lost skills: find-similar-patterns, pr-review-subagents-* These skills were created in the previous session but never committed. Recovered from conversation history. - find-similar-patterns: Methodology for finding similar code patterns - pr-review-subagents-available: Catalog of pr-review-* agents with scope boundaries - pr-review-subagents-guidelines: Best practices for writing/improving reviewers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove pr-review-* changes (moved to separate PR #1759) The pr-review-consistency.md and pr-review-types.md improvements belong in PR #1759, not this auto-improver feature branch. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Move auto-improver to private plugin repo Move agent and skills to inkeep/internal-cc-plugins for CI/CD-only loading: - Removed: .claude/agents/closed-pr-review-auto-improver.md - Removed: .agents/skills/{find-similar-patterns,pr-review-subagents-available,pr-review-subagents-guidelines}/ Updated workflow: - Added step to clone inkeep/internal-cc-plugins - Added --plugin-dir flag to load agent from plugin Prerequisites before merging: 1. Create private repo: inkeep/internal-cc-plugins 2. Push plugin content to new repo 3. Add GH_PAT_PLUGINS secret to inkeep/agents Co-Authored-By: Claude <noreply@anthropic.com> * Switch from PAT to GitHub App for cross-repo auth GitHub Apps provide better security and maintainability: - 8-hour token lifetime (vs days/infinite for PATs) - No user account dependency (survives personnel changes) - Zero manual rotation (tokens generated fresh each run) - Scales to N plugins without additional credentials Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add workflow_dispatch trigger for testing historical PRs - Add manual trigger with pr_number input - Add Get PR Metadata step to fetch data via API (works for both triggers) - Update all PR references to use the new metadata outputs - Enables testing against historical PRs like #1737 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review feedback: robustness improvements Fixes from Claude Code review: 1. Add merge validation for workflow_dispatch (Major) - Prevents analyzing unmerged PRs via manual trigger - Validates PR is merged before proceeding 2. Use unique HEREDOC delimiters (Major) - Prevents collision if PR body/comments contain "EOF" - Uses unique suffixes like __BODY_DELIM_7f3a9b2c__ 3. Pin claude-code-action to SHA (Major) - Aligns with claude-code-review.yml for consistency - Tracks issue #892 for AJV validation bug 4. Add concurrency control (Minor) - Prevents race conditions on concurrent runs - Groups by PR number, doesn't cancel in-progress 5. Add shell error handling (Minor) - set -eo pipefail in all shell blocks - Fail fast on command errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add debug artifact upload for troubleshooting Uploads execution logs when workflow fails, matching pattern from claude-code-review.yml. 7-day retention. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
No description provided.