fix: address Copilot review of #198 (scope-denied sync result, legacy ref migration write, lockfile)

[alvarosanchez]

Follow-up to #198: addresses the three unresolved GitHub Copilot review threads left on that PR after it was merged.

1. src/worker.ts — scope-denied sync must surface the operator message

readTrustedConfig caught InvocationScopeDeniedError and fell back to the remembered (or empty) config. For a scheduled sync target whose company has mappings but no saved plugin-config row, startSync therefore never reached createUnexpectedSyncErrorResult, so COMPANY_SCOPE_DENIED_SYNC_MESSAGE was never persisted — and with a worker-local fallback token the sync could even continue silently.

• readTrustedConfig/getResolvedConfig gained a requireCompanyScope option: when the host denies the company scope and it never delivered a config for that company, the error is rethrown.
• startSync opts in and converts the rethrown scope-denied error into createUnexpectedSyncErrorResult(...), which records the message already documented in README.md:380.
• Settings/registration data paths keep the lenient fallback, so the UI still renders an empty config instead of erroring.

2. src/ui/index.tsx — legacy bare-UUID refs never got migrated

readPluginConfig normalized legacy bare-UUID refs into { type: "secret_ref" } before patchPluginConfig compared current vs next, so the githubTokenNeedsConfigSync migration effect saw an identical object, returned early, and the host row stayed a bare UUID (binding_missing forever).

• Added hasLegacyPluginSecretRefs() in src/ui/plugin-config.ts.
• patchPluginConfig now reads the raw row (readRawPluginConfig) and only takes the no-op shortcut when the stored row is already in binding form, so a legacy row always forces the upgrade write.

3. package-lock.json

The repo does keep an npm lockfile (Renovate maintains it alongside pnpm-lock.yaml), so it was regenerated rather than removed. The SDK range (^2026.831.1) and engines.node (>=24.21.0) drift Copilot flagged was already resolved on main by #199 and #202; only the root version was still 0.17.0. Regenerated with npm install --package-lock-only on Node 24.21.0 — the only resulting delta is the version bump to 0.17.1.

Tests

• scheduled sync records the scope-denied operator message for a company the host refuses to scope and never configured — also asserts no GitHub request is made with a worker-local fallback token, that the settings data path stays lenient, and that the sync succeeds once the host delivers the config via configChanged.
• patchPluginConfig rewrites a legacy bare-UUID secret ref as a binding even when the normalized config is unchanged — plus the follow-up no-op assertion once the row is a binding.
• hasLegacyPluginSecretRefs only flags secret refs stored as bare secret-id strings.

pnpm typecheck, pnpm test (337 passing) and pnpm build are green locally on Node 24.21.0.

[Copilot]

🟡 Changes recommended

Unresolved comments remain regarding legacy-ref compatibility and raw scope-denial diagnostics.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This follow-up fixes scope-denied sync reporting, legacy secret-reference migration, and lockfile metadata.

Changes:

• Prevents fallback-token use when company scope is denied.
• Forces migration writes for legacy secret references.
• Adds regression tests and updates package-lock.json to 0.17.1.

File summaries

File	Description
tests/plugin.spec.ts	Adds regression coverage for sync errors and secret-ref migration.
src/worker.ts	Handles denied company scope during scheduled syncs.
src/ui/plugin-config.ts	Detects legacy secret-reference formats.
src/ui/index.tsx	Migrates legacy references through config writes.
package-lock.json	Updates package version metadata.

Review details

Suppressed comments (1)

src/worker.ts:22298

• Passing the original scope-denial exception here also stores its raw message (for example, company context is required) in errorDetails. The UI renders that field as “Raw error” (src/ui/index.tsx:8324-8327), so this path still surfaces the host error alongside the requested operator guidance; sanitize or suppress raw diagnostics for scope-denied failures while retaining COMPANY_SCOPE_DENIED_SYNC_MESSAGE.

    return createUnexpectedSyncErrorResult(ctx, trigger, error, targetCompanyId);

• Files reviewed: 4/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[alvarosanchez]

Note on the suppressed review comment (src/worker.ts:22298, raw scope-denial message in errorDetails): keeping it is intentional. createUnexpectedSyncErrorResult already overrides the user-facing message with COMPANY_SCOPE_DENIED_SYNC_MESSAGE; errorDetails.rawMessage is the diagnostics block the settings UI renders under "Raw error", and company context is required is host diagnostic text, not a secret or a token. Suppressing it would remove the only signal distinguishing a denied invocation scope from other configuration failures when an operator reports the issue, and every other configuration-phase failure keeps its raw message, so special-casing this one would be inconsistent. No change made.

[Copilot]

🟢 Approval recommended

Reviewed changes and tests show no approval-blocking issues.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Lite