[Protocol] Add more types - #3658
Merged
Merged
Conversation
…al into shared protocol - openapi.json: grow AccountType/AccountSpecifics with broker/bank/asset variants (BrokerAccount/BankAccount/AssetAccount); add NodeEndpoint (real wire data carried inside the QR pairing payload); extract UserActionConfiguration as a real named schema and add ProposedActionEntry/ActionProposal on top of it. Regenerated Python + TypeScript models. AccountView/AutomationView/AccountKind/AutomationRunStatus/Holding were promoted and then un-promoted in the same branch after review: each is a client-computed projection over a real wire type (Account/AutomationState), never itself serialized, so none belongs in the shared wire contract. `octobot_client_ts` hand-declares them again; `Holding` specifically is gone for good (not just relocated) since it added no information over the wire's own `DetailedAsset` (`free`/`used` were always derived from `available`, never independently wire-sourced). - octobot_client_ts: `accountViewOf`/`automationViewOf` unchanged in behavior; `AccountView.holdings`/`AutomationView.holdings` are now `DetailedAsset[]` instead of the removed `Holding[]`. `automationViewOf` no longer duplicates the free/used mapping that used to live in both it and `accountHoldingsFromNodeState` (nothing ever consumed `Automation.holdings` in that shape). - decodeActionProposal now throws a distinguishable UnsupportedActionProposalVersionError for a recognised-but-unsupported `v`, instead of the same generic error as garbage input. - connectReadOnlyDevice's multi-entry after:'previous-confirmed' chaining is now complete and consistent (accounts.create/update/delete, automations.create/update) via a shared chainEntries() helper - automations.update previously omitted the chain between strategy_edit and automation_edit entirely. - automationStrategyRefOf/automationStrategyRefsOf: the per-automation strategy-ref recovery is now O(actions) once via automationStrategyRefsOf when resolving many automations, with automationStrategyRefOf kept as its own targeted single-pass scan for one-off lookups (both share a small extraction helper, neither penalizes the other's use case). - exchangeConfigIdOf exported so a caller building its own account view on top of AccountSpecifics doesn't have to re-derive the same rule. Verified: protocol Python tests pass (129); octobot_client_ts npm run build + npm test (761/761) pass.
Herklos
force-pushed
the
feature/add-proposals-proto
branch
from
August 24, 2026 15:53
83bb17f to
be8a78b
Compare
Herklos
marked this pull request as ready for review
August 24, 2026 15:59
| "test:watch": "vitest" | ||
| }, | ||
| "dependencies": { | ||
| "@drakkar.software/octobot-client": "file:../../../../client/octobot_client_ts", |
Contributor
Author
There was a problem hiding this comment.
It needs the octobot PR to be merged first
| } | ||
| }, | ||
| "BrokerAccount": { | ||
| "description": "BrokerAccount", |
Member
There was a problem hiding this comment.
just to be sure we don't forget it: could we add a description to all AccountTypes so that we can't mix them?
Contributor
Author
There was a problem hiding this comment.
Added — AccountType's description now spells out what distinguishes all six values (each already has the correct $ref+description convention on its own account_type property, so narrowing itself was never at risk; this was purely missing docs):
Account category discriminator.
generic: a placeholder account created before its real kind is known (e.g. mid-onboarding), not a semantic category of its own.exchange: a centralized or DEX trading account, live-synced via a provider connection.blockchain: a wallet address tracked read-only on-chain.broker: a brokerage account for trading stocks, commodities, forex, and similar instruments (eToro-style platforms), which may be live-synced via a provider connection.bank: a cash/deposit account, always manually entered, never live-synced.asset: a manually-entered, single tracked non-liquid asset (a watch, a car, real estate, and similar).
Regenerated via npm run generate:all (129/129 Python model tests green, no shape change).
Member
There was a problem hiding this comment.
can we put the definition of each account type in the relevant account type and not refer other account types in the generic one ?
Contributor
Author
There was a problem hiding this comment.
Right, sorry, it's up !
Herklos
force-pushed
the
feature/add-proposals-proto
branch
from
August 25, 2026 12:13
4227849 to
6f361f8
Compare
Contributor
Author
|
it's up |
…sals Adds a clipboard-based alternative to the QR hand-off for action proposals, and a paste-and-execute consumer for it in the node's own web interface, so a proposal a read-only phone session can't send directly has a path that doesn't depend on QR framing size or a second scanning device. octobot_client_ts (0.7.0 -> 0.8.0): - New protocol/proposalSummary.ts: describeProposedAction(configuration), promoted verbatim from Astrolab2's mobile2 copy now that the node web interface needs the identical wording for the identical entry. Exported from both the root package and ./protocol; barrelParity.test.ts extended to cover it the same way qrFrames/proposal already are. node_web_interface (first consumer of octobot-client in this repo, wired via a file: link matching docs/package.json's existing precedent — the package is not yet published at 0.8.0): - New src/lib/debug/action-proposal.ts: parseProposalText (empty / a raw QR frame / an unsupported future version / structurally invalid, using decodeActionProposal + isQrFrame) and runActionProposal, which submits each entry via POST /api/v1/debug/ and polls GET /api/v1/debug/ to `completed` before advancing to the next entry — full per-step status feedback, unlike octobot-sdk's executeActionProposal, which only waits before an entry tagged `after: 'previous-confirmed'` since its local outbox has no other way to observe progress. A failed or timed-out step stops the chain and marks the rest skipped; aborting (dialog closed mid-run) resolves rather than rejects, since an intentional stop isn't a failure. - New src/components/Debug/dialogs/PasteProposalDialog.tsx: a paste box next to ExecuteActionDialog in the Debug view, showing a live per-step list (with a connector for `after: 'previous-confirmed'` entries) while it runs. - DebugToolbar/DebugView: new "Paste proposal" button, hidden in imported- snapshot mode like Execute already is. - package.json: sdk:build/prebuild/pretest scripts, mirroring docs/ package.json's own sdk:build hook for the same file: dependency — npm's pre<script> lifecycle runs it automatically before `npm run build`/ `npm test`, including the tentacle exporter's own `npm run build` call, so the sibling package is always built before this one needs its dist. Note: this makes the mechanism correct, but the `packages/copy` CI job building this tentacle still can't go fully green on this branch today — octobot_client_ts's own dependency on octobot-protocol@^0.8.0 can't resolve via a real `npm install` until that version is genuinely published (same root cause, same remedy, as the Astrolab2 side's pnpm-lockfile block: wait for this PR to merge and publish for real). `packages/copy` is not a required status check on this repo, so it doesn't block merging. packages/protocol/openapi.json: - Each account-kind schema (GenericAccount/ExchangeAccount/BlockchainAccount/ BrokerAccount/BankAccount/AssetAccount) now documents its own meaning on its own `description` field, addressing review feedback that the shared `AccountType` discriminator needed this — moved per-review-request from an initial version that put all six definitions in one place (on `AccountType` itself, cross-referencing the others), which per review discussion was the wrong spot: `AccountType`'s own description stays a plain "Account category discriminator." Each schema's own `account_type` property already had the correct $ref+description discriminator convention throughout, so narrowing itself was never at risk — this is purely relocated documentation. Regenerated via `npm run generate:all` (129/129 Python model tests pass). Known, deliberately unaddressed follow-up: runActionProposal duplicates (with intentionally different semantics — full-confirm-per-step vs. barrier-only-wait, per-step timeout vs. one shared deadline, faster local backoff) the state machine octobot-sdk's executeActionProposal already implements for the phone's own local-outbox path. Worth a shared extraction into octobot-client (parameterized by submit/poll I/O and a timeout/backoff policy) as a follow-up, not folded in here since it would mean touching octobot-sdk's already-shipped, tested execution path for a need this feature doesn't itself have. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Herklos
force-pushed
the
feature/add-proposals-proto
branch
from
August 25, 2026 12:58
6f361f8 to
cbcd589
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.