Skip to content

docs: design notes for build-flag capture at release, verify at patch - #3701

Closed
eseidel wants to merge 2 commits into
mainfrom
eseidel/flag-capture-design
Closed

docs: design notes for build-flag capture at release, verify at patch#3701
eseidel wants to merge 2 commits into
mainfrom
eseidel/flag-capture-design

Conversation

@eseidel

@eseidel eseidel commented Apr 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

From Claude:

Draft — seeking design review before I write the implementation.

Design doc at NOTES_ON_FLAG_CAPTURE.md.

What this proposes

Record the set of flags used at `shorebird release` time that affect the AOT snapshot, so `shorebird patch` can:

  1. Auto-apply flags whose values we can safely store (e.g. `--obfuscate`, `--flavor`, `--dart-define`).
  2. Require flags whose values we can't store (e.g. `--split-debug-info`, `--target` — paths).
  3. Fail with a specific diff before linking when something's off (turn today's generic VM-section hash mismatch into "your `--dart-define=SERVER_URL` value differs").

Follows on from #3695 (incident), #3698 (obfuscate-after-`--` drop), #3699 (CLI-side hint), #3700 (this issue).

What I want review on before implementing

  1. Flag classification. The doc has a table bucketing every shorebird-recognized flag into `record value` / `record presence only` / `don't record`. Please shout if anything is misclassified. Unknown flags forwarded via `rest` default to presence-only.
  2. Dart-define-from-file handling. Recommending we expand-and-record k/v pairs from the file (consistent with the privacy rule: values are already in the binary). Conservative alternative of keys-only is flagged as an open question — reviewer's call.
  3. Storage. Explicitly deferred in the doc — three options listed (supplement artifact / release metadata field / dedicated endpoint) with tradeoffs. My lean is the supplement artifact for this PR, but I'd like a cloud-team opinion before writing storage code.
  4. Privacy rule. "If the value is baked into the shipped binary, recording it is not a new disclosure." Explicit and worth signing off on.

Plan after review

  • Land this design doc.
  • Implement the release-side capture and patch-side verifier against a stub storage interface, in a follow-up PR.
  • Wire up the chosen storage once (3) above is resolved.
  • End-to-end tests only after storage lands.

Test plan

  • CSpell clean on the new doc.
  • No code changes in this PR.

eseidel added 2 commits April 19, 2026 09:59
@eseidel
docs: design notes for release/patch build-flag capture
7ae3901
@eseidel
docs: tighten flag taxonomy, correct key-in-binary claim, explain why…
d0f8c61 
… not to hash

- Scope is specifically flags that affect the AOT snapshot (VM data /
  instructions sections). Build-name, build-number, flavor, tree-shake-icons,
  split-per-abi, export-method don't — remove them from the captured set.
  Build-name/number are separately auto-applied via
  buildNameAndNumberArgsFromReleaseVersion.
- Verified experimentally: dart-define keys are stripped from the AOT binary
  only when used in a const context. In a non-const context the key literal
  survives. Updated doc to reflect this honestly.
- Add the practical argument against hashing paths: path hashes change with
  the build machine, so they'd generate pure noise rather than useful
  equality signal. Record presence for paths; record plaintext for values
  we can safely disclose.
@eseidel

eseidel commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

This should turn into a PRD first. Closing for now.

@eseidel eseidel closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eseidel @bdero