feat(contracts): complete local web adapter path#349
Conversation
cssbruno
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fb5eda69a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| out = append(out, event) | ||
| continue | ||
| } | ||
| fresh, err := seen.MarkIfNew(ctx, event.ID) |
There was a problem hiding this comment.
When a seen store is configured, this call records every fresh ID before PublishEnvelopesForRole runs. If a worker subscriber then returns an error and the source accepts Nack, the durable source will redeliver the batch, but those IDs are already considered duplicates and dispatchEventBatch will ack duplicate-only batches without invoking subscribers. This drops events after transient subscriber failures; defer marking until after successful dispatch/ack or make the seen-store write rollbackable.
Useful? React with 👍 / 👎.
| case contractsaddon.ImportPath: | ||
| return contractsaddon.Addon(), true |
There was a problem hiding this comment.
Treating contractsaddon.Addon() as an AST-parsable built-in exposes the existing config.Addons, needsExecutableLoad = parseAddons(...) overwrite: when a config has an earlier dynamic field such as AppName: os.Getenv("APP") and then Addons: []gowdk.Addon{contractsaddon.Addon()}, this new case returns false from parseAddons and clears the earlier needsExecutableLoad, so LoadConfigFile no longer evaluates the executable config and silently uses zero values for those dynamic fields. Preserve any prior needsExecutableLoad value when parsing built-in addons.
Useful? React with 👍 / 👎.
| Cond: notNil("err"), | ||
| Body: block( | ||
| writeNoStoreHandlerErrorExprStmt(id("err"), sel("http", "StatusInternalServerError")), | ||
| writeNoStoreHandlerJSONErrorExprStmt(id("err"), sel("http", "StatusInternalServerError")), |
There was a problem hiding this comment.
This change makes contract adapter execution errors JSON, but requests that fail earlier while decoding command/query input still go through contractDecodeInputStmts → ifErrReturnInvalidForm(), which writes WriteNoStoreError text/plain. For any contract with scanned input fields, a bad scalar value or unknown submitted field therefore returns invalid form as a non-JSON response even though the new contract says adapter failures use {"error":"..."} JSON; use the JSON error helper on this path too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a50729320
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err != nil { | ||
| return contracts.EventBatch{}, err |
There was a problem hiding this comment.
When the opportunistic drain has already appended at least one valid message, a decode error from a later tryNextMessage now returns an empty batch error here. For Core NATS subscriptions the message has already been consumed and this adapter intentionally provides no Ack/Nack hook, so the earlier valid events in events are never dispatched and cannot be redelivered whenever a malformed or undecodable message follows them in the same receive window. Return the accumulated batch when len(events) > 0 or otherwise avoid consuming additional messages before the current batch is handed off.
Useful? React with 👍 / 👎.
| define([]ast.Expr{id("values")}, values), | ||
| define([]ast.Expr{id("input"), id("err")}, call(sel(contractDecoderName(exposure)), id("values"))), | ||
| ifErrReturnInvalidForm(), | ||
| ifErrReturnInvalidJSONForm(), |
There was a problem hiding this comment.
Switching the contract decoder error to ifErrReturnInvalidJSONForm only covers failures after request.ParseForm succeeds; command contract routes still prepend actionParseFormStmts, which writes the non-JSON WriteNoStoreError response for malformed URL-encoded bodies, oversized bodies, and CSRF failures. In those cases a g:command endpoint still violates the new documented stable application/json {"error":"..."} error shape, so contract command parsing/CSRF should use JSON-specific error writers too.
Useful? React with 👍 / 👎.
| } | ||
| record := Record{ | ||
| ID: store.nextID(), | ||
| ID: event.ID, |
There was a problem hiding this comment.
Using event.ID as the file outbox record key makes the Ack/Nack maps operate on event IDs rather than unique persisted rows (decodeRecordsLocked keys acked by record.ID, and removeRecordsLocked/markRecordsFailedLocked update every matching record). If a caller stores two pending envelopes with the same durable event ID and the batch size only delivers one of them, acking or nacking that one also removes, retries, or dead-letters the other undelivered row. Keep a separate unique outbox record ID and store the durable event ID in its own field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd29f75f4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return nil | ||
| } | ||
| file, err := os.OpenFile(store.path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o600) |
There was a problem hiding this comment.
When MarkSeen or MarkIfNew rewrites an existing seen-store file, this opens the live path with O_TRUNC and then streams JSON lines back into it. If the process is interrupted or the filesystem returns a partial-write error after truncation, the seen store can be left empty or with an invalid trailing JSON line; subsequent Seen calls then either forget delivered IDs or fail on readRecordsLocked, which defeats worker deduplication for the file outbox path. Please use the same temp-file-and-rename pattern as the outbox writer so each update either preserves the old file or installs a complete new one.
Useful? React with 👍 / 👎.
1 participant
Summary
g:command/g:query, contracts addon config parsing, and build-report verification for name/kind/path/guards metadata.HandlerErrormessages for client-safe failures, JSON contract input decode failures, and JSON command form parse, oversized body, and CSRF failures.RunContractEventWorkerWithSeenStorehelpers so duplicate event IDs can be skipped while fresh IDs are marked seen only after dispatch and source ack succeed.CloseRead.go listfailures, and scoped-script errors.Issue Closure
Closes #110
Closes #111
Closes #112
Closes #113
Closes #114
Closes #155
Closes #161
Closes #180
Verification
scripts/test-go-modules.shwhen Go code or compiler behavior changed.go build ./cmd/gowdkwhen CLI, compiler, runtime, addon, or release behavior changed.node --check editors/vscode/extension.jswhen editor files changed. N/A: no editor files changed.Commands run:
go test ./runtime/response ./runtime/contracts ./runtime/contracts/fileoutbox ./runtime/contracts/membroker ./runtime/contracts/sse -count=1go test ./internal/appgen ./internal/buildgen ./internal/compiler ./internal/project ./internal/contractscan ./cmd/gowdk -count=1go test ./internal/appgen -run 'TestGenerateWiresCSRFForCommandContracts|TestGeneratedBinaryContractAdaptersReturnJSONErrors|TestGeneratedBinaryContractCommandCSRFReturnsJSONError|TestGeneratedGoMatchesGoldenFixture' -count=1go test ./internal/appgen -count=1go test ./runtime/contracts/fileoutbox -count=1go test ./runtime/contracts/fileoutbox -run 'TestSeenStore' -count=1go test ./internal/appgen ./runtime/contracts/fileoutbox -count=1go test ./runtime/contracts ./runtime/contracts/fileoutbox ./internal/project -count=1go test ./... -count=1inruntime/contracts/redisstreamgo test ./... -count=1inruntime/contracts/natsbrokergo test ./... -count=1inruntime/contracts/websocketfanoutgo test ./...scripts/test-go-modules.shgo build ./cmd/gowdkgo run ./cmd/gowdk check --config examples/contracts/gowdk.config.go examples/contracts/patients.page.gwdkgo run ./cmd/gowdk build --config examples/contracts/gowdk.config.go --out /tmp/gowdk-contracts-build --app /tmp/gowdk-contracts-app --bin /tmp/gowdk-contracts-site examples/contracts/patients.page.gwdktest -x /tmp/gowdk-contracts-site && rg -n '"(name|kind|path|guards|method|contract|adapter|role)"|gwdk.contract|patients.CreatePatient|patients.GetPatientPage' /tmp/gowdk-contracts-build/gowdk-build-report.json /tmp/gowdk-contracts-build/openapi.json /tmp/gowdk-contracts-build/asyncapi.jsongo run ./cmd/gowdk check --ssr examples/pages/*.gwdk examples/actions/*.gwdk examples/partials/*.gwdk examples/api/*.gwdk examples/ssr/*.gwdk examples/go-interop/*.gwdk examples/components/base/*.gwdk examples/components/css/*.gwdk examples/components/assets/*.gwdk examples/embed/*.gwdk examples/css/*.gwdk examples/tailwind/*.gwdk examples/contracts/*.gwdkGitHub CI:
CI / Verifypassed on latest headc4c36a8via workflow-dispatch run27468043897; CodeQL checks also passed on latest headc4c36a8.LLM Assistance
Ackboth succeed; docs call out that subscribers must still tolerate redelivery outside the configured window, after seen-store write failures, and across concurrent workers.