Repository navigation
Conversation
Resync the public contract against the backend branch that adds
POST /opencompany/instances/{slug}/update: add it to the generated public
route registry and the namespace manifest, and bump the pinned operation/
route counts (237 -> 238) in the sync test.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Comment |
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0128 · 203,028 in / 11,146 out · 9,390 cached (5%) · flash, ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 124 embedded
critique: $0.0046 · 84,910 in / 1,772 out · 4,048 cached (5%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0048 · 90,520 in / 2,530 out · 5,342 cached (6%) · gpt-5.6-luna
tests: $0.0016 · 15,941 in / 1,097 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0011 · 7,263 in / 2,587 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| ("POST", "/opencompany/instances/{slug}/custom-domain/verify"), | ||
| ("POST", "/opencompany/instances/{slug}/resume"), | ||
| ("POST", "/opencompany/instances/{slug}/suspend"), | ||
| ("POST", "/opencompany/instances/{slug}/update"), |
There was a problem hiding this comment.
Add the typed OpenCompany update method
This newly exposed public operation has no corresponding method on OpenCompanyApi (the client currently provides create, delete, suspend, resume, and custom-domain methods only). Callers must use the raw transport for a route that the generated public contract says is supported, contrary to the repository's one-typed-method-per-public-operation contract; add the request type, typed method, and route test alongside this registry entry.
[RULE] missing-typed-method ·
| assert_eq!(manifest["source"]["excludedAdminOperationCount"], 44); | ||
| assert_eq!(manifest["source"]["excludedWebhookOperationCount"], 12); | ||
| assert_eq!(rust_routes.len(), 237); | ||
| assert_eq!(rust_routes.len(), 238); |
There was a problem hiding this comment.
Regenerate the Rust route registry for the new route
The manifest includes POST /opencompany/instances/{slug}/update, but the generated PUBLIC_ROUTES registry does not contain it. Consequently, rust_routes.len() and rust_routes == manifest_routes fail in every test run. Regenerate the route registry so it includes the new public operation, rather than only increasing the expected count here.
[RULE] generated-route-parity ·
There was a problem hiding this comment.
Resolved — the review agent found this finding fixed in the new code, as of 06ba4e4.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
| "POST /opencompany/instances/{slug}/custom-domain/verify", | ||
| "POST /opencompany/instances/{slug}/resume", | ||
| "POST /opencompany/instances/{slug}/suspend", | ||
| "POST /opencompany/instances/{slug}/update", |
There was a problem hiding this comment.
Add the typed client method for the new update route
This exposes POST /opencompany/instances/{slug}/update in the public manifest without adding the matching Rust route entry, request/response types, typed namespace method, and route test. The generated-route parity test will also detect that the checked-in registry does not match the manifest. Complete the contract change across the Rust client and generated registry, or do not expose the route here.
[RULE] missing-typed-method ·
How this change flows0 changed behaviours across 6 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 10 further behaviours left out to keep the diagram readable. flowchart LR
n0["..._api_key_request_uses_openapi_field_names"]:::impacted
n1["path_segments_are_encoded_on_typed_routes"]:::impacted
n2["create"]:::impacted
n3["try_from"]:::impacted
n4["get_feedback"]:::impacted
n5["...ejects_the_machine_only_connections_scope"]:::impacted
n0 -->|calls| n2
n0 -->|tests| n2
n1 -->|calls| n4
n1 -->|tests| n4
n5 -->|calls| n3
n5 -->|tests| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
…ance-update # Conflicts: # api/tinyhumans.backend.json # tests/openapi_sync.rs
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0119 · 206,442 in / 14,048 out · 16,268 cached (8%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 124 embedded
critique: $0.0072 · 121,745 in / 7,993 out · 7,098 cached (6%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0032 · 60,926 in / 2,461 out · 7,122 cached (12%) · gpt-5.6-luna
tests: $0.0009 · 16,211 in / 1,321 out · 1,024 cached (6%) · deepseek/deepseek-v4-flash
description: $0.0006 · 7,560 in / 2,273 out · 1,024 cached (14%) · deepseek/deepseek-v4-flash
| assert_eq!(manifest["source"]["excludedAdminOperationCount"], 44); | ||
| assert_eq!(manifest["source"]["excludedWebhookOperationCount"], 12); | ||
| assert_eq!(rust_routes.len(), 203); | ||
| assert_eq!(rust_routes.len(), 204); |
There was a problem hiding this comment.
Regenerate the Rust route registry for the new route
The checked-in PUBLIC_ROUTES contains no POST /opencompany/instances/{slug}/update, so it remains at 203 entries while this assertion requires 204 and rust_routes == manifest_routes cannot pass. Regenerate the generated route registry so the new public route is present in PUBLIC_ROUTES (and remains absent from UNEXPOSED_ROUTES).
[RULE] generated-route-registry-drift ·
| "POST /opencompany/instances/{slug}/custom-domain/verify", | ||
| "POST /opencompany/instances/{slug}/resume", | ||
| "POST /opencompany/instances/{slug}/suspend", | ||
| "POST /opencompany/instances/{slug}/update", |
There was a problem hiding this comment.
Add the typed OpenCompany update method
This adds the route to the public SDK contract, but OpenCompanyApi has no method for it. Callers are forced to use the raw transport, contrary to the repository's requirement that every changed public operation have a matching typed method, request/response types, and route test. Add the typed update operation before exposing this route.
[RULE] missing-typed-method ·
| "POST /opencompany/instances/{slug}/custom-domain/verify", | ||
| "POST /opencompany/instances/{slug}/resume", | ||
| "POST /opencompany/instances/{slug}/suspend", | ||
| "POST /opencompany/instances/{slug}/update", |
There was a problem hiding this comment.
Add the typed client method for the new update route
The new public route is present in the manifest, but no update_instance-style method exists under src/api/opencompany.rs. This leaves the operation available only through the raw escape hatch and does not preserve the typed SDK surface expected for namespace operations. Add the corresponding client method and request type.
[RULE] missing-typed-method ·
| "POST /opencompany/instances/{slug}/custom-domain/verify", | ||
| "POST /opencompany/instances/{slug}/resume", | ||
| "POST /opencompany/instances/{slug}/suspend", | ||
| "POST /opencompany/instances/{slug}/update", |
There was a problem hiding this comment.
Add the typed method for the OpenCompany instance update route
Adding this manifest entry without adding the matching OpenCompanyApi operation makes the contract and typed API inconsistent. The new endpoint needs a typed request/response path and a route test, rather than being exposed only through raw HTTP.
[RULE] missing-typed-method ·
| assert_eq!(manifest["source"]["operationCount"], 203); | ||
| // 203 -> 204: `POST /opencompany/instances/{slug}/update`, the | ||
| // owner-triggered "update to latest" for a hosted company. | ||
| assert_eq!(manifest["source"]["operationCount"], 204); |
There was a problem hiding this comment.
Add the typed OpenCompany update method
The manifest now counts POST /opencompany/instances/{slug}/update, but OpenCompanyApi has no corresponding method or request/response type. This leaves the newly advertised operation available only through the raw escape hatch and violates the SDK's typed-operation contract; add the typed method and its route test.
[RULE] missing-typed-method ·
| "POST /opencompany/instances/{slug}/custom-domain/verify", | ||
| "POST /opencompany/instances/{slug}/resume", | ||
| "POST /opencompany/instances/{slug}/suspend", | ||
| "POST /opencompany/instances/{slug}/update", |
There was a problem hiding this comment.
Regenerate the Rust route registry for the new route
This manifest adds POST /opencompany/instances/{slug}/update, but the pull request changes only the manifest. The repository's generated PUBLIC_ROUTES registry must be regenerated in the same change; otherwise the manifest and Rust transport allowlist disagree and the OpenAPI parity test will fail.
[RULE] generated-surface-sync ·
| ("POST", "/opencompany/instances/{slug}/custom-domain/verify"), | ||
| ("POST", "/opencompany/instances/{slug}/resume"), | ||
| ("POST", "/opencompany/instances/{slug}/suspend"), | ||
| ("POST", "/opencompany/instances/{slug}/update"), |
There was a problem hiding this comment.
Add the typed OpenCompany update method
This exposes POST /opencompany/instances/{slug}/update in the public route registry without adding the corresponding OpenCompanyApi method, request/response types, and route test required for a contract change. Callers are forced to use the raw transport and lose the typed SDK surface and its normal response handling.
[RULE] missing-typed-method ·
| ("POST", "/opencompany/instances/{slug}/custom-domain/verify"), | ||
| ("POST", "/opencompany/instances/{slug}/resume"), | ||
| ("POST", "/opencompany/instances/{slug}/suspend"), | ||
| ("POST", "/opencompany/instances/{slug}/update"), |
There was a problem hiding this comment.
Add the typed client method for the new update route
The new public route has no matching typed client method in the changed surface. Add the OpenCompany update operation with its request and response types and a route test so the generated registry and typed API remain consistent.
[RULE] missing-typed-method ·
| ("POST", "/opencompany/instances/{slug}/custom-domain/verify"), | ||
| ("POST", "/opencompany/instances/{slug}/resume"), | ||
| ("POST", "/opencompany/instances/{slug}/suspend"), | ||
| ("POST", "/opencompany/instances/{slug}/update"), |
There was a problem hiding this comment.
Add the typed method for the instance update route
Registering this route alone leaves POST /opencompany/instances/{slug}/update available only through the raw escape hatch. The repository contract requires a matching typed OpenCompany method, DTOs, and route test for newly exposed operations.
[RULE] missing-typed-method ·
| ("POST", "/opencompany/instances/{slug}/custom-domain/verify"), | ||
| ("POST", "/opencompany/instances/{slug}/resume"), | ||
| ("POST", "/opencompany/instances/{slug}/suspend"), | ||
| ("POST", "/opencompany/instances/{slug}/update"), |
There was a problem hiding this comment.
Add typed method for the opencompany instance update route
The new public route POST /opencompany/instances/{slug}/update is added to PUBLIC_ROUTES, making it available via the raw HTTP escape hatch, but no corresponding typed method is added to OpenCompanyApi in src/api/opencompany.rs. Per repository rules ("Add matching Rust methods, request/response types, and route tests"), every new public route with a namespace change should have a typed method and a route test. Without this, callers must use the raw client and forfeit envelope unwrapping and typed error handling. Add pub async fn update_instance(...) to OpenCompanyApi and a wiremock test in tests/opencompany.rs similar to usage_gets_instance_usage_summary.
[RULE] missing-typed-method ·
|
Triaged the TinySweeper findings on the current head ( The CRITICAL "Regenerate the Rust route registry" is a stale false positive. The registry already contains the route —
TinySweeper raised it on an earlier push and its index is behind (it notes "2 memory calls timed out, so this review saw part of what the engine holds"), and it already self-resolved one copy of the finding. It does not reproduce on The one substantive item (MEDIUM): a typed The merge conflict from the medulla/orchestration removal on |
Review status — all findings triaged (verified against the code)TL;DR: the merge conflict is resolved, every gate is green ( 1. CRITICAL "Regenerate the Rust route registry" — false positive (does not reproduce on the current head
|
Summary
Resync the public contract against the backend branch that adds
POST /opencompany/instances/{slug}/update(owner-triggered "update to latest" for a hosted company).Changes
src/generated_public_routes.rs: add("POST", "/opencompany/instances/{slug}/update")to the public route registry.api/tinyhumans.backend.json: add the route to the OpenCompany namespace and bump the pinned operation/route counts (255→256 paths, 237→238 operations).tests/openapi_sync.rs: bump the pinnedoperationCount/rust_routes.len()(237→238) with a comment noting the added route.How it was generated
Per
AGENTS.md, from the backend branch's raw spec:npm run swagger -- /tmp/spec.jsonthennode scripts/sync-openapi.mjs --input /tmp/spec.json— not from the deployed spec, which doesn't have this route yet.Testing
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings,cargo test— all green (the route-registry ↔ manifest match test passes with the bumped counts).Notes
Pairs with tinyhumansai/backend#1349, which adds the route. This is the generic public contract only — no substrate details.