Fix Trakt shared credentials UX, headers panel, and watchlist seed reset by electather · Pull Request #1041 · electather/nama

electather · 2026-07-05T17:38:51Z

Summary

Fixes Trakt shared credential flow: dedupes "add credential" button, corrects token-status reporting, cleans up empty-state messaging/UI, adds shared client_id/secret verification for user connections.
Updates headers panel UI for better usability/accessibility.
Resets watchlist seed on new connections so it's immediately visible.

Test plan

Manually verify Trakt shared credentials empty state and add-credential flow
Confirm token status accurately reflects Trakt connection state
Verify headers panel usability/accessibility changes
Confirm new connection watchlist is visible immediately

…empty state fix: remove redundant success toast for personal API key fallback policy fix: correct Trakt shared credential testing to accurately report token status fix: update shared credentials empty state message for clarity fix: adjust shared credentials empty state UI for better user experience feat: add shared client_id/secret verification for user connection in Trakt plugin

…bility

github-actions · 2026-07-05T17:42:58Z

Fallow combined report

No GitHub PR/MR findings.

Generated by fallow.

github-actions · 2026-07-05T17:43:49Z

Code Review — PR #1041

Overview

Four logical fixes bundled together: Trakt shared-credential verification, headers panel table→list refactor, shared-credentials empty-state dedup, watchlist seed reset on new connection. Changesets are correct (four separate files, right scopes and bump levels).

🔴 BLOCKING — Design-doc sync

PR description links no design document. Under repo policy every substantive implementation PR must link the relevant doc.

The `verifyShared` hook is a new extension of the plugin SDK surface.
`definePlugin({…, verifyShared, …})` registers a hook type that was not present before. The plugin architecture design doc (docs/2026-04-19-plugin-architecture-design.md) specifies what lifecycle hooks plugins may implement. Adding a new hook type to the plugin definition schema is a public-surface extension — it must be documented there (or in the advanced-admin doc, depending on where verify-shared fits in the spec).

Action required: Add a link to the design doc(s) this PR belongs to in the description, and either confirm verifyShared is already spec'd in one of them (quote the passage) or add a sentence to the relevant doc in this same PR.

🔴 BLOCKING — `allowlist-panel.tsx`: accessibility + layout regression

// Before (correct accessible pattern):
<Label className="flex items-start gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50">
  <RadioGroupItem value="inherit" className="mt-0.5" />
  <span>…</span>
</Label>

// After:
<RadioGroupItem value="inherit">
  <span>…</span>
</RadioGroupItem>

Two problems:

Layout styles lost. The flex row, gap, padding, and hover:bg-muted/50 interactive area came from the <Label> wrapper. RadioGroupItem carries none of those — the rows will be unstyled small radio circles with the text packed inside the <button>.
Radix RadioGroup.Item renders as <button role="radio">. Putting a <span> inside makes it label-text-inside-button, not the standard label-associates-with-radio pattern. Works for click, but the visual affordance (full-row hover highlight) is gone.

The Label import was removed entirely, but the correct fix preserves the wrapper with its classes. Either keep the <Label> or replicate its layout+hover on the RadioGroupItem via className.

🟡 Non-blocking issues

`verifyShared` — unhandled network error

const res = await c.fetch(`${BASE}/oauth/device/code`, { … });

If c.fetch throws (timeout, DNS failure, TLS error) the exception propagates rather than returning { ok: false }. Add a try/catch:

try {
  const res = await c.fetch(…);
  …
} catch (err) {
  return { ok: false, message: "network error" };
}

`verifyShared` — hardcoded English error strings

"client id missing", "invalid client id", "Trakt ${res.status}" are English literals. If the UI surfaces these to end users they bypass i18n. If they're internal-only log strings, document that with a comment so a future reader doesn't localize them by mistake.

`writeConnection` — seed-lock race note

resetWatchlistSeed (→ clearSeedLock) deletes the userWatchlistSeed row. If a concurrent in-flight seed goroutine already holds the lock and is mid-fetch, clearing the row lets a second caller re-acquire it via trySeedLock and start a parallel seed. Low probability in practice (connection write and watchlist GET racing within milliseconds), but worth acknowledging. The existing comment explains the intent well; optionally add a brief note that this is safe because trySeedLock uses onConflictDoNothing.

No automated tests

All verification is manual. writeConnection + resetWatchlistSeed and verifyShared are both unit-testable:

verifyShared: mock c.fetch returning 200/401/network error, assert ok and message.
writeConnection: assert clearSeedLock is called (or check hasUserSeeded returns false after write).

Not a hard block for a bug-fix PR, but worth adding before this lands on main given the cross-module side effect.

✅ Clean

Changeset files: four separate files, correct packages, correct bump levels, good user-facing sentences.
uninstall-dialog.tsx: useEffect dep narrowed to uninstall.reset — correct; React Query's reset is a stable reference.
header-dialog.tsx: Checkbox swap and onCheckedChange={(v) => setPreserveValue(v === true)} correctly handles the "indeterminate" case.
headers-panel.tsx: table→div-list, icon-only edit button with aria-label — accessibility improves for the edit action.
section.tsx: removing onAdd/atCapacity from EmptyRow correctly dedupes the button; top-level header already owns it.
i18n: key renames (_prefix → _title, _edit → _edit_aria) are consistent across en.json and fa.json. Removed keys (toast_fallback_saved, column headers) match removed code exactly.
Fallow baseline: not modified — good. The new resetWatchlistSeed export is immediately consumed.

github-actions · 2026-07-05T17:44:02Z

        className="gap-1.5"
      >
-        <Label className="flex items-start gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50">
-          <RadioGroupItem value="inherit" className="mt-0.5" />
+        <RadioGroupItem value="inherit">
          <span>{m.admin_plugins_allowlist_mode_inherit()}</span>
-        </Label>
-        <Label className="flex items-start gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50">
-          <RadioGroupItem value="restrict" className="mt-0.5" />
+        </RadioGroupItem>
+        <RadioGroupItem value="restrict">


BLOCKING — layout styles lost + incorrect Radix usage

RadioGroupItem renders as <button role="radio">. The <Label> wrapper was doing two jobs here: (1) the flex/gap/padding layout and (2) the full-row hover:bg-muted/50 interactive area. Both are gone.

Putting <span> inside RadioGroupItem works for click semantics but the visual result is a small circle with text crammed inside a <button>, not a styled selectable row.

Suggested fix — keep the label wrapper, just tighten it:

Suggested change

className="gap-1.5"

>

<Label className="flex items-start gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50">

<RadioGroupItem value="inherit" className="mt-0.5" />

<RadioGroupItem value="inherit">

<span>{m.admin_plugins_allowlist_mode_inherit()}</span>

</Label>

<Label className="flex items-start gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50">

<RadioGroupItem value="restrict" className="mt-0.5" />

</RadioGroupItem>

<RadioGroupItem value="restrict">

<Label className="flex items-center gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50 cursor-pointer">

<RadioGroupItem value="inherit" />

{m.admin_plugins_allowlist_mode_inherit()}

</Label>

<Label className="flex items-center gap-2 rounded-md px-2 py-1.5 text-sm font-normal hover:bg-muted/50 cursor-pointer">

<RadioGroupItem value="restrict" />

{m.admin_plugins_allowlist_mode_restrict()}

</Label>

This also means the Label import should be restored in the import block.

github-actions · 2026-07-05T17:44:10Z

+export async function verifyShared(ctx: unknown): Promise<{ ok: boolean; message?: string }> {
+  const c = ctx as Ctx;
+  const shared = c.sharedCredentials as TraktSharedCreds | null;
+  if (!shared?.clientId) return { ok: false, message: "client id missing" };
+  const res = await c.fetch(`${BASE}/oauth/device/code`, {
+    method: "POST",
+    headers: { "content-type": "application/json" },
+    body: JSON.stringify({ client_id: shared.clientId }),
+  });
+  if (res.ok) return { ok: true };
+  if (res.status === 401 || res.status === 403) return { ok: false, message: "invalid client id" };


Non-blocking — unhandled network error

c.fetch can throw on timeout, DNS failure, or TLS error. The exception propagates uncaught instead of returning { ok: false }. Callers expecting a result shape will see an unhandled rejection.

Suggested change

export async function verifyShared(ctx: unknown): Promise<{ ok: boolean; message?: string }> {

const c = ctx as Ctx;

const shared = c.sharedCredentials as TraktSharedCreds | null;

if (!shared?.clientId) return { ok: false, message: "client id missing" };

const res = await c.fetch(`${BASE}/oauth/device/code`, {

method: "POST",

headers: { "content-type": "application/json" },

body: JSON.stringify({ client_id: shared.clientId }),

});

if (res.ok) return { ok: true };

if (res.status === 401 || res.status === 403) return { ok: false, message: "invalid client id" };

try {

const res = await c.fetch(`${BASE}/oauth/device/code`, {

method: "POST",

headers: { "content-type": "application/json" },

body: JSON.stringify({ client_id: shared.clientId }),

});

if (res.ok) return { ok: true };

if (res.status === 401 || res.status === 403) return { ok: false, message: "invalid client id" };

return { ok: false, message: `Trakt ${res.status}` };

} catch {

return { ok: false, message: "network error" };

}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fab9757fc6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-05T17:46:06Z

+  // already gave up on (locked after an earlier, possibly empty, run).
+  // Clearing the lock lets the next GET re-fetch instead of waiting for
+  // the 6-hourly cron sync.
+  await resetWatchlistSeed(args.userId);


Preserve seeded users when resetting the lock

When a user who already has watchlist rows adds any connection, this deletes the user_watchlist_seed row via clearSeedLock. The 6-hour sync job sources rows from listSeededUserIds() (apps/server/src/watchlist/jobs/sync-plugin-watchlist.ts), and the first-page read only recreates the marker when hasActiveRows() is false (apps/server/src/watchlist/internal/reads.ts), so seeded users with existing rows are dropped from scheduled plugin sync indefinitely. Use a separate retry marker or explicitly re-seed/sync without deleting the scheduler marker.

Useful? React with 👍 / 👎.

electather added 3 commits July 4, 2026 18:02

fix: update headers panel UI for better usability and accessibility

598fdad

fix: reset watchlist seed on new connections to ensure immediate visi…

fab9757

…bility

github-actions Bot added the status: ready-for-review Ready for reviewer attention label Jul 5, 2026

electather added type: bug Something is broken or behaving unexpectedly scope: client @ent-mcp/client package scope: server @ent-mcp/server package complexity: medium Multi-file change, 2-8h labels Jul 5, 2026

github-actions Bot reviewed Jul 5, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Trakt shared credentials UX, headers panel, and watchlist seed reset - #1041

Fix Trakt shared credentials UX, headers panel, and watchlist seed reset#1041
electather wants to merge 3 commits into
mainfrom
fix/make-it-work

electather commented Jul 5, 2026

Uh oh!

github-actions Bot commented Jul 5, 2026

Uh oh!

github-actions Bot commented Jul 5, 2026

Uh oh!

github-actions Bot Jul 5, 2026

Uh oh!

github-actions Bot Jul 5, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

electather commented Jul 5, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented Jul 5, 2026

Fallow combined report

Uh oh!

github-actions Bot commented Jul 5, 2026

Code Review — PR #1041

Overview

🔴 BLOCKING — Design-doc sync

🔴 BLOCKING — allowlist-panel.tsx: accessibility + layout regression

🟡 Non-blocking issues

verifyShared — unhandled network error

verifyShared — hardcoded English error strings

writeConnection — seed-lock race note

No automated tests

✅ Clean

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

🔴 BLOCKING — `allowlist-panel.tsx`: accessibility + layout regression

`verifyShared` — unhandled network error

`verifyShared` — hardcoded English error strings

`writeConnection` — seed-lock race note