Skip to content

fix(helper)!: drop claimed identity, version the protocol - #615

Draft
Alex Shabalin (alex-sparus) wants to merge 1 commit into
alex/fix-helper-b1-b2from
alex/fix-helper-c2
Draft

fix(helper)!: drop claimed identity, version the protocol#615
Alex Shabalin (alex-sparus) wants to merge 1 commit into
alex/fix-helper-b1-b2from
alex/fix-helper-c2

Conversation

@alex-sparus

@alex-sparus Alex Shabalin (alex-sparus) commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • new helper protocol: determining the identity from the socket, not from claimed values
  • introduce helper protocol

internal code C2

Test Plan

  • Manual testing of a CI-build artifact

Docs

  • Docs updated (companion PR in darkmatter/nixmac-web: #___)
  • No docs update needed

@alex-sparus

Alex Shabalin (alex-sparus) commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by jj-ryu.

@darkmatter

darkmatter Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🎨 Storybook preview

Open Storybook preview

Updated for 4d769aa


⚠️ Detected UI changes (1)

These stories' HTML snapshots changed. I've added screenshots + links to the changed stories below. Review them carefully then accept the changes to regenerate baselines and include them in this PR:

Flows/Evolve › Playground

Flows/Evolve › Playground


Accept UI changes

  • Click here to accept these changes

Alternatively, you can run bun run test:update-snapshots locally to re-generate the baselines and then push the changes to this PR.

What does this do?

The screenshots above show UI changes detected by the Storybook
snapshot tests run on this PR. Each image is the rendered output of
a Storybook story from the code in this PR branch; the snapshot
test compared it against the committed baseline in
__snapshots__/ and flagged the difference.

Checking the box tells the darkmatter[bot] to regenerate the
baselines from this PR's current code and commit them directly to
this branch. The new baselines become the source of truth for
future runs — only accept after confirming the visual changes are
intentional.

Comparison baseline: the committed __snapshots__/ files on this
PR branch (carried forward from develop). Accept updates them in
place on this branch.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

No Linear issue ID found in this PR's title, description, or branch name (expected something like ENG-123). Add one so this work is traceable in Linear, or add #no-linear to the PR description to acknowledge it's intentionally untracked.

⚠️ PR is marked WIP / draft — do not merge until ready for review.

📋 PR Overview

Lines changed 844 (+684 / -160)
Files 0 added, 7 modified, 0 deleted
Draft / WIP yes
Has Test Plan yes
Linear issue no
No Test Plan Needed no
New UI components no
New Storybook stories no
New Rust modules no
New TS source files no
New tests no
package.json touched no
Cargo.toml touched no
Infra / CI touched no

🔬 Coverage

Report Lines Statements Functions Branches
apps/native/coverage/coverage-summary.json 35.5% 35.1% 30.6% 29.4%

Generated by 🚫 dangerJS against d5c9e7e

// The operation is deliberately invalid: getting the skew
// classification rather than an unknown-variant parse error proves
// the version gate wins before the rest of the shape is interpreted.
for version in [1, 3, 99] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This will have to be updated each time we rev the protocol but could be fixed by using a const which is "current protocol version".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, really sorry how it turned out with the version - it's completely removed later. The plan has changed mid-stack and I didn't spend time cleaning up the middle

// cannot answer the probe, so the osascript path takes over instead of
// the activation request coming back as an unparseable request.
if !status.responding {
info!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a warn instead of an info

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, same here - the decision to use the osascript instead of the helper has become much more nuanced later. I was hoping there wasn't that much code overwritten later but looks like I was wrong

// could not have parsed the request — so falling back is safe
// until the installed helper is replaced by a current one.
if helper_protocol::is_protocol_skew(&error) {
info!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is also replaced

Requests no longer carry userName/userId/home/nixPath: the daemon
derives the account from the authenticated socket peer via getpwuid_r,
so a client has nothing left to assert about itself. Unknown fields
are hard errors on both sides.

Every request and response carries a required protocolVersion, checked
for exact equality before any other field is interpreted. Skew is
classified — typed ProtocolSkew client-side, prefix-matched error
marker daemon-side — so callers pick a recovery without parsing
display text. Responses also embed a diagnostic helper build version.

The v2 activation envelope is deliberately unparseable by the v1
request parser: a resident v1 daemon must not execute a v2 activation
and answer with a versionless response, or the app's recovery would
re-run a finished activation. A skewed daemon fails the status probe
and the app uses the interactive prompt until the installed helper is
replaced.

BREAKING CHANGE: protocol v2 rejects every v1 wire shape; v1 helpers
and v2 apps cannot interoperate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants