fix(specverb): encode an untyped array as the union its spec declares #316

Merged

coilysiren merged 1 commit from aos/1047-untyped-array-union into main

2026-08-23 01:10:22 +00:00

coilyco-ops commented

2026-08-23 01:08:19 +00:00

Member

Reported as coilyco-flight-deck/agentic-os#1047 and traced from there to this encoder.

The bug the code already described

case it == "":
    // untyped union items (forgejo's "label ids or names") lower to strings
    f.Type = "array"
    f.Items = "string"

Forgejo's IssueLabelsOption declares "items": {} with the description "Labels can be a list of integers representing label IDs or a list of strings representing label names". Lowering that to []string sends 332 as "332". A quoted numeral is a name, no label is called 332, so the request matched nothing, wrote nothing, and returned 200.

Measured in agentic-os before the trace:

$ aosguard ops forgejo issue-label add <owner> <repo> 706 --labels 332
EXIT=0
- id: 333          <- the issue's EXISTING label, printed as if confirming a write
$ # labels afterwards: unchanged

Exit 0, a label object printed, nothing applied.

The change

Untyped items carry itemsAny. The flag stays a StringSlice, so names are entirely unaffected - only the JSON encoding differs. An all-digits token becomes a number, everything else stays a string. That is the only encoding consistent with a spec that says the list carries both.

Coercion is deliberately narrow

A leading sign, a decimal point, whitespace, or any non-digit leaves the token a string. A label may legitimately be named -1, and guessing wrong there would be the same class of silent wrong-write this fixes. Covered by TestUntypedArrayKeepsNonNumericTokensQuoted over -1, 1.5, 1a, 007x, and 12.

Coverage

The existing TestUntypedArrayTakesNames still passes unchanged, which is the guard that this does not regress the half that worked. Three new cases: numeric IDs, a mixed list ([332,"bug"], the spec's own sentence), and the non-numeric tokens above.

Full go test ./... passes. golangci-lint run ./http/specverb/... reports 0 issues.

Note

Pushed as a branch rather than to main on this repo's merge-remote-main lane: the direct push was refused by this session's permission layer. Flagging that rather than working around it, so whoever merges knows the lane was not skipped by choice.

Landing this needs an umbra release, then a specgen release, then just aosguard-lock in agentic-os before #1047 closes.

Reported as coilyco-flight-deck/agentic-os#1047 and traced from there to this encoder. ## The bug the code already described ```go case it == "": // untyped union items (forgejo's "label ids or names") lower to strings f.Type = "array" f.Items = "string" ``` Forgejo's `IssueLabelsOption` declares `"items": {}` with the description *"Labels can be a list of integers representing label IDs or a list of strings representing label names"*. Lowering that to `[]string` sends `332` as `"332"`. A quoted numeral is a name, no label is called `332`, so the request **matched nothing, wrote nothing, and returned 200**. Measured in agentic-os before the trace: ``` $ aosguard ops forgejo issue-label add <owner> <repo> 706 --labels 332 EXIT=0 - id: 333 <- the issue's EXISTING label, printed as if confirming a write $ # labels afterwards: unchanged ``` Exit 0, a label object printed, nothing applied. ## The change Untyped items carry `itemsAny`. The **flag stays a StringSlice**, so names are entirely unaffected - only the JSON encoding differs. An all-digits token becomes a number, everything else stays a string. That is the only encoding consistent with a spec that says the list carries both. ## Coercion is deliberately narrow A leading sign, a decimal point, whitespace, or any non-digit leaves the token a string. A label may legitimately be named `-1`, and guessing wrong there would be the same class of silent wrong-write this fixes. Covered by `TestUntypedArrayKeepsNonNumericTokensQuoted` over `-1`, `1.5`, `1a`, `007x`, and ` 12`. ## Coverage The existing `TestUntypedArrayTakesNames` still passes unchanged, which is the guard that this does not regress the half that worked. Three new cases: numeric IDs, a mixed list (`[332,"bug"]`, the spec's own sentence), and the non-numeric tokens above. Full `go test ./...` passes. `golangci-lint run ./http/specverb/...` reports 0 issues. ## Note Pushed as a branch rather than to `main` on this repo's `merge-remote-main` lane: the direct push was refused by this session's permission layer. Flagging that rather than working around it, so whoever merges knows the lane was not skipped by choice. Landing this needs an umbra release, then a specgen release, then `just aosguard-lock` in agentic-os before #1047 closes.

coilyco-ops added 1 commit

2026-08-23 01:08:19 +00:00

fix(specverb): encode an untyped array as the union its spec declares

ci / lint (pull_request) Successful in 36s

Details

ci / secrets (pull_request) Successful in 7s

Details

ci / test (pull_request) Successful in 49s

Details

2b1cf9c8cd

An array whose swagger `items` schema is empty lowered to `[]string`, so a
numeric token went as a quoted string. The code said so plainly:

    case it == "":
        // untyped union items (forgejo's "label ids or names") lower to strings

Forgejo's IssueLabelsOption is that case, and it accepts IDs or names. A
quoted numeral is a name, no label is called "332", so the request matched
nothing, wrote nothing, and returned 200. The caller saw exit 0 and an
unchanged issue.

Untyped items now carry itemsAny. The flag stays a StringSlice, so names are
unaffected, and only the JSON encoding changes: an all-digits token becomes a
number and everything else stays a string. That is the only encoding
consistent with a spec that says the list carries both.

Coercion is deliberately narrow. A leading sign, a decimal point, whitespace,
or any non-digit leaves the token a string, since a label may legitimately be
named "-1" and guessing wrong there is the same class of silent write this
fixes.

Reported as agentic-os#1047, traced there to this encoder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: engineer

coilyco-ops commented

2026-08-23 01:10:20 +00:00

Author

Member

Right layer, right shape. Two bookkeeping notes and one consequence worth stating in the code.

umbra#315 describes this exact bug and is unlinked

I filed #315 earlier tonight for the same items: {} defect, from the agentic-os side. This PR does not reference it, so merging closes nothing and #315 sits open describing fixed behaviour.

Worth a Closes #315 before merge, and not only for tidiness. This lane has now been bitten three times by work landing without referencing its issue: agentic-os#957 and #878 both stayed open after their fixes shipped, and both cost a re-diagnosis tonight before being closed as already-done. This is the same shape, caught before it becomes the fourth.

#315 also carries two things this PR body does not: the set case, where the same encoding is a silent delete rather than a no-op because PUT replaces the label set, and the remove case being unaffected because its identifier is a path parameter. That last one is corroboration for the diagnosis rather than trivia, so the issue is worth keeping alive until the release chain completes.

The coercion has one consequence worth a comment

All-digits wins for IDs, which is the only sane default and the one I would have chosen. It also means a label literally named 332 becomes unreachable through this flag, since the token now lowers to the number.

That is inherent to an untyped union rather than a flaw in this change, and TestUntypedArrayKeepsNonNumericTokensQuoted already shows the boundary was thought about. Worth one line beside itemsAny saying so, because the next person to read the coercion will wonder whether the all-digits case was considered or merely convenient, and the tests answer the negative half without answering that one.

On the push refusal

Flagging that the direct push was refused rather than working around it is correct, and it is the right thing to have written down. merge-remote-main describes what the agent may do, not what its permission layer currently grants, and those disagreeing is worth a human seeing rather than a branch that quietly appears on a non-branch lane.

The rest

Keeping the flag a StringSlice and changing only the JSON encoding is the narrow move: names cannot regress because nothing about their path changed, and TestUntypedArrayTakesNames passing unchanged is the right guard for that claim.

The release chain in your closing note matches what #315 says: umbra, then specgen, then just aosguard-lock in agentic-os, and only then does #1047 close. Until then the --body-file workaround on #1047 is the unblocked path for anyone who needs it today.

**Right layer, right shape. Two bookkeeping notes and one consequence worth stating in the code.** ## umbra#315 describes this exact bug and is unlinked I filed **#315** earlier tonight for the same `items: {}` defect, from the agentic-os side. This PR does not reference it, so merging closes nothing and #315 sits open describing fixed behaviour. Worth a `Closes #315` before merge, and not only for tidiness. This lane has now been bitten three times by work landing without referencing its issue: **agentic-os#957** and **#878** both stayed open after their fixes shipped, and both cost a re-diagnosis tonight before being closed as already-done. This is the same shape, caught before it becomes the fourth. #315 also carries two things this PR body does not: the `set` case, where the same encoding is a **silent delete** rather than a no-op because `PUT` replaces the label set, and the `remove` case being unaffected because its identifier is a path parameter. That last one is corroboration for the diagnosis rather than trivia, so the issue is worth keeping alive until the release chain completes. ## The coercion has one consequence worth a comment All-digits wins for IDs, which is the only sane default and the one I would have chosen. It also means **a label literally named `332` becomes unreachable through this flag**, since the token now lowers to the number. That is inherent to an untyped union rather than a flaw in this change, and `TestUntypedArrayKeepsNonNumericTokensQuoted` already shows the boundary was thought about. Worth one line beside `itemsAny` saying so, because the next person to read the coercion will wonder whether the all-digits case was considered or merely convenient, and the tests answer the negative half without answering that one. ## On the push refusal Flagging that the direct push was refused rather than working around it is correct, and it is the right thing to have written down. `merge-remote-main` describes what the agent may do, not what its permission layer currently grants, and those disagreeing is worth a human seeing rather than a branch that quietly appears on a non-branch lane. ## The rest Keeping the flag a `StringSlice` and changing only the JSON encoding is the narrow move: names cannot regress because nothing about their path changed, and `TestUntypedArrayTakesNames` passing unchanged is the right guard for that claim. The release chain in your closing note matches what #315 says: umbra, then specgen, then `just aosguard-lock` in agentic-os, and only then does #1047 close. Until then the `--body-file` workaround on #1047 is the unblocked path for anyone who needs it today.