Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,19 @@ jobs:
- name: Run Go tests (targeted non-live package scope)
run: task test:go:safe

# Generated code was never verified in CI, which is how the sanitizer's
# nondeterministic operationId suffixing went unnoticed.
- name: Verify generated models and client are current
run: task models:verify && task client:verify

- name: Verify OpenAPI spec coverage artifact is current
run: task quality:spec-coverage:verify

# Catches commands that no live test proves work, including ones whose only
# live test skips itself when the call fails.
- name: Verify CLI live coverage has not regressed
run: task quality:cli-live-coverage:verify

docs-site:
name: Docs Site
runs-on: ubuntu-latest
Expand Down
52 changes: 52 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,58 @@
git push --no-verify --force-with-lease
```

### Tests must not reconfigure the repository they run in

`internal/git/gittest` snapshots the repository-scoped git configuration before a package's tests and
compares it afterwards. `TestMain` in `internal/git/execgit`, `internal/cli` and
`tests/integration/live` fails the package when anything changed, naming the exact keys.

This is not hypothetical. `Backend.Clone` persists `http.extraHeader` into the repository it clones
into so later fetches carry authentication. A test that pointed it at the working copy instead of a
temporary directory wrote this into the project's own `.git/config`:

```
http.extraheader = Authorization: Basic <base64 of dummy-user:dummy-password>
user.name = Test User
user.email = test@example.local
remote.upstream.url = https://example.local/scm/PRJ/upstream.git
```

An unscoped `http.extraHeader` is attached to every HTTP request git makes, and an explicit
`Authorization` header beats any credential helper, so every push to GitHub sent
`dummy-user:dummy-password` and was rejected with *"Password authentication is not supported for Git
operations"* — a message that reads like a bad token and sends you hunting in the wrong place. The
identity override meanwhile authored real commits as `Test User <test@example.local>`.

**Any test that shells out to git must operate on a directory it created**, normally `t.TempDir()`.
If the guard fires, look for a git invocation missing `-C` or a helper defaulting to the current
directory. It reports rather than repairs; undo damage with `git config --local --unset <key>`.

### CLI live coverage artifact

`docs/quality/cli-live-coverage.json` records which CLI commands the live suite actually proves work
against a real Bitbucket. CI verifies it via `task quality:cli-live-coverage:verify` (CI-safe, no live
infra needed — it is static analysis of the Cobra tree and the live test sources).

It fails when:

- a command that used to be covered loses its live coverage,
- a new command arrives with no live test invoking it, or
- a command becomes **masked** — its only live coverage comes from a test that calls `t.Skip` when the
call fails, so the suite passes whether or not the command works.

That last case is the one that matters. `bb pr task *` called an endpoint Atlassian removed in
Bitbucket 8.0, and the live tests hid it behind
`if strings.Contains(err.Error(), "not_found") { t.Skipf(...) }` — CI stayed green for years. A skipped
test is not a passing test. Fix the command or the test; do not add a skip.

When you add a command, add a live test that runs it and asserts, then:

```bash
task quality:cli-live-coverage:update
git add docs/quality/cli-live-coverage.json
```

### OpenAPI spec coverage artifact

`docs/quality/spec-coverage.json` is a separate committed artifact that does **not** depend on coverage profiles or live tests. If you change the OpenAPI spec, the generated client, or how `internal/services` calls the API, regenerate it and commit the result:
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ bb --json auth status
run in the stack, so the newest published release is not automatically the supported one.
Set `BITBUCKET_VERSION_TARGET` if you want to record a version for your own environment.
- API contract source: a version-pinned Atlassian OpenAPI artifact
(`docs/reference/atlassian/bitbucket-9.4-openapi.json`). This fixes the endpoint and payload
(`docs/reference/atlassian/bitbucket-10.2-openapi.json`). This fixes the endpoint and payload
shapes the generated client is built from — it is the provenance of the spec, not a statement
about which server versions work. Behavior is established by live tests, not the spec.
- CLI identity and machine contract: `bb` / `bb.machine` `v2`
Expand Down
35 changes: 25 additions & 10 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ version: '3'

vars:
OAPI_CODEGEN_VERSION: v2.4.1
BITBUCKET_OPENAPI_URL: https://dac-static.atlassian.com/server/bitbucket/9.4.swagger.v3.json?_v=1.637.27
BITBUCKET_OPENAPI_TARGET: docs/reference/atlassian/bitbucket-9.4-openapi.json
BITBUCKET_OPENAPI_TMP: .tmp/bitbucket-9.4-openapi.json
BITBUCKET_OPENAPI_SANITIZED_TMP: .tmp/bitbucket-9.4-openapi.sanitized.json
BITBUCKET_OPENAPI_URL: https://dac-static.atlassian.com/server/bitbucket/10.2.swagger.v3.json
BITBUCKET_OPENAPI_TARGET: docs/reference/atlassian/bitbucket-10.2-openapi.json
BITBUCKET_OPENAPI_TMP: .tmp/bitbucket-10.2-openapi.json
BITBUCKET_OPENAPI_SANITIZED_TMP: .tmp/bitbucket-10.2-openapi.sanitized.json
DOCS_PROJECT_DIR: docs
COVERAGE_BASE_REF: origin/main
COVERAGE_MIN_GLOBAL_COMBINED: "85"
Expand All @@ -20,11 +20,11 @@ tasks:
cmd: task --list

docs:refresh-openapi:
desc: Refresh vendored Atlassian Bitbucket 9.4 OpenAPI reference
desc: Refresh vendored Atlassian Bitbucket 10.2 OpenAPI reference
cmds:
- mkdir -p .tmp docs/reference/atlassian
- curl -fsSL '{{.BITBUCKET_OPENAPI_URL}}' -o '{{.BITBUCKET_OPENAPI_TMP}}'
- python3 -c "import json; d=json.load(open('.tmp/bitbucket-9.4-openapi.json', 'r', encoding='utf-8')); assert d.get('openapi') == '3.0.1', 'unexpected openapi version'; assert d.get('info', {}).get('version') == '9.4', 'unexpected API version'; print('validated openapi paths=' + str(len(d.get('paths', {}))))"
- python3 -c "import json; d=json.load(open('.tmp/bitbucket-10.2-openapi.json', 'r', encoding='utf-8')); assert d.get('openapi') == '3.0.1', 'unexpected openapi version'; assert d.get('info', {}).get('version') == '10.2', 'unexpected API version'; print('validated openapi paths=' + str(len(d.get('paths', {}))))"
- cp '{{.BITBUCKET_OPENAPI_TMP}}' '{{.BITBUCKET_OPENAPI_TARGET}}'

docs:build:
Expand Down Expand Up @@ -101,10 +101,10 @@ tasks:
- task docs:deploy-version VERSION={{.VERSION}}

models:generate:
desc: Generate Go models from vendored Bitbucket 9.4 OpenAPI spec
desc: Generate Go models from vendored Bitbucket 10.2 OpenAPI spec
cmds:
- mkdir -p internal/models/generated
- go run ./tools/openapi-sanitize -in docs/reference/atlassian/bitbucket-9.4-openapi.json -out {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- go run ./tools/openapi-sanitize -in docs/reference/atlassian/bitbucket-10.2-openapi.json -out {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@{{.OAPI_CODEGEN_VERSION}} -config tools/oapi-codegen/bitbucket-types.yaml {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- go run ./tools/oapi-fix-generated -file internal/models/generated/bitbucket_types.gen.go
- gofmt -w internal/models/generated/bitbucket_types.gen.go
Expand All @@ -116,10 +116,10 @@ tasks:
- git diff --exit-code -- internal/models/generated/bitbucket_types.gen.go

client:generate:
desc: Generate Go OpenAPI client from vendored Bitbucket 9.4 OpenAPI spec
desc: Generate Go OpenAPI client from vendored Bitbucket 10.2 OpenAPI spec
cmds:
- mkdir -p internal/openapi/generated
- go run ./tools/openapi-sanitize -in docs/reference/atlassian/bitbucket-9.4-openapi.json -out {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- go run ./tools/openapi-sanitize -in docs/reference/atlassian/bitbucket-10.2-openapi.json -out {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@{{.OAPI_CODEGEN_VERSION}} -config tools/oapi-codegen/bitbucket-client.yaml {{.BITBUCKET_OPENAPI_SANITIZED_TMP}}
- gofmt -w internal/openapi/generated/bitbucket_client.gen.go

Expand Down Expand Up @@ -227,6 +227,21 @@ tasks:
cmds:
- go run ./tools/quality-report -spec-coverage -verify-report -spec-coverage-file docs/quality/spec-coverage.json

quality:cli-live-coverage:
desc: Print which CLI commands the live suite proves work against a real Bitbucket
cmds:
- go run ./tools/cli-live-coverage

quality:cli-live-coverage:update:
desc: Update the committed CLI live coverage baseline
cmds:
- go run ./tools/cli-live-coverage -write

quality:cli-live-coverage:verify:
desc: Fail when a command loses live coverage, arrives without it, or is masked by a skip-on-error test (CI-safe, no live execution)
cmds:
- go run ./tools/cli-live-coverage -verify

test:unit:
desc: Run Go unit tests
cmd: go test ./cmd/... ./internal/... ./tools/...
Expand Down
117 changes: 116 additions & 1 deletion docs/openapi/fixes.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
version: 1
updated_at: "2026-04-08"
source_openapi: docs/reference/atlassian/bitbucket-9.4-openapi.json
source_openapi: docs/reference/atlassian/bitbucket-10.2-openapi.json
fixes:
- id: OPENAPI-001
area: spec-path-parameters
Expand Down Expand Up @@ -115,3 +115,118 @@ fixes:
commands:
- go test ./internal/services/pullrequestactivity ./internal/cli ./internal/mcp
- go test -tags=live ./tests/integration/live -run TestLiveCLIRepoListAndComments -count=1

- id: OPENAPI-007
area: pull-request-activities-anchor-path
description: Accept string anchor paths on pull request activity comments, which the spec models as an object.
reference:
type: endpoint
value: /api/latest/projects/{projectKey}/repos/{repositorySlug}/pull-requests/{pullRequestId}/activities
upstream_issue: >
The activity timeline serialises an inline comment's anchor.path (and anchor.srcPath) as a plain
string, while RestComment.Anchor.Path is an object with name/parent/extension/components. Verified
against Bitbucket Data Center 10.2.1. A single inline comment therefore made the whole activity page
fail to decode, so pull requests carrying inline review comments returned no comments at all.
change:
files:
- internal/services/pullrequestactivity/service.go
- internal/services/pullrequestactivity/threads_test.go
- tests/integration/live/pr_review_visibility_live_test.go
detail: >
Before unmarshalling a comment into the generated model, string anchor paths are rewritten into the
documented object shape (name, parent, extension, components), recursively across replies and parent
comments. Payloads already using the object form are left untouched.
verification:
commands:
- go test ./internal/services/pullrequestactivity -run 'AnchorPaths|PathObject' -count=1
- go test -tags=live ./tests/integration/live -run TestLivePullRequestReviewVisibility -count=1

- id: OPENAPI-008
area: not-found-route-vs-resource
description: Distinguish a retired endpoint from a missing resource, which the spec models identically as 404.
reference:
type: generation
value: 404 response semantics across all Bitbucket endpoints
upstream_issue: >
The OpenAPI spec declares a single 404 response per operation, but Bitbucket answers two
different situations with it. A resource that does not exist returns the Bitbucket error
envelope, which always carries an "errors" array
({"errors":[{...,"exceptionName":"...NoSuchPullRequestException"}]}). A route the application
never registered never reaches that layer, so the servlet container answers with a status
document instead ({"message":"HTTP 404 Not Found","status-code":404,"sub-code":-1}). That
document is content-negotiated: clients sending Accept application/json receive the JSON form
above, while clients that do not receive the XML equivalent
(<status><status-code>404</status-code>...</status>). The detection therefore keys on the
absence of the "errors" array rather than on the content type, so both forms are covered.
Without the distinction a client reports a plain "not found" for a call that could never
succeed, which is how the retired pull request task endpoint went unnoticed after Bitbucket
8.0 removed it.
change:
files:
- internal/openapi/errors.go
- internal/openapi/errors_test.go
- internal/services/pullrequestactivity/service.go
- tests/integration/live/pr_review_visibility_live_test.go
detail: >
MapStatusError inspects the body of a 404 and attaches ErrRouteMissing as the cause when the
payload is not a Bitbucket error envelope. Callers use openapi.IsRouteMissing to degrade only
for features the server does not expose, and to report everything else. Verified against
Bitbucket Data Center 10.2.1 for retired endpoints, unknown routes, missing pull requests and
missing repositories.
verification:
commands:
- go test ./internal/openapi -run IsRouteMissing -count=1
- go test ./internal/services/pullrequestactivity -run TrySummarize -count=1
- go test -tags=live ./tests/integration/live -run TestLiveRouteMissingClassification -count=1

- id: OPENAPI-009
area: spec-operationid-nondeterminism
description: Make operationId collision suffixes deterministic so the generated client is reproducible.
reference:
type: generation
value: tools/openapi-sanitize operationId suffixing
upstream_issue: >
Bitbucket reuses operationIds across unrelated endpoints (six "get", two "getStatus", and so on),
so the sanitizer suffixes collisions. It assigned those suffixes while ranging over the spec's
paths map, and Go randomises map iteration, so the same spec produced a different
operationId-to-endpoint mapping on every run. Whether Get3WithResponse called
/basicauth/latest/config or a pull request endpoint was decided by chance at generation time.
Nothing caught it because models:verify and client:verify are not run by CI.
change:
files:
- tools/openapi-sanitize/main.go
- tools/openapi-sanitize/main_test.go
detail: >
Paths are sorted before assignment, making the mapping a pure function of the spec. Collision
suffixing also now keeps incrementing until the canonical id is free, because Bitbucket 10.2
ships an operation already named "get_2" and blind suffixing produced duplicate Go declarations.
verification:
commands:
- go test ./tools/openapi-sanitize -count=1
- task client:verify
- task models:verify

- id: OPENAPI-010
area: unreachable-recursive-schemas
description: Exclude Bitbucket 10.2 domain schemas that Go cannot express and no operation uses.
reference:
type: generation
value: components.schemas Comment, CommentThread, PullRequest, PullRequestParticipant
upstream_issue: >
Bitbucket 10.2 added four schemas that reference each other through required, non-nullable
$refs (Comment.thread <-> CommentThread.rootComment, PullRequest.author <->
PullRequestParticipant.pullRequest). Go cannot size such a value cycle, so generation produced
code that would not compile. No path operation references any of them; the wire types remain
RestComment and RestPullRequest.
change:
files:
- tools/oapi-codegen/bitbucket-types.yaml
- tools/oapi-codegen/bitbucket-client.yaml
detail: >
Both codegen configurations exclude the four schemas. If a future spec makes them reachable
from an operation, generation will fail loudly rather than silently omitting a used type.
verification:
commands:
- task models:generate
- task client:generate
- go build ./...
Loading