# Reviewing what the agent pulled in

> A coding agent adds packages fast and with confidence. Review each one in proportion to its risk, with the OpenSSF Scorecard checks as the security step and a written skip list.

Page: https://schubergphilis.github.io/ai-training/coding-with-agents/dependency-review/
License: CC BY-SA 4.0, https://creativecommons.org/licenses/by-sa/4.0/

An agent spent an afternoon on a feature and the pull request is green.
Somewhere in the diff the lockfile grew, because the agent added a package
for date formatting and a forty-line one for zero-padding numbers, the
first of which brought in a third it never mentioned, and it bumped the
router the project already used. The tests pass. In this lesson we decide what to do
with those four changes. We work out which questions each one gets and how
much time it deserves, and we end with a written verdict that the next
person can reuse.

The method here follows the public review guideline for external code that
the Fuchsia project publishes (Review process for external Rust crates, Fuchsia documentation). It adds a separate
governance dimension and uses the OpenSSF Scorecard checks as the security
step. The Fuchsia guideline covers a large codebase with many reviewers.
Yours covers one pull request, and the agent that wrote it adds another
package tomorrow if you let it.

## The lockfile knows what changed

Start from the lockfile. The summary says "added `date-fmt` for the due
dates", and the lockfile says what was installed. In npm,
`package-lock.json` records every package in the installed tree at its
exact version, so a later install can build the same tree again, and its
root entry lists the direct dependencies (The npm package lock file, npm CLI documentation). Everything
else under `node_modules/` is transitive: a package your code never
imports, pulled in by one that it does.

The fixture for this lesson is a pair of lockfiles under
`site/examples/coding-with-agents/dependency-review/fixture/` with
invented package names, so nothing here is a claim about a real project.
`classify.py` diffs them and labels every change with one of three kinds.

#### Checkpoint: Predict the classification

The root entry of `package-lock.after.json` lists `date-fmt`,
`microroute-js` and `zero-pad-left-lite`. `week-of-year-iso` appears only
as a dependency of `date-fmt`, and `microroute-js` moved from 4.19.2 to
4.21.2. What does the script print? Type all four lines in the format
`name version: kind`.

```python
for name, version, kind in classify(before, after):
    print(f"{name} {version}: {kind}")
```

Each kind gets a different review. A new direct dependency is an
architecture decision, while somebody upstream decided the transitive one
for you, and a version bump changes code you already accepted once.

The agent can draw up the list for you. Start a new session, one that
never saw the feature being built, and ask it to name every package the
lockfile diff added or bumped and what each one is for. A session that
didn't make the choices has nothing to defend, so it describes the diff as
it is (Best practices for Claude Code, Claude Code documentation). The list is where the review starts,
and you write the verdict at the end.

## Principles that keep the review affordable

A full review of every package would take longer than writing the feature.
The guideline this method follows starts with principles that decide where
the time goes.

**Effort in proportion to risk.** Find the biggest risks the external code
brings in, and spend the review there. Compare with the risk of writing the
same thing yourselves. A date formatter on an internal dashboard and a
token parser on the login path are different reviews, even at the same
line count.

**Count the future cost.** A convenient package today is an upgrade,
a security advisory, and a maintainer question next year. When a dependency
solves a general class of problems well, it repays its upkeep every time you
meet another problem in that class. One that saves you eleven lines does
not.

**Be wary of surprises.** Code or an API that confuses you at review time
confuses every reader after you. Confusion is a cost you pay many times.

**No default outcome.** The same principles govern "use the library" and
"write it ourselves". Writing it yourself is also a decision that needs the
same questions, because your own code needs tests and a maintainer too.

**Give back, and share.** Report real findings upstream for the parts you
use. Write the assessment down where the next team can find it, so that
nobody reviews the same package twice.

## Six dimensions, and which of them apply

The review has six dimensions. A change of each kind gets a different
subset.

| Change                    | Architecture | Quality | Governance and support | Security | Code | License            |
| ------------------------- | ------------ | ------- | ---------------------- | -------- | ---- | ------------------ |
| New direct dependency     | yes          | yes     | yes                    | yes      | yes  | yes                |
| New transitive dependency | no           | yes     | yes                    | yes      | yes  | yes                |
| Version bump              | no           | no      | no                     | yes      | yes  | if license changed |

Architecture applies only to a direct dependency, because that's the one
you chose. You can't answer whether `week-of-year-iso` fits your
architecture, and the question you can answer is whether you accept
`date-fmt` with `week-of-year-iso` attached.
A version bump skips the questions that were answered when the package
came in and keeps the two that a new release can change: what the new
code does, and whether it has a known vulnerability.

#### Checkpoint: Which dimensions for a bump?

The agent bumped `microroute-js` from 4.19.2 to 4.21.2 while it was working
on something else. Which dimensions do you review?

- All six. A new version is a new package.
- Security and code, plus license if it changed.
- None. The agent bumped it to fix a vulnerability, so it is an improvement.
- Architecture only, because the version constraint in the manifest is an architecture decision.

## The questions, per dimension

Each dimension is a short list of questions. Write the answers down, in
one line each, in the pull request or in an assessment file in the
repository.

**Architecture.** Check whether the codebase already has something
similar. Count the sub-dependencies it brings and their size, and compare
the cost of reviewing and updating them with the benefit. The API has to
make sense where you call it (from async code, say) on the platform you
run. Its docs, or the lack of them, tell you about undocumented invariants
and unsafe code.

**Quality.** If the maintainer walked away, would we fork it and maintain
it ourselves? Are there real tests, and do they pass in CI? Duplicated code
and needless complexity are the two warning signs to look for.

**Governance and support.** More than one maintainer, or one? Authors with a
track record? Many packages that depend on it? Recent activity? A
foundation behind it, or one vendor? A project has **open
governance** when it is clear how its decisions are made. A project can be
led by one company and still have open governance, if the decision process
is written down and followed. A project can have many contributors and no
open governance, if decisions happen in a room nobody can see into.

#### Checkpoint: Which project has open governance?

Which of these projects has open governance, as defined above?

- A project with two hundred contributors, where the founder merges what they like and there is no written process.
- A project where every contributor signs their copyright over to the lead company.
- A project under a foundation, which is enough on its own.
- A project led by one company, with a published process for proposals, a named group that decides, and a public record of each decision.

**Security.** Run the OpenSSF Scorecard checks, covered in the next
section. Ask the human questions too: does the package run code at install
or build time, and does it need network access to do its job? In the Rust
ecosystem, for example, Cargo compiles a `build.rs` file into a program and
runs that program before it builds the package (Build Scripts, The Cargo Book).
That's a normal place for legitimate work and a place a reviewer reads.

**Code.** Find the risks. Code that needs a specialist to read
(concurrency, cryptography, network protocols, unsafe blocks), code on your
critical path, and code that's too complicated to follow all get read.
The rest gets skipped, and the skip list is below.

**License.** What does it require: attribution, reproducing a notice file,
sharing changes under the same terms? Does that fit your project's policy?
A version bump gets this question only if the license changed.

#### Checkpoint: Which dimension asks this?

Buckets:

- Architecture
- Quality
- Governance and support
- Security
- Code
- License

Items:

- Do we already have a library in the codebase that does this?
- Would we fork and maintain it if the maintainer walked away?
- Is it clear how the project decides what gets merged?
- Are release artifacts signed, and are there open vulnerabilities?
- Can we follow the function our request handler calls on every request?
- Must we reproduce a notice file when we ship?

## The security checks

OpenSSF Scorecard is an open-source tool that runs a set of automated
checks against a public repository and scores each check from 0 to 10
(OpenSSF Scorecard checks, Open Source Security Foundation). It also publishes precomputed results for many
public projects, so for a well-known package you can look up the result at
`https://scorecard.dev/viewer/?uri=github.com/<owner>/<repo>` and skip
running anything. For a package it hasn't scored, install the `scorecard`
command, export a GitHub token as `GITHUB_AUTH_TOKEN` so the API doesn't
rate-limit you, and run it against the repository.

```sh
scorecard --repo=github.com/<owner>/<repo> --format=json
```

The check names below are Scorecard's, as documented at the time of
writing. The list changes between releases, so read the current docs
before you quote a name in a review.

| Check                    | What it asks                                                                              |
| ------------------------ | ----------------------------------------------------------------------------------------- |
| `Vulnerabilities`        | Are there open, unfixed vulnerabilities in the project or its dependencies, in the Open Source Vulnerabilities (OSV) database? |
| `Dependency-Update-Tool` | Is a tool such as Dependabot or Renovate configured to keep dependencies current?         |
| `Maintained`             | Has there been commit or issue activity in the last 90 days, and is the repo not archived? |
| `Binary-Artifacts`       | Does the source repository contain executable binaries nobody can review?                 |
| `Branch-Protection`      | Are the default and release branches protected?                                           |
| `Dangerous-Workflow`     | Do the CI workflows contain dangerous patterns, such as checking out untrusted pull-request code or passing untrusted input to a script? |
| `Code-Review`            | Is human review required before a change is merged?                                       |
| `Token-Permissions`      | Do workflow tokens follow least privilege?                                                |
| `Signed-Releases`        | Are release artifacts cryptographically signed, or published with provenance?             |

The JSON output has a top-level `score` and a `checks` list. Each check
has a `name`, a `score` and a one-line `reason`. `triage.py` in the fixture
directory reads a hand-written result file in that format, for an invented
project, and prints every check that scored below 5, then a summary line.

#### Checkpoint: Predict the summary line

The fixture has ten checks. `Binary-Artifacts`, `Dangerous-Workflow`,
`Token-Permissions` and `Vulnerabilities` score 10. `Pinned-Dependencies`
scores 5. `Branch-Protection` scores 3, `Code-Review` 2, and
`Dependency-Update-Tool`, `Maintained` and `Signed-Releases` 0. What's the
last line the script prints?

```python
findings = [c for c in result["checks"] if c["score"] < THRESHOLD]
print(f"{len(findings)} of {len(result['checks'])} checks below {THRESHOLD}")
```

A low score is a finding, and a finding is a question for you to answer
before you reach a verdict.
`Maintained` at 0 for a package that's complete and has had nothing to fix
means something different from `Maintained` at 0 for a parser that
handles untrusted input. `Signed-Releases` at 0 tells you to pin the
version you reviewed and to read how the project publishes a release.
Write the finding, write what you concluded from it, and move on.

## Code: what to skip

The code dimension is where a review becomes expensive, so it is also
where the skip list does the most work. Read the risky parts named above.
Skip everything on this list, and write in the review that you skipped it:

- Code style. The project's style is the project's business.
- Code that hasn't changed since somebody on your team reviewed it.
- Individual test cases. Check that tests exist and run. Don't read them.
- Platform-specific code for platforms you never build for.
- Docs that aren't about the API or about how the implementation
  works.

Skipping is a decision, and it is recorded. "Skipped the Windows path
handling, we build for Linux only" is a line the next reviewer can trust,
and silence about it is not.

#### Pitfall: Reading the whole package

A reviewer opened the `date-fmt` repository, started at the top of
`src/`, and after two hours had read the locale tables, the plural rules
and the test suite for time zones, and had run out of time before reaching
`format()`, the one function the code calls. The rule: start from the
function your code calls, follow it as far as your critical path goes,
and skip the rest on purpose. A reviewer who reads everything doesn't
reach the part that matters.

## Writing the verdict

The output of the review is a verdict with reasons: accept, accept with
conditions, or reject and write it ourselves. "No default outcome"
applies to both outcomes here. Rejecting a small package and writing the eleven lines
yourself is a fine outcome, and so is accepting a large one because it
solves a class of problems you have more of. A verdict without the reasons
fails the next person, who can't reuse it.

#### Checkpoint: One function, one maintainer

Of the four changes, one is a small package, `zero-pad-left-lite`: about
forty lines, one maintainer, no tests, last commit three years ago,
`Maintained` at 0. The agent imported it for one function, which pads a
number with leading zeros. The function
works. What's your verdict?

- Accept. The tests pass and the function works.
- Reject, and ask the agent to replace the import with the same function written in your codebase, with a test.
- Accept with a condition: pin the exact version so nothing changes without a review.
- Reject the whole pull request until the agent removes every dependency it added.

## Automation, and giving back

Automation takes some of this off the reviewer's desk.

Commit the lockfile and install from it in CI. The lockfile is the record
of what was reviewed, and an install that ignores it can bring in a version
nobody looked at. A new package in the diff of the lockfile is then the
event that starts a review, and nothing gets in without one.

Set a release age floor. Some package managers can refuse to install a
version published less than a set time ago. pnpm has a `minimumReleaseAge`
setting in minutes, applied to direct dependencies and to transitive ones
alike (Dependency Resolution Settings, pnpm documentation). Its docs give the reason, which is to lower the
chance of installing a compromised or broken package before anyone has
noticed. Since pnpm 11 the setting defaults to 1440 minutes, one day
(Mitigating supply chain attacks, pnpm documentation) (Dependency Resolution Settings, pnpm documentation). Left at that default, pnpm still
installs a version younger than the floor when no older one fits the
requested range. Setting `minimumReleaseAge` yourself makes that install
fail instead (Dependency Resolution Settings, pnpm documentation). A project that waits a day before
installing a new version gets the benefit of everyone else's attention
during that day.

Record a rejection where the next agent reads it. A verdict in a pull
request is found by the person who searches for it. A line in the project
instructions ("pad numbers with `str.zfill`, don't add a package for it")
is read at the start of every session (How Claude remembers your project, Claude Code documentation). The second
time an agent proposes the same package is the moment to add the line
(The AI-native SDLC playbook, Claude Academy). A check that fails the build when the
lockfile changed and the assessment file didn't turns the review from a
habit into a gate.

Giving back is the last principle, and the easiest one to skip. If you
read the critical path and found a real bug, file it upstream. If you
found nothing, share your written assessment anyway, in the repository or
with the project, because it saves the next reviewer the same hour.

## Exercise

Pick one dependency a coding agent added to a project you work on, or the
first direct dependency of any open-source project you use. Write a
one-page assessment as a Markdown file: the package name and version at the
top, then one short table per dimension from this lesson, each with the
questions that apply and a one-line answer to each. End with a verdict:
accept, accept with conditions, or reject, and the reasons. Writing it once
makes the next review a diff against this one instead of a fresh start.

A good result fits on one page, answers every question that applies to a
new direct dependency, names what you skipped and why, and has a verdict
that a colleague could act on without asking you.

Then get a second opinion from an agent. A **skill** is a packaged
procedure, a Markdown file of instructions that the agent loads when a
task matches its description. The
[Customizing agents](https://schubergphilis.github.io/ai-training/customizing-agents/) course explains how skills
work, in a later lesson, and you need none of that to install one. The
`sbp-dependency-audit` skill in the public
[`schubergphilis/agents.md`](https://github.com/schubergphilis/agents.md/blob/main/skills/sbp-dependency-audit/SKILL.md)
repository on GitHub walks an agent through a dependency audit
(sbp-dependency-audit: dependency audit skill for coding agents, schubergphilis/agents.md on GitHub). Its first step is an inventory: what the project
declares, what the code imports, and how many transitive packages the
lockfile adds. It then sorts each direct dependency into keep, inline,
replace with something native (the language's standard library or a
library already in the tree), vendor, or remove. Only a package it decides
to keep gets a supply-chain risk rating, from maintenance activity, known
vulnerabilities, trust signals, and what the package does at install or
run time. Expect an empty risk section for a package it wants to inline.
The procedure skips that step on purpose. Its last step is an action plan,
and the skill as written also executes the plan, from removing packages to
committing the result. Copy the `skills/sbp-dependency-audit/` directory
into the place your coding agent reads skills from (for Claude Code,
`.claude/skills/` in the project (Extend Claude with skills, Claude Code documentation), and other agents
document their own directory), start a new session in the project, and ask
the agent to audit the dependency you reviewed, using that skill. In a
project you work on, the request is for the report only, with the
instruction that the agent may not change any file. Running it on a copy
of the repository is the other safe option.

Compare its report with your page, and write the comparison under your
verdict. Note which dimension took you the longest to answer, and whether
the time matched the risk. Which of the six dimensions did the agent cover,
and which did it skip or fold into another? Which questions did it answer
differently from you, and which of you was right? A claim about the last
release or the number of maintainers is a fact you can check at the source.
The agent's five categories and your three verdicts are different lists, so
first translate its category into accept, accept with conditions, or
reject. Then, where that differs from your verdict, write down why. Last,
name what you would still check by hand after reading the agent's report.

Reflect: which parts of a dependency review can you hand to an agent from
now on, and which parts stay with you?

Stretch: Run Scorecard on the same package, read the function your code calls on its critical path, and add a security section and a one-line verdict to the assessment.

## Recap

1. Start from the lockfile diff, and classify each change as a new direct dependency, a new transitive dependency, or a version bump. Each kind gets a different review.
2. Spend review effort in proportion to risk, count the future cost, be wary of surprises, and apply the same principles to writing it yourself, because the review has no default outcome.
3. The dimensions are architecture (direct dependencies only), quality, governance and support, security, code, and license. A version bump keeps only security, code, and license if it changed.
4. Run or look up the OpenSSF Scorecard checks, and treat a low score as a question to answer in writing.
5. Read the code on your critical path and the parts that need a specialist. Skip style, unchanged reviewed code, individual tests, foreign platforms and non-API documentation, and say that you skipped them.
6. Commit the lockfile, consider a release age floor, write the verdict with reasons, record a rejection in the project instructions, and give real findings back upstream.

## References

- Review process for external Rust crates, Fuchsia documentation, https://fuchsia.dev/fuchsia-src/development/languages/rust/external_crates/review
- The npm package lock file, npm CLI documentation, https://docs.npmjs.com/cli/v11/configuring-npm/package-lock-json
- Best practices for Claude Code, Claude Code documentation, https://code.claude.com/docs/en/best-practices
- Build Scripts, The Cargo Book, https://doc.rust-lang.org/cargo/reference/build-scripts.html
- OpenSSF Scorecard checks, Open Source Security Foundation, https://scorecard.dev/
- Dependency Resolution Settings, pnpm documentation, https://pnpm.io/settings/dependency-resolution
- Mitigating supply chain attacks, pnpm documentation, https://pnpm.io/supply-chain-security
- How Claude remembers your project, Claude Code documentation, https://code.claude.com/docs/en/memory
- The AI-native SDLC playbook, Claude Academy, https://academy.claude.com/courses/ai-native-sdlc-playbook
- sbp-dependency-audit: dependency audit skill for coding agents, schubergphilis/agents.md on GitHub, https://github.com/schubergphilis/agents.md/blob/main/skills/sbp-dependency-audit/SKILL.md
- Extend Claude with skills, Claude Code documentation, https://code.claude.com/docs/en/skills