#38 (v0.4.0) made repo the canonical repo-identifier and kept name as an alias. Its body also named number as canonical for the issue/PR index. Most issue tools already use number canonical (with index aliased via the shim) — but the PR tools do not.
Finding
pr_merge (and other PR tools) advertise and read index as their canonical id field:
So a caller primed on the "number" convention (GitHub/idiomatic) must send index for PR tools but number for issue tools — inconsistent, and the shim only aliases index→number (one-directional), so sending number to pr_merge does not fill index.
Flip the PR tools (pr_merge, pr_get, pr_comment, pr_files_diff, and any other tool whose canonical is index) to canonical number, matching the issue tools.
Make the shim bidirectional for number↔index (mirror what #38 did for name↔repo), so both spellings resolve for every tool regardless of which the struct declares.
Keep index as an accepted alias.
Acceptance
All issue AND PR tools advertise number in their schema.
Round-trip tests: number and index both resolve; explicit number wins.
task check passes.
Refs
Follow-up to #38 (v0.4.0). Shim: internal/tools/tool.gonormalizeAliases.
## Context
#38 (v0.4.0) made `repo` the canonical repo-identifier and kept `name` as an alias. Its body also named `number` as canonical for the issue/PR index. Most **issue** tools already use `number` canonical (with `index` aliased via the shim) — but the **PR tools do not**.
## Finding
`pr_merge` (and other PR tools) advertise and read `index` as their canonical id field:
```go
// internal/tools/pr_merge.go
"index":{"type":"integer","minimum":1},
...
"required":["owner","repo","index"]
...
Index int `json:"index"`
```
So a caller primed on the "`number`" convention (GitHub/idiomatic) must send `index` for PR tools but `number` for issue tools — inconsistent, and the shim only aliases `index`→`number` (one-directional), so sending `number` to `pr_merge` does **not** fill `index`.
## Proposal
Finish the #38 normalization for the id field:
1. Flip the PR tools (`pr_merge`, `pr_get`, `pr_comment`, `pr_files_diff`, and any other tool whose canonical is `index`) to canonical `number`, matching the issue tools.
2. Make the shim **bidirectional** for `number`↔`index` (mirror what #38 did for `name`↔`repo`), so both spellings resolve for every tool regardless of which the struct declares.
3. Keep `index` as an accepted alias.
## Acceptance
- [ ] All issue AND PR tools advertise `number` in their schema.
- [ ] `index` still resolves (back-compat) on every tool.
- [ ] Bidirectional `number`↔`index` shim, mirroring `name`↔`repo`.
- [ ] Round-trip tests: `number` and `index` both resolve; explicit `number` wins.
- [ ] `task check` passes.
## Refs
- Follow-up to #38 (v0.4.0). Shim: `internal/tools/tool.go` `normalizeAliases`.
Scope was smaller than the issue estimated: on inspection, pr_merge was the only tool still advertising index — pr_get, pr_comment, pr_files_diff already use canonical number. So this was a one-file flip:
No shim change needed. The existing one-directional index→number alias already covers legacy index callers, and after the flip no tool has canonical index, so a bidirectional number↔index shim would be dead code — skipped it (min viable).
Test TestPRMergeNumberCanonical: schema advertises number, and both {"number":7} and {"index":7} resolve to /pulls/7/merge.
task check green.
Result: the id arg is number uniformly across all per-issue/PR tools; index remains an accepted alias. Deploying via CD.
Done in v0.4.1 (commit on `main`).
**Scope was smaller than the issue estimated:** on inspection, `pr_merge` was the **only** tool still advertising `index` — `pr_get`, `pr_comment`, `pr_files_diff` already use canonical `number`. So this was a one-file flip:
- `pr_merge` schema property + `required` + struct field/tag `index` → `number`.
- **No shim change needed.** The existing one-directional `index`→`number` alias already covers legacy `index` callers, and after the flip no tool has canonical `index`, so a bidirectional `number`↔`index` shim would be dead code — skipped it (min viable).
- Test `TestPRMergeNumberCanonical`: schema advertises `number`, and both `{"number":7}` and `{"index":7}` resolve to `/pulls/7/merge`.
- `task check` green.
Result: the id arg is `number` uniformly across all per-issue/PR tools; `index` remains an accepted alias. Deploying via CD.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Context
#38 (v0.4.0) made
repothe canonical repo-identifier and keptnameas an alias. Its body also namednumberas canonical for the issue/PR index. Most issue tools already usenumbercanonical (withindexaliased via the shim) — but the PR tools do not.Finding
pr_merge(and other PR tools) advertise and readindexas their canonical id field:So a caller primed on the "
number" convention (GitHub/idiomatic) must sendindexfor PR tools butnumberfor issue tools — inconsistent, and the shim only aliasesindex→number(one-directional), so sendingnumbertopr_mergedoes not fillindex.Proposal
Finish the #38 normalization for the id field:
pr_merge,pr_get,pr_comment,pr_files_diff, and any other tool whose canonical isindex) to canonicalnumber, matching the issue tools.number↔index(mirror what #38 did forname↔repo), so both spellings resolve for every tool regardless of which the struct declares.indexas an accepted alias.Acceptance
numberin their schema.indexstill resolves (back-compat) on every tool.number↔indexshim, mirroringname↔repo.numberandindexboth resolve; explicitnumberwins.task checkpasses.Refs
internal/tools/tool.gonormalizeAliases.Done in v0.4.1 (commit on
main).Scope was smaller than the issue estimated: on inspection,
pr_mergewas the only tool still advertisingindex—pr_get,pr_comment,pr_files_diffalready use canonicalnumber. So this was a one-file flip:pr_mergeschema property +required+ struct field/tagindex→number.index→numberalias already covers legacyindexcallers, and after the flip no tool has canonicalindex, so a bidirectionalnumber↔indexshim would be dead code — skipped it (min viable).TestPRMergeNumberCanonical: schema advertisesnumber, and both{"number":7}and{"index":7}resolve to/pulls/7/merge.task checkgreen.Result: the id arg is
numberuniformly across all per-issue/PR tools;indexremains an accepted alias. Deploying via CD.