Migrate point_of_sale spec to deployConfig + transformRemoteToLocal#6909
Migrate point_of_sale spec to deployConfig + transformRemoteToLocal#6909ryancbahan wants to merge 1 commit into02-27-rcb_contract-migration-1from
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
Remove transformConfig, set deployConfig (configWithoutFirstClassFields pass-through) and transformRemoteToLocal directly. transformLocalToRemote is now undefined — TOML shape is sent directly to the server. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ryancbahan
force-pushed
the
rcb/contract-point-of-sale
branch
from
036d663 to
e0af67f
Compare
Coverage report
Test suite run success3781 tests passing in 1449 suites. Report generated by 🧪jest coverage report action from e0af67f |
![binks-code-reviewer[bot]](https://avatars.githubusercontent.com/in/2788732?s=60&v=4){kind=small}
| const {name, ...rest} = configWithoutFirstClassFields(config) | ||
| return rest | ||
| }, | ||
| transformRemoteToLocal: (content: object) => ({pos: {embedded: (content as {embedded: boolean}).embedded}}), |
There was a problem hiding this comment.
Remote/local config shape mismatch (likely breaking reverse transform)
deployConfig now returns the TOML shape (nested under pos), but transformRemoteToLocal still assumes the remote object is flat and reads content.embedded. If the server stores/returns what we now send ({pos:{embedded:true}}), then content.embedded will be undefined and local config becomes {pos:{embedded: undefined}}, which is incorrect and can break downstream validation/behavior.
Evidence in code:
// deploy sends nested:
return rest // likely { pos: { embedded: true } }
// reverse expects flat:
(content as {embedded: boolean}).embeddedImpact:
- Merchants may see POS config flip/disappear after fetch/pull/sync.
- CLI could produce invalid local TOML and re-deploy incorrectly (config drift).
- Affects all users of the
point_of_saleconfig extension once server returns nested shape.
| const {name, ...rest} = configWithoutFirstClassFields(config) | ||
| return rest | ||
| }, | ||
| transformRemoteToLocal: (content: object) => ({pos: {embedded: (content as {embedded: boolean}).embedded}}), |
There was a problem hiding this comment.
Reverse transform is unsafe on unexpected remote payload
transformRemoteToLocal force-reads (content as {embedded: boolean}).embedded without validating the payload. If the platform returns null, {}, {pos:{...}}, or embedded is not boolean, the result can become {pos:{embedded: undefined}} or lead to errors during parsing/validation.
Impact:
- Pull/sync can generate invalid local TOML or break workflows like
deploy,pull, config hydration. - Increases risk of clients re-uploading corrupted config.
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - 2 findings 📋 History✅ 2 findings |
1 participant
Summary
transformConfigfrom point_of_sale spec, setdeployConfig(pass-through withconfigWithoutFirstClassFields) andtransformRemoteToLocaldirectlytransformLocalToRemoteis nowundefined— TOML shape sent directly to serverTest plan
transformLocalToRemoteisundefined🤖 Generated with Claude Code