Repository navigation
Read a workflow stage as JSON in every SDK - #285
Conversation
HEY served a workflow stage only as its web page, so GetWorkflowStage was modelled as an HTML blob and Go, Rust, Kotlin and Swift each parsed the page for the stage's threads. HEY now answers the stage as JSON: the stage, and each thread as the topic JSON HEY answers elsewhere with its staging id and email and note counts. Model that answer (WorkflowStageDetail, StagedTopic) and send the read as JSON. The hand-written stage read keeps its signature and its view in every SDK, reduced from the JSON: topic_id from id, subject from name, entry_count from emails_count, in HEY's order. The HTML parsers go, with Rust's scraper and ego-tree and Go's direct golang.org/x/net dependency. The generators' text/html path stays, covered by their fixtures and by core tests on a route of their own. Rust, Kotlin and Swift lose WorkflowStageView::parse, and every generated get_stage answers the stage rather than a string: noted in MIGRATING.md.
The crate no longer depends on it, and cargo deny --locked refuses a lock file that would have to change.
90c8279 to
76ce9f0
Compare
|
🤖 Rebased onto |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
The API-breaking JSON contract depends on an inaccessible upstream PR, so a maintainer must verify its response shape and deployment before merging.
No open issues 🎉
What changed in this PR
Moves workflow-stage reads from HTML scraping to typed JSON across all five SDKs, aligning the API → SDK → CLI pipeline while preserving the hand-written stage-view interfaces. Deployment depends on the server support described in basecamp/haystack#9103.
Changes:
- Models stage details and staged topics in Smithy and regenerates all SDK responses.
- Replaces HTML parsers with JSON-to-view conversions and removes unused parser dependencies.
- Updates unit tests, conformance fixtures, and migration documentation while retaining generic HTML transport coverage.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
typescript/tests/client.test.ts |
Tests stage JSON and retains HTML transport coverage. |
typescript/src/generated/schema.ts |
Adds typed stage and topic responses. |
typescript/src/generated/operations.ts |
Removes stage HTML metadata. |
swift/Tests/HeyTests/WorkflowStageTests.swift |
Removes obsolete HTML-parser tests. |
swift/Tests/HeyTests/WorkflowsServiceTests.swift |
Tests JSON conversion and error reporting. |
swift/Tests/HeyTests/RouteTests.swift |
Asserts the stage route uses JSON. |
swift/Tests/HeyTests/RequestTests.swift |
Tests JSON requests and generic HTML support. |
swift/Tests/HeyTests/CacheTests.swift |
Uses a test HTML route for cache coverage. |
swift/Sources/Hey/Services/WorkflowsService+Conveniences.swift |
Converts JSON details into the existing stage view. |
swift/Sources/Hey/Services/Html.swift |
Deletes the HTML parser. |
swift/Sources/Hey/Generated/Services/WorkflowsService.swift |
Returns typed stage JSON. |
swift/Sources/Hey/Generated/Routes.swift |
Marks the stage route as non-HTML. |
swift/Sources/Hey/Generated/Models/WorkflowStageDetail.swift |
Adds the stage-detail model. |
swift/Sources/Hey/Generated/Models/StagedTopic.swift |
Adds staged-topic fields and counts. |
swift/Sources/Hey/Generated/Models/GetWorkflowStageResponseContent.swift |
Aliases the JSON response model. |
spec/shape-fingerprint.json |
Records the stage response fingerprint. |
spec/hey.smithy |
Defines the JSON stage contract. |
rust/hey-sdk/tests/services_workflows.rs |
Replaces parser tests with JSON-view coverage. |
rust/hey-sdk/src/services/workflows.rs |
Replaces scraping with typed conversion. |
rust/hey-sdk/src/generated/types.rs |
Adds stage and staged-topic types. |
rust/hey-sdk/src/generated/services/workflows.rs |
Returns typed stage JSON. |
rust/hey-sdk/src/generated/routes.rs |
Disables HTML for the stage route. |
rust/hey-sdk/src/client.rs |
Preserves HTML coverage with a dedicated test route. |
rust/hey-sdk/PARITY.md |
Documents JSON-stage parity with Go. |
rust/hey-sdk/Cargo.toml |
Removes HTML-parser dependencies. |
rust/Cargo.lock |
Removes unused parser dependency entries. |
README.md |
Updates the browser-form service description. |
openapi.json |
Publishes the generated JSON response schemas. |
MIGRATING.md |
Documents breaking APIs and conversion replacements. |
kotlin/sdk/src/commonTest/kotlin/com/basecamp/hey/WorkflowStageTest.kt |
Removes obsolete HTML-parser tests. |
kotlin/sdk/src/commonTest/kotlin/com/basecamp/hey/WorkflowsServiceTest.kt |
Tests JSON conversion and missing stages. |
kotlin/sdk/src/commonTest/kotlin/com/basecamp/hey/RouteTest.kt |
Asserts the stage route uses JSON. |
kotlin/sdk/src/commonTest/kotlin/com/basecamp/hey/RequestTest.kt |
Tests JSON requests and generic HTML support. |
kotlin/sdk/src/commonTest/kotlin/com/basecamp/hey/HooksTest.kt |
Checks JSON decoding errors reach hooks. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/services/WorkflowsService.kt |
Converts JSON details into the existing view. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/internal/Html.kt |
Deletes the HTML parser. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/generated/services/WorkflowsService.kt |
Returns typed stage JSON. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/generated/Routes.kt |
Disables HTML for the stage route. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/generated/models/WorkflowStageDetail.kt |
Adds the stage-detail model. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/generated/models/StagedTopic.kt |
Adds staged-topic fields and counts. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/hey/generated/models/GetWorkflowStageResponseContent.kt |
Aliases the JSON response model. |
go/pkg/hey/workflows.go |
Replaces parsing with JSON-to-view conversion. |
go/pkg/hey/services_test.go |
Tests JSON reads and non-JSON rejection. |
go/pkg/hey/retry_policy_test.go |
Renames and documents the existing retry test. |
go/pkg/generated/client.gen.go |
Adds stage models and JSON response decoding. |
go/go.mod |
Makes golang.org/x/net indirect. |
conformance/tests/paths.json |
Verifies JSON requests and stage-view fields. |
conformance/runner/typescript/runner.ts |
Updates the stage-view exclusion check. |
conformance/runner/typescript/README.md |
Explains the JSON-stage coverage exclusion. |
conformance/runner/typescript/not-applicable.json |
Updates the excluded fixture and rationale. |
conformance/runner/typescript/fixture-inventory.json |
Renames the stage fixture entry. |
conformance/runner/rust/Cargo.lock |
Removes unused parser dependency entries. |
AGENTS.md |
Updates documentation for retained HTML support. |
Review effort: Balanced
|
Marked ready now that basecamp/haystack#9103 has merged (02b092e9bb). There are no review threads. Codex's code and security reviews completed on 76ce9f0 with no findings. Copilot reviewed 76ce9f0 and has no open issues.
Open for a decision: when to merge relative to haystack's deploy, for @jeremy. This PR drops the HTML parser in every SDK, so a release cut before 02b092e9bb reaches production would read the stage from a HEY that doesn't serve it as JSON yet. |
Why
hey-sdk read a workflow stage's threads out of the stage's web page, because HEY served the stage only as HTML. That broke hey-cli's "nothing scrapes a web page" rule and the API → SDK → CLI pipeline: the SDK carried an HTML parser per language (Go
golang.org/x/net/html, Rustscraper, hand-written ones in Kotlin and Swift), and every change to the page's markup could break them quietly. HEY now serves the stage as JSON.Depends on basecamp/haystack#9103. Keep this a draft until that merges and deploys: against a HEY without it,
GET /workflows/{id}/stages/{id}.jsonhas no JSON to answer.Used by basecamp/hey-cli#380 (
hey workflow stage view). Its command code needs no change: Go'sWorkflowsService.GetStage(ctx, workflowID, stageID) (*WorkflowStageView, error)keeps its signature, struct fields and JSON tags.What changes
Spec.
GetWorkflowStageanswersWorkflowStageDetail(id,name,created_at,updated_at,workflow_id,app_url,topics), each topic aStagedTopic: theTopicmembers as_topic.jbuilderemits them (noentries), plusstaging_id(required),emails_count,notes_count. Smithy has no mixins in this spec, soStagedTopiclists the topic members itself. The@mediaType("text/html") blob WorkflowStageHTMLis gone. Not paginated, as HEY's isn't.Hand-written stage read, every SDK. Same public types and fields, reduced from the JSON in HEY's order:
TopicID/topic_id←id,Subject/subject←name,EntryCount/entry_count←emails_count(the email count the old parser read off the card's "N emails, M notes" line),StagingID/staging_id←staging_id.GetStagesends the generated request as is (.json,Accept: application/json) and refuses a 200 with no JSON body.parseWorkflowStageHTML,findNode,attr,visibleNodeTextandisVisuallyHiddenare gone (nothing else used them), along with an orphanedworkflowStageIDRecomment.golang.org/x/netdrops to an indirect requirement (oapi-codegen still pulls it in).Workflows::stageisget_stageplusFrom<WorkflowStageDetail> for WorkflowStageView. The scraper parser is gone, and with it thescraperandego-treedependencies.stageisgetStageplusWorkflowStageView.from(detail).internal/Html.ktis gone.stageisgetStageplusWorkflowStageView(detail).Services/Html.swiftis gone.getWorkflowStageanswers the typed stage.The generators'
text/htmlpath stays. No modelled operation uses it now. It's still covered by the generator fixtures (Rust, Kotlin, Swift) and by the core tests, which now use a route of their own (Rust: a testRoute; Kotlin:GET_WORKFLOW_STAGE.copy(html = true); Swift: a copy withhtml = true; TypeScript: the stage's metadata marked HTML for the test's duration). AGENTS.md no longer gives the workflow stage as the HTML example.Conformance. The
paths.jsonstage case now mocks HEY's JSON and asserts/workflows/8801/stages/5512.json,Accept: application/json, and the view: both threads' staging and topic ids, the full thread's subject and email count, the sparse thread's defaults. The Go, Rust, Kotlin and Swift runners already send it through the hand-writtenstage. TypeScript keeps it not-applicable, since its assertions are on the hand-written view, which TypeScript doesn't have. The exclusion's reason and the runner's check on it are updated.Drift. No snapshot or provenance change.
GET /workflows/{workflow_id}/stages/{id}was already inspec/route-snapshot.json(its route takes(.:format)) and already modelled, so the reverse check accounts for it. The forward check strips.jsonbefore comparing. The haystack PR adds a view, not a route, sodrift-regenagainst it would pin provenance to an unmerged SHA for no change in the snapshot. I didn't run it.Public API changes (breaking for Rust, Kotlin, Swift, TypeScript; none in Go's
pkg/hey)WorkflowStageView::parse(&str, i64)removed.Workflows::get_stagereturnsGetWorkflowStageResponseContent(=WorkflowStageDetail), notString.GetWorkflowStageOutputPayloadremoved.WorkflowStageDetail,StagedTopicandimpl From<WorkflowStageDetail> for WorkflowStageViewadded.Workflows::stage,WorkflowStageViewandWorkflowStageTopicare unchanged.WorkflowStageView.parse(String, Long)removed;getStagereturnsGetWorkflowStageResponseContent;WorkflowStageView.fromadded.WorkflowStageView.parse(_:stageId:)removed;getStagereturnsGetWorkflowStageResponseContent;WorkflowStageView.init(_:)added.getWorkflowStageresolvesdatato the stage object, not astring.go/pkg/heyis unchanged;go/pkg/generatedgainsJSON200and the new types and losesGetWorkflowStageOutputPayload.parsecan't stay on any honest terms: its input was the page, and keeping it would keep the parsers this PR exists to remove. The generatedget_stagereturn type breaks the Rust API anyway. So this PR carries thebreakinglabel (which skipsapi-compat-rust, and also Go'sapi-compat; Go'spkg/heysurface is unchanged:apidiffagainstmain'sgo/pkg/heyreports no change, compatible or not). MIGRATING.md has the before/after.