fix(metrics): send pg4ol + live ADDIN_VERSION in client-version header - #106
fix(metrics): send pg4ol + live ADDIN_VERSION in client-version header#106dobby-coder[bot] wants to merge 2 commits into
Conversation
The Yivi-hosting dialog's PostGuard client was constructing its own headers object with the v0.2.0-rewrite-era `pg4outlook` token and a hardcoded `ADDIN_VERSION = "0.1.0"`. PR #11 already fixed the token in the taskpane via `clientHeaders()`, but the dialog runtime reintroduced the bug. As a result, all dialog-flow encrypt requests were: - attributed to a non-existent client-id in PKG's per-client Prometheus counter, and - missing `X-Cryptify-Source: outlook`, so cryptify's `detect_channel` fell back to website-channel classification for dialog uploads. The taskpane compose and read views had the same stale `0.1.0` literal, so every release reported the same client version regardless of the actual deployed build. Fix both at the source: - yivi-dialog: drop the inline headers literal and call `clientHeaders(ADDIN_VERSION)` like the taskpane already does. - webpack: inject `process.env.ADDIN_VERSION` from `package.json` via the existing DefinePlugin. release-please bumps `package.json` natively, so this stays in sync without extra config. - pkg-client: export `ADDIN_VERSION` as the single source of truth. - compose-view / read-view / yivi-dialog: import the constant; remove the three `const ADDIN_VERSION = "0.1.0"` literals. Closes #103.
The build-time-configuration section enumerated four URLs baked in via webpack DefinePlugin. This PR adds ADDIN_VERSION as a fifth value, sourced from package.json instead of .env; update the section to match.
|
Rules pass found one docs-drift gap and fixed it in this PR:
Other rule checks were clean, with a few flagged-then-dismissed:
|
|
/dobby I think we added some different code in the mean time to fix this issue. Can you figure out if this pr is still necessary? |
|
On it, @rubenhensen! I'll investigate the recent code changes and check whether the PR is still needed or if the work has already been covered. |
|
No, this PR is no longer necessary — the fix already landed on Confirmed on current
|
| What #106 set out to fix | State on master today |
|---|---|
yivi-dialog.ts sending pg4outlook (regression of #11) |
✅ Fixed — grep -rn pg4outlook src/ is empty; dialog now calls clientHeaders(ADDIN_VERSION) (yivi-dialog.ts:139), so it sends pg4ol + X-Cryptify-Source: outlook |
Stale ADDIN_VERSION = "0.1.0" in 3 files |
✅ Fixed — single source of truth ADDIN_VERSION exported from pkg-client.ts:13 via process.env.ADDIN_VERSION, injected by DefinePlugin from package.json (webpack.config.js:100) |
#105 even left POSTGUARD_VERSION out of scope for the same reason #106 did, and #103 is already closed.
One tiny thing #105 didn't carry
#106 also bumped a docs line: CLAUDE.md:23 still reads "Four URLs are baked into the bundle via webpack DefinePlugin" and doesn't list ADDIN_VERSION as the fifth DefinePlugin-injected value. That's a minor docs-drift gap — not worth keeping this whole PR open for.
Recommendation
Close #106. Its code changes are fully superseded by #105 and would now just conflict. If the CLAUDE.md doc note is worth keeping, I can open a one-line follow-up PR to update that count + add the ADDIN_VERSION bullet — just say the word.
|
Hey @rubenhensen — looks like this PR is already taken care of! The same fix landed on master via #105, so #106 is now redundant. I confirmed on current master: no left, uses , and is a single source of truth injected from via . The only extra thing this PR carries is a small docs tweak (Four→Five DefinePlugin values) — if that's worth keeping, it could land as a one-liner follow-up PR. Otherwise, safe to close this one. 🎉 |
Closes #103.
Summary
Two related defects in the
X-PostGuard-Client-Versionheader that the launchevent's Yivi dialog and the taskpane sent to PKG / Cryptify. Sends still succeeded but per-client metrics were silently misattributed.1. Wrong client-id token (regression of #11)
src/yivi-dialog/yivi-dialog.tsconstructed its own headers object withpg4outlook— the v0.2.0-rewrite-era bug PR #11 had already fixed in the taskpane. The taskpane usesclientHeaders()fromsrc/lib/pkg-client.ts, which produces the canonical pair (pg4ol+X-Cryptify-Source: outlook). The dialog now calls the same helper.Without
X-Cryptify-Source, cryptify'sdetect_channelfalls back to website-channel classification because the add-in is served fromaddin.*.postguard.eu— so dialog uploads were being counted as website uploads.2. Stale
ADDIN_VERSION = "0.1.0"Hardcoded in three files (
yivi-dialog.ts,compose-view.ts,read-view.ts). The fourth field of the client-version header is meant to track the deployed extension — every release was reporting0.1.0, so PKG could not distinguish current vs. historical clients.Replaced with the same DefinePlugin mechanism webpack already uses for
PKG_URL,CRYPTIFY_URL, etc.:webpack.config.jsreadsversionfrompackage.jsonand injects it asprocess.env.ADDIN_VERSION. release-please'srelease-type: nodebumpspackage.jsonnatively, so this stays in sync without extra config.Changes
webpack.config.jspackage.jsonversion; injectprocess.env.ADDIN_VERSIONviaDefinePlugin.src/lib/pkg-client.tsADDIN_VERSION(single source of truth).src/yivi-dialog/yivi-dialog.tsclientHeaders(ADDIN_VERSION)instead of inline header literal; drop0.1.0constant.src/taskpane/compose-view.tsADDIN_VERSION; drop0.1.0constant.src/taskpane/read-view.tsADDIN_VERSION; drop0.1.0constant.Verification
grep -rn "pg4outlook" src/→ empty.grep -rn "pg4ol" src/→ onlysrc/lib/pkg-client.ts:16(CLIENT_ID).grep -rn 'ADDIN_VERSION' src/→ only imports from../lib/pkg-client; no string literals.dist/yivi-dialog.jsnow contains the literalOutlook,1.0,pg4ol,0.4.0—pg4outlookis gone.npm run lint,npx tsc --noEmit,npm run build,npm run validateall pass. (The pre-existing size-limit warning on the WASM-heavy bundles is the documented baseline, not a regression.)Not in scope
recipientsKey/guessContentType/ Office.js promise wrappers into shared lib modules) — left for a separate issue/PR.POSTGUARD_VERSION/POSTGUARD_HEADER_VALUEconstants (0.1.0inlaunchevent.ts,compose-view.ts,mime.ts) are a different protocol-level versioning constant used in the internet message MIME header — unrelated to the add-in release version.Manual smoke test (not executable here — requires Outlook desktop)
Outlook,1.0,pg4ol,<current-version>in theX-PostGuard-Client-Versionheader on both PKG and Cryptify requests, andX-Cryptify-Source: outlookon the Cryptify upload.Marking as draft because the manual smoke test against a live Outlook desktop client cannot be executed from this environment. Ready to flip out of draft after a maintainer verifies the headers in the browser/network capture.