diff --git a/.claude/hooks/ruff-format.sh b/.claude/hooks/ruff-format.sh new file mode 100755 index 000000000..de66133b7 --- /dev/null +++ b/.claude/hooks/ruff-format.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# PostToolUse: reject an edit that leaves a Python file unformatted; exit 2 hands the message back to Claude. +set -euo pipefail +command -v jq > /dev/null || { echo "ruff hook: jq not found, skipping (ruff check still runs in nox -s lint)." >&2; exit 0; } +file=$(jq -r '.tool_input.file_path // empty') +case "$file" in + *.py) ;; + *) exit 0 ;; +esac +[ -f "$file" ] || exit 0 +case "$file" in + "${CLAUDE_PROJECT_DIR:-$PWD}"/*) ;; + *) exit 0 ;; +esac +if [ -x "${CLAUDE_PROJECT_DIR:-.}/.venv/bin/ruff" ]; then + ruff=("${CLAUDE_PROJECT_DIR:-.}/.venv/bin/ruff") +elif command -v uvx > /dev/null; then + ruff=(uvx --quiet "ruff~=0.8.4") +else + echo "ruff hook: no ruff found, skipping (ruff check still runs in nox -s lint)." >&2 + exit 0 +fi +if out=$("${ruff[@]}" format --check --quiet "$file" 2>&1); then + exit 0 +fi +echo "ruff: $file is not formatted. Run 'nox -s format' (or 'ruff format $file') and re-check.${out:+ $out}" >&2 +exit 2 diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 000000000..93af454f4 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,12 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { "type": "command", "command": "\"${CLAUDE_PROJECT_DIR}\"/.claude/hooks/ruff-format.sh" } + ] + } + ] + } +} diff --git a/.claude/skills/ai-native-sdlc/SKILL.md b/.claude/skills/ai-native-sdlc/SKILL.md new file mode 100644 index 000000000..1798cb043 --- /dev/null +++ b/.claude/skills/ai-native-sdlc/SKILL.md @@ -0,0 +1,47 @@ +--- +name: ai-native-sdlc +description: Entry point for the AI-native SDLC workflow (intent.md -> spec.md -> plan.md -> build -> verify -> review -> release). Use when starting any new feature, change or incident, when asked "where do we start", "what's the next step", or "which play should we run", or when deciding which sdlc-* skill applies to the current state of a change. +--- + +# AI-native SDLC + +Humans stay accountable for every judgment call. Claude does the drafting, checking and plumbing, and every stage leaves a committed Markdown artifact that the next stage consumes. + +## The artifact chain + +Each change gets a folder in the repo: `intent//`. + +| Artifact | Written by | Approved by | Skill | +|---|---|---|---| +| `intent.md` | Originator + Claude | Maintainer | `sdlc-intent` | +| `spec.md` | Claude, from the intent | Maintainer (area owners for flagged concerns) | `sdlc-spec` | +| `plan.md` | Claude in plan mode | Engineer (tech lead if higher risk) | `sdlc-plan` | +| code + tests | Claude, verified by its own feedback loop | Code owner at PR | `sdlc-feedback-loop`, `sdlc-pr-review` | +| release | Maintainer, following `RELEASE.md` | Code owner on the release PR | none | + +## Pick the next step from what exists + +Look in `intent//` and the repo, then route: + +1. No folder, only an idea or a ticket: `sdlc-intent`. +2. `intent.md` approved, no `spec.md`: `sdlc-spec`. +3. `spec.md` approved, no `plan.md`: `sdlc-plan`. +4. `plan.md` approved: build. Use `sdlc-feedback-loop` so the session verifies its own work, and `sdlc-subagents` to parallelise. +5. Diff ready: `sdlc-pr-review`. +6. Releasing: follow `RELEASE.md`. + +Foundations that any stage leans on: `sdlc-claude-md` (shared context in `AGENTS.md`), `sdlc-policy-skill` (organization policy as skills). + +## Prerequisites + +`sdlc-spec` needs an approved `intent.md`; `sdlc-plan` works from a spec or, for small changes, a plain +description; `sdlc-subagents` and `sdlc-pr-review` lean on `AGENTS.md`, `REVIEW.md` and `nox -s ci`. If a +prerequisite is missing, say so and offer to run that skill first instead of working around it. + +## Ground rules for every stage + +- Put the artifact in the repo and name its author. Git history is the audit record. +- One source of truth per artifact. If Jira, ServiceNow or a requirements tool already holds the record, either declare it authoritative and write outcomes back through MCP, or link both ways (record ID in the file, commit SHA in the record). +- Advisory controls (skills, `CLAUDE.md`) get a deterministic backstop where the rule must always hold: a hook at the edit, a required check at the PR. +- A human makes each accept or reject decision that moves an artifact to the next stage. Never mark one approved on their behalf. +- Every play has a leading and a lagging indicator. Name them when you set the play up so the team can tell whether it worked. diff --git a/.claude/skills/sdlc-claude-md/SKILL.md b/.claude/skills/sdlc-claude-md/SKILL.md new file mode 100644 index 000000000..53a92bf8c --- /dev/null +++ b/.claude/skills/sdlc-claude-md/SKILL.md @@ -0,0 +1,63 @@ +--- +name: sdlc-claude-md +description: Create, trim or audit a repo's CLAUDE.md so it holds exactly what a new joiner needs on day one (commands, conventions, architecture, recurring mistakes, verification). Use when a repo has no CLAUDE.md, when it has grown stale or long, when Claude repeats a mistake, or when asked to "set up CLAUDE.md" or "update our CLAUDE.md". +--- + +# The CLAUDE.md + +`CLAUDE.md` is the context a new joiner would need, read by the agent at the start of every session. It is version controlled and reviewed like code. Keep it under a page: Claude reads all of it every session, and stale lines waste context. + +## Create + +1. Survey the repo: build files, `Makefile`/package scripts, CI config, README, directory layout, test setup. +2. Draft with `/init` if available, or by hand, then cut it down to day-one essentials. +3. Verify every command by running it (or asking the engineer). Never list a command you have not seen work, and note a healthy output for each. +4. Ask the engineer who knows the codebase best for the "things Claude gets wrong" list; pull more from recent PR review comments and reverted changes. +5. Place it at the repo root and commit, with code owners set so changes get reviewed. + +## Template + +```markdown +# + +## Commands +- Build: +- Test: (unit), (integration, needs ...) +- Lint: (runs in CI; fix before pushing) +- Run: () + +## Conventions +- +- + +## Architecture +- + +## Things Claude gets wrong +- + +## Verifying your work +- Build: (must finish with "") +- Test: (all green; never skip or delete a failing test) +- Lint: (zero warnings) +Run all three before reporting any task complete, and paste the output. +If a test fails, fix the code, not the test. +``` + +If artifacts live in `intent/`, add a line saying `intent/` holds `intent.md`, `spec.md` and `plan.md` per change. + +## Audit an existing file + +- Remove anything stale, obvious from the code, or already enforced by CI or a hook. +- Move multi-step procedures into a skill (`sdlc-policy-skill`); move must-always-hold rules behind a hook (`.claude/settings.json`) or a CI check. +- Flag missing sections, unverifiable commands, and contradictions with `README` or CI. +- Report the diff and why; do not silently drop lines someone may rely on. + +## Maintenance rule + +When Claude makes the same mistake twice, the correction goes into `CLAUDE.md` in the same PR. PR review should also flag changes that make `CLAUDE.md` outdated (`sdlc-pr-review`). Trimming changes agent behavior, so watch the next few PRs for regressions. + +## Measure + +- Leading: how often Claude repeats a mistake `CLAUDE.md` should have caught (track corrections in Git history). +- Lagging: time to first merged PR for a new team member. diff --git a/.claude/skills/sdlc-feedback-loop/SKILL.md b/.claude/skills/sdlc-feedback-loop/SKILL.md new file mode 100644 index 000000000..db2525b95 --- /dev/null +++ b/.claude/skills/sdlc-feedback-loop/SKILL.md @@ -0,0 +1,41 @@ +--- +name: sdlc-feedback-loop +description: Give Claude a way to verify its own work (tests, build, lint, screenshot diff) and protect that check from being weakened. Use when starting implementation or a bug fix, when asked to "set up verification", "fix this bug test-first", or when Claude reports work done without proof; also for UI changes that need a visual check. +--- + +# Give Claude a feedback loop + +Always give the session a way to check its own work before a person sees it. A late signal (CI minutes later, a tester days later) turns the engineer into the bottleneck. + +## Set up + +1. Wrap verification in one command per concern that exits non-zero on failure (`make test`, `npm test`, `make lint`, `make build`). Create the targets if only a command sequence exists today. +2. List them in `CLAUDE.md` (see `sdlc-claude-md`) with a sample of healthy output, plus the instruction: run the tests before reporting a task complete and show the output. +3. State a quantifiable target for the task so Claude can check without asking: "all tests in X pass", "screenshot matches the attached mock", "the endpoint returns 200 with the new field". Take targets from `plan.md` Proof when it exists. +4. For UI work, give Claude a browser or screenshot tool (e.g. via MCP) and the mock: implement, screenshot, compare, adjust. Two or three rounds is normal. + +## Bug fixes: failing test first + +1. Reproduce the bug as a test. Run it and confirm it fails for the reason expected. +2. Commit that test. +3. Only then fix the code, without editing the test. A test that existed before the fix, and that the agent could not rewrite, is proof the bug is gone. + +## Protect the loop + +An agent fixing code must not be able to weaken the check on that code. Either: + +- block edits to existing test files once the failing-test commit exists, with a `PreToolUse` hook (`sdlc-policy-skill` shows the pattern), or +- check the diff in review: new regression tests are expected, but reject any change after the failing-test commit that weakens, skips or rewrites a test (`sdlc-pr-review`). + +## Report format + +When finishing, paste the literal output of the build, test and lint commands (or the screenshot diff). Evidence comes from the toolchain, not from the agent's summary. + +## Governance + +Verification before "done" and the block on editing tests are enforced as hooks where the organization wants them guaranteed. Evidence lives in the session transcript and in the PR's check run; the code owner reviews intent and risk because mechanical evidence is already attached. + +## Measure + +- Leading: first-pass CI success rate of agent-written changes. +- Lagging: review time per PR (should fall) and change failure rate from the incident tracker. diff --git a/.claude/skills/sdlc-intent/SKILL.md b/.claude/skills/sdlc-intent/SKILL.md new file mode 100644 index 000000000..21f754778 --- /dev/null +++ b/.claude/skills/sdlc-intent/SKILL.md @@ -0,0 +1,52 @@ +--- +name: sdlc-intent +description: Turn an idea, ticket, pain point or incident into a committed intent.md proto-spec by interviewing the originator. Use when someone describes a problem or wants a new feature and there is no intent.md yet, when asked to "capture intent", "write up this idea", or "start a new change". +--- + +# Capture as intent.md + +`intent.md` is a short proto-spec in the originator's own terms: what is wanted, why, and under which constraints. It is human readable and machine actionable, and it kicks off the rest of the chain. + +## Steps + +1. Let the originator describe the problem in their own words. No formal language needed. If the source is a ticket or incident, read it (via MCP or the pasted text) and start from there. +2. Interview until the idea is concrete, asking what an analyst would: who is affected, what they cannot do today, what better looks like, what is out of scope, which systems are touched, which constraints apply (security, compliance, existing auth), how success is observed. Ask a few questions at a time, not a form. +3. Check for an agreed template (`intent/README.md` or an organization skill) and use it. Otherwise use the template below. +4. Write the draft to `intent//intent.md` (kebab-case slug, create the folder). +5. Read it back and have the originator correct anything misunderstood. Do not invent answers: unresolved items go under Open questions. +6. Commit when the originator is satisfied (follow the repo's commit conventions). The product owner picks it up from there and decides accept or close. + +## Template + +```markdown +# Intent: +Author: . Status: draft. + +## Problem + + +## Proposed outcome + + +## Affected users and systems + + +## Constraints + + +## Open questions + +``` + +## Rules + +- Stay in the problem space. No design, no implementation, no file names. +- If the originator is not a Git user, commit through the version-control connector on their behalf and record them as author in the header. +- Track an incident-sourced intent the same way, and say in the header that it came from monitoring (`Author: monitoring agent`) so the product owner knows to review harder. + +## Measure + +- Leading: time from first conversation to a committed `intent.md` (Git timestamps). +- Lagging: share of intents the product owner accepts into design instead of closing, and how many edits `intent.md` receives after the first `spec.md` commit. + +Next step once accepted: `sdlc-spec`. diff --git a/.claude/skills/sdlc-plan/SKILL.md b/.claude/skills/sdlc-plan/SKILL.md new file mode 100644 index 000000000..8c50f9915 --- /dev/null +++ b/.claude/skills/sdlc-plan/SKILL.md @@ -0,0 +1,54 @@ +--- +name: sdlc-plan +description: Produce and commit a plan.md before any code is written, working in plan mode from the approved spec.md and intent.md. Use when starting implementation of a change, when asked to "plan this", "make an implementation plan", or before editing code for a change that has an intent/spec; also use to keep plan.md in sync when implementation departs from it. +--- + +# Plan mode as the default starting point + +Work starts with a written, reviewable plan. Plan mode lets Claude read the codebase without changing anything, so course corrections are edits to a document. + +## Steps + +1. Work in plan mode (Shift+Tab, or start the session with `--permission-mode plan`). Do not edit files until the engineer accepts the plan. +2. Read `intent//intent.md`, `spec.md` (the requirement numbers R1.. and concerns), `CLAUDE.md` and the code the change touches. If a spec exists, a plan without it is a smell; if neither exists, ask whether to run `sdlc-intent` / `sdlc-spec` first or to plan from a plain description. +3. Draft the plan using the template below. +4. Let the engineer interrogate it. Be ready to answer: what could this break, which step is riskiest, which alternatives were rejected and why. +5. Iterate until someone who never saw the conversation could implement the change from the plan alone. +6. Commit the approved plan as `intent//plan.md`. Then implement; with a solid plan this is often one pass. +7. If implementation departs from the plan, update `plan.md` in the same commit. Consider a hook that flags source changes with no matching plan update. + +## Template + +```markdown +# Plan: (from intent.md and spec.md) + +## Files that change +<paths, marking new files> + +## Order of work +1. <step> (R1, R2). +2. <step> (R4). + +## Risks +<what could break; cite spec concerns by number> + +## Proof +<which tests or checks cover which requirements, and how UI changes are verified> +``` + +## Rules + +- Cite requirement numbers on every step and every proof line so a reviewer can check the plan against the spec line by line. +- The Proof section must be executable by Claude (tests, build, screenshot compare); see `sdlc-feedback-loop`. +- Independent steps (different files) can run in parallel sessions; mark them so `sdlc-subagents` can split the work. +- Plans for changes the organization classes as higher risk go to a tech lead or architect before acceptance. +- Existing records (Jira, requirements tools) stay authoritative when the organization says so: note the record ID in the plan header, and write outcomes back through the connector. Otherwise link both ways. + +## Auto mode + +Once guardrails mature (a tuned `CLAUDE.md`, policy skills, blocking hooks, a test suite Claude can run), routine work with a tight spec and a small blast radius can run in auto mode after the plan is approved. The plan is still reviewed first. + +## Measure + +- Leading: share of changes that merge from the first implementation pass; time from plan approval to merged PR. +- Lagging: rework cycles per change; how often the merged diff still matches `plan.md`. diff --git a/.claude/skills/sdlc-policy-skill/SKILL.md b/.claude/skills/sdlc-policy-skill/SKILL.md new file mode 100644 index 000000000..01be1fc6e --- /dev/null +++ b/.claude/skills/sdlc-policy-skill/SKILL.md @@ -0,0 +1,52 @@ +--- +name: sdlc-policy-skill +description: Write a Claude skill that encodes an organization policy or standard (security, API design, brand, compliance, UX), with a trigger test and a deterministic backstop. Use when asked to "turn this policy into a skill", "encode our standard", "make Claude follow our X guidelines", or when a rule is enforced inconsistently today. +--- + +# Skills as institutional knowledge + +A skill makes institutional knowledge operational: explicit, version controlled, applied broadly, updated centrally. It is an advisory control. Write one for knowledge that must be applied consistently; keep general project context in `CLAUDE.md` and one-off instructions in a prompt. + +## Decide where it belongs + +| Knowledge | Home | +|---|---| +| Conventions every session needs (commands, layout) | `CLAUDE.md` | +| A policy with an owner, applied when a certain task happens | **a skill** | +| A rule that must hold with no exceptions | skill + hook (+ required CI check) | +| A recurring job needing its own context and tools | subagent (`sdlc-subagents`) | + +## Steps + +1. Pick one policy that is enforced inconsistently today. Get its written source of truth and its named owner. +2. Create `.claude/skills/<name>/SKILL.md` (ships with the repo) or put it in a plugin for organization-wide distribution. +3. Frontmatter: `name`, and a `description` that says what the skill does and exactly when to trigger (task types, file types, verbs users would say). The description decides whether it ever loads. +4. Body: numbered, testable rules in imperative form; cite the policy section each rule comes from; name any script to run and require its output in the summary. +5. Test triggering: ask Claude for the relevant task in 4-5 different phrasings, including ones that never mention the policy, and confirm the skill loads each time. Tighten the description until it does. +6. Have the policy owner sign off. Policy changes become skill changes, reviewed like code; engineers pick up the new version in their next session. +7. Back it up (below). + +## Template + +```markdown +--- +name: <kebab-name> +description: <What it enforces>. Use whenever <task types and triggers>. +--- +# <Title> +When you <do the task>: +1. <Rule>. (Policy §x.y) +2. <Rule>. +Run <script> and include its output in your summary. +``` + +## Add a deterministic backstop for must-hold rules + +- **Hook at the edit**: a `PostToolUse` hook (matcher `Edit|Write`) that runs the same script the skill names, only for the files that changed. Exit code 2 feeds the script's message back to Claude so it fixes the line. Keep build-phase hooks fast and scoped; heavy checks belong at commit or PR. +- **Required check at the PR**: run the same script in CI as a required status check. It also catches changes that bypassed the hook (a shell command rewriting a file, a different tool). +- A hook that asks a human for approval is a gate, not a guardrail; keep those at release, not mid-build. + +## Measure + +- Leading: time from policy owner approving a change to the updated skill merging. +- Lagging: PR review findings that cite the policy. They should trend to zero; if not, the skill is not triggering or has drifted from the official text. diff --git a/.claude/skills/sdlc-pr-review/SKILL.md b/.claude/skills/sdlc-pr-review/SKILL.md new file mode 100644 index 000000000..db76d171c --- /dev/null +++ b/.claude/skills/sdlc-pr-review/SKILL.md @@ -0,0 +1,62 @@ +--- +name: sdlc-pr-review +description: Set up REVIEW.md and run structured AI review passes (bugs, security, compliance against spec.md/plan.md) on a diff or PR with severity-ranked findings, or address review comments on a PR. Use when asked to "review this PR/diff", "set up PR review", "write REVIEW.md", "address the review comments", or before a PR is marked ready. +--- + +# AI in the PR review loop + +Claude both gives and receives reviews. Every PR gets the same review passes with findings ranked by severity, so human attention moves up to whether the change does what the plan intended and whether the risk is acceptable. Separation of duties holds: the agent that wrote the code has no way to approve it. Approval comes from a human code owner through branch protection. + +## Mode A: set up `REVIEW.md` (tech lead) + +1. Ask which passes the organization cares about. Default: Bugs, Security, Compliance (change matches `spec.md`, `plan.md` and design principles). +2. Define what Important means (breaks behavior, leaks data, breaches policy) and treat style and naming as nits. +3. Cap nits (e.g. five per review, summarize the rest as a count) and list what not to report (generated paths, anything CI already enforces). +4. Commit `REVIEW.md` at the repo root. + +```markdown +# Review instructions +## Passes +Run three passes and tag each finding with its pass: +- Bugs: logic errors, broken edge cases, subtle regressions +- Security: injection risks, authentication gaps, PII in logs +- Compliance: the change matches spec.md, plan.md and our design principles +## What Important means here +Reserve Important for findings that would break behavior, leak data +or breach a policy. Style and naming are nits. +## Cap the nits +Report at most five nits per review; summarize the rest as a count. +## Do not report +Generated files and anything CI already enforces. +``` + +Hosting options: the managed Code Review service (admin enables it per repository) or the `claude-code-action` in your own CI when you need control of the pipeline or model access through your own cloud agreement. + +## Mode B: review a diff or PR + +1. Get the diff (`git diff <base>...HEAD`, or `gh pr diff`). Read `REVIEW.md`, `CLAUDE.md`, and the change's `intent/<slug>/spec.md` and `plan.md` if present. The compliance pass depends on the committed plan, which a reviewer looking at the diff alone would not have open. +2. Run each pass in `REVIEW.md` over the full diff, citing requirement numbers (R1..) and plan steps where relevant. +3. Check that tests were not weakened and that `plan.md` was updated if the implementation departed from it. +4. Report a table sorted by severity: + + | Severity | File:Line | Issue | + |---|---|---| + | Important | `path:22` | Compliance: <finding, cite plan/spec> | + | Nit | `path:8` | Bugs: <finding> | + +5. Findings do not approve or block on their own. State plainly that approval is for the code owner. + +## Mode C: address review comments + +When tagged on a comment, make the fix, push, and reply on the thread so it records both the request and the change. To shepherd your own PR: sweep unresolved comments and failing checks, fix, push, repeat until green and waiting only on code-owner approval. Never approve or merge your own PR. + +## Feedback into the system + +- A mistake flagged a second time goes into `CLAUDE.md` in that PR (`sdlc-claude-md`). +- Flag when a change makes `CLAUDE.md` outdated. +- Monthly: rate findings, cap noisy categories in `REVIEW.md`. + +## Measure + +- Leading: time to first review (minutes); share of review comments resolved without a human touching the branch. +- Lagging: defects and vulnerabilities caught before merge vs escaping to production. diff --git a/.claude/skills/sdlc-spec/SKILL.md b/.claude/skills/sdlc-spec/SKILL.md new file mode 100644 index 000000000..fc4df26e2 --- /dev/null +++ b/.claude/skills/sdlc-spec/SKILL.md @@ -0,0 +1,59 @@ +--- +name: sdlc-spec +description: Turn an approved intent.md into a requirements-and-design spec.md with numbered requirements, a constraint-coverage table and flagged areas of concern, applying the organization's policy skills. Use when an intent.md has been accepted and needs a spec, or when asked to "write the spec", "do requirements and design", or "convert this intent into requirements". +--- + +# Requirements and design: intent.md to spec.md + +Requirements and design happen in one session instead of two hand-offs. The product owner reviews the spec; they do not write it. The goal is a spec engineering can plan against, with every concern surfaced early. + +## Before you start + +- Locate `intent/<slug>/intent.md` and confirm it is approved (ask if unclear). No intent, run `sdlc-intent` first. +- List the policy skills available (`.claude/skills/`, plugins) for brand, security, compliance and UX, and apply every one that is relevant. If none exist, say so, because the spec will rest on stated constraints only. +- Read `CLAUDE.md` and the parts of the codebase the intent touches so the design integrates with what exists. + +## Produce `intent/<slug>/spec.md` + +```markdown +# Spec: <title> +From: intent.md (<author>). Status: ready for product owner review. +Skills applied: <skill names>. + +## Requirements +- R1. <testable statement> +- R2. ... + +## Design +<endpoints, components, data flow, where each piece lives in the existing system> + +## How each constraint in the intent is met +| Constraint | Met by | +| :---- | :---- | +| <from intent> | <requirement or design element> | + +## Open questions from the intent +- <question>. <answered here, or carried forward as concern N> + +## Areas of concern +1. <concern, why it matters>. Owner: <policy owner>. +``` + +## Rules + +- Number every requirement (R1, R2, ...). The plan and the review cite these numbers. +- Make requirements testable: observable behavior, a measurable limit, an exact field set. +- Every constraint in the intent must appear in the coverage table. If one cannot be met, it becomes a concern, not a silent omission. +- When two policies conflict, do not pick a winner. Describe the clash, propose a resolution, name the owner. +- Carry unanswered intent questions forward as concerns with an owner; never guess. +- Record which skills (and versions) were applied, so the audit trail shows the live policy was read. +- Do not write code in this stage. + +## Hand-off + +Tell the product owner to resolve flagged concerns with their policy owners first (these are the points an analyst would have escalated), then review the spec against the original idea. A human decides whether the spec and intent progress to build; ask a technical lead to weigh in for higher-risk changes. Commit `spec.md` next to `intent.md` once accepted. Next step: `sdlc-plan`. + +## Measure + +- Leading: elapsed time between the `intent.md` commit and the `spec.md` commit. +- Lagging: `spec.md` commits dated after the first `plan.md` commit for the same change (requirements rework). diff --git a/.claude/skills/sdlc-subagents/SKILL.md b/.claude/skills/sdlc-subagents/SKILL.md new file mode 100644 index 000000000..594c836e4 --- /dev/null +++ b/.claude/skills/sdlc-subagents/SKILL.md @@ -0,0 +1,54 @@ +--- +name: sdlc-subagents +description: Split a plan into independent streams for parallel Claude sessions in separate git worktrees, and define reusable subagents (verifier, simplifier, researcher) in .claude/agents/. Use when asked to "work on these in parallel", "set up worktrees", "create a verifier agent", or when one engineer has several independent tasks in flight. +--- + +# Parallel sessions and subagents + +A **parallel session** is a full Claude Code instance on its own task in its own git worktree; sessions share nothing except the engineer steering them. A **subagent** is a scoped helper inside one session, with its own context window and tool limits, for jobs that recur across tasks. Sessions raise how many tasks are in flight; subagents keep each session focused. The engineer's job becomes steering and reviewing. + +## Prerequisites + +`CLAUDE.md` exists (every session reads it). A feedback loop helps, since sessions that can verify themselves need less supervision (`sdlc-feedback-loop`). Permission settings should be tuned so safe commands do not stall sessions on prompts. + +## Split the work + +1. Read `plan.md` (or the task list). Group steps by the files they touch. +2. Independent groups become parallel sessions. Tasks that share files run in one session, sequentially. +3. Start with two or three sessions. The ceiling is how many streams the engineer can review properly; add more only while review keeps up. +4. Give each its own worktree and branch: + ```bash + claude --worktree feature-auth + claude --worktree fix-rate-limit + ``` +5. Give each session its slice of the plan and its own proof (tests or checks) so it can finish without the others. + +## Define subagents + +Markdown files in `.claude/agents/`, checked into Git so the team shares them. Each needs a name, a description of when to use it, and the minimal tools it may touch. + +```markdown +--- +name: verifier +description: Runs the app and checks the change works before the session reports done +tools: Bash, Read +--- +Start the app with <run command>. Exercise the changed behavior and the two +nearest neighboring flows. Report what you ran, what you saw, and any +behavior that does not match plan.md. Do not fix anything; report only. +``` + +Useful starting set: + +- **verifier**: fresh-context final check, so the verdict is not colored by the assumptions that produced the code. This complements, and does not replace, the feedback loop running throughout the task. +- **code-simplifier**: strips needless complexity after the main agent finishes. +- **researcher**: explores the codebase and reports back without flooding the main context. + +## Governance + +More sessions means more output, so controls must live in repo configuration (hooks, permission settings) that all sessions inherit. Activity is attributed to the engineer who ran it and exported via OpenTelemetry if configured. + +## Measure + +- Leading: concurrent sessions per engineer while review quality holds; share of the day spent steering vs waiting. +- Lagging: changes merged per engineer per week, read alongside the rework rate from PR history. diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000..065eceaaa --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +end_of_line = lf +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true + +[*.py] +indent_style = space +indent_size = 4 + +[*.{json,toml,yml,yaml,md}] +indent_style = space +indent_size = 2 diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 000000000..06ab2b7b3 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,46 @@ +name: Bug report +description: A b2 command behaves incorrectly. +labels: [bug] +type: Bug +body: + - type: input + id: version + attributes: + label: b2 version + description: Output of `b2 version`, and how you installed it (pip, uv, binary, Docker, Homebrew). + validations: + required: true + - type: input + id: platform + attributes: + label: OS and Python version + placeholder: macOS 15 arm64, Python 3.13 (or "standalone binary") + validations: + required: true + - type: input + id: command + attributes: + label: Entry point + description: Which command you ran, `b2` or a pinned ApiVer such as `b2v4`. + placeholder: b2v4 + - type: textarea + id: what + attributes: + label: What happened, and what did you expect? + description: Include the exit code if it matters. + validations: + required: true + - type: textarea + id: repro + attributes: + label: Steps to reproduce + description: The exact commands. Never paste application keys, auth tokens or SSE-C keys. + render: shell + validations: + required: true + - type: textarea + id: logs + attributes: + label: Output or logs + description: Run with `--verbose` if you can. Redact keys, tokens and bucket names you consider private. + render: text diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 000000000..83aa66082 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: false +contact_links: + - name: Report a security vulnerability + url: https://github.com/Backblaze/B2_Command_Line_Tool/security/advisories/new + about: Report privately. Do not open a public issue. See SECURITY.md. + - name: Problem with the B2 service or your account + url: https://help.backblaze.com/hc/en-us/requests/new + about: Contact Backblaze support for service, billing or account issues. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 000000000..a8502025a --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,24 @@ +name: Feature request +description: A command, flag or B2 capability the CLI should support. +labels: [enhancement] +type: Feature +body: + - type: textarea + id: problem + attributes: + label: Problem + description: What can you not do today, and who is affected? + validations: + required: true + - type: textarea + id: outcome + attributes: + label: Proposed outcome + description: The command line you would like to run and what it should print. Link the B2 API docs for the calls involved if you know them. + validations: + required: true + - type: textarea + id: constraints + attributes: + label: Constraints and alternatives + description: Compatibility with existing scripts, capabilities needed, whether b2sdk already supports it, workarounds. diff --git a/.github/ISSUE_TEMPLATE/task.yml b/.github/ISSUE_TEMPLATE/task.yml new file mode 100644 index 000000000..6f4e86b86 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/task.yml @@ -0,0 +1,40 @@ +name: Task +description: Maintenance, tooling, CI, docs or release work that is neither a bug nor a new feature. +type: Task +body: + - type: textarea + id: goal + attributes: + label: What needs doing, and why? + description: The outcome and the reason it matters now. + validations: + required: true + - type: textarea + id: done + attributes: + label: Done when + description: Checkable conditions, one per line. + value: | + - [ ] + validations: + required: true + - type: textarea + id: scope + attributes: + label: Scope and context + description: Files, workflows or settings involved; related issues and PRs; what is out of scope. + - type: dropdown + id: area + attributes: + label: Area + options: + - CI and workflows + - Docs + - Release + - Tests + - Bundles and Docker + - Dependencies + - Repository settings + - Other + validations: + required: true diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..d1a015b90 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,34 @@ +Summary: + +## Motivation + +Why this change is needed. Link the issue (`Fixes #123`) and, if one exists, `intent/<slug>/`. + +## Changes + +What changed, and which ApiVers (`b2`, `b2v3`, `b2v4`, `b2v5`) see it. If this PR is stacked on another, +name it and the merge order. + +## Testing + +Paste the tail of `nox -s ci` and the output of the changed command run end to end. For each behavior +this PR adds or fixes, name the test that fails when the change is reverted. List the live-B2 +integration tests you ran, and mark the ones you could not as NOT RUN. + +## Risk + +What could break for existing scripts: flags, output, exit codes, stderr. Name the riskiest part of the +diff. + +## How to revert + +Whether reverting this commit is enough, or whether users would need to pin the previous release. + +## Checklist + +- [ ] `nox -s ci` passes locally. +- [ ] A fresh-context review (`/code-review`, `/security-review` or a subagent) ran on the diff; findings are fixed or answered below. +- [ ] A `changelog.d` fragment exists with the right type, or a maintainer added the `-changelog` label. +- [ ] New or changed behavior is pinned by a test in every ApiVer it affects. +- [ ] Help text, README usage and `AGENTS.md` are updated if needed. +- [ ] The description matches the final code. diff --git a/.github/security-review-instructions.txt b/.github/security-review-instructions.txt new file mode 100644 index 000000000..dfee0ca9a --- /dev/null +++ b/.github/security-review-instructions.txt @@ -0,0 +1,10 @@ +This is the Backblaze B2 command-line tool (Python, built on b2sdk). Pay special attention to: + +- Application keys, authorization tokens and SSE-C keys reaching logs (--verbose, --debug-logs, + b2_cli.log), error messages or stderr, or stdout in commands that do not document it. `account get` + and `key create` print credentials on purpose; flag only new or undocumented output. +- Commands that delete or overwrite data (rm, sync --delete, file and bucket updates) acting beyond the + bucket, prefix or versions the user asked for, or ignoring --dry-run. +- The local account info database (B2_ACCOUNT_INFO): permissions, copying, or printing it. +- GitHub workflow changes: secrets exposed to untrusted code, broadened permissions, untrusted PR fields + in run: steps, and third-party actions not pinned to a commit SHA. diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml new file mode 100644 index 000000000..bfe53cec3 --- /dev/null +++ b/.github/workflows/ai-review.yml @@ -0,0 +1,88 @@ +name: AI review + +# Advisory AI code review and security review on every PR (see REVIEW.md). Both jobs are no-ops until a +# maintainer adds the ANTHROPIC_API_KEY repository secret and installs the Claude GitHub App (code-review +# authenticates through it); fork PRs never receive the secret. + +on: + pull_request: + types: [opened, synchronize, ready_for_review, reopened] + +permissions: + contents: read + pull-requests: write + +jobs: + code-review: + concurrency: + group: ai-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.user.login != 'dependabot[bot]' && !github.event.pull_request.draft }} + timeout-minutes: 30 + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + id-token: write + env: + HAS_KEY: ${{ secrets.ANTHROPIC_API_KEY != '' }} + steps: + - name: Skip without an API key + if: env.HAS_KEY != 'true' + run: echo 'Skipped because ANTHROPIC_API_KEY is not set for this repository or PR.' + - uses: actions/checkout@v4 + if: env.HAS_KEY == 'true' + with: + fetch-depth: 0 + - uses: anthropics/claude-code-action@925afb53776cf1a2b738c6bdbed37b4d4000ef13 # v1.0.262 + if: env.HAS_KEY == 'true' + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Review this pull request following REVIEW.md, AGENTS.md and doc/review-lessons.md in this + checkout. The working directory holds the PR merged onto its base branch. + + - Read the PR description and the linked issue, and check the diff against the whole repo: + does it solve the stated problem, fit the system, and avoid breaking anything else? For a + bug fix, check that it addresses the root cause. + - Find candidate issues, then verify each one by reading the code it depends on before + posting. Post only findings you verified; drop style, diagnostic and cleanup-only remarks. + - Do not repeat a finding already posted on this PR, resolved or not. If this is not the + first review, review only the commits since the last one. + - Post each finding as an inline comment tagged Important, Nit or Pre-existing, citing + file:line and the input or sequence that triggers it. + - Finish with one top-level comment that starts with the count + (for example "1 Important, 0 Nit, 0 Pre-existing" or "No issues found"). + + Use `mcp__github_inline_comment__create_inline_comment` (with `confirmed: true`) for inline + findings and `gh pr comment` for the summary. Only post GitHub comments. + claude_args: | + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Read,Grep,Glob" + + security-review: + if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.user.login != 'dependabot[bot]' && !github.event.pull_request.draft }} + timeout-minutes: 30 + runs-on: ubuntu-latest + env: + HAS_KEY: ${{ secrets.ANTHROPIC_API_KEY != '' }} + steps: + - name: Skip without an API key + if: env.HAS_KEY != 'true' + run: echo 'Skipped because ANTHROPIC_API_KEY is not set for this repository or PR.' + - uses: actions/checkout@v4 + if: env.HAS_KEY == 'true' + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 2 + - uses: anthropics/claude-code-security-review@0c6a49f1fa56a1d472575da86a94dbc1edb78eda # main, 2026-02-11 + if: env.HAS_KEY == 'true' + with: + comment-pr: true + claude-api-key: ${{ secrets.ANTHROPIC_API_KEY }} + exclude-directories: doc,changelog.d,intent,.claude + # Without this the action scans only the first commit of each PR. + run-every-commit: true + custom-security-scan-instructions: .github/security-review-instructions.txt diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..31513687c --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,128 @@ +# B2 Command Line Tool + +The `b2` CLI for Backblaze B2 (PyPI `b2`), built on [`b2sdk`](https://github.com/Backblaze/b2-sdk-python). +This file is the map for coding agents and new contributors; `CLAUDE.md` points here. Keep it under about +150 lines and link out for detail. + +## Commands + +Install [nox](https://nox.thea.codes) and [uv](https://docs.astral.sh/uv/) (`uv` 0.13.4 or newer). + +- Full local gate (CI's lint, build and test jobs on Python 3.12): `nox -s ci`. Run it before every push and + paste the tail in the PR. `NOX_CI_PYTHON`, `NOX_CI_BASE` and `NOX_CI_SKIP_CHANGELOG` adjust it (see + `noxfile.py`). It does not run the doc, docker or bundle jobs. +- Format: `nox -s format` (ruff). Lint: `nox -s lint` (ruff check, license check, + `test/static`, `uv lock --check`). +- Unit tests: `nox -s unit-3.12`. They run once per ApiVer (`--cli b2v3`, `b2v4`, `b2v5`) and need no + credentials. +- Integration tests: `nox -s integration-3.12 -- -m "not require_secrets"` runs offline. With + `B2_TEST_APPLICATION_KEY` and `B2_TEST_APPLICATION_KEY_ID` set, `-- -m require_secrets --cleanup` runs + against live B2. Use a throwaway account. Leftover buckets: `nox -s cleanup_buckets`. +- Changelog: `nox -s towncrier_check` (fragment names and text) and `nox -s changelog_check` (this branch + adds a fragment). See [CONTRIBUTING.md](CONTRIBUTING.md#changelog). +- Build: `nox -s build`. Docs: `nox --non-interactive -s doc`. Bundle: `nox -s bundle`. +- Spelling: `nox -s ci` runs codespell 2.4.1 on the files your branch changes. +- Dependencies: change `pyproject.toml` with `uv add` / `uv lock`; `uv.lock` must stay in sync. +- Review health: `nox -s review_metrics -- --days 30` (human approvals, comments and wait per merged PR, + reverts as the guardrail); reviewer evals in [doc/reviewer-evals.md](doc/reviewer-evals.md). + +## Architecture + +| Path | Role | +| --- | --- | +| `b2/_internal/console_tool.py` | Every command class (`Command`, `B2`, subcommands), argument definitions, output. Most changes land here. | +| `b2/_internal/b2v3/`, `b2v4/`, `b2v5/` | ApiVer entry points. Each `registry.py` registers the commands for that version and may override old behavior. `b2` is `b2v5`. | +| `b2/_internal/_cli/` | Shared argument types, completers, `obj_dumps` (JSON output), autocomplete install and cache. | +| `b2/_internal/_utils/` | `b2://` and `b2id://` URI parsing. | +| `test/unit/` | Offline tests through `ConsoleTool` with b2sdk's `RawSimulator`; `console_tool/` holds per-command files. | +| `test/integration/` | Runs the CLI as a subprocess (`--sut` can point at a bundled binary) against live B2. | +| `test/static/` | License headers and other static checks. | +| `.sdkharness/` | Contract checks run by the central SDK harness against the B2 simulator on loopback. | +| `changelog.d/` | towncrier fragments; `CHANGELOG.md` is built from them at release. | +| `scripts/` | Repo tooling run by nox (`review_metrics.py`). | +| `.github/workflows/ai-review.yml` | Advisory AI code review and security review on PRs, off until `ANTHROPIC_API_KEY` is set and the Claude GitHub App is installed. | +| `doc/` | Sphinx docs (`doc/source`), plus Markdown pages for niche topics (`authorization_realm.md`). | +| `intent/` | Per-change `intent.md`, `spec.md`, `plan.md` (see Workflow). | + +The SDK does the B2 work; the CLI parses arguments, calls `b2sdk.v3`, and prints. A behavior that belongs +in the SDK (retries, wire shapes, large-file logic) is fixed in `b2-sdk-python`, then picked up here by +bumping the `b2sdk` pin. + +## Conventions + +- Python 3.10+ (`requires-python`); CI tests 3.10 to 3.14 on Linux, macOS and Windows, and PyPy 3.10 on Linux. +- ApiVer is a compatibility promise. `b2vN` keeps its commands, flags, output and exit codes; a change + to them goes into a new ApiVer or behind a deprecation, never into an existing `b2vN` in place. Check + the `registry.py` of every version a command is registered in. +- Command help comes from the class docstring and is part of the product (`--help`, README usage, Sphinx). + Developer-only flags and environment variables (`--environment`, `--dev`, `B2_ENVIRONMENT`) stay out of the main help. +- Messages that tell the user to run something use the binary name the user invoked + (`self.console_tool.b2_binary_name` in a `Command`), never a hard-coded `b2`. +- Every source file starts with the Backblaze license header (`test/static` checks it). +- Comments are short and explain non-obvious decisions. Ruff config is in `pyproject.toml`. + +## Things agents get wrong here + +Each line was raised by reviewers on at least two merged PRs. + +1. **Missing changelog fragment.** Every PR adds a `changelog.d` file: `{issue}.{type}.md` or + `+{slug}.{type}.md`. Only a `uv.lock`-only change or the `-changelog` label skips it. +2. **Fragment text and type.** One complete sentence ending in a full stop ("fixed #157" is rejected). + CI, tests, build and tooling go in `infrastructure`; docs in `doc`. User-facing sections are for users. +3. **Unannounced behavior changes.** A new exit code, changed output or new stderr text breaks scripts. + Say so in the fragment (`changed`), and keep old ApiVers unchanged. +4. **Tests that do not pin the fix.** Revert the change and the new test must fail. Cover each input + branch (file and stdin `-`, `b2://` and `b2id://`, every ApiVer the command is registered in), in a + test placed with its topic, not appended to an unrelated block. +5. **stdout and stderr are a contract.** `--quiet` prints nothing on stdout. JSON output stays parseable. + Warnings and deprecations go to stderr, and users monitor stderr, so do not add noise there. +6. **Developer knobs in user help.** Keep internal flags and env vars out of `--help` and README; document + them in `doc/` instead. +7. **Integration tests that share state.** They run in parallel against one live account: no fixed bucket + names, no assumptions about bucket list order, clean up what you create. Before blaming a PR for a red + integration job, check whether `master` is red too. +8. **b2sdk pins.** Depend on a released `b2sdk` within the declared range, never a git URL (the release + session refuses one). Fix SDK behavior in the SDK. +9. **CI hygiene.** Pin third-party actions to a commit SHA, drop runner images GitHub retires, keep + `cd.yml` and `ci.yml` runner versions aligned. +10. **Proof that did not run.** A skipped check is NOT RUN, never PASS. Tests can pass under nox and fail + under bare pytest (forked tests); run them through nox. +11. **PR text that disagrees with code.** Re-read the description after the last commit. +12. **Undocumented options.** Every new flag goes in the command docstring with what it does, its + choices and default, plus costs or limits users would otherwise learn from support (transactions, + size limits, inclusive ranges). +13. **Happy-path-only tests.** Test the failure paths and assert the error message; parametrize instead of + copying a test to vary one input. +14. **Unsafe destructive defaults.** Commands that delete (`rm`, `sync --delete`) must not be easy to + trigger by accident, and filters must be tested together with deletion. A safer default is still a + breaking change: add an opt-in flag in the current major. +15. **Unrelated changes in the PR.** Refactors, formatter runs and side fixes go in their own PR or an + issue; explain the reason for dependency and config changes in the description. + +The full list, by theme and with the PRs where each came up, is in +[doc/review-lessons.md](doc/review-lessons.md). Add to it when a review teaches something new. + +## Workflow (AI-native SDLC) + +- Non-trivial changes start in `intent/<slug>/`: `intent.md` (problem) → `spec.md` (numbered requirements + R1..) → `plan.md` (steps cite R numbers). A human approves each before the next. Templates in + [`intent/README.md`](intent/README.md). Bug fixes with a linked issue may skip straight to a failing test. +- Bug fixes: commit the failing regression test first, then the fix, without editing the test. +- Skills in `.claude/skills/` (`ai-native-sdlc` routes to the right one). A `PostToolUse` hook in + `.claude/settings.json` rejects edits that leave a Python file unformatted. +- Before opening a PR: run the command you changed end to end (`python -m b2 ...` against the simulator + or a throwaway live bucket) and paste the output; then review your own diff in a fresh context with + `/code-review` (bugs), `/simplify` (a smaller diff) and `/security-review`, or a subagent that did not + write the code. Fix or answer what they find and say in the PR what they reported. When the same + finding keeps coming back, fix the cause (a ruff rule, a test, a line here) rather than each instance. +- Keep PRs small and reversible; say how to revert in the PR template. +- Review: [REVIEW.md](REVIEW.md). Every finding gets fixed or resolved with a reason. Agents never approve + or merge their own PRs; a maintainer approves. +- The person directing an agent owns its PR: they count as one of its reviewers and fix what it breaks. +- Releases: [RELEASE.md](RELEASE.md). Security reports: [SECURITY.md](SECURITY.md). + +## Verifying your work + +Run `nox -s ci` and paste the tail of its output. codespell (changed files), lint, `towncrier_check`, +`changelog_check`, build, unit tests and the offline integration tests must pass. If a test fails, fix the code, not the test. +List the live-B2 integration tests you ran, and say NOT RUN for the ones you could not. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 000000000..2e37e75ad --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,63 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, religion, or sexual identity and +orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment include: + +- Demonstrating empathy and kindness toward other people +- Being respectful of differing opinions, viewpoints, and experiences +- Giving and gracefully accepting constructive feedback +- Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +- Focusing on what is best for the overall community + +Examples of unacceptable behavior include: + +- The use of sexualized language or imagery, and sexual attention or advances of + any kind +- Trolling, insulting or derogatory comments, and personal or political attacks +- Public or private harassment +- Publishing others' private information, such as a physical or email address, + without their explicit permission +- Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Project maintainers are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the project maintainers responsible for enforcement at +**conduct@backblaze.com**. All complaints will be reviewed and investigated +promptly and fairly. All maintainers are obligated to respect the privacy and +security of the reporter of any incident. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +https://www.contributor-covenant.org/version/2/1/code_of_conduct.html. + +[homepage]: https://www.contributor-covenant.org diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 75402ac08..867c81e77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,7 +52,49 @@ These files can either be created manually, or using `towncrier` e.g. `towncrier create` also takes care of duplicates automatically (if there is more than 1 news fragment of one type for a given github issue). + +The entry is one or more complete sentences ending with a full stop, written for users. CI, test, build and tooling +changes use `infrastructure`; documentation changes use `doc`. Behavior changes that can affect scripts (exit codes, +output, new stderr messages) must say so. Check your fragments locally with: + + nox -s towncrier_check changelog_check + +A PR that only changes `uv.lock` needs no entry; otherwise a maintainer can add the `-changelog` label to skip the +check. +## Working with coding agents + +Most of the process here is written for people and coding agents alike: + +* [AGENTS.md](AGENTS.md) (read by Claude Code through `CLAUDE.md`): commands, architecture, conventions and the + mistakes reviewers keep finding. Read it before your first PR. +* [REVIEW.md](REVIEW.md): how PRs are reviewed, the severity of findings and how to answer them. +* [doc/review-lessons.md](doc/review-lessons.md): what reviewers asked for on past PRs, by theme. Check your PR + against it before asking for review. +* [doc/reviewer-evals.md](doc/reviewer-evals.md) and `nox -s review_metrics`: how the review process itself is + measured. An AI code review and security review comment on PRs once maintainers enable them; answer their + findings like any other review comment. +* [intent/README.md](intent/README.md): larger changes start with an `intent.md`, then `spec.md` and `plan.md`, + each approved by a person before the next. Bug fixes with a clear issue can start from a failing test. +* `.claude/skills/`: the AI-native SDLC playbook skills (`ai-native-sdlc` is the entry point). + +If an agent wrote your PR, you own it: you review it before asking others to, and you answer the review. + +## Local CI + +`nox -s ci` runs what CI's lint, build and test jobs run, on Python 3.12: codespell on the files you changed, `lint`, `towncrier_check`, +`changelog_check`, `build`, unit tests and the integration tests that need no credentials (plus the live-B2 ones +when `B2_TEST_APPLICATION_KEY` and `B2_TEST_APPLICATION_KEY_ID` are set). Run it before every push. Set +`NOX_CI_PYTHON` to pick the interpreter, `NOX_CI_BASE` for a base other than `origin/master`, and +`NOX_CI_SKIP_CHANGELOG=1` when the PR has the `-changelog` label. Note that `nox -s lint` reformats files in place +rather than failing on formatting, so commit what it changes. + +## Releases and security + +Maintainers release by following [RELEASE.md](RELEASE.md). Report vulnerabilities privately as described in +[SECURITY.md](SECURITY.md), never in a public issue. Everyone taking part follows the +[Code of Conduct](CODE_OF_CONDUCT.md). + ## Developer Info You'll need to have [nox](https://github.com/theacodes/nox) and [uv](https://docs.astral.sh/uv/) installed: diff --git a/README.release.md b/README.release.md deleted file mode 100644 index 3bd2d6668..000000000 --- a/README.release.md +++ /dev/null @@ -1,5 +0,0 @@ -# Release Process - -- Run `nox -s make_release_commit -- X.Y.Z` where `X.Y.Z` is the version you're releasing -- Copy the main usage string (from `b2 --help`) to `README.md`. Handy command for consistent format: `COLUMNS=4000 b2 --help | awk '/^usages:/ {p=1; next} p {sub(/^ */, "", $0); print}'` - diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 000000000..3e4175b83 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,64 @@ +# Releasing the B2 CLI + +A release is a changelog commit on `master` (titled `release X.Y.Z` or `vX.Y.Z`, for example `9ea898a`, +v5.0.2) and a `vX.Y.Z` tag on it. The version comes from the git tag (hatch-vcs); no file holds it. Pushing the tag runs +[`cd.yml`](.github/workflows/cd.yml), which publishes everything. + +## Before you start + +- `master` is green in CI, including the live-B2 integration jobs. +- `pyproject.toml` depends on a released `b2sdk` (no git URL); `make_release_commit` refuses otherwise. + If the release needs a new SDK, release `b2-sdk-python` first and bump the pin in its own PR. +- `changelog.d/` holds a fragment for every change since the last tag, and `nox -s towncrier_check` + passes. Review the wording: it becomes the GitHub release body. `towncrier build --draft --version X.Y.Z` + previews the section. + +## Versioning + +Semantic versioning. A `removed` or breaking `changed` fragment needs a major bump and, usually, a new +ApiVer (`b2vN`) so scripts on the old interface keep working. `added`, `changed` or `deprecated` means +minor; only `fixed`, `doc` and `infrastructure` means patch. Maintainers usually batch trivial changes into the next +release rather than cutting one for each. + +## Steps + +1. On an up-to-date `master` with a clean tree, run `nox -s make_release_commit -- X.Y.Z` (no leading + `v`). It runs `towncrier build`, which writes the `## [X.Y.Z]` section of `CHANGELOG.md` and deletes + the fragments. +2. Update the usage block in `README.md` from `b2 --help`: + `COLUMNS=4000 b2 --help | awk '/^usages:/ {p=1; next} p {sub(/^ */, "", $0); print}'` +3. Commit as `release X.Y.Z`. Today it is pushed to `master` directly, as `make_release_commit` prints; + `intent/review-process/intent.md` proposes a PR instead, so CI and a maintainer see it before the tag. +4. Once it is on `master` and CI there is green, tag that commit and push the tag: + `git tag vX.Y.Z && git push upstream vX.Y.Z`. +5. Watch `cd.yml` to the end. The release is done when every job passed. + +## What `cd.yml` publishes + +| Job | Output | +| --- | --- | +| `deploy` | sdist and wheel on PyPI (skipped for pre-release tags such as `v5.1.0rc1`), GitHub release with the changelog section as its body | +| `deploy-linux-bundle` | Standalone Linux binaries (x86_64 and arm64), with hashes, attached to the GitHub release | +| `deploy-windows-bundle` | Signed Windows binary, with hashes, attached to the GitHub release | +| `deploy-docker` | `backblazeit/b2` Docker image (not for pre-releases) | + +Runner images in `cd.yml` must match the bundle jobs in `ci.yml`, so what was tested is what ships. + +## Staged releases + +New or risky behavior can ship first as a pre-release tag (`vX.Y.ZrcN`, `aN`, `bN` or `devN`). `cd.yml` +then publishes the GitHub release and standalone binaries but skips PyPI and Docker, so early users can try +it before `pip install b2` picks it up. ApiVer is the CLI's flag mechanism: incompatible behavior goes to +the next `b2vN`, and scripts pinned to an older one keep the old behavior. + +## If something goes wrong + +- A failed `cd.yml` job can be re-run from the Actions tab without a new tag. +- To roll back a bad release in one step, yank it on PyPI (`pip` then skips it unless pinned) and mark the + GitHub release as a pre-release; then fix forward. +- After a post-release fix, ask which layer should have caught it (a test, `REVIEW.md`, the AI review) and + add the case to [doc/reviewer-evals.md](doc/reviewer-evals.md). +- Never move or delete a pushed tag that reached PyPI; PyPI does not accept the same version twice. Fix + forward with a patch release. +- A security fix ships as a patch release with a `fixed` fragment and a GitHub security advisory (see + [SECURITY.md](SECURITY.md)). diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 000000000..d7e56fbbf --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,133 @@ +# Review instructions + +For human reviewers and review agents. Approval always comes from a maintainer; an agent's findings never +approve or block on their own, and the agent that wrote a change never reviews it. Every PR also gets an +automated AI review and security review (`.github/workflows/ai-review.yml`) once a maintainer adds the +`ANTHROPIC_API_KEY` secret and installs the Claude GitHub App; their findings follow the same rules as +everyone else's. + +Review has two jobs: find and fix problems before they ship, and keep the people who own this code +understanding it. + +## What failure looks like here + +The B2 CLI fails by breaking a script that depends on it: a changed flag, output format, exit code or +stderr line in an ApiVer that promised to keep it. It also fails by sending the wrong request through +b2sdk (wrong bucket, file, encryption or retention setting), by deleting or overwriting data the user did +not ask to touch (`rm`, `sync --delete`, `--replace-newer`), by leaving buckets, keys or unfinished large +files behind, or by leaking a credential into output, logs or `b2_cli.log`. A test that passes either way +is also a failure: it claims coverage it does not give. + +## Review depth + +Match the effort to the blast radius of the paths the PR touches. + +| Depth | Paths | What it means | +| --- | --- | --- | +| Quick | `*.md`, `doc/`, `changelog.d/`, `intent/`, `test/` only | Read the diff; check claims against the code they describe. | +| Standard | `b2/_internal/` (except below), `noxfile.py` test sessions, `.sdkharness/`, b2sdk bumps | All passes below; run `nox -s ci`. | +| Deep | `rm`, `sync`, `delete-*`, file and bucket updates, `account authorize` and account info, `b2v*/registry.py`, `.github/workflows/`, `noxfile.py` bundle/sign/release sessions | All passes, read every caller and the b2sdk code involved, and exercise the command end to end (simulator or live B2). | + +## Skip + +Do not review or comment on: `uv.lock` (check only that it matches `pyproject.toml`), `CHANGELOG.md` +(generated at release), `doc/source/subcommands/` and other generated docs, vendored license files, and +anything ruff, codespell, the license-header test or `towncrier_check` already enforces. Style, +diagnostic-only and cleanup-only remarks are not findings. + +## Before the passes + +1. Read the linked issue and, if present, `intent/<slug>/spec.md` and `plan.md`. Keep + [doc/review-lessons.md](doc/review-lessons.md) at hand: it lists what earlier reviews found. +2. Check out the PR merged onto current `master` and run `nox -s ci`. Say what you ran. +3. For each behavior the PR claims to fix or add, revert that line and confirm a test fails. Name any + behavior no test pins. +4. Check the diff against the whole repo, not only the changed lines: does it solve the stated problem, + fit the system, and avoid breaking callers elsewhere? For a bug fix, check that it addresses the root + cause rather than one symptom, and look for the same bug in sibling commands. + +## Passes + +Tag each finding with its pass. + +- **Bugs**: logic errors, wrong defaults, error paths that swallow failures or exit 0, unclosed files or + progress bars, threads and pools left running. +- **Compatibility**: every ApiVer a command is registered in (`b2/_internal/b2v*/registry.py`) keeps its + flags, output and exit codes; deprecations warn on stderr; `b2` (latest) changes are announced in a + `changed` or `removed` fragment that says what users must do. +- **Output**: stdout vs stderr is deliberate; `--quiet` prints nothing on stdout; JSON output stays valid + and stable; help text reads well and keeps developer-only knobs out; messages use the invoked binary name + (`self.console_tool.b2_binary_name`), not a hard-coded `b2`. +- **Data safety**: destructive commands need the flags the user gave, honor `--dry-run`, and touch only + the bucket, prefix or versions requested. +- **Security**: application keys, auth tokens and SSE-C keys never reach logs (`--verbose`, + `--debug-logs`, `b2_cli.log`), error messages or stderr, and stdout only where a command documents it + (`account get` prints the key and auth token, `key create` prints the new key); no new command or flag + prints them without saying so in its help. The account info database is never copied, and nothing new + is sent outside B2. +- **Compliance**: the change matches the issue, `spec.md` and `plan.md`; the PR description matches the + code; a `changelog.d` fragment exists, has the right type and reads well for users; `AGENTS.md` is + updated when the change makes it stale or breaks a convention in it. + +## Severity + +- **Important**: breaks existing scripts or an ApiVer promise, deletes or leaks data or credentials, + sends a wrong request, or leaves resources behind. A claimed behavior with no test pinning it is + Important. +- **Nit**: style, naming, comments. At most five per review; summarize the rest as a count. +- **Pre-existing**: a real bug this PR did not introduce. Report it so it is not lost, but it does not + block this PR; suggest an issue. + +## Verifying findings (review agents) + +Find candidates broadly, then verify each one before posting: read the code it depends on, and drop it +unless the input or sequence that triggers it is real. When several review agents run, a separate +verifying pass decides which findings are posted; unverified findings are not posted. Prefer one real +Important over ten speculative remarks. + +## Evidence + +Every finding cites `file:line` and says what goes wrong, with the command line or sequence that +triggers it. Do not infer behavior from a function's name; read it, including the b2sdk code it calls. +If you could not verify something (no live B2 credentials, no simulator), say so. + +## Converging + +- Round 1 reviews the whole diff. From round 3 on, review only the commits since your last review. +- Name the class of bug, not one instance, so the author fixes every occurrence in one round. +- Never repeat a finding already posted on the PR, resolved or not. +- After three rounds with Important findings still open, stop and ask the author to reconsider the + approach instead of posting more line comments. +- A red live-B2 integration job that is also red on `master` is not a finding against the PR. + +## Answering findings + +Every finding gets an answer before merge: fix it, or resolve the thread with a one-line reason +(false positive, intentional, out of scope with an issue link). Do not resolve a thread silently. A +finding that a later commit fixes is resolved by that commit; say which one. + +When the same finding shows up on several PRs, fix it at the source in the same change: a ruff rule or +test, a line in `AGENTS.md`, or an entry in [doc/review-lessons.md](doc/review-lessons.md). + +## What people still review + +Automated review does not replace these, and they are the reason a maintainer approves: + +- Whether the change should be built at all: scope, direction and fit with the CLI's interface. Raise + direction changes early in the issue or the first review, not as blocking comments on a finished PR; + closing a PR is cheap. +- Risky code: the Deep paths above, especially destructive commands, credentials and CI workflows. +- Patterns others will copy: a new command layout, argument type or output format. +- The evidence: test output, commands run and their results, and the reviewer findings. If you cannot + say what the checks would miss, review the code itself. +- Ownership: the person who lands a change owns fixing what it breaks, whoever wrote it. + +For a trivial diff in code you do not know, ask Claude for an explainer of that slice of the code instead +of reading it line by line. + +## Format + +Start with a count (`2 Important, 1 Nit, 0 Pre-existing`, or `No issues found`), then what you checked +and against what (issue number, live B2 or simulator, commands run). Then a table sorted by severity with +the columns `Severity | Pass | File:Line | Finding`. Do not report generated files or what CI already +enforces (ruff, codespell, license headers, `towncrier_check`). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 000000000..c827654b3 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,72 @@ +# Security Policy + +Security-response owner: Backblaze Security. + +## Supported Versions + +The latest release of the `b2` CLI is supported. Fixes ship as a new release on top of the latest one; +older releases are not patched in place. Every ApiVer entry point in that release (`b2`, `b2v3`, `b2v4`, +`b2v5`) receives the fix. See [`RELEASE.md`](RELEASE.md). + +| Version | Supported | +|---|---| +| Latest release (`5.x`) | Yes | +| Older releases | No | + +## Reporting a Vulnerability + +**Do not file a public GitHub issue for a security vulnerability.** + +Instead, report it privately via GitHub Security Advisories: + +1. Go to the [Security tab](https://github.com/Backblaze/B2_Command_Line_Tool/security/advisories/new) of this repository +2. Click **Report a vulnerability** +3. Provide a clear description, the affected version, reproduction steps, and impact assessment + +You can also email **security@backblaze.com** if you cannot use GitHub Security Advisories. + +## What to expect + +- Acknowledgement within 3 business days +- Initial assessment within 7 business days +- Coordinated disclosure once a fix is available, shipped as a patch release with a changelog entry and a + GitHub security advisory + +## Scope + +In scope: + +- Credential-handling flaws in the CLI: application keys, authorization tokens or SSE-C keys exposed in + output, logs (`--verbose`, `--debug-logs`, `b2_cli.log`), errors, shell history or to unintended hosts +- Commands that delete, overwrite or expose data beyond what the user asked for (`rm`, `sync`, file and + bucket updates) +- The published artifacts: PyPI packages, standalone binaries and the `backblazeit/b2` Docker image + +Out of scope: + +- Vulnerabilities in the Backblaze B2 service itself — report those to Backblaze directly +- Vulnerabilities in `b2sdk` — report them to [b2-sdk-python](https://github.com/Backblaze/b2-sdk-python/security) +- Credentials the user passes or stores insecurely, such as a master key in a shared script +- Issues requiring root or local access on the machine running the CLI + +## Security-relevant areas + +- **Credentials**: `b2 account authorize` reads the key from arguments, a prompt, or + `B2_APPLICATION_KEY_ID` / `B2_APPLICATION_KEY`, and stores the resulting authorization in a local SQLite + database (`B2_ACCOUNT_INFO`, or the default path; `b2 account get` shows which). Anyone who can read that file + can act as that key. +- **SSE-C keys**: passed through `B2_SOURCE_SSE_C_KEY_B64` / `B2_DESTINATION_SSE_C_KEY_B64` and sent to + B2 in request headers. They are not persisted. +- **Logs**: `--debug-logs` writes `b2_cli.log` in the current directory; do not run it inside a directory + you sync. +- **Transport**: all B2 API traffic uses HTTPS through `b2sdk`. +- **Tests and CI**: live integration tests read `B2_TEST_APPLICATION_KEY` and `B2_TEST_APPLICATION_KEY_ID` + from repository secrets. The `.sdkharness` checks refuse any non-loopback endpoint and drop ambient + `B2_*` variables. Releases sign the Windows binary and publish hashes for every bundle. + +## Best practices for users + +- Use application keys scoped to specific buckets, prefixes and capabilities. +- Prefer environment variables or the prompt over passing keys as arguments. +- Protect the account info database and run `b2 account clear` on shared machines. +- Rotate application keys regularly, and verify the published hashes of standalone binaries. diff --git a/changelog.d/+ai-native-sdlc.infrastructure.md b/changelog.d/+ai-native-sdlc.infrastructure.md new file mode 100644 index 000000000..d9c2fd4a9 --- /dev/null +++ b/changelog.d/+ai-native-sdlc.infrastructure.md @@ -0,0 +1 @@ +Add `AGENTS.md`, `REVIEW.md`, `RELEASE.md`, `SECURITY.md`, a code of conduct, issue forms, a PR template, the AI-native SDLC playbook skills, and the `nox -s ci` and `nox -s changelog_check` sessions. diff --git a/changelog.d/+ai-review.infrastructure.md b/changelog.d/+ai-review.infrastructure.md new file mode 100644 index 000000000..a8f60016d --- /dev/null +++ b/changelog.d/+ai-review.infrastructure.md @@ -0,0 +1 @@ +Add an advisory AI code review and security review workflow (inactive until `ANTHROPIC_API_KEY` is set), review depth and verification rules in `REVIEW.md`, a reviewer eval set in `doc/reviewer-evals.md`, and a `nox -s review_metrics` report. diff --git a/changelog.d/+review-lessons.doc.md b/changelog.d/+review-lessons.doc.md new file mode 100644 index 000000000..11c0ae1c6 --- /dev/null +++ b/changelog.d/+review-lessons.doc.md @@ -0,0 +1 @@ +Add `doc/review-lessons.md`, a checklist of what reviewers asked for on all merged PRs, grouped by theme with links to the PRs. diff --git a/doc/review-lessons.md b/doc/review-lessons.md new file mode 100644 index 000000000..3b6f4cd95 --- /dev/null +++ b/doc/review-lessons.md @@ -0,0 +1,138 @@ +# Review lessons + +What reviewers asked for on the merged PRs of this repository, grouped by theme, with the PRs where it came +up. It comes from all 692 reviewer comments (inline, review and conversation, excluding the PR author's own +replies and bots) on the 571 PRs merged up to #1191. Copilot had not reviewed any merged PR at that point. + +Use it as a checklist before asking for review. When a review teaches something that will matter again, +add a line here with the PR link in the same change. Rules that apply to every change, and that came up on +at least two PRs, are also in [AGENTS.md](../AGENTS.md#things-agents-get-wrong-here). + +PRs before #550 predate the split of the library into +[b2-sdk-python](https://github.com/Backblaze/b2-sdk-python); lessons from them about SDK internals (retries, +upload URLs, account info storage) now belong there, and are kept here only when they still apply to the CLI. + +## Compatibility and defaults + +- Changing a default or removing behavior that scripts can rely on is a major-version change, even when the + old behavior is dangerous. Ship a safer opt-in flag in the current major. (#370, #667) +- Do not document a bug as expected behavior; once documented it becomes part of the interface. (#667) +- Do not document a future mode or option before it exists; that commits the next release to it. (#667) +- New warnings on stderr can set off users' alerts. Deprecations go to stderr by default, so add them + deliberately and keep old ApiVers quiet. (#1000) +- A change that makes a script exit non-zero where it used to exit 0 must say so in the release note. + (#1165) +- Leave room in a new interface for the next obvious option (for example `--sourceFileId` now, so a name + option can be added later without breaking it). (#586) + +## Data safety + +- Destructive operations must not be easy to trigger by accident: an empty sync source must not delete the + destination (`--allow-empty-source`). (#370) +- Decide and test how filters interact with deletion: is an excluded file deleted at the destination? + (#153) +- Never catch broad exceptions around code that can delete or reset user state; Ctrl+C during a slow read + must not wipe account settings. Catch the specific exceptions. (#130) +- When a download fails, remove the partial local file, and test every destination type that inherits the + behavior. (#533) +- Report unusable input clearly rather than silently skipping it; users expect all of their data to be + synced. (#424, #1165) + +## Tests + +- Every new behavior and every bug fix needs a test; reviewers block on this more than on anything else. + (#48, #153, #179, #227, #307, #332, #424, #470, #512) +- Test failure paths, not only the happy path, and assert the error message, not just that something was + raised. (#295, #501, #411) +- When an assertion is removed, the function now accepts new input: add a test for that input. (#332) +- One command per test case, with a name that says what it checks, so a failure list reads on its own. + (#89, #924) +- Do not copy a test to vary one input; extract a helper or parametrize. (#295) +- Cover each input branch: stdin `-` as well as a file, files and directories for filters, both sides of a + boundary. (#1165, #407, #337) +- Know which layer you are testing: unit tests run against the simulator (`test/unit`), integration tests + against live B2 (`test/integration`). Behavior both depend on needs both. (#424, #470, #512) +- The simulator can differ from B2 (for example hiding a file that does not exist). Check live behavior + before trusting a simulator-only test. (#426) +- Use reserved domains (`example.com`) in tests, never real ones like `foo.com`. (#303) +- Use the existing test helpers (`_make_local_file`, `_run_command`) instead of new ad-hoc setup. (#512) +- Put a new test in the section it belongs to, not in an unrelated block. (#1163) +- Mixing forked and non-forked tests in one module breaks under pytest 8; keep them in separate modules. + (#1054) + +## Help text and documentation + +- Every new option is documented in the command's docstring: what it does, its choices and its default. + (#154, #375, #407, #758) +- Document cost and limits users would otherwise discover from support or their bill: listing a bucket + incurs transactions; copies over 5 GB are not supported. (#423, #586) +- Say whether ranges are inclusive or exclusive; HTTP byte ranges are inclusive at the end. (#465, #534, + #586) +- Write help for the user, not the implementation: "accepts a JSON string" rather than "formatted JSON". + (#905, #407, #375) +- Keep developer-only knobs (`--environment`, `--dev`, `B2_ENVIRONMENT`) out of the main help and README; + document them under `doc/`. (#1163) +- Keep the README in sync with the commands it describes, including paths that changed (profiles moved the + account database). (#451, #1163) +- `b2 --help` should stay compact and readable. (#629) + +## Output and errors + +- `--quiet` means nothing on stdout, like `curl` and `wget`; it implies `--no-progress`. (#924, #929) +- An invalid combination of arguments is an error, not a silent fallback or a warning. (#244, #629) +- Error messages tell the user what to run next, using the binary name they invoked. (#1080, #398) +- Format exception messages in the exception class, not at the call site, and define exceptions at module + level so callers can catch them. Group related exceptions under a common base class. (#202, #303, #459, + #899) +- Never log authorization tokens or keys; log the URL that helps find the request server-side. (#287) +- When falling back (for example to ASCII when locales are not set), log it so `--verbose` shows why. + (#768) + +## B2 semantics + +- Paginate list calls to the end (`nextFileName`, `nextApplicationKeyId`); a CLI user listing keys or files + wants all of them. (#451) +- Stream large listings instead of building a list in memory; buckets can hold close to a billion files. + (#423) +- B2 file name limits are in UTF-8 bytes, not characters. (#424) +- B2 names always use `/`; do not use `os.path` on them, which uses `\` on Windows. (#407) +- Hidden files still count toward bucket size; hide markers count as files with size 0. (#423, #426) +- Restricted application keys: give a clear error when the bucket on the command line is outside the key's + restriction, and document the capabilities each command needs. (#459, #470, #471, #532) + +## Python style + +These are mostly enforced by ruff now; they still show up in review. + +- No mutable default arguments; use `None` and create the value inside. (#139, #899) +- Compare with `is not None` when `None` is the sentinel; an object may define truthiness later. (#173) +- Do not shadow builtins (`file`, `id`, `dir`, `format`). (#83, #93, #139, #650) +- Prefer `@classmethod` to `@staticmethod`; make a value used in two places a constant. (#459, #346, #337) +- Trailing commas on multi-line literals keep diffs to one line. (#191, #412, #459, #512) +- Remove leftover debug prints, commented-out code and stale comments. (#103, #300, #337) +- Split long functions by responsibility (for example one signing function per OS). (#746) +- Use the database's own features (`INSERT OR IGNORE`) instead of reimplementing them. (#459) + +## Shell, CI and release + +- Quote every shell expansion (`"$var"`, `"$(...)"`). (#62, #398) +- Pin third-party GitHub Actions to a commit SHA. (#760) +- Make skipped CI steps say so (`else echo "skip"`), and do not fail PRs on optional services such as + coverage upload. (#650) +- Check dependency changes early, not only at package build time (license check, version conflicts with + b2sdk). (#650, #769) +- Pin around known-bad releases (for example `b2sdk>=1.22.1`), keep CLI and SDK version bounds + compatible, and require the b2sdk version that ships a feature the CLI uses. (#897, #769, #800, #667) +- Release-tag regular expressions must match every PEP 440 pre-release form: `(a|b|rc|dev)[0-9]+$`. + (#1108) +- Infrastructure-only changes the user cannot notice stay out of user-facing changelog sections. (#763) +- Credit outside contributors in the changelog entry. (#743) + +## PR scope + +- Keep unrelated fixes, refactors and formatter runs out of a feature PR; open a separate PR or an issue. + (#370, #451, #459, #558) +- Explain the reason for a change in the PR description, especially for dependency, domain or config + changes. (#212, #550, #714) +- File an issue for every follow-up deferred during review, so nothing is lost. (#650, #512) +- Merge current `master` before asking for review when CI failures may come from it. (#213, #436, #512) diff --git a/doc/reviewer-evals.md b/doc/reviewer-evals.md new file mode 100644 index 000000000..c01fd6976 --- /dev/null +++ b/doc/reviewer-evals.md @@ -0,0 +1,49 @@ +# Reviewer evals + +A set of real bugs, used to check that a reviewer (the AI review in `.github/workflows/ai-review.yml`, a +review agent, or `REVIEW.md` itself) catches what matters, without burying authors in noise. Rescore the +reviewer whenever `REVIEW.md`, `AGENTS.md`, the review prompt, its tools or its model change, and keep a +change only if the score does not drop. Catching real bugs comes first; fewer review rounds second. + +## Cases + +Each case is a bug that a human reviewer caught or that shipped and was fixed later. A maintainer confirms +each one (the bug and where it lived) before it joins the set. To run a case, check out the commit that +contains the bug (the PR head before the fix, or the parent of the fix), give the reviewer that diff, and +count it as caught only if a finding names the right `file:line` and the failure. Paths are given as they +are today; older cases lived in `b2/console_tool.py`. + +| Case | Bug | Where | Commit with the bug | Source | +| --- | --- | --- | --- | --- | +| E1 | A helper returned `False` where callers unpack `found, altered`; the command fails with `TypeError` | `console_tool.py`, `ReplicationRuleChanger` | #802 head before review | Caught in review, #802 | +| E2 | `update-bucket` without `--lifecycle-rules` cleared the bucket's existing lifecycle rules | `console_tool.py`, `BucketUpdate` / `UpdateBucket` | parent of #925 | Shipped, fixed in #925 | +| E3 | `file upload --incremental-mode` on stdin or a pipe silently did a full upload and exited 0 | `console_tool.py`, `UploadFileMixin` | parent of #1165 | Shipped, issue #1164 | +| E4 | `list-keys` printed only the first page of keys | `console_tool.py`, key listing (`KeyListBase`) | #451 head before review | Caught in review, #451 | +| E5 | `sync` with an empty source and `--delete` deleted every file at the destination | `console_tool.py`, `Sync` | parent of #370 | Shipped, fixed in #370 | +| E6 | Developer-only realm settings (`B2_ENVIRONMENT`) were documented in the main `--help` | `console_tool.py`, README | #1163 head before review | Caught in review, #1163 | + +Add a case for every revert and every post-release fix PR, in the same change as the fix when possible. +Bugs in library code (retries, upload URLs, account info storage) belong in +[b2-sdk-python](https://github.com/Backblaze/b2-sdk-python)'s set instead. + +## Clean PRs + +Recall is half the score. Also run the reviewer on known-clean PRs (merged, with no follow-up fix or +revert, for example docs-only and dependency-bump PRs) and count findings tagged Important that would have +led to a harmful or pointless change. That count should be zero. + +## Noise on live PRs + +For each finding the reviewer posts on a real PR, record how the author answered it: **fixed**, **wrong** +(false positive), **intentional**, or **didn't matter**. Use those words in the thread reply when resolving +(see [REVIEW.md](../REVIEW.md#answering-findings)) so they can be counted. A rising share of wrong and +didn't-matter answers means the reviewer needs tuning, even if recall is fine. + +## Reporting + +Record each scoring run as a row: date, what changed (prompt, model, `REVIEW.md` commit), cases caught out +of the total, Important findings on clean PRs, and median review rounds to pass from +`nox -s review_metrics`. + +| Date | Change | Caught | Important on clean PRs | Median rounds | +| --- | --- | --- | --- | --- | diff --git a/intent/README.md b/intent/README.md new file mode 100644 index 000000000..f137727f5 --- /dev/null +++ b/intent/README.md @@ -0,0 +1,65 @@ +# Change intents + +Each non-trivial change gets a folder `intent/<change-slug>/` holding three files, written in order. A +human approves each one before the next is written; the approval is a commit or a PR review, not an +agent's edit to the status line. + +| File | Written by | Approved by | Holds | +| --- | --- | --- | --- | +| `intent.md` | Originator (with Claude) | Maintainer | Problem, outcome, constraints, open questions. No design. | +| `spec.md` | Claude, from the intent | Maintainer; area owner for flagged concerns | Numbered requirements R1.., design, constraint table, concerns. | +| `plan.md` | Claude in plan mode | Engineer doing the work | Files, ordered steps citing R numbers, risks, proof. | + +Small fixes that already have a GitHub issue with a clear reproduction can skip `intent/` and start from a +failing test. Link the GitHub issue in the `intent.md` header when one exists, and the intent folder in the +issue. + +The `ai-native-sdlc` skill in `.claude/skills/` looks at what exists and routes to the next step. + +## Templates + +### intent.md + +```markdown +# Intent: <short title> +Author: <name>. Issue: #<n>. Status: draft. + +## Problem +## Proposed outcome +## Affected users and APIs +## Constraints +## Open questions +``` + +### spec.md + +```markdown +# Spec: <title> +From: intent.md (<author>). Status: ready for review. + +## Requirements +- R1. <testable statement: command, flags, output, exit code> + +## Design +<which commands and ApiVers change, new flags, output shape, b2sdk calls> + +## How each constraint is met +| Constraint | Met by | +| --- | --- | + +## Areas of concern +1. <concern>. Owner: <@handle>. +``` + +### plan.md + +```markdown +# Plan: <title> + +## Files that change +## Order of work +1. <step> (R1). +## Risks +## Proof +- R1: <test name, and whether it fails when the change is reverted> +``` diff --git a/intent/review-process/intent.md b/intent/review-process/intent.md new file mode 100644 index 000000000..54beb2173 --- /dev/null +++ b/intent/review-process/intent.md @@ -0,0 +1,111 @@ +# Intent: review and merge process scaled by risk +Author: Gonzalo Peña-Castellanos (with Claude). Issue: #1192. Status: draft, for the maintainers. +Source: [How the Claude Code team does code review](https://claude.dev/blog/solving-code-review-at-anthropic/), +applied to the B2 CLI; the same proposal as Backblaze/blazer#152, adapted. + +## Problem + +More B2 CLI PRs are now written with coding agents. Reviewers repeatedly ask for the same things +(changelog fragments, fragment types, tests per input branch, stdout/stderr discipline), and a PR's +live-B2 integration jobs can fail for reasons unrelated to it. + +Today: + +- Every PR gets the same human attention whatever its risk; a docs fix and a change to `rm`, `sync` or + credential handling are treated alike. +- There is no `.github/CODEOWNERS`, so nothing routes a change to the person who knows that area. +- Which branch protection applies to `master` could not be confirmed without admin access. +- Review findings can be left unanswered at merge. +- Nothing measures whether a reviewer change (REVIEW.md, AGENTS.md, model) catches more or fewer real + bugs. + +The PR that adds this file already covers what needs no settings change: a short `AGENTS.md`/`CLAUDE.md`, +a formatter hook, `REVIEW.md` (failure modes, review depth by path, paths to skip, severities, verified +findings with `file:line` evidence, a count-first summary, round caps, what people still review), Risk and +How-to-revert in the PR template, `/code-review`, `/simplify` and `/security-review` before a PR opens, an +advisory AI code review and security review workflow (`.github/workflows/ai-review.yml`, inactive until the +secret is set), a seed eval set (`doc/reviewer-evals.md`), and a metrics report (`nox -s review_metrics`). +On the last 120 days it reports 24 merged PRs, a median of 0 human approvals per PR (p95 1), and a median +of 5.8 hours (p95 147) to the first human review. + +## Proposed outcome + +Human review time goes where the risk is, with no rise in reverts or post-release fixes: + +1. **Risk tiers set by the change, not the author.** A first proposal to accept or change: + + | Tier | Paths | Human approvals | + | --- | --- | --- | + | Low | `*.md` docs, `doc/`, `test/` only, `changelog.d/`, `intent/` | None once CI and automated review are green, for authors on the trusted list | + | Medium | `b2/_internal/` (except below), `.sdkharness/`, `pyproject.toml`/`uv.lock` b2sdk bumps | One maintainer | + | High | Destructive commands (`rm`, `sync`, `delete-*`), authorization and account info, `b2v*/registry.py` (ApiVer promises), `.github/workflows/`, `noxfile.py` release/bundle/sign sessions, release commits | One maintainer always; two for workflow and signing changes | + + Authors outside the trusted list always need a person. A bot's approval never fills a human slot. + A check (a path classifier first, an AI classifier later) sets the tier from the diff, and a second + check can still ask for a person when a Low change looks risky. Proposed `.github/CODEOWNERS`, with + owners to be filled in by the maintainers: + + ```text + * @Backblaze/<cli-maintainers> + /.github/workflows/ @Backblaze/<cli-maintainers> @Backblaze/<security> + /noxfile.py @Backblaze/<cli-maintainers> + /b2/_internal/b2v*/registry.py @Backblaze/<cli-maintainers> + /SECURITY.md @Backblaze/<security> + ``` +2. **Approvals cover one commit.** New commits pushed after an approval (agent pushes, hand-resolved + merges) need re-approval for Medium and High tiers ("dismiss stale approvals" plus "require approval of + the most recent push"). +3. **Every review thread resolved before merge** (GitHub "require conversation resolution"). +4. **Automated AI review and security review on every PR**, with open Important findings blocking merge. + `ai-review.yml` is in place; a maintainer adds the `ANTHROPIC_API_KEY` secret and installs the Claude + GitHub App to turn it on. Start + advisory, tune REVIEW.md for a few weeks, then decide on blocking and on the review depth (Deep for the + High-tier paths). Decide whether it replaces or runs beside Copilot review. +5. **Merge queue**, so checks rerun on `master` plus the change, and two PRs that pass alone cannot + combine into a broken `master`. The live-B2 jobs make each queue run slow; decide whether the queue + runs them or only the offline jobs. +6. **Past bugs become reviewer evals.** `doc/reviewer-evals.md` seeds the set with six cases (for + example #802, a tuple-unpack crash caught in review; #925, `update-bucket` clearing lifecycle rules; + #1164, incremental mode silently ignored on stdin). Maintainers confirm each case; every revert and + post-release fix adds one. Score the reviewer whenever REVIEW.md, AGENTS.md, the prompt or the model + changes, also on known-clean PRs, and classify each live finding as fixed, wrong, intentional or + didn't matter to measure noise. +7. **Measure it.** Human approvals per PR (median, p95), human comments per PR, review rounds to pass, + time a PR waits on a person (`nox -s review_metrics`); reverts and post-release fix PRs as the + guardrail. +8. **Circuit breakers.** If reverts or post-release fixes rise over a month, tighten the tiers + automatically (Low needs one person again) until a maintainer reviews why. +9. **Nightly review of existing code.** A scheduled run of the AI reviewer over the destructive commands + and credential handling finds old bugs before users do; findings become issues, not PR comments. +10. **Staged releases and incident reviews.** Use pre-release tags (`vX.Y.ZrcN`, binaries and GitHub + release only) for risky behavior before PyPI, keep one-step rollback (yank), and after each + post-release fix ask which layer should have caught it (see `RELEASE.md`). + +## Affected users and systems + +- Contributors and agents opening PRs; the maintainers. +- Repo settings: branch protection or rulesets on `master`, merge queue, conversation resolution, + a new `CODEOWNERS`. +- Workflows: `ci.yml`, `ai-review.yml`, possibly a risk-tier check and a nightly review. +- CLI users only indirectly: fewer regressions and faster fixes. + +## Constraints + +- Settings changes need a repo admin; this intent proposes them, it does not apply them. +- Separation of duties: the agent that wrote a change never approves it, and the person directing it + counts as one reviewer. +- Live-B2 secrets are only available to trusted runs; external contributors' PRs run the offline jobs. +- Start strict and loosen with evidence; any tier can be tightened if reverts rise. +- Cost of running an AI reviewer per push in CI. + +## Open questions + +- Who are the code owners, per area, and who is on the trusted-author list for the Low tier? +- Do the tier paths above match where the risk is? Which paths always need two people? +- Should Dependabot PRs keep running the full matrix with live-B2 secrets, or be gated like other + Backblaze repos? +- Is a merge queue available on this org's plan, and who enables it? (repo admin) +- Where should the eval set live and run: this repo, or the central SDK harness? +- Who owns the `ANTHROPIC_API_KEY` secret, the Claude GitHub App installation and the review budget, and which model and depth should the + review use? +- What revert or post-release-fix rate trips the circuit breaker? diff --git a/noxfile.py b/noxfile.py index 0a1ba8cb0..0822f6581 100644 --- a/noxfile.py +++ b/noxfile.py @@ -65,7 +65,7 @@ def _detect_python_nox_id() -> str: PYTHON_DEFAULT_VERSION = PYTHON_VERSIONS[-2] if len(PYTHON_VERSIONS) > 1 else PYTHON_VERSIONS[0] -PY_PATHS = ['b2', 'test', 'noxfile.py'] +PY_PATHS = ['b2', 'test', 'scripts', 'noxfile.py'] DOCKER_TEMPLATE = pathlib.Path('docker/Dockerfile.template') @@ -922,3 +922,101 @@ def towncrier_check(session): session.error( 'Found errors in the changelog.d directory. Check logs above for more information' ) + + +def _branch_files(session, base: str, diff_filter: str, *paths: str) -> list[str]: + """ + Files changed since the merge base with `base`, including uncommitted and untracked ones. + """ + try: + merge_base = subprocess.check_output(['git', 'merge-base', base, 'HEAD'], text=True).strip() + files = subprocess.check_output( + [ + 'git', + 'diff', + f'--diff-filter={diff_filter}', + '--name-only', + '-z', + merge_base, + '--', + *paths, + ], + text=True, + ).split('\0') + files += subprocess.check_output( + ['git', 'ls-files', '-z', '--others', '--exclude-standard', '--', *paths], text=True + ).split('\0') + except subprocess.CalledProcessError: + session.error(f'Cannot diff against {base!r}; fetch it or pass another base ref.') + return sorted(set(filter(None, files))) + + +@nox.session(python=False) +def changelog_check(session): + """ + Fail if the branch adds no changelog.d entry, like the CI lint job. + Pass the base ref as a posarg (default: $NOX_CI_BASE or origin/master). + """ + base = session.posargs[0] if session.posargs else os.environ.get('NOX_CI_BASE', 'origin/master') + if _branch_files(session, base, 'ACMR') == ['uv.lock']: + session.log('Skipping changelog validation for lockfile-only change.') + return + if not _branch_files(session, base, 'A', 'changelog.d'): + session.error( + f'No changelog.d entry added since {base}. Add one (see CONTRIBUTING.md) ' + 'or ask a maintainer for the "-changelog" label.' + ) + + +@nox.session(python=False) +def ci(session): + """ + Local gate that mirrors the CI lint, build and test jobs on one Python version (default 3.12, + as in ci.yml). It does not run the doc, docker or bundle jobs. + Set NOX_CI_BASE for a base other than origin/master, and NOX_CI_SKIP_CHANGELOG=1 when the PR + has the "-changelog" label. Integration tests that need B2 credentials run only when they are set. + """ + python = os.environ.get('NOX_CI_PYTHON', '3.12') + base = os.environ.get('NOX_CI_BASE', 'origin/master') + nox_cmd = [sys.executable, '-m', 'nox'] + # Child sessions without an explicit version (lint, build, ...) use the same interpreter. + env = {'NOX_PYTHONS': python} + # Spell-check only what this branch changes; older files still carry known typos. + changed = [ + path + for path in _branch_files(session, base, 'ACMR') + if os.path.isfile(path) and os.path.basename(path) != '.DS_Store' + ] + if changed: + session.run( + 'uvx', 'codespell==2.4.1', '-L', 'datas,re-use,re-using', *changed, external=True + ) + session.run(*nox_cmd, '-s', 'lint', 'towncrier_check', 'build', env=env) + if os.environ.get('NOX_CI_SKIP_CHANGELOG'): + session.log('NOT RUN: changelog_check (NOX_CI_SKIP_CHANGELOG is set).') + else: + session.run(*nox_cmd, '-s', 'changelog_check', '--', base, env=env) + session.run(*nox_cmd, '-s', f'unit-{python}', env=env) + session.run(*nox_cmd, '-s', f'integration-{python}', '--', '-m', 'not require_secrets', env=env) + if os.environ.get('B2_TEST_APPLICATION_KEY') and os.environ.get('B2_TEST_APPLICATION_KEY_ID'): + session.run( + *nox_cmd, + '-s', + f'integration-{python}', + '--', + '-m', + 'require_secrets', + '--cleanup', + env=env, + ) + else: + session.log('NOT RUN: integration tests with secrets (B2_TEST_APPLICATION_KEY* unset).') + + +@nox.session(python=False) +def review_metrics(session): + """ + Report human review time per merged PR, with reverts as the guardrail (needs `gh`). + Pass `-- --days N` to change the window (default 30). + """ + session.run(sys.executable, 'scripts/review_metrics.py', *session.posargs) diff --git a/scripts/review_metrics.py b/scripts/review_metrics.py new file mode 100644 index 000000000..0fb032d99 --- /dev/null +++ b/scripts/review_metrics.py @@ -0,0 +1,124 @@ +###################################################################### +# +# File: scripts/review_metrics.py +# +# Copyright 2026 Backblaze Inc. All Rights Reserved. +# +# License https://www.backblaze.com/using_b2_code.html +# +###################################################################### +""" +Report how much human review merged PRs needed, with reverts as the guardrail. + +Run it through `nox -s review_metrics -- --days 30`. Needs an authenticated `gh` CLI. Counts use the +first 100 reviews and conversation comments of each PR. +""" + +from __future__ import annotations + +import argparse +import datetime +import json +import statistics +import subprocess +import sys + +REPO = 'Backblaze/B2_Command_Line_Tool' +QUERY = """ +query($q: String!, $endCursor: String) { + search(query: $q, type: ISSUE, first: 50, after: $endCursor) { + pageInfo { hasNextPage endCursor } + nodes { + ... on PullRequest { + number title createdAt mergedAt author { __typename login } + reviews(first: 100) { + nodes { author { __typename login } state submittedAt comments { totalCount } } + } + comments(first: 100) { nodes { author { __typename login } } } + } + } + } +} +""" + + +def is_bot(actor: dict | None) -> bool: + # GraphQL drops the "[bot]" suffix from Bot logins, so check the type too. + if not actor or actor.get('__typename') == 'Bot': + return True + login = actor.get('login') or '' + return login.endswith('[bot]') or 'copilot' in login.lower() or login == 'coveralls' + + +def is_human_reviewer(actor: dict | None, author: str | None) -> bool: + return not is_bot(actor) and actor['login'] != author + + +def fetch(days: int) -> list[dict]: + since = (datetime.date.today() - datetime.timedelta(days=days)).isoformat() + variables = {'q': f'repo:{REPO} is:pr is:merged merged:>={since}', 'endCursor': None} + prs = [] + while True: + # The query and its variables go on stdin, so the command line stays constant. + body = json.dumps({'query': QUERY, 'variables': variables}) + out = subprocess.run( + ['gh', 'api', 'graphql', '--input', '-'], + input=body, + capture_output=True, + text=True, + check=True, + ).stdout + search = json.loads(out)['data']['search'] + prs += [node for node in search['nodes'] if node] + if not search['pageInfo']['hasNextPage']: + return prs + variables['endCursor'] = search['pageInfo']['endCursor'] + + +def parse_time(value: str) -> datetime.datetime: + return datetime.datetime.fromisoformat(value.replace('Z', '+00:00')) + + +def summarize(values: list[float]) -> str: + if not values: + return 'n/a' + values = sorted(values) + p95 = values[min(len(values) - 1, int(round(0.95 * (len(values) - 1))))] + return f'median {statistics.median(values):.1f}, p95 {p95:.1f}' + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--days', type=int, default=30, help='Merged in the last N days.') + args = parser.parse_args(argv) + + prs = fetch(args.days) + approvals, comments, waits, rounds = [], [], [], [] + reverts = 0 + for pr in prs: + author = (pr['author'] or {}).get('login') + reviews = pr['reviews']['nodes'] + human = [r for r in reviews if is_human_reviewer(r['author'], author)] + approvals.append(len({r['author']['login'] for r in human if r['state'] == 'APPROVED'})) + conversation = [ + c for c in pr['comments']['nodes'] if is_human_reviewer(c['author'], author) + ] + comments.append(sum(r['comments']['totalCount'] for r in human) + len(conversation)) + submitted = [parse_time(r['submittedAt']) for r in human if r['submittedAt']] + if submitted: + waits.append((min(submitted) - parse_time(pr['createdAt'])).total_seconds() / 3600) + bot_reviews = [r for r in reviews if is_bot(r['author'])] + rounds.append(len(bot_reviews)) + reverts += pr['title'].lower().startswith('revert') + + print(f'{len(prs)} PRs merged in the last {args.days} days') + print(f'Distinct human approvers per PR: {summarize(approvals)}') + print(f'Human comments per PR: {summarize(comments)}') + print(f'Hours to first human review: {summarize(waits)}') + print(f'Automated review rounds per PR: {summarize(rounds)}') + print(f'Reverts (guardrail): {reverts}') + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/test/unit/console_tool/test_upload_file.py b/test/unit/console_tool/test_upload_file.py index 27b75095c..1431da52b 100644 --- a/test/unit/console_tool/test_upload_file.py +++ b/test/unit/console_tool/test_upload_file.py @@ -96,7 +96,15 @@ def test_upload_file__incremental_mode_on_named_pipe_is_refused(b2_cli, bucket, # No writer needed: get_input_stream only stats the FIFO (points_to_fifo check), # and CommandError is raised before file_identifier_to_read_stream ever opens it. b2_cli.run( - ['file', 'upload', '--no-progress', '--incremental-mode', 'my-bucket', str(local_file), 'dst.txt'], + [ + 'file', + 'upload', + '--no-progress', + '--incremental-mode', + 'my-bucket', + str(local_file), + 'dst.txt', + ], expected_status=1, expected_stderr=( 'ERROR: incremental mode is not supported for streamed sources; '