Skip to content

Commit 5aac8b0

Browse files
Merge branch 'main' into copilot/fix-dependabot-fileupdater-error
2 parents bfd9f8e + b0c2af4 commit 5aac8b0

File tree

10 files changed

+711
-325
lines changed

10 files changed

+711
-325
lines changed

.github/copilot-instructions.md

Lines changed: 18 additions & 321 deletions
Large diffs are not rendered by default.
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
applyTo:
3+
- "**/file_fetcher.rb"
4+
- "**/file_fetcher/**"
5+
- "**/file_parser.rb"
6+
- "**/file_parser/**"
7+
- "**/file_updater.rb"
8+
- "**/file_updater/**"
9+
- "**/update_checker.rb"
10+
- "**/update_checker/**"
11+
- "**/metadata_finder.rb"
12+
- "**/metadata_finder/**"
13+
- "**/version.rb"
14+
- "**/requirement.rb"
15+
---
16+
17+
# Core Class Structure Pattern
18+
19+
**CRITICAL**: All Dependabot core classes with nested helper classes must follow the exact pattern to avoid "superclass mismatch" errors. This pattern is used consistently across all established ecosystems (bundler, npm_and_yarn, go_modules, etc.).
20+
21+
## Main Class Structure
22+
23+
Applies to FileFetcher, FileParser, FileUpdater, UpdateChecker, etc.
24+
25+
```ruby
26+
# {ecosystem}/lib/dependabot/{ecosystem}/file_updater.rb (or file_fetcher.rb, file_parser.rb, etc.)
27+
require "dependabot/file_updaters"
28+
require "dependabot/file_updaters/base"
29+
30+
module Dependabot
31+
module {Ecosystem}
32+
class FileUpdater < Dependabot::FileUpdaters::Base
33+
# require_relative statements go INSIDE the class
34+
require_relative "file_updater/helper_class"
35+
36+
# Main logic here...
37+
end
38+
end
39+
end
40+
41+
Dependabot::FileUpdaters.register("{ecosystem}", Dependabot::{Ecosystem}::FileUpdater)
42+
```
43+
44+
## Helper Class Structure
45+
46+
```ruby
47+
# {ecosystem}/lib/dependabot/{ecosystem}/file_updater/helper_class.rb
48+
require "dependabot/{ecosystem}/file_updater"
49+
50+
module Dependabot
51+
module {Ecosystem}
52+
class FileUpdater < Dependabot::FileUpdaters::Base
53+
class HelperClass
54+
# Helper logic nested INSIDE the main class
55+
end
56+
end
57+
end
58+
end
59+
```
60+
61+
## Key Rules
62+
63+
1. **Main classes** inherit from appropriate base: `Dependabot::FileUpdaters::Base`, `Dependabot::FileFetchers::Base`, etc.
64+
2. **Helper classes** are nested inside the main class.
65+
3. **`require_relative`** statements go INSIDE the main class, not at module level.
66+
4. **Helper classes require the main file** first: `require "dependabot/{ecosystem}/file_updater"`.
67+
5. **Never define multiple top-level classes** with the same name in the same namespace.
68+
6. **Backward compatibility** can use static methods that delegate to instance methods.
69+
70+
## Applies To
71+
72+
- **FileFetcher** and its helpers (e.g., `FileFetcher::GitCommitChecker`)
73+
- **FileParser** and its helpers (e.g., `FileParser::ManifestParser`)
74+
- **FileUpdater** and its helpers (e.g., `FileUpdater::LockfileUpdater`)
75+
- **UpdateChecker** and its helpers (e.g., `UpdateChecker::VersionResolver`)
76+
- **MetadataFinder** and its helpers
77+
- **Version** and **Requirement** classes (if they have nested classes)
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
---
2+
applyTo: "**/*.rb"
3+
---
4+
5+
# Code Quality Guidelines
6+
7+
## Pre-Commit Checklist
8+
9+
**Do NOT commit or push changes until ALL of the following pass inside a Docker container:**
10+
11+
1. `rubocop` — Passes with no offenses (use `rubocop -A` to auto-fix)
12+
2. `bundle exec srb tc` — Sorbet type checking passes (for production code)
13+
3. `rspec spec/path/to/relevant_spec.rb` — Tests covering your changes pass
14+
15+
Run these via `bin/test`:
16+
17+
- `bin/test {ecosystem} rubocop`
18+
- `bin/test {ecosystem} spec/path/to/spec.rb`
19+
20+
For Sorbet, run from the repo root inside the container:
21+
22+
- `bin/docker-dev-shell {ecosystem}` then `cd /home/dependabot && bundle exec srb tc`
23+
24+
When delegating work to sub-agents, each agent must validate its own changes before committing.
25+
26+
## RuboCop Best Practices
27+
28+
Avoid adding RuboCop exceptions unless absolutely necessary. Resolve offenses using proper coding practices:
29+
30+
- **Method extraction** — Break large methods into smaller, focused methods
31+
- **Class extraction** — Split large classes into single-responsibility classes
32+
- **Reduce complexity** — Simplify conditional logic and nested structures
33+
- **Improve naming** — Use clear, descriptive variable and method names
34+
- **Refactor long parameter lists** — Use parameter objects or configuration classes
35+
- **Extract constants** — Move magic numbers and strings to named constants
36+
37+
If a RuboCop exception is truly unavoidable, provide clear justification in a comment explaining why the rule cannot be followed and what alternatives were considered.
38+
39+
## Sorbet Type Checking
40+
41+
- All new files must use `# typed: strict` at minimum
42+
- Existing files below `strict`: upgrade to `strict` when making changes
43+
- Add explicit type signatures for method parameters and return values
44+
- Always validate with `bundle exec srb tc`
45+
46+
### Autocorrect Usage
47+
48+
Use `bundle exec srb tc -a` **cautiously** — it often creates incorrect fixes for complex cases.
49+
50+
Autocorrect is acceptable for simple cases:
51+
52+
- Missing `override.` annotations
53+
- `T.let` declarations for instance variables
54+
- Constant type annotations
55+
56+
Always manually resolve complex type mismatches, method signature issues, and structural problems. Review any autocorrected changes carefully.
57+
58+
## Code Comments
59+
60+
Prioritize self-documenting code over comments. Prefer extracting well-named methods over adding explanatory comments.
61+
62+
### DO Comment
63+
64+
- **Business logic context** — Explain *why* something is done when not obvious
65+
- **Complex algorithms** — Document the approach or concepts
66+
- **Workarounds** — Explain why a non-obvious solution was necessary
67+
- **External constraints** — API limitations, ecosystem-specific behaviors
68+
- **TODOs** — With issue references when possible
69+
70+
### DON'T Comment
71+
72+
- **Implementation decisions** — Don't explain what was *not* implemented
73+
- **Obvious code** — Don't restate what the code clearly does
74+
- **Apologies or justifications** — Suggests code quality issues
75+
- **Outdated information** — Remove comments that no longer apply
76+
- **Version history** — Use git history instead
77+
78+
### Example
79+
80+
```ruby
81+
# Good: Explains WHY
82+
# Retry up to 3 times due to GitHub API rate limiting
83+
retry_count = 3
84+
85+
# Bad: Explains WHAT (obvious from code)
86+
# Set retry count to 3
87+
retry_count = 3
88+
89+
# Good: Documents external constraint
90+
# GitHub API requires User-Agent header or returns 403
91+
headers["User-Agent"] = "Dependabot/1.0"
92+
```
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
---
2+
applyTo:
3+
- "**/*_spec.rb"
4+
- "**/spec/**"
5+
- "**/spec_helper.rb"
6+
---
7+
8+
# Testing Guidelines
9+
10+
## Docker-Based Testing
11+
12+
All tests must run inside Docker containers. The development environment, dependencies, and native helpers are containerized and will not work on the host system. Never attempt to run tests directly on your machine.
13+
14+
## Quick Testing with `bin/test`
15+
16+
The `bin/test` script is the fastest way to run tests. It handles container setup automatically.
17+
18+
```bash
19+
# Run a specific spec file
20+
bin/test {ecosystem} spec/dependabot/{ecosystem}/file_updater_spec.rb
21+
22+
# Run the full ecosystem test suite
23+
bin/test {ecosystem}
24+
25+
# Test common/ changes (uses the bundler container internally)
26+
bin/test common spec/dependabot/file_fetchers/base_spec.rb
27+
28+
# Test updater/ changes (note: use --workdir updater, it maps to dependabot-updater in the container)
29+
bin/test --workdir updater {ecosystem} rspec spec/path/to/spec.rb
30+
```
31+
32+
The first run builds the Docker image and can take several minutes. Subsequent runs reuse the cached image and are much faster.
33+
34+
## Interactive Development with `bin/docker-dev-shell`
35+
36+
For iterative development where you need to run multiple commands, use the interactive shell:
37+
38+
```bash
39+
# Start an interactive container shell
40+
bin/docker-dev-shell {ecosystem}
41+
42+
# Inside the container:
43+
cd {ecosystem} && rspec spec
44+
45+
# For updater tests (the folder is renamed inside containers):
46+
cd dependabot-updater && rspec spec
47+
```
48+
49+
## GitHub Actions / CI (Non-Interactive)
50+
51+
In CI environments where interactive shells are unavailable, use the non-interactive pattern:
52+
53+
```bash
54+
# Build the ecosystem image
55+
script/build {ecosystem}
56+
57+
# Run tests inside the container
58+
docker run --rm --env "CI=true" ghcr.io/dependabot/dependabot-updater-{ecosystem} bash -c \
59+
"cd /home/dependabot/{ecosystem} && rspec spec"
60+
```
61+
62+
## Testing `updater/` and `common/` Changes
63+
64+
The `updater/` directory is renamed to `dependabot-updater/` inside containers. Always use the container path when running updater tests.
65+
66+
```bash
67+
# Test updater changes
68+
bin/test --workdir updater {ecosystem} rspec spec/path/to/spec.rb
69+
70+
# Test common changes (runs inside the bundler container)
71+
bin/test common spec/path/to/spec.rb
72+
```
73+
74+
## Test Coverage Requirements
75+
76+
- All changes must be covered by tests to prevent regressions.
77+
- Add tests for new functionality **before** implementing the feature.
78+
- When fixing bugs, add a test that reproduces the issue **first**.
79+
- All existing tests must continue to pass after your changes.
80+
81+
### Rules for Test Design
82+
83+
- **NEVER** test private methods directly — tests should only exercise public interfaces.
84+
- **NEVER** modify production code visibility to accommodate tests. If a test needs access to a private method, the test design is wrong.
85+
- **NEVER** add public methods solely for testing — this pollutes the production API.
86+
- Tests should verify behavior through public APIs and production code paths (e.g., `fetch_files`), not isolated helper methods.
87+
88+
## Fixture-Based Testing
89+
90+
Use real dependency files as fixtures. Helper methods in `spec_helper.rb` generate realistic test data:
91+
92+
```ruby
93+
let(:dependency_files) { bundler_project_dependency_files("example_project") }
94+
```
95+
96+
Fixtures live in `{ecosystem}/spec/fixtures/`.
97+
98+
## Mocking External Calls
99+
100+
- Mock HTTP requests to package registries using **VCR** or **WebMock**.
101+
- Use realistic registry responses to catch edge cases.
102+
- Never make real network calls in tests.
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
applyTo: "updater/**"
3+
---
4+
5+
# Updater & Native Helpers
6+
7+
## Container Path Mapping
8+
9+
The `updater/` directory on the host is mounted as `dependabot-updater/` inside containers. Always use `dependabot-updater` when referencing paths in container commands.
10+
11+
## Running Updater Tests
12+
13+
```bash
14+
# Quick one-off test run:
15+
bin/test --workdir updater {ecosystem} rspec spec/path/to/spec.rb
16+
17+
# Interactive development:
18+
bin/docker-dev-shell {ecosystem}
19+
# Then inside the container:
20+
cd dependabot-updater && rspec spec
21+
```
22+
23+
## Native Helpers
24+
25+
- Located in `{ecosystem}/helpers/`
26+
- Run exclusively within containers — they will not work on the host
27+
- Rebuild after changes: `{ecosystem}/helpers/build`
28+
- Changes are **not** automatically reflected in the container — you must rebuild
29+
30+
## Debugging
31+
32+
All debugging commands must be run inside the dev container. Start one first:
33+
34+
```bash
35+
bin/docker-dev-shell {ecosystem}
36+
```
37+
38+
Then use `bin/dry-run.rb` to test against real repositories:
39+
40+
```bash
41+
# Enable helper debug output
42+
DEBUG_HELPERS=true bin/dry-run.rb {ecosystem} {repo}
43+
44+
# Debug a specific native helper function
45+
DEBUG_FUNCTION=function_name bin/dry-run.rb {ecosystem} {repo}
46+
47+
# Profile performance
48+
bin/dry-run.rb {ecosystem} {repo} --profile
49+
```

updater/lib/dependabot/updater/group_update_creation.rb

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,34 @@ def cleanup_workspace
534534

535535
sig { params(group: Dependabot::DependencyGroup).returns(T::Boolean) }
536536
def pr_exists_for_dependency_group?(group)
537-
job.existing_group_pull_requests.any? { |pr| pr["dependency-group-name"] == group.name }
537+
!find_existing_group_pr(group).nil?
538+
end
539+
540+
sig { params(group: Dependabot::DependencyGroup).returns(T.nilable(T::Hash[String, T.untyped])) }
541+
def find_existing_group_pr(group)
542+
job.existing_group_pull_requests.find do |pr|
543+
next false unless pr["dependency-group-name"] == group.name
544+
545+
existing_pr_covers_job_directories?(pr)
546+
end
547+
end
548+
549+
sig { params(pull_request: T::Hash[String, T.untyped]).returns(T::Boolean) }
550+
def existing_pr_covers_job_directories?(pull_request)
551+
dependencies = pull_request["dependencies"]
552+
553+
# Old PRs without directory info — treat as match (backward compat).
554+
# Only enforce directory matching when ALL dependencies include a directory,
555+
# consistent with DependencyChange#matches_existing_pr? and PullRequest#using_directory?.
556+
return true if dependencies.nil? || !dependencies.all? { |dep| dep["directory"] }
557+
558+
pr_directories = dependencies.filter_map { |dep| dep["directory"] }
559+
job_directories = job.source.directories || [job.source.directory || "/"]
560+
normalized_job_dirs = job_directories.map { |d| Pathname.new(d).cleanpath.to_s }.uniq
561+
normalized_pr_dirs = pr_directories.map { |d| Pathname.new(d).cleanpath.to_s }.uniq
562+
563+
# Match only when the PR directories exactly match the job directories
564+
normalized_job_dirs.sort == normalized_pr_dirs.sort
538565
end
539566

540567
sig do

updater/lib/dependabot/updater/operations/group_update_all_versions.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ def run_grouped_dependency_updates
8585
# Preprocess to discover existing group PRs and add their dependencies to the handled list before processing
8686
# the rest of the groups. This prevents multiple PRs from being created for the same dependency.
8787
groups_without_pr = dependency_snapshot.groups.filter_map do |group|
88-
if pr_exists_for_dependency_group?(group)
89-
existing_pr = job.existing_group_pull_requests.find { |pr| pr["dependency-group-name"] == group.name }
90-
pr_number = existing_pr["pr_number"] if existing_pr
88+
existing_pr = find_existing_group_pr(group)
89+
if existing_pr
90+
pr_number = existing_pr["pr_number"]
9191

9292
Dependabot.logger.info(
9393
"Detected existing pull request ##{pr_number} for the dependency group '#{group.name}'."

0 commit comments

Comments
 (0)