Skip to content

Orb catalog route: named 503s for kind and shape refusals (CatalogKindInvalid, orb_catalog_kind_invalid, orb_catalog_invalid) - #4084

Open
jaylfc wants to merge 2 commits into
devfrom
exec/tsk-epdulm
Open

jaylfc wants to merge 2 commits into
devfrom
exec/tsk-epdulm

Conversation

@jaylfc

@jaylfc jaylfc commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Orb catalog route: named 503s for kind and shape refusals (CatalogKindInvalid, orb_catalog_kind_invalid, orb_catalog_invalid)

Autonomous build of board card tsk-epdulm.

test: harness-measured red for tsk-epdulm

RED-FIRST proof measured by the harness (measure_red.sh): the changed tests run
against product code at base 3811f78, then against this branch.

The base fails at COLLECTION: the tests import a symbol this branch adds
(measure_red.sh checked the diff adds its top-level definition).

RED on the base:

E   ImportError: cannot import name 'CatalogInvalid' from 'tinyagentos.orb.catalog_sig' (/tmp/measure-red-hMLer2/wt/tinyagentos/orb/catalog_sig.py)
ERROR tests/test_routes_orb.py
1 error in 0.62s

GREEN on the fix:

83 passed in 56.38s

Orb catalog route: named 503s for kind and shape refusals (CatalogKind

Docs-Reviewed: executor rescue commit -- the model left these edits uncommitted and wrote no commit message, so doc drift was NOT assessed by the model; the lead reviews docs at PR time.

Files:
tests/orb/test_catalog_sig.py | 20 ++++++++++
tests/test_routes_orb.py | 88 ++++++++++++++++++++++++++++++++++++++++++
tinyagentos/orb/catalog_sig.py | 16 ++++++--
tinyagentos/routes/orb.py | 8 +++-
4 files changed, 127 insertions(+), 5 deletions(-)

In-house review (pre-PR)

in-house-review: UNAVAILABLE (no reviewer gave a verdict)

reviewer output

(none)

In-house-Review: UNAVAILABLE model=none head=a65a99097 needs-lead=0


Summary by Gitar

  • New error types:
    • Added CatalogKindInvalid and CatalogInvalid exceptions in catalog_sig.py
  • Validation:
    • verify_catalog now raises CatalogInvalid for non-object JSON and invalid expires values
  • Route errors:
    • Mapped kind and invalid catalog errors to 503 orb_catalog_kind_invalid and orb_catalog_invalid in orb.py
    • Remote fetch failures with these errors no longer fall back to cache
  • Tests:
    • Added route tests for the new 503 responses and catalog signature tests for CatalogInvalid

This will update automatically on new commits.

Docs-Reviewed: executor rescue commit -- the model left these edits uncommitted and wrote no commit message, so doc drift was NOT assessed by the model; the lead reviews docs at PR time.
RED-FIRST proof measured by the harness (measure_red.sh): the changed tests run
against product code at base 3811f78, then against this branch.

The base fails at COLLECTION: the tests import a symbol this branch adds
(measure_red.sh checked the diff adds its top-level definition).

RED on the base:
```
E   ImportError: cannot import name 'CatalogInvalid' from 'tinyagentos.orb.catalog_sig' (/tmp/measure-red-hMLer2/wt/tinyagentos/orb/catalog_sig.py)
ERROR tests/test_routes_orb.py
1 error in 0.62s
```

GREEN on the fix:
```
83 passed in 56.38s
```
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between e3efe6d and 3c21fc8.

Files selected for processing: 4
  • tests/orb/test_catalog_sig.py
  • tests/test_routes_orb.py
  • tinyagentos/orb/catalog_sig.py
  • tinyagentos/routes/orb.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@gitar-bot

gitar-bot Bot commented Oct 31, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@jaylfc jaylfc added the needs-jay Automerge refused (sensitive path / unreadable compare): Jay merges. Not counted in the lane PR cap. label Oct 31, 2026
@jaylfc

jaylfc commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Lead triage 2026-11-01: changelog-gate red (no fragment for catalog_sig.py / routes/orb.py). Fix-forward card tsk-w2jhic.

@jaylfc

jaylfc commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Lead review (taosc-dev), card tsk-epdulm: BLOCK, 3 must-fix findings. Applies to #4092 too, which carries the same code plus the changelog fragment.

  1. Bug: a kind refusal is still an unhandled 500. tinyagentos/orb/catalog.py is not changed. check_firmware_kind still raises plain OrbError("catalog kind %r is not a firmware catalog"), so the new CatalogKindInvalid route mapping is never reached in production. Card step 2 requires check_firmware_kind to raise CatalogKindInvalid for a non-firmware kind (its non-dict case stays plain OrbError).
  2. Bug: the cache-fallback tuple is not extended. Card step 3 says BOTH exception maps. Only the outer (remote) except gained the two classes. The inner except under except CatalogFetchError: (remote fetch failed, falling back to load_cached_catalog) still lists only the five old classes. So a cached doc with a bad kind, or a cached array or bad expires, reached through the fallback is still a 500.
  3. Tests do not exercise the real path. test_catalog_admin_remote_fetch_bad_kind and test_catalog_admin_cache_only_bad_kind monkeypatch fetch_catalog / load_cached_catalog to raise CatalogKindInvalid themselves, so they pass with finding 1 unfixed. The card asks for (a) a remote fetch serving a correctly signed {"serial": 5, "kind": "orb-desktop-apps", "apps": []} and (b) that doc cached in cache-only mode. They must run through the real fetch_catalog / load_cached_catalog (stub only the HTTP layer and sign with a test key, like the existing signed-catalog cases), so they FAIL on this head. Add one fallback case for finding 2: remote CatalogFetchError plus a cached bad-kind doc gives 503 orb_catalog_kind_invalid.

Red-first: the harness red is an ImportError at collection, not the card's AssertionError on 500 != 503. Once the tests go through the real path, a 500 != 503 assertion red on base is available: paste that one in the commit body AND the PR body.

Fine as is: the catalog_sig.py classes and the verify_catalog changes; no bare OrbError caught.

Fix on ONE PR only (#4092, which has the changelog), and close the other.

@jaylfc jaylfc added the lead-blocked Lead has blocked this PR; gate_merge.sh refuses at exit 10. label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lead-blocked Lead has blocked this PR; gate_merge.sh refuses at exit 10. needs-jay Automerge refused (sensitive path / unreadable compare): Jay merges. Not counted in the lane PR cap.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant