Skip to content

Add scanned_manifests_path metadata to snapshots #14406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brrygrdn merged 2 commits into from
Mar 12, 2026
Merged

Add scanned_manifests_path metadata to snapshots #14406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
846b944
Add scanned_manifests_path metadata to snapshots
brrygrdn Mar 10, 2026
0c3042b
Switch to using a string representation
brrygrdn Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion updater/lib/github_api/dependency_submission.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,17 @@ def payload
url: SNAPSHOT_DETECTOR_URL
},
manifests: manifests,
# TODO: Move use of metadata to a Dependabot-specific object
#
# We are using the existing job metadata as a bag-of-values for error handling
# and job tracking that is specific to Dependabot-created submissions.
#
# In future, we should extend the public API schema with a validated object to
# harden this contract.
Comment on lines +111 to +117
Copy link
Copy Markdown
Contributor Author

@brrygrdn brrygrdn Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are tracking this internally, I'm just adding a note here to make sure we keep line of sight that using metadata as a bag of values is something we will need to migrate off of in future.

The values we store here are all of short-term relevant, e.g. they only matter to the actual POST request that submits the snapshot or for state reporting when this snapshot is on the tip of a branch so we don't need to worry about backfilling or handling historical values.

metadata: {
status: status.serialize,
reason: reason
reason: reason,
scanned_manifest_path: scanned_manifest_path
}.compact
}
end
Expand Down Expand Up @@ -179,5 +187,16 @@ def manifests
}
}
end

# Returns a synopsis of the scan performed in the format `ecosystem::manifest_path`, e.g.
# - `golang::/`
# - `rubygems::rails_app/`
#
sig do
returns(String)
end
def scanned_manifest_path
"#{GithubApi::EcosystemMapper.ecosystem_for(package_manager)}::#{File.dirname(manifest_file.path)}"
end
Comment on lines +191 to +200
Copy link
Copy Markdown
Contributor Author

@brrygrdn brrygrdn Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 We could use the job_correlator for this but it has some handling to ensure we deterministically hash long paths that I don't want to undo as it would change how historical snapshots are compared using these keys.

Rather than overload the responsibility of this value, which is firmly in the snapshot domain, with the requirement to track the job 'receipt' of a snapshot, let's keep these isolated.

end
end
4 changes: 4 additions & 0 deletions updater/spec/dependabot/update_graph_processor_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@
# It should contain the expected metadata
expect(payload[:metadata][:status]).to eql(GithubApi::DependencySubmission::SnapshotStatus::FAILED.serialize)
expect(payload[:metadata][:reason]).to eql("dependency_file_not_evaluatable")
expect(payload[:metadata][:scanned_manifest_path]).to eql("rubygems::/")
end

it "correctly snapshots the second directory" do
Expand Down Expand Up @@ -418,6 +419,7 @@
# We should have metadata indicating a successful snapshot
expect(payload[:metadata][:status]).to eql(GithubApi::DependencySubmission::SnapshotStatus::SUCCESS.serialize)
expect(payload[:metadata][:reason]).to be_nil
expect(payload[:metadata][:scanned_manifest_path]).to eql("rubygems::/subproject")
end
end
end
Expand Down Expand Up @@ -557,6 +559,7 @@
# It should contain the expected metadata
expect(payload[:metadata][:status]).to eq(GithubApi::DependencySubmission::SnapshotStatus::SKIPPED.serialize)
expect(payload[:metadata][:reason]).to eq(GithubApi::DependencySubmission::EMPTY_REASON_NO_MANIFESTS)
expect(payload[:metadata][:scanned_manifest_path]).to eql("rubygems::/")
end

update_graph_processor.run
Expand Down Expand Up @@ -665,6 +668,7 @@ def fetch_subdependencies(_dependency)
# We should have metadata indicating a successful snapshot
expect(payload[:metadata][:status]).to eql(GithubApi::DependencySubmission::SnapshotStatus::DEGRADED.serialize)
expect(payload[:metadata][:reason]).to eql(GithubApi::DependencySubmission::DEGRADED_REASON_SUBDEPENDENCY_ERR)
expect(payload[:metadata][:scanned_manifest_path]).to eql("rubygems::/")
end

expect(service).to receive(:record_update_job_warning) do |args|
Expand Down
5 changes: 5 additions & 0 deletions updater/spec/github_api/dependency_submission_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@
expect(payload[:detector][:version]).to eq(Dependabot::VERSION)
expect(payload[:job][:correlator]).to eq("dependabot-bundler")
expect(payload[:job][:id]).to eq("9999")

# Check dependabot-specific metadata keys
expect(payload[:metadata][:status]).to eql("ok")
expect(payload[:metadata][:reason]).to be_nil
expect(payload[:metadata][:scanned_manifest_path]).to eql("rubygems::/")
end

it "affixes to use the updater sha if available" do
Expand Down
Loading