Skip to content

N data repos via topic discovery (ADR-0023)#69

Merged
danielmoraes merged 5 commits into
mainfrom
feat/n-repo-registry
Jul 12, 2026
Merged

N data repos via topic discovery (ADR-0023)#69
danielmoraes merged 5 commits into
mainfrom
feat/n-repo-registry

Conversation

@danielmoraes

Copy link
Copy Markdown
Contributor

Summary

Generalizes the hardwired two-repo model (personal + one `BULLETIN_TEAM_REPO`) to N data repos per user, discovered via the `steward-data` GitHub topic. Sharing = GitHub repo permissions, nothing else.

Four commits, each deployable:

  1. Registry + primitives — `repos.ts`/`repos.server.ts` (topic search ∪ home repo, 60s per-token cache, `requireDataRepo` gate), GitHub client grows search/topics/collaborators/orgs.
  2. Routes + payloads — canonical `/r/:owner/:repo/:dashboard` (legacy `/d/`, `/team/` 301), `BoardScope` deleted, `/sync` `/run` `/dashboards` take an explicit repo, pointer prompts always name the repo. `BULLETIN_TEAM_REPO` removed.
  3. Repo identity UI — rail groups per repo with lock/globe visibility, collaborator avatar stack, manage-access link out to GitHub; new "Add data repo" flow (create from template in any owner / register existing by tagging).
  4. Templates + scripts + ADR — template tiers collapse to built-in / this-repo; `routines:sync` home-vs-shared classification with the generalized runner rule; ADR-0023 + glossary.

Notes for cutover

  • Existing data repos need the `steward-data` topic added (one `gh repo edit --add-topic` each) to appear in the rail.
  • Existing cloud routines keep working; they get recreated with repo-claused prompts during the rename cutover (ops runbook).

🤖 Generated with Claude Code

https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G

…023 groundwork)

New repos.ts/repos.server.ts: DataRepo model, listDataRepos (topic search
unioned with the conventional home repo, 60s per-token cache), requireDataRepo
gate for client-supplied repos. GitHub client grows searchReposByTopic,
getRepoMeta, listCollaborators (best-effort), topics read/union-write, and
listUserOrgs. No callers yet — behavior unchanged.

Claude-Session: https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G
Boards now live at one canonical URL shape for any discovered data repo;
/ keeps the home repo's default board. Legacy /d/* and /team/* become 301
stubs. BoardScope and the personal/team dichotomy are gone: BoardRef carries
repo+shared, /sync //run //dashboards take an explicit repo gated by
requireDataRepo, the rail renders one group per repo via loadSidebar, and
pointer-prompt commands always name the repo. Setup tags the generated repo
with the discovery topic. BULLETIN_TEAM_REPO is removed.

Claude-Session: https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G
Each rail group now carries its repo's identity: lock/globe visibility
glyph, a collaborator avatar stack when the repo is actually shared, and a
hover-revealed jump to GitHub — access settings for admins, the repo page
otherwise. Indicators and a link out; GitHub stays the sharing surface.

New 'Add data repo' verb in the rail: create a private repo from the
template under any owner the viewer can create in, or register an existing
data repo by tagging it with the discovery topic (requires
data/routines.yaml and push access). Both invalidate the registry cache so
the group appears immediately.

Claude-Session: https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G
Template sources are now built-in (product bundle, everywhere) or this
repo (scoped to the repo's boards) — the team tier is gone. routines:sync,
routine, and routine-trigger classify repos as home vs shared instead of
personal vs team; every pointer prompt names its repo; shared cloud names
carry the owner so slugs can't collide across repos. run-routine accepts
legacy unclaused prompts as home-repo runs. ADR-0023 records the model;
CONTEXT.md glossary gains Home repo / Shared repo / Topic and drops Team
repo.

Claude-Session: https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This change replaces personal/team dashboard scoping with an N-data-repository model. GitHub topic search discovers readable repositories, while the home repository remains separately identifiable. Canonical repository routes, repository creation and registration flows, sidebar grouping, and repo-based dashboard CRUD are added. Dashboard, routine execution, synchronization, template discovery, and runner ownership now use explicit repository identifiers and shared-state flags. Legacy routes redirect to canonical repository URLs, with supporting tests, locales, mocks, documentation, and environment configuration updated.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Viewer
  participant DashboardSidebar
  participant DataReposRoute
  participant GitHub
  Viewer->>DashboardSidebar: open add data repo
  DashboardSidebar->>DataReposRoute: submit create or register request
  DataReposRoute->>GitHub: create, probe, or tag repository
  GitHub-->>DataReposRoute: repository result
  DataReposRoute-->>DashboardSidebar: success or structured error
  DashboardSidebar->>DashboardSidebar: refresh discovered repository groups
Loading
sequenceDiagram
  participant Viewer
  participant RepoDashboard
  participant RequireDataRepo
  participant DashboardServer
  Viewer->>RepoDashboard: request repository dashboard
  RepoDashboard->>RequireDataRepo: validate repository
  RequireDataRepo-->>RepoDashboard: authorized data repository
  RepoDashboard->>DashboardServer: load dashboard and sidebar
  DashboardServer-->>RepoDashboard: dashboard view and repository groups
  RepoDashboard-->>Viewer: render canonical dashboard
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: discovering N data repos via topic search for ADR-0023.
Description check ✅ Passed The description is clearly related to the changeset and matches the repo-discovery, routing, UI, and script updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/n-repo-registry

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Sentry


Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

🔗 Vercel preview: https://bulletin-eiaoex1pb-form-factory.vercel.app (dc0813c)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/app/routes/setup.tsx (1)

19-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the owner from dataRepo when creating the repo.

resolveHomeRepo() keeps the session override as a full owner/repo, but this path still drops the owner with split("/")[1] and passes auth.login to generateFromTemplate(). If the override points at a different owner, setup creates the repo in the wrong account.

🔧 Suggested fix
   const dataRepo = resolveHomeRepo(auth.login, auth.dataRepo)
-  const name = dataRepo.split("/")[1]
-  if (!name) throw new Response("Bad data repo name", { status: 400 })
+  const [owner, name] = dataRepo.split("/")
+  if (!owner || !name) throw new Response("Bad data repo name", { status: 400 })

   await generateFromTemplate(
     auth.token,
     env().BULLETIN_DATA_REPO_TEMPLATE,
-    auth.login,
+    owner,
     name,
   )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/routes/setup.tsx` around lines 19 - 39, The setup action
incorrectly derives only the repository name and always passes auth.login as the
owner. Update the dataRepo parsing and generateFromTemplate call in action to
use both owner and repository segments from resolveHomeRepo(auth.login,
auth.dataRepo), preserving the existing validation for malformed values and
creating the repository under the configured owner.
🧹 Nitpick comments (5)
apps/web/app/routes/data-repos.ts (1)

47-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

owner lacks format validation, unlike name.

name is constrained with a GitHub-plausible regex, but owner only requires non-empty. A malformed owner (e.g. containing /) would produce a confusing GitHub error later mismapped by the generic status-code branches (403/404/422) rather than a clean validation error.

♻️ Suggested owner validation
-    owner: z.string().min(1),
+    owner: z
+      .string()
+      .regex(/^[A-Za-z0-9-]+$/, "not a plausible GitHub owner name"),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/routes/data-repos.ts` around lines 47 - 48, Update the
create-intent schema near the owner field to apply the same GitHub-plausible
format validation used by name, while retaining the non-empty constraint. Reject
malformed owners such as values containing “/” during input validation before
the GitHub request flow.
apps/web/app/routes/settings.tsx (1)

26-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale doc comment after switching to loadSidebar.

The comment still explains the "never crash the settings page" invariant in terms of listDashboards degrading to null, but the loader no longer calls listDashboards directly — the actual guard is now the .catch(() => ({ repos: [], complete: false })) on loadSidebar. Worth updating so the comment matches the real degrade path.

✏️ Suggested comment update
- * also fetches the sibling boards the rail lists. No board is current here, so
- * the rail lights nothing (it reads as "off-board") and the header carries a
- * plain way back. Boards are best-effort: `listDashboards` already degrades to
- * null when the repo/dir is missing (pre-setup, reached via the account menu),
- * and a transient GitHub blip degrades to an empty rail rather than crashing
- * the one page that must never trap the user — never a redirect.
+ * also fetches the sidebar the rail lists. No board is current here, so
+ * the rail lights nothing (it reads as "off-board") and the header carries a
+ * plain way back. The sidebar is best-effort: `loadSidebar` is wrapped in a
+ * `.catch` that falls back to an empty, incomplete listing, so a transient
+ * GitHub blip (or pre-setup missing repo) degrades to an empty rail rather
+ * than crashing the one page that must never trap the user — never a redirect.

Also applies to: 38-45

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/routes/settings.tsx` around lines 26 - 35, Update the Settings
route’s doc comment to describe the current loadSidebar-based loading path,
including its catch fallback to empty repos with complete false, instead of
attributing graceful degradation to direct listDashboards behavior. Preserve the
existing explanation of the shared NavShell, off-board state, and no-redirect
behavior.
scripts/routine.ts (1)

73-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the redundant fallback in promptRepo. repo is already set to repoArg ?? personalRepo, so repo ?? personalRepo here just repeats the same defaulting and obscures the intent. const promptRepo = repo is enough.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/routine.ts` around lines 73 - 78, Remove the redundant fallback from
promptRepo in the routine prompt construction, assigning it directly from repo.
Preserve the existing clause generation and personalRepo defaulting through
repoArg ?? personalRepo.
apps/web/app/lib/dashboard.server.ts (1)

203-232: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Bound repository enrichment concurrency.

This launches 2 × repository count GitHub requests simultaneously. At N-repository scale, burst throttling will silently appear as empty board lists and missing collaborators. Process repositories in bounded batches or through a concurrency-limited mapper.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/lib/dashboard.server.ts` around lines 203 - 232, Update
loadSidebar to process listing.repos through a bounded-concurrency mapper or
batching mechanism, limiting the number of repositories enriched simultaneously
while preserving the existing per-repository Promise.all behavior and failure
fallback for dashboards. Return the same repos and listing.complete values
without launching all repository enrichment requests at once.
apps/web/app/components/add-routine-dialog.tsx (1)

517-529: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update stale "Team / Private" doc comments to match the new repo/builtin tiers.

The badge assignment here correctly reflects the simplified two-tier model (repo/builtin), but TemplateCard's badge prop doc still says "Translated source label (Built-in / Team / Private)", and the runner prop doc above still says "Set on team boards" — both predate ADR-0023's home/shared model. Worth a quick pass to keep terminology consistent with the rest of the PR.

♻️ Suggested doc updates
-  /** Translated source label (Built-in / Team / Private). */
+  /** Translated source label (Built-in / Repo). */
   badge: string
-  /** Set on team boards: the login whose Claude account owns the routine's
-      cloud resource (ADR-0010/0016). Stamped on the routine and surfaced
-      as a hint. */
+  /** Set on shared (non-home) boards: the login whose Claude account owns
+      the routine's cloud resource (ADR-0010/0016/0023). Stamped on the
+      routine and surfaced as a hint. */
   runner?: string

As per coding guidelines, **/*: Use terminology from CONTEXT.md exactly when working on the project.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/components/add-routine-dialog.tsx` around lines 517 - 529,
Update the TemplateCard prop documentation near the templates.map usage to
replace the outdated “Built-in / Team / Private” badge terminology with the
current repo/builtin tiers, and revise the runner prop comment from “team
boards” to the ADR-0023 home/shared terminology. Use the exact terms defined in
CONTEXT.md while leaving the badge behavior unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/components/add-data-repo-dialog.tsx`:
- Around line 177-201: Associate the owner Label with the Select control by
giving the SelectTrigger an appropriate id and setting the Label’s htmlFor to
that same id. Update the owner field markup around the Label and SelectTrigger
while preserving the existing selection behavior.
- Line 1: Update the submit success handling in the add-data repository dialog
to consume or guard each submit.data result before resetting/closing, preventing
a reopened dialog from processing stale success data again. When selecting the
post-submit route, preserve the original submit mode so register submissions use
their intended destination instead of always navigating to /r/${repo}/main.

In `@apps/web/app/components/dashboard-sidebar.tsx`:
- Around line 157-160: Update the add-repository button’s className in the
relevant sidebar component to use Tailwind v4’s important suffix syntax for the
margin-top utility, changing the existing important mt-0.5 utility while
preserving all other classes and behavior.

In `@apps/web/app/lib/repos.ts`:
- Around line 1-9: Rename the new board-addressing utility and its related
comments from “board” to “dashboard,” including the boardHref symbol and
references around the DEFAULT_DASHBOARD declaration, using the terminology
defined in CONTEXT.md. Update all call sites and exports consistently while
preserving behavior.

In `@apps/web/app/routes/data-repos.ts`:
- Around line 130-138: Update the getFile call in the data-repository validation
flow to remove the catch that converts every failure to null. Rely on getFile’s
existing null result for missing files, while allowing non-404 failures to
propagate instead of returning the not-data-repo result.

In `@apps/web/app/routes/team`.$dashboard.tsx:
- Around line 19-27: Update the legacy destination logic around listDataRepos so
authentication failures and incomplete topic-search results are not converted
into an empty repos list. Only issue the 301 redirect through boardHref when
repository discovery completed successfully and exactly one shared repository
exists; otherwise use a temporary fallback redirect, preserving the existing
destination resolution for the complete case.

In `@apps/web/app/routes/team.tsx`:
- Around line 15-23: Update the repository discovery flow around listDataRepos
so errors, including revoked-token failures, are not converted into an empty
repository list and permanently redirected. Preserve and inspect the returned
complete status, avoiding the 301 redirect to "/" when discovery is incomplete;
only apply the existing shared-repository redirect logic after a complete
successful lookup.

In `@CONTEXT.md`:
- Around line 81-87: The glossary entry currently labels non-home data
repositories as “Shared (data) repo,” conflicting with the existing “Shared
repo” definition. Rename this concept to a distinct canonical term and update
the entry’s references consistently, preserving the definition that it covers
any readable data repository other than the viewer’s home repository.
- Around line 35-37: Update the route description in CONTEXT.md to use
“dashboard” instead of “board,” and rewrite the malformed sentence so it clearly
states that the home repository’s main dashboard renders at the default `/`
route. Preserve the existing `/r/<owner>/<repo>/<slug>` route description and
ADR-0023 reference.

In `@docs/adr/0023-n-data-repos-topic-discovery.md`:
- Around line 37-40: Update the terminology in the ADR passage to use
“dashboard” instead of “board,” including the phrase describing the repo’s
scoped resources and any related references, while preserving the distinction
between built-in and this-repo sources.
- Around line 49-51: Update the ADR’s cloud resource identity from owner/slug to
a collision-free owner/repository/slug format, ensuring repository names are
included for repos sharing a slug. Update the corresponding routine scripts
together so resource creation and orphan cleanup use the same new identity and
migration behavior.

In `@scripts/routine-trigger.ts`:
- Around line 111-120: Update the shared-repository check near the routine
runner validation to compare repo against the resolved home-repository string,
including the configured BULLETIN_DATA_REPO_PREFIX, rather than constructing
bulletin-data-${login} directly. Reuse the existing home-repo resolution symbol
or logic, and preserve the runner ownership check for genuinely shared
repositories.

In `@scripts/routines-sync.ts`:
- Around line 140-148: Validate that either repo or login is available before
deriving dataRepo or entering reconciliation. When both are absent, fail
immediately and do not create routines, pointerPrompt messages, or orphan rules;
preserve the existing repo derivation and routine behavior when a repository can
be resolved.
- Around line 9-14: Update the shared-repository determination in the routine
sync flow to use the app’s existing home-repo resolver, including
BULLETIN_DATA_REPO_PREFIX and session override support, instead of constructing
a hard-coded bulletin-data repository name. Preserve the existing behavior that
runner-less routines in the resolved home repo are enacted.

---

Outside diff comments:
In `@apps/web/app/routes/setup.tsx`:
- Around line 19-39: The setup action incorrectly derives only the repository
name and always passes auth.login as the owner. Update the dataRepo parsing and
generateFromTemplate call in action to use both owner and repository segments
from resolveHomeRepo(auth.login, auth.dataRepo), preserving the existing
validation for malformed values and creating the repository under the configured
owner.

---

Nitpick comments:
In `@apps/web/app/components/add-routine-dialog.tsx`:
- Around line 517-529: Update the TemplateCard prop documentation near the
templates.map usage to replace the outdated “Built-in / Team / Private” badge
terminology with the current repo/builtin tiers, and revise the runner prop
comment from “team boards” to the ADR-0023 home/shared terminology. Use the
exact terms defined in CONTEXT.md while leaving the badge behavior unchanged.

In `@apps/web/app/lib/dashboard.server.ts`:
- Around line 203-232: Update loadSidebar to process listing.repos through a
bounded-concurrency mapper or batching mechanism, limiting the number of
repositories enriched simultaneously while preserving the existing
per-repository Promise.all behavior and failure fallback for dashboards. Return
the same repos and listing.complete values without launching all repository
enrichment requests at once.

In `@apps/web/app/routes/data-repos.ts`:
- Around line 47-48: Update the create-intent schema near the owner field to
apply the same GitHub-plausible format validation used by name, while retaining
the non-empty constraint. Reject malformed owners such as values containing “/”
during input validation before the GitHub request flow.

In `@apps/web/app/routes/settings.tsx`:
- Around line 26-35: Update the Settings route’s doc comment to describe the
current loadSidebar-based loading path, including its catch fallback to empty
repos with complete false, instead of attributing graceful degradation to direct
listDashboards behavior. Preserve the existing explanation of the shared
NavShell, off-board state, and no-redirect behavior.

In `@scripts/routine.ts`:
- Around line 73-78: Remove the redundant fallback from promptRepo in the
routine prompt construction, assigning it directly from repo. Preserve the
existing clause generation and personalRepo defaulting through repoArg ??
personalRepo.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e382d39-acf2-435e-97ba-3f119924d65c

📥 Commits

Reviewing files that changed from the base of the PR and between 0d8a17b and 5c2ad07.

📒 Files selected for processing (49)
  • .claude/skills/run-routine/SKILL.md
  • CONTEXT.md
  • apps/web/app/components/add-data-repo-dialog.browser.test.tsx
  • apps/web/app/components/add-data-repo-dialog.tsx
  • apps/web/app/components/add-routine-dialog.browser.test.tsx
  • apps/web/app/components/add-routine-dialog.tsx
  • apps/web/app/components/dashboard-board.browser.test.tsx
  • apps/web/app/components/dashboard-board.tsx
  • apps/web/app/components/dashboard-shell.tsx
  • apps/web/app/components/dashboard-sidebar.browser.test.tsx
  • apps/web/app/components/dashboard-sidebar.tsx
  • apps/web/app/components/nav-shell.tsx
  • apps/web/app/components/new-dashboard-dialog.tsx
  • apps/web/app/components/repo-group-header.tsx
  • apps/web/app/components/sync-panel.tsx
  • apps/web/app/components/widget-card.browser.test.tsx
  • apps/web/app/components/widget-card.tsx
  • apps/web/app/lib/board.ts
  • apps/web/app/lib/dashboard.server.test.ts
  • apps/web/app/lib/dashboard.server.ts
  • apps/web/app/lib/env.server.ts
  • apps/web/app/lib/github.server.test.ts
  • apps/web/app/lib/github.server.ts
  • apps/web/app/lib/repos.server.test.ts
  • apps/web/app/lib/repos.server.ts
  • apps/web/app/lib/repos.ts
  • apps/web/app/lib/templates.server.ts
  • apps/web/app/lib/templates.ts
  • apps/web/app/locales/en.ts
  • apps/web/app/locales/pt-br.ts
  • apps/web/app/mocks/github.ts
  • apps/web/app/routes.ts
  • apps/web/app/routes/d.$dashboard.tsx
  • apps/web/app/routes/dashboards.ts
  • apps/web/app/routes/data-repos.ts
  • apps/web/app/routes/dev-dialog.tsx
  • apps/web/app/routes/home.tsx
  • apps/web/app/routes/r.$owner.$repo.$dashboard.tsx
  • apps/web/app/routes/run.ts
  • apps/web/app/routes/settings.tsx
  • apps/web/app/routes/setup.tsx
  • apps/web/app/routes/sync.ts
  • apps/web/app/routes/team.$dashboard.tsx
  • apps/web/app/routes/team.tsx
  • docs/adr/0010-team-data-repo.md
  • docs/adr/0023-n-data-repos-topic-discovery.md
  • scripts/routine-trigger.ts
  • scripts/routine.ts
  • scripts/routines-sync.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Form-Factory/kb (manual)
💤 Files with no reviewable changes (1)
  • apps/web/app/lib/board.ts

@@ -0,0 +1,282 @@
import { useCallback, useEffect, useState } from "react"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- target file ---\n'
git ls-files 'apps/web/app/components/add-data-repo-dialog.tsx' 'apps/web/app/routes/**' 'apps/web/app/**/*board*' 'apps/web/app/**/*dashboard*' 'apps/web/app/**/*repo*' | sed -n '1,200p'

printf '\n--- add-data-repo-dialog.tsx outline ---\n'
ast-grep outline apps/web/app/components/add-data-repo-dialog.tsx --view expanded || true

printf '\n--- search for DEFAULT_DASHBOARD and navigate(/r/ ---\n'
rg -n 'DEFAULT_DASHBOARD|navigate\(/r/|onNavigate\?\(\)|submit\.data|fetcher|mode' apps/web/app/components/add-data-repo-dialog.tsx apps/web/app/routes apps/web/app -g '!**/node_modules/**' || true

Repository: Form-Factory/bulletin

Length of output: 250


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- file list ---'
git ls-files 'apps/web/app/components/add-data-repo-dialog.tsx' 'apps/web/app/routes/**' 'apps/web/app/**/route.tsx' 'apps/web/app/**/route.ts' 'apps/web/app/**/routes/**' | sed -n '1,240p'

printf '\n%s\n' '--- add-data-repo-dialog.tsx outline ---'
ast-grep outline apps/web/app/components/add-data-repo-dialog.tsx --view expanded || true

printf '\n%s\n' '--- direct references ---'
rg -n 'DEFAULT_DASHBOARD|navigate\(/r/|onNavigate\?\(\)|submit\.data|fetcher|mode|add-data-repo' apps/web/app/components/add-data-repo-dialog.tsx apps/web/app/routes apps/web/app -g '!**/node_modules/**' || true

Repository: Form-Factory/bulletin

Length of output: 24863


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- apps/web/app/components/add-data-repo-dialog.tsx (selected lines) ---'
sed -n '1,220p' apps/web/app/components/add-data-repo-dialog.tsx

printf '\n%s\n' '--- apps/web/app/lib/repos.ts ---'
sed -n '1,200p' apps/web/app/lib/repos.ts

printf '\n%s\n' '--- apps/web/app/routes/r.$owner.$repo.$dashboard.tsx ---'
sed -n '1,220p' apps/web/app/routes/r.\$owner.\$repo.\$dashboard.tsx

printf '\n%s\n' '--- apps/web/app/routes/home.tsx (route defaults) ---'
sed -n '1,120p' apps/web/app/routes/home.tsx

Repository: Form-Factory/bulletin

Length of output: 17599


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- apps/web/app/components/add-data-repo-dialog.tsx ---'
sed -n '1,220p' apps/web/app/components/add-data-repo-dialog.tsx

printf '\n%s\n' '--- apps/web/app/lib/repos.ts ---'
sed -n '1,220p' apps/web/app/lib/repos.ts

printf '\n%s\n' '--- apps/web/app/routes/r.$owner.$repo.$dashboard.tsx ---'
sed -n '1,220p' apps/web/app/routes/r.\$owner.\$repo.\$dashboard.tsx

Repository: Form-Factory/bulletin

Length of output: 13017


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,340p' apps/web/app/components/add-data-repo-dialog.tsx

Repository: Form-Factory/bulletin

Length of output: 9814


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- apps/web/app/routes/data-repos.ts ---'
sed -n '1,260p' apps/web/app/routes/data-repos.ts

printf '\n%s\n' '--- any tests for add-data-repo-dialog ---'
sed -n '1,240p' apps/web/app/components/add-data-repo-dialog.browser.test.tsx

Repository: Form-Factory/bulletin

Length of output: 7613


Handle each success once submit.data is reused after close, so reopening can rerun the success effect and immediately close the dialog again. Guard or clear the handled result before resetting, and preserve the original submit mode when choosing the post-submit route — the register path shouldn’t always go to /r/${repo}/main.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/components/add-data-repo-dialog.tsx` at line 1, Update the
submit success handling in the add-data repository dialog to consume or guard
each submit.data result before resetting/closing, preventing a reopened dialog
from processing stale success data again. When selecting the post-submit route,
preserve the original submit mode so register submissions use their intended
destination instead of always navigating to /r/${repo}/main.

Comment on lines +177 to +201
<div className="grid gap-2">
<Label>{t("addRepo.owner")}</Label>
<Select
value={effectiveOwner}
onValueChange={(next) => {
if (typeof next === "string") setOwner(next)
}}
>
<SelectTrigger className="w-full font-mono">
<SelectValue />
</SelectTrigger>
<SelectContent>
{[login, ...(owners.data?.orgs ?? [])]
.filter((option): option is string => option != null)
.map((option) => (
<SelectItem
key={option}
value={option}
className="font-mono"
>
{option}
</SelectItem>
))}
</SelectContent>
</Select>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Associate the owner label with its select trigger.

Without an association, assistive technology announces the selected owner but not the field’s purpose.

Proposed fix
-<Label>{t("addRepo.owner")}</Label>
+<Label htmlFor="data-repo-owner">{t("addRepo.owner")}</Label>
...
-<SelectTrigger className="w-full font-mono">
+<SelectTrigger id="data-repo-owner" className="w-full font-mono">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div className="grid gap-2">
<Label>{t("addRepo.owner")}</Label>
<Select
value={effectiveOwner}
onValueChange={(next) => {
if (typeof next === "string") setOwner(next)
}}
>
<SelectTrigger className="w-full font-mono">
<SelectValue />
</SelectTrigger>
<SelectContent>
{[login, ...(owners.data?.orgs ?? [])]
.filter((option): option is string => option != null)
.map((option) => (
<SelectItem
key={option}
value={option}
className="font-mono"
>
{option}
</SelectItem>
))}
</SelectContent>
</Select>
<div className="grid gap-2">
<Label htmlFor="data-repo-owner">{t("addRepo.owner")}</Label>
<Select
value={effectiveOwner}
onValueChange={(next) => {
if (typeof next === "string") setOwner(next)
}}
>
<SelectTrigger id="data-repo-owner" className="w-full font-mono">
<SelectValue />
</SelectTrigger>
<SelectContent>
{[login, ...(owners.data?.orgs ?? [])]
.filter((option): option is string => option != null)
.map((option) => (
<SelectItem
key={option}
value={option}
className="font-mono"
>
{option}
</SelectItem>
))}
</SelectContent>
</Select>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/components/add-data-repo-dialog.tsx` around lines 177 - 201,
Associate the owner Label with the Select control by giving the SelectTrigger an
appropriate id and setting the Label’s htmlFor to that same id. Update the owner
field markup around the Label and SelectTrigger while preserving the existing
selection behavior.

Comment on lines +157 to +160
<button
type="button"
onClick={() => setAddingRepo(true)}
className="!mt-0.5 flex w-full cursor-pointer items-center gap-2.5 rounded-md px-2.5 py-1.5 text-sm text-ink-dim transition-colors outline-none hover:bg-sidebar-accent/60 hover:text-foreground focus-visible:ring-3 focus-visible:ring-ring/50"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '"tailwindcss"|tailwindcss@' package.json pnpm-lock.yaml
rg -n '!mt-0\.5' apps/web/app/components/dashboard-sidebar.tsx

Repository: Form-Factory/bulletin

Length of output: 500


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== tailwind-related files ==\n'
fd -a -t f 'tailwind.*|postcss.*|vite.*|globals.css|app.css' . || true

printf '\n== package.json excerpts ==\n'
rg -n '"tailwindcss"|tailwind|postcss|`@tailwind`' package.json pnpm-lock.yaml apps/web -g '!**/node_modules/**' || true

printf '\n== dashboard-sidebar surrounding lines ==\n'
sed -n '130,180p' apps/web/app/components/dashboard-sidebar.tsx

Repository: Form-Factory/bulletin

Length of output: 6615


🌐 Web query:

Tailwind CSS v4 important modifier syntax prefix postfix official docs

💡 Result:

In Tailwind CSS v4, the syntax for the important modifier has changed to improve clarity and consistency [1]. To apply!important to a specific utility class, you now append an exclamation mark (!) to the end of the class name [1]. For example:

Regarding the global configuration for important (which previously used the important: true setting in tailwind.config.js), Tailwind CSS v4 provides a new way to mark all utility declarations as!important via your CSS import [2]. You can add the important flag after the @import statement for Tailwind CSS [3][2]: @import "tailwindcss" important; If you are using individual imports, you can apply it to the utilities layer [4]: @layer theme, base, components, utilities; @import "tailwindcss/theme.css" layer(theme); @import "tailwindcss/utilities.css" layer(utilities) important; While the previous tailwind.config.js important: true setting is maintained for backwards compatibility [2], the new CSS-native approach is the recommended method for v4 [3][2]. Similarly, if you need to use a selector-based strategy (e.g., important: "#app"), you should now perform this nesting directly in your CSS [4].

Citations:


Use Tailwind v4 important suffix here

Tailwind v4 expects the important marker as a suffix, so this should be mt-0.5! to keep the add-repo button spacing consistent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/components/dashboard-sidebar.tsx` around lines 157 - 160, Update
the add-repository button’s className in the relevant sidebar component to use
Tailwind v4’s important suffix syntax for the margin-top utility, changing the
existing important mt-0.5 utility while preserving all other classes and
behavior.

Comment thread apps/web/app/lib/repos.ts
Comment on lines +1 to +9
/**
* Repo + board addressing for the N-data-repo model (ADR-0023): every board
* lives in some data repo the viewer can read; which repos those are is
* discovered by GitHub topic, and sharing is repo permissions — nothing
* else. Shared by server loaders and client navigation.
*/

/** Slug of the board `/` renders; every data repo starts with it. */
export const DEFAULT_DASHBOARD = "main"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Rename boardHref to dashboardHref.

The glossary requires “dashboard” and explicitly avoids “board.” Rename this new utility and its comments before the API spreads further.

As per coding guidelines, use terminology from CONTEXT.md exactly.

Also applies to: 43-55

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/lib/repos.ts` around lines 1 - 9, Rename the new
board-addressing utility and its related comments from “board” to “dashboard,”
including the boardHref symbol and references around the DEFAULT_DASHBOARD
declaration, using the terminology defined in CONTEXT.md. Update all call sites
and exports consistently while preserving behavior.

Source: Coding guidelines

Comment on lines +130 to +138
const routines = await getFile(
auth.token,
ref.full,
"data/routines.yaml",
"main",
).catch(() => null)
if (!routines) {
return { ok: false, error: "not-data-repo" } satisfies DataRepoResult
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep run --pattern 'export async function getFile($$$) { $$$ }' --lang typescript apps/web/app/lib/github.server.ts

Repository: Form-Factory/bulletin

Length of output: 159


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg 'apps/web/app/(lib/github\.server\.ts|routes/data-repos\.ts|routes/(run|dashboards)\.ts)$'

echo
echo "== getFile definition search =="
rg -n "getFile" apps/web/app -g '!**/*.map'

echo
echo "== github.server.ts outline =="
ast-grep outline apps/web/app/lib/github.server.ts --view expanded || true

echo
echo "== relevant snippets =="
for f in \
  apps/web/app/lib/github.server.ts \
  apps/web/app/routes/data-repos.ts \
  apps/web/app/routes/run.ts \
  apps/web/app/routes/dashboards.ts
do
  echo "----- $f -----"
  sed -n '1,260p' "$f" | nl -ba | sed -n '1,260p'
done

Repository: Form-Factory/bulletin

Length of output: 6296


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== github.server.ts getFile =="
sed -n '340,390p' apps/web/app/lib/github.server.ts

echo
echo "== data-repos.ts register flow =="
sed -n '118,145p' apps/web/app/routes/data-repos.ts

echo
echo "== run.ts getFile usage =="
sed -n '50,90p' apps/web/app/routes/run.ts

echo
echo "== dashboards.ts getFile usage =="
sed -n '88,112p' apps/web/app/routes/dashboards.ts

Repository: Form-Factory/bulletin

Length of output: 4892


Do not swallow non-404 getFile failures here. getFile already returns null for a missing file; this .catch(() => null) also hides 5xx, network, and parse errors, so a transient GitHub issue is reported as not-data-repo.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/routes/data-repos.ts` around lines 130 - 138, Update the getFile
call in the data-repository validation flow to remove the catch that converts
every failure to null. Rely on getFile’s existing null result for missing files,
while allowing non-404 failures to propagate instead of returning the
not-data-repo result.

Comment on lines +37 to +40
name the repo explicitly. Template tiers collapse to two sources:
**built-in** (the product repo bundle, available everywhere) and **this
repo** (scoped to the repo's own boards, shadowing same-named built-ins)
— ADR-0021's discovery mechanics are unchanged.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use “dashboard,” not “board.”

Line 39 conflicts with the glossary’s explicit _Avoid_: board terminology.

As per coding guidelines, use terminology from CONTEXT.md exactly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/adr/0023-n-data-repos-topic-discovery.md` around lines 37 - 40, Update
the terminology in the ADR passage to use “dashboard” instead of “board,”
including the phrase describing the repo’s scoped resources and any related
references, while preserving the distinction between built-in and this-repo
sources.

Source: Coding guidelines

Comment on lines +49 to +51
home-repo runs. Cloud resource names carry the owner for shared repos
(`bulletin-<owner>-<slug>`) so two repos' slugs can't collide on one
Claude account; orphan cleanup keys off the prompt's repo clause, as

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Include the repository name in cloud resource identities.

bulletin-<owner>-<slug> still collides when one owner has multiple data repos containing the same slug. Define a collision-free owner/repo/slug identity and migrate both routine scripts together.

🧰 Tools
🪛 LanguageTool

[grammar] ~50-~50: Ensure spelling is correct
Context: ...so two repos' slugs can't collide on one Claude account; orphan cleanup keys off the pr...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/adr/0023-n-data-repos-topic-discovery.md` around lines 49 - 51, Update
the ADR’s cloud resource identity from owner/slug to a collision-free
owner/repository/slug format, ensuring repository names are included for repos
sharing a slug. Update the corresponding routine scripts together so resource
creation and orphan cleanup use the same new identity and migration behavior.

Comment thread scripts/routine-trigger.ts
Comment thread scripts/routines-sync.ts
Comment thread scripts/routines-sync.ts
Comment on lines +140 to 148
const dataRepo =
repo ?? (login != null ? `${login}/bulletin-data-${login}` : null)

/** The stable pointer prompt — created once, never edited (ADR-0005).
Team prompts carry the repo so the dispatcher targets it (ADR-0010). */
Always names the repo: with N data repos (ADR-0023) an unclaused
prompt is ambiguous, so every command is explicit. */
function pointerPrompt(routine: Routine): string {
return teamMode
? `Run the bulletin routine \`${routine.slug}\` in \`${repo}\` — follow the run-routine skill.`
: `Run the bulletin routine \`${routine.slug}\` — follow the run-routine skill.`
return `Run the bulletin routine \`${routine.slug}\` in \`${dataRepo ?? "(unknown repo)"}\` — follow the run-routine skill.`
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail before creating routines with an unknown repository.

When both repo and login are absent, this creates unusable (unknown repo) prompts and a null orphan rule. Exit before reconciliation instead.

Proposed fix
 const dataRepo =
   repo ?? (login != null ? `${login}/bulletin-data-${login}` : null)
+
+if (dataRepo == null) {
+  console.error("routines-sync: cannot determine the target data repo.")
+  process.exit(1)
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const dataRepo =
repo ?? (login != null ? `${login}/bulletin-data-${login}` : null)
/** The stable pointer prompt created once, never edited (ADR-0005).
Team prompts carry the repo so the dispatcher targets it (ADR-0010). */
Always names the repo: with N data repos (ADR-0023) an unclaused
prompt is ambiguous, so every command is explicit. */
function pointerPrompt(routine: Routine): string {
return teamMode
? `Run the bulletin routine \`${routine.slug}\` in \`${repo}\` — follow the run-routine skill.`
: `Run the bulletin routine \`${routine.slug}\` — follow the run-routine skill.`
return `Run the bulletin routine \`${routine.slug}\` in \`${dataRepo ?? "(unknown repo)"}\` — follow the run-routine skill.`
}
const dataRepo =
repo ?? (login != null ? `${login}/bulletin-data-${login}` : null)
if (dataRepo == null) {
console.error("routines-sync: cannot determine the target data repo.")
process.exit(1)
}
/** The stable pointer prompt created once, never edited (ADR-0005).
Always names the repo: with N data repos (ADR-0023) an unclaused
prompt is ambiguous, so every command is explicit. */
function pointerPrompt(routine: Routine): string {
return `Run the bulletin routine \`${routine.slug}\` in \`${dataRepo ?? "(unknown repo)"}\` — follow the run-routine skill.`
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/routines-sync.ts` around lines 140 - 148, Validate that either repo
or login is available before deriving dataRepo or entering reconciliation. When
both are absent, fail immediately and do not create routines, pointerPrompt
messages, or orphan rules; preserve the existing repo derivation and routine
behavior when a repository can be resolved.

Security:
- registry no longer admits a stranger's public topic-tagged repo — a repo
  qualifies only if private, viewer-owned, or push-granted (isEligible),
  applied in both discovery and the /r gate (was: any public repo injected
  into every user's rail)
- default board 'main' is now protected in EVERY repo, not just the
  viewer's home — a collaborator can't delete another user's default board

Correctness:
- requireDataRepo maps GitHubError → 401 re-auth / 503 degrade instead of
  crashing the /r loader and /sync //run //dashboards actions
- legacy /d/* and /team/* redirects are 302 (not cached 301) so
  session/transient-derived targets re-resolve each visit
- create/delete dashboard surface a read-only collaborator's 403/404 as a
  clean 'denied', not a 500
- register-existing lands on the repo's real first board (or /), never a
  guessed /main that 404s
- shared cloud names carry the full repo tag (owner AND name) so two repos
  can't collide on one account
- requireDataRepo no longer invalidates the registry cache on the lag
  fallback (was burning search quota on every page view of a lagging repo)

+6 regression tests.

Claude-Session: https://claude.ai/code/session_01PZpPYdXrwmYKBFat6DFV5G
@danielmoraes
danielmoraes merged commit aaa82a7 into main Jul 12, 2026
4 checks passed
@danielmoraes
danielmoraes deleted the feat/n-repo-registry branch July 12, 2026 22:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/data-repo.ts`:
- Around line 19-26: Update repoTag so normalization remains cloud-name-safe
while adding a deterministic uniqueness guard, such as a short hash derived from
the original repoFull value, ensuring distinct valid repository names like
foo.bar/baz and foo-bar/baz produce different tags. Preserve the shared output
contract used by routines-sync and routine-trigger.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e8b7ef5e-bc3f-4c17-986c-34b8a5192b90

📥 Commits

Reviewing files that changed from the base of the PR and between 5c2ad07 and 6ef71ef.

📒 Files selected for processing (13)
  • apps/web/app/components/add-data-repo-dialog.tsx
  • apps/web/app/components/dashboard-sidebar.browser.test.tsx
  • apps/web/app/components/dashboard-sidebar.tsx
  • apps/web/app/lib/repos.server.test.ts
  • apps/web/app/lib/repos.server.ts
  • apps/web/app/routes/d.$dashboard.tsx
  • apps/web/app/routes/dashboards.ts
  • apps/web/app/routes/data-repos.ts
  • apps/web/app/routes/team.$dashboard.tsx
  • apps/web/app/routes/team.tsx
  • scripts/data-repo.ts
  • scripts/routine-trigger.ts
  • scripts/routines-sync.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Form-Factory/kb (manual)
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/web/app/routes/team.tsx
  • apps/web/app/routes/d.$dashboard.tsx
  • apps/web/app/components/add-data-repo-dialog.tsx
  • apps/web/app/routes/team.$dashboard.tsx
  • scripts/routines-sync.ts
  • apps/web/app/lib/repos.server.ts
  • apps/web/app/components/dashboard-sidebar.tsx

Comment thread scripts/data-repo.ts
Comment on lines +19 to +26
/** A repo's full owner+name folded into a cloud-name-safe tag, so two
shared repos — even two under the same owner — can't collide as cloud
routine names on one Claude account (ADR-0023). Shared by routines-sync
and routine-trigger so the sync'd resource and its trigger agree. */
export function repoTag(repoFull: string): string {
return repoFull.toLowerCase().replace(/[^a-z0-9]+/g, "-")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What characters are allowed in GitHub repository and owner (username/org) names?

💡 Result:

GitHub repository and owner (username/organization) names have specific constraints regarding length and character sets. Repository Names - Allowed Characters: ASCII letters (a-z, A-Z), digits (0-9), hyphens (-), underscores (_), and periods (.) [1][2]. - Maximum Length: 100 characters [1][3]. - Automatic Handling: Non-ASCII characters are typically sanitized and replaced with hyphens [1][2]. - Restrictions: - Cannot end with the string.git (the suffix is automatically removed if present) [1][2]. - Cannot end with the string.wiki (this results in an error) [1]. - Names consisting only of a single period (.) or double period (..) are reserved [2]. Owner (Username/Organization) Names - Allowed Characters: Alphanumeric characters (a-z, A-Z, 0-9) and hyphens (-) [4][5]. - Maximum Length: 39 characters [4]. - Restrictions: - Cannot start or end with a hyphen [4]. - Cannot contain consecutive hyphens [4]. - Usernames are generally not case-sensitive [6][7]. Note that these rules apply to standard GitHub.com accounts. Enterprise Managed Users (EMU) may have different normalization rules or constraints managed by an Identity Provider (IdP) [4][5]. Organization owners can also implement additional restrictions on repository naming conventions using repository policies [8].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== files ==\n'
git ls-files scripts/data-repo.ts scripts/data-repos.ts scripts | sed -n '1,120p'

printf '\n== outline: scripts/data-repo.ts ==\n'
ast-grep outline scripts/data-repo.ts --view expanded || true

printf '\n== relevant search ==\n'
rg -n "repoTag|cloudName|routine-trigger|routines-sync|isEligible|name regex|owner" scripts src . --glob '!**/node_modules/**' || true

printf '\n== targeted read: scripts/data-repo.ts ==\n'
cat -n scripts/data-repo.ts | sed -n '1,220p'

printf '\n== targeted read: scripts/data-repos.ts ==\n'
if [ -f scripts/data-repos.ts ]; then
  cat -n scripts/data-repos.ts | sed -n '1,260p'
fi

Repository: Form-Factory/bulletin

Length of output: 26415


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== call sites for repoTag ==\n'
rg -n "repoTag\\(" -S . --glob '!**/node_modules/**' || true

printf '\n== references to cloudName / routine identity ==\n'
rg -n "cloudName|routine.*name|trigger.*name|routine.*trigger" scripts src . --glob '!**/node_modules/**' || true

Repository: Form-Factory/bulletin

Length of output: 18835


repoTag can still collide on valid repo names
foo.bar/baz and foo-bar/baz both normalize to the same tag because every non-alphanumeric run becomes -. That breaks the collision-free routine-name guarantee; add a short hash suffix or another uniqueness guard.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/data-repo.ts` around lines 19 - 26, Update repoTag so normalization
remains cloud-name-safe while adding a deterministic uniqueness guard, such as a
short hash derived from the original repoFull value, ensuring distinct valid
repository names like foo.bar/baz and foo-bar/baz produce different tags.
Preserve the shared output contract used by routines-sync and routine-trigger.

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.

1 participant