diff --git a/CLAUDE.md b/CLAUDE.md index 1bc5c42..955df1c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,47 +168,104 @@ as the origin address (same physical address, just registered under the sandbox account) and auto-saves the resulting `warehouse_id` into `TEST_SHIPSTATION_SIGNIFY_WAREHOUSE_ID` / `TEST_SHIPSTATION_OAKSTREET_WAREHOUSE_ID`. -**There is no way to list store IDs, in test mode or production - confirmed a dead -end.** `list_stores()` used to call `GET /v2/stores`, which doesn't exist (plain 404, -"No route matched with those values"). Checked against ShipStation's own V2 OpenAPI -reference: there is no Stores/Marketplaces section at all - `store_id` only ever shows -up as an *input* field on label/shipment requests, never as a listable resource. Their -own help docs confirm the only ways to get a store_id are ShipStation support looking -it up, or the legacy V1 API (different auth - key+secret Basic Auth, not the single -V2 API-Key header this app uses everywhere). `list_stores()` now raises a clear error -saying so instead of a confusing 404; the Data-menu item was relabeled "About -ShipStation Store IDs..." accordingly. **The only real fix**: log into the -ShipStation account's own UI (Settings > Store Setup) - for test mode, that means the -TEST/sandbox account specifically, not production - find or create a manual store, -and copy its store_id into `TEST_SHIPSTATION_SIGNIFY_STORE_ID` / -`TEST_SHIPSTATION_OAKSTREET_STORE_ID` by hand. This was caught mid-debugging: a -"Return label creation failed" / "invalid store" error on `store_id: 367672` (Oak -Street's *production* `se-367672`, prefix stripped in the echoed error per point 8 -above) - happening because `TEST_SHIPSTATION_OAKSTREET_STORE_ID` was still blank. -The fix requires **two separate manual stores in the sandbox account** (one per -company), not one shared - mirrors production, which already has two independent -store IDs (`SHIPSTATION_SIGNIFY_STORE_ID`/`SHIPSTATION_OAKSTREET_STORE_ID`), and the -`TEST_*` settings already have two independent slots for exactly this. There was no -way to confirm via API whether one store could technically be shared across -companies in the sandbox (still no listing/inspection endpoint) - moot anyway, since -per-company stores is the already-intended design. +**SOLVED, 2026-10-01: test mode can NEVER have a store_id - it's a platform limit, +not a configuration gap.** Confirmed directly in ShipEngine/ShipStation's own +dashboard (the actual separate sandbox account, reached via its own distinct sandbox +login - not `ship15.shipstation.com`, which is the production web UI regardless of +how a store there is named): the Sandbox environment's "Connect order sources" flow +explicitly says to **switch to Production to connect order sources**. Stores/Order +Sources structurally do not exist in sandbox at all - unlike carriers and warehouses +(both confirmed to have real sandbox equivalents), this is one ShipStation resource +type the sandbox environment simply doesn't support. No amount of searching the UI, +guessing IDs, or chasing V1 API tricks was ever going to produce a valid +`TEST_SHIPSTATION_*_STORE_ID`, because there is no such thing to find. -**`store_id` is schema-optional, but don't take that as license to drop it**: checked -ShipStation's own request schema for `POST /v2/labels` - `shipment.store_id` is NOT -in the list of required fields (unlike `shipment.ship_to`), and nothing in the schema -ties a store to a specific carrier/warehouse. But this app's own store_id check -exists for a real, previously-confirmed reason (see Return Labels point 1 above): a -label can come back `status: completed` from the API while being invisible anywhere -in ShipStation's own UI. The schema being lenient doesn't mean this account's actual -behavior is - keep requiring it. **Confirmed live, 2026-09-18** (three direct -`POST /v2/labels` probes against the sandbox key, harmless - sandbox labels, no real -cost): omitting `store_id` entirely succeeds cleanly (`200`, `status: "completed"`, -real label `se-201075505`) - so it genuinely is optional API-side, exactly as the -schema says. Whether that label is actually visible in ShipStation's own UI (the -thing that would tell us if the "invisible label" concern really extends to -store_id, or was specific to unlinked return labels) still needs a human to check the -sandbox account's Orders/Shipments list for `external_shipment_id: -STORE-PROBE-no-store-id-at-all`. +**The fix**: `store_id` is now only enforced in PRODUCTION +(`_validate_return_label_prerequisites()` in `shipstation_send.py` checks +`config.is_shipstation_test_mode()` before raising), and left out of the request +entirely when blank (`_create_dummy_outbound_label()` / +`create_return_label_from_dummy()`) rather than sent empty. This was already known to +be API-safe - `shipment.store_id` is not in ShipStation's own required-fields schema, +and a live probe on 2026-09-18 confirmed omitting it succeeds cleanly (`200`, +`status: "completed"`). **Confirmed end-to-end, 2026-10-01**: both Step 1 (dummy +shipment) and Step 2 (return label) now succeed in test mode for both companies with +real sandbox labels (`se-206662629`/`se-206662656` Signify, +`se-206662683`/`se-206662696` Oak Street). Production behavior is unchanged - +store_id is still required there, where it's achievable and still matters for the +originally-confirmed visibility reason (Return Labels point 1 above). + +**Two stores were created in the wrong account along the way** - `ship15.shipstation.com` +turned out to be the unified web UI for *all* accounts, test or production, so +creating a store while logged into the normal company login created it in +production regardless of name (`OAKYTEST` id 381922, `Signify Test` id 381921 - +confirmed in the same `GET /stores` V1 API response as the real production Signify/ +Oak Street stores, proving shared account). Those two got typed into +`TEST_SHIPSTATION_*_STORE_ID` before the mistake was caught, which actively broke +things again until cleared back to blank - **if Step 1 starts failing with "invalid +store" again, check those two settings are still blank before anything else.** +`list_stores()` still raises a clear dead-end error for the unrelated reason it +always did - `GET /v2/stores` plain doesn't exist in the V2 API (confirmed against +ShipStation's own OpenAPI reference: no Stores/Marketplaces section at all, +`store_id` only ever an *input* field) - that part of the finding stands, it just +turned out not to matter once test mode stopped needing a store_id at all. + +**Ceiling discovered 2026-10-02: omitting store_id also silently discards +`external_shipment_id`, so sandbox can verify label *creation* but never ticket +*traceability*.** Confirmed by comparing real labels: querying actual historical +production labels (e.g. real ticket `AR-161051`, created months ago) shows +`external_shipment_id` round-tripping correctly - so the code's placement of that +field is fine and this was never a general bug. But the two sandbox labels created +once store_id started being omitted both came back with ShipStation's own +auto-generated `external_shipment_id` (`SEAuto-...`), not the ticket number we sent - +and `external_order_id`/"Order #" was blank on both, same as the "invisible in the +UI" concern this app's store_id check was originally written to prevent. The +mechanism: no store means no backing Order object, and ShipStation apparently won't +honor a custom `external_shipment_id` without one to attach it to. This isn't +fixable in code - it's the same platform ceiling as the store_id dead-end itself, +just one layer deeper. **Net effect: sandbox can prove the API calls mechanically +succeed (labels get created, no errors), but can never prove ticket-number +traceability works - that joins the branded-email step as something only +verifiable in production.** Treat a successful sandbox Step 1/Step 2 as "the +request shape and credentials are right," not as "this ticket's correlation back to +AR-###### will work" - the latter has only ever been confirmed in production. + +**FOUND AND FIXED, 2026-10-01: `external_shipment_id` was in the wrong place in the +request the whole time - a real, longstanding bug, unrelated to any of the +store_id/test-mode work above.** Both `_create_dummy_outbound_label()` and +`create_return_label_from_dummy()` put it at the TOP level of the `POST /v2/labels` +payload. Confirmed against ShipStation's own request schema: it's only ever a field +ON the shipment object (`shipment.external_shipment_id`) - there is no top-level +equivalent for this endpoint, so ShipStation silently ignored ours and +auto-generated its own `SEAuto-...` placeholder instead, on every single label this +flow has ever created. **Fixed** by moving it inside the `shipment` dict in both +functions. **Confirmed end-to-end on a real production ticket (`AR-166098`, +2026-10-01)**: `external_shipment_id` now correctly reads `AR-166098-DUMMY` on the +dummy and `AR-166098` on the return label, and - the actual point of all of this - +**ShipStation's "Order #" column now correctly shows `AR-166098`**, linked to the +ticket's real pre-existing Order record. This had never been confirmed working +before; `CLAUDE.md`'s own "Known-pending" section had been flagging this exact +end-to-end verification as outstanding for a reason. The dummy's `-DUMMY`-suffixed +external_shipment_id will never show an Order # of its own (it doesn't match any +real Order's number, and isn't meant to - it's throwaway per its own docstring); the +real return label's bare ticket number is what matters, and that's the one that +links up. + +**Also confirmed correct on that same real ticket, despite how it reads in +ShipStation's UI**: the return label's `ship_from`/`ship_to` are right +(`ship_from` = the real customer's address, `ship_to` = the company warehouse - +exactly the direction a return should go), verified via a direct `GET +/v2/shipments/{id}` call (the ground truth) rather than trusting ShipStation's own +"Return Details" UI tab, which mislabels the warehouse/return-destination address as +"Ship From Address" in that specific summary panel - confusing UI copy on +ShipStation's end, not a bug in this code. If this ever looks wrong again, check the +real shipment data via the API before assuming the code regressed. + +**Correlation note**: this app's own tracking-number pull +(`shipstation_service.py::_extract_ticket_number()`) matches `shipment_number`/ +`external_shipment_id` against `TICKET_NUMBER_REGEX` using `fullmatch`, not a +substring search - so the dummy's `-DUMMY` suffix was never going to match anyway, +by design, regardless of the bug above. Only the real return label's bare ticket +number needs to (and now does) match correctly. **A separate, unrelated test-mode gap found 2026-09-18**: `TEST_SIGNIFY_RETURN_CARRIER_ID`/ `TEST_OAKSTREET_RETURN_CARRIER_ID` were blank (only the `*_SERVICE_CODE` halves had @@ -219,26 +276,6 @@ into both settings. Worth knowing for next time: `TEST_SIGNIFY_RETURN_SERVICE_CO `TEST_OAKSTREET_RETURN_SERVICE_CODE` being set doesn't imply their carrier-ID counterparts are - check both halves of a `TEST_*_RETURN_*` pair, not just one. -**The newer ShipStation web UI's URL is NOT the `se-` store ID - confirmed, don't -reuse it.** Creating a manual store at `ship15.shipstation.com/settings/stores/...` -shows a UUID in the URL (e.g. `081cf783-32b2-4b52-b9f8-767531d0ac47`), not an -`se-XXXXX` ID. This is ShipStation's newer/redesigned web UI - that UUID is its own -internal routing ID, a completely different ID space from the V2 API's store IDs, not -just a missing `se-` prefix (the `se-` prefix quirk documented above only ever -applies to genuinely `se-`-shaped IDs missing their prefix, not arbitrary UUIDs). -Confirmed by direct probe: a known-fake-but-correctly-shaped ID (`se-999999999`) -gets the expected clean `400 "invalid store"`, but that UUID (tried both raw and -with `se-` prepended) gets a `500 "An unexpected error occurred"` instead - a -different failure shape entirely, meaning the API doesn't even recognize it as a -candidate ID, let alone a wrong one. **Don't paste that URL UUID into -`TEST_SHIPSTATION_*_STORE_ID` and expect it to work.** The real `se-` ID for a -store created in this newer UI needs to come from somewhere else - check the store's -own settings *page content* (not the URL) for an API/Integration section that -displays it as text, or fall back to ShipStation support (their own help docs say -this explicitly: "contact our support team and tell them the name of the manual -store" is a valid way to get a store_id when the List Stores API isn't an option - -see the store_id dead-end note above). - **Production readiness check, 2026-09-16 (read-only, no labels created, no cost)**: before a first real production test of the return-label flow, ran `GET /v2/carriers`, `GET /v2/warehouses`, and `GET /v2/carriers/{id}/services` directly against the @@ -272,6 +309,27 @@ untouched. **Deliberately gated to Test Mode** - the same flow against productio settings would create a real, paid UPS shipment to a fake address, not just a sandbox test label. +**Bug found and fixed, 2026-10-02: per-ticket write-backs were hardcoded to +`source == "jira"`**, so they silently no-op'd for `source="test"` tickets. +`mark_shipstation_sent()`, `save_dummy_outbound_label_id()`, and `save_pack_data()` +in `workers.py` all looked up `WHERE source == "jira" AND ticket_number == ...` - +harmless for real tickets, but meant step 1 of the return-label flow on a test +ticket would genuinely succeed in ShipStation (a real label got created) while the +app silently failed to remember it, making "Create Return Label" always restart at +step 1 no matter how many times it was run - and the exact same symptom reproduced +in PRODUCTION too, since it had nothing to do with store_id or test mode at all, +just this lookup. **Fixed** by matching on `ticket_number` alone - `source` is only +ever `"jira"` or `"test"`, and `ticket_number` is already unique across both (test +tickets use the `TEST-EMAIL-` prefix specifically so they can't collide with a real +JIRA number), so there was never a real reason to restrict these three to `"jira"`. +Confirmed end-to-end after the fix: create dummy -> persist -> reload from DB -> +create real return label all succeeded in one real (sandbox) run. **Side effect +worth knowing**: because this bug made every "Create Return Label" click on an +already-stepped-through test ticket silently restart step 1, testing this in +PRODUCTION (while chasing what looked like a store_id problem) may have created an +extra, real, paid dummy shipment under a `TEST-EMAIL-*` ticket - worth checking the +production account for stray dummy shipments and voiding any unwanted ones. + **Real limitations of ShipStation's sandbox (confirmed via their own docs, not assumed) that constrain what test mode can actually verify:** - Branded Labels / Branded Tracking Pages are NOT available in sandbox. This directly @@ -360,22 +418,20 @@ change without a deliberate separate conversation about it. ## Known-pending / not yet built -- **Immediate next step, as of the last working session**: the warehouse_id gap is - fixed and confirmed (both `TEST_SHIPSTATION_*_WAREHOUSE_ID` settings are populated). - The store_id gap is still open and turned out to be more involved than the - warehouse one: two manual stores were created in the sandbox account, one per - company (see ShipStation Test Mode above for why two, not one), but ShipStation's - newer web UI (`ship15.shipstation.com`) only exposes a UUID in the URL, not the - `se-XXXXX` ID this app's `TEST_SHIPSTATION_*_STORE_ID` settings need - confirmed via - direct API probe that this UUID is not a usable store_id at all (a different, - `500`-shaped failure than a genuinely wrong-but-well-formed ID gets). The real - `se-` ID for each new store still needs to be found (store settings page content, - or ShipStation support) before the emailed-return-label flow can be tested - end-to-end in test mode. Also still open: whether a label with NO store_id at all - is actually visible in ShipStation's UI - confirmed live that the API accepts a - request with the field omitted (see ShipStation Test Mode above), which, if it - turns out to be visible too, could make chasing the real store_id unnecessary for - test mode specifically. +- **RESOLVED 2026-10-01, was the longest-running open item**: the warehouse_id and + store_id test-mode gaps are both fixed and confirmed end-to-end. Warehouse_id just + needed `TEST_SHIPSTATION_*_WAREHOUSE_ID` populated (done). Store_id turned out to + be structurally impossible to fix via settings at all - ShipStation's sandbox + environment cannot have stores/Order Sources, full stop (confirmed in the real + sandbox dashboard, which says to switch to Production to connect one) - so the code + now only requires store_id in production and omits it entirely in test mode. Both + Step 1 and Step 2 of the emailed-return-label flow are confirmed working end-to-end + in test mode for both companies as of this fix (see ShipStation Test Mode above for + the real sandbox label IDs from that test). **This flow is now genuinely testable + in sandbox for the first time** - if there's still an appetite to verify the + branded-email step too, remember that part specifically still requires production + (Branded Labels/Tracking Pages aren't available in sandbox - see the sandbox + limitations list below). - **EOD JIRA push**: deliberately deferred. `packed`, `serial_numbers`, `tracking_numbers`, `shipping_method`, `assignee` are all captured and ready for it whenever it's prioritized - would need the exact JIRA custom field IDs (outbound @@ -383,12 +439,15 @@ change without a deliberate separate conversation about it. - **Bulk JIRA -> ShipStation import**: still an external, separate process (their own CSV-conversion tool) - this app only handles the single-ticket emergency send case. A bigger, riskier build if ever tackled (rate limits, bulk automation behavior). -- **Return-label flow real-world verification**: the two-step dummy-then-return flow - is fully tested at the code level (unit tests covering both success and - void-on-failure paths), but end-to-end confirmation that it behaves correctly with - ShipStation's actual production API, for the actual team, is still in progress as of - the last working session - don't assume it's fully proven in production just because - the code is correct and tests pass. +- **RESOLVED 2026-10-01: return-label flow real-world verification.** Confirmed + end-to-end on a real production ticket (`AR-166098`) for the first time: Step 1 + (dummy) and Step 2 (return label) both succeeded, Order # correctly shows the real + ticket number in ShipStation, and ship_from/ship_to are correct for a return. This + surfaced and fixed a real bug along the way (external_shipment_id placement - see + ShipStation Test Mode above) that had been silently breaking ticket traceability on + every label this flow ever created, sandbox or production, until now. Test labels + from this verification were voided by the team since real tickets were about to be + worked for real. - **JIRA write access / auto-cancellation**: explicitly declined by the team so far (cancellation requires a JIRA account and an audited reason) - don't build this without a deliberate, separate conversation about it first. diff --git a/app/services/shipstation_send.py b/app/services/shipstation_send.py index 54c3684..ff0d34b 100644 --- a/app/services/shipstation_send.py +++ b/app/services/shipstation_send.py @@ -623,7 +623,16 @@ def _create_dummy_outbound_label( shipment: dict = { "carrier_id": carrier_id, "service_code": service_code, - "store_id": store_id, + # Confirmed against ShipStation's own request schema for POST /v2/labels: + # external_shipment_id is a field ON the shipment object + # (shipment.external_shipment_id), there is no top-level equivalent for + # this endpoint. Putting it at the top level (as this code used to) means + # ShipStation silently ignores it and auto-generates its own "SEAuto-..." + # placeholder instead - confirmed live: a real production dummy shipment + # came back with external_shipment_id "SEAuto-..." instead of the ticket + # number we sent, which is almost certainly why "Order #" never showed up + # in ShipStation's UI for labels from this flow specifically. + "external_shipment_id": external_shipment_id, "ship_to": { "name": info.get("name", ""), "phone": info.get("phone", ""), @@ -636,13 +645,17 @@ def _create_dummy_outbound_label( }, "packages": [_build_return_package(1.0, 1.0, 1.0, 1.0)], } + # Left out entirely when blank (test mode only - see + # _validate_return_label_prerequisites) rather than sent as an empty string, + # matching the confirmed-working shape from a live probe. + if store_id: + shipment["store_id"] = store_id if warehouse_id: shipment["warehouse_id"] = warehouse_id else: shipment["ship_from"] = _return_address_for_order(order) payload = { - "external_shipment_id": external_shipment_id, "shipment": shipment, } return _post_to_labels(payload, api_key) @@ -662,8 +675,17 @@ def _validate_return_label_prerequisites(order: Order) -> tuple[str, str, str, s "Add them in Settings under Return Labels." ) + # store_id is only enforced in PRODUCTION. Confirmed directly in ShipStation's own + # dashboard: the Sandbox environment cannot connect/create Order Sources ("stores") + # at all - it explicitly says to switch to Production to do that. So no valid + # TEST_SHIPSTATION_*_STORE_ID can ever exist; requiring one in test mode would make + # this flow permanently untestable in sandbox. Confirmed via a live API probe that + # omitting store_id entirely still succeeds (schema-optional, not just a guess) - + # see _create_dummy_outbound_label()/create_return_label_from_dummy() for where it's + # left out of the request when blank. Still required in production, where it's + # achievable and matters for real (see the error message below). store_id = _store_id_for_company(order.company) - if not store_id: + if not store_id and not config.is_shipstation_test_mode(): raise ShipStationSendError( f"No ShipStation Store ID configured for '{order.company}'. Add it in Settings " "under ShipStation. A label created without one may not show up anywhere in " @@ -738,33 +760,42 @@ def create_return_label_from_dummy( ticket_number = order.ticket_number or order.external_id - payload = { + shipment: dict = { + "carrier_id": carrier_id, + "service_code": service_code, + # Nested here, not top-level - see _create_dummy_outbound_label() for why + # (confirmed against ShipStation's own schema; a top-level + # external_shipment_id is silently ignored on POST /v2/labels). "external_shipment_id": ticket_number, + "ship_to": return_address, + "ship_from": { + "name": info.get("name", ""), + "phone": info.get("phone", ""), + "address_line1": info.get("address1", ""), + "address_line2": info.get("address2", "") or None, + "city_locality": info.get("city", ""), + "state_province": info.get("state", ""), + "postal_code": info.get("zip", ""), + "country_code": "US", + }, + "packages": [ + _build_return_package( + p.get("weight_oz", 1.0), p.get("length", 0), p.get("width", 0), p.get("height", 0) + ) + for p in packages + ], + } + # Left out entirely when blank (test mode only - see + # _validate_return_label_prerequisites), matching the confirmed-working shape + # from a live probe, rather than sent as an empty string. + if store_id: + shipment["store_id"] = store_id + + payload = { "is_return_label": True, "outbound_label_id": dummy_label_id, "charge_event": resolved_charge_event, - "shipment": { - "carrier_id": carrier_id, - "service_code": service_code, - "store_id": store_id, - "ship_to": return_address, - "ship_from": { - "name": info.get("name", ""), - "phone": info.get("phone", ""), - "address_line1": info.get("address1", ""), - "address_line2": info.get("address2", "") or None, - "city_locality": info.get("city", ""), - "state_province": info.get("state", ""), - "postal_code": info.get("zip", ""), - "country_code": "US", - }, - "packages": [ - _build_return_package( - p.get("weight_oz", 1.0), p.get("length", 0), p.get("width", 0), p.get("height", 0) - ) - for p in packages - ], - }, + "shipment": shipment, } data = _post_to_labels(payload, api_key) diff --git a/app/workers.py b/app/workers.py index c845720..cf08257 100644 --- a/app/workers.py +++ b/app/workers.py @@ -443,11 +443,16 @@ def load_orders_by_view() -> Tuple[List[Order], List[Order], List[Order]]: def mark_shipstation_sent(ticket_number: str) -> None: """Stamps shipstation_sent_at after a CONFIRMED emergency API send - - called once ShipStation's own response confirms creation succeeded.""" + called once ShipStation's own response confirms creation succeeded. + + Matches by ticket_number alone, not source == "jira" - source is only + ever "jira" or "test" (synthetic tickets from create_test_shipment_order()), + and ticket_number is already unique across both, so restricting to "jira" + here just means this silently no-ops for test tickets instead of erroring.""" session = get_session() try: order = session.execute( - select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number) + select(Order).where(Order.ticket_number == ticket_number) ).scalar_one_or_none() if order is not None: order.shipstation_sent_at = dt.datetime.now() @@ -459,11 +464,14 @@ def mark_shipstation_sent(ticket_number: str) -> None: def save_dummy_outbound_label_id(ticket_number: str, label_id: str) -> None: """Persists step 1's result (the dummy shipment's label_id) so step 2 (the real return label) can be triggered separately - including in a - later session, after the dummy has been verified in ShipStation.""" + later session, after the dummy has been verified in ShipStation. + + Matches by ticket_number alone - see mark_shipstation_sent() above for why + this must not be restricted to source == "jira".""" session = get_session() try: order = session.execute( - select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number) + select(Order).where(Order.ticket_number == ticket_number) ).scalar_one_or_none() if order is not None: order.dummy_outbound_label_id = label_id @@ -478,11 +486,14 @@ def save_pack_data(ticket_number: str, serial_numbers: dict, packed: bool) -> No dialog. Staff-entered data, not sourced from JIRA - this is the beginning of the eventual end-of-day push back to JIRA (deferred for now), so nothing here gets overwritten by a JIRA re-import. + + Matches by ticket_number alone, not source == "jira" - see + mark_shipstation_sent() for why. """ session = get_session() try: order = session.execute( - select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number) + select(Order).where(Order.ticket_number == ticket_number) ).scalar_one_or_none() if order is None: return