Feature/hub sync by jefeish · Pull Request #1089 · github-community-projects/safe-settings

jefeish · 2026-09-28T15:56:39Z

No description provided.

…moval' into feature/hub-sync

🔀 - Generated by Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

✅ - Generated by Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Unauthenticated mutation routes, unmerged-PR propagation, broken defaults, and nonfunctional enrichment must be resolved before approval.

Review effort: Balanced
Findings: 8 · 16 · 2

Open (26)

Normalize property_name before prefix checks · New
Dashboard API routes lack authentication and authorization · New
Process only merged pull requests · New
Fix class method spacing for Standard lint · New
Remove unused results variable · New
Normalize property_name aliases before filtering · New
Remove redirect incompatible with static export · New
Replace obsolete next export command · New
Include organization in hub repository matching · New
Verify hub repository owner for check-suite events · New
Include configured owner in hub predicate · New
Default hub path duplicates CONFIG_PATH · New
Preserve empty hub URL prefix · New
Use github.rest.apps for installation access · New
Support both enrichment configuration shapes · New
Normalize custom properties before enrichment · New
Use github.rest.repos for validation errors · New
Use github.rest.repos to remove error topics · New
Resolve top-level enrichment configuration and custom properties · New
Preserve empty base path environment value · New

And 6 more that still need to be addressed.

What changed in this PR

Adds hub-and-spoke synchronization, a static dashboard, tech-asset enrichment, supporting APIs, tests, and documentation.

Changes:

Adds hub synchronization, installation caching, import routes, and configuration.
Introduces a Next.js dashboard for organizations, logs, configuration, and status.
Adds tech-asset enrichment through a plugin and GitHub Actions workflow.

File	Description
`ui/src/app/utils/basePath.js`	Adds URL-prefix handling.
`ui/src/app/not-found.jsx`	Adds a 404 page.
`ui/src/app/layout.jsx`	Defines root layout and metadata.
`ui/src/app/hooks/useHydrated.js`	Adds hydration-state hook.
`ui/src/app/hooks/useClientSafe.js`	Adds client-safe formatting hooks.
`ui/src/app/globals.css`	Adds global theme styles.
`ui/src/app/dashboard/safe-settings-hub/page.jsx`	Adds hub-content page.
`ui/src/app/dashboard/page.jsx`	Adds dashboard landing page.
`ui/src/app/dashboard/organizations/page.jsx`	Adds organizations page.
`ui/src/app/dashboard/logs/page.jsx`	Adds log viewer.
`ui/src/app/dashboard/help/page.jsx`	Adds help page.
`ui/src/app/dashboard/env/page.jsx`	Adds environment page.
`ui/src/app/components/TitleBar.jsx`	Adds dashboard navigation.
`ui/src/app/components/TitleBar.css`	Styles navigation and header.
`ui/src/app/components/ThemeToggle.jsx`	Adds theme-toggle control.
`ui/src/app/components/ThemeContext.jsx`	Adds persistent theme context.
`ui/src/app/components/HubOrgGraph.jsx`	Visualizes hub organizations.
`ui/src/app/components/EnvVariables.jsx`	Displays environment settings.
`ui/src/app/api/logs/route.js`	Adds static-export log response.
`ui/src/app/[slug]/route.js`	Adds example static route.
`ui/shield.svg`	Adds shield artwork.
`ui/README.md`	Documents UI setup.
`ui/public/favicon.svg`	Adds SVG favicon.
`ui/public/favicon.ico`	Adds ICO favicon.
`ui/package.json`	Defines UI dependencies and scripts.
`ui/next.config.js`	Configures static export and base path.
`ui/favicon.svg`	Adds alternate favicon asset.
`ui/.gitignore`	Adds UI ignore rules.
`ui/.eslintrc.json`	Configures Next.js linting.
`test/unit/lib/settings.test.js`	Reformats settings tests.
`test/unit/lib/routes.test.js`	Adds route tests.
`test/unit/lib/plugins/rulesets.test.js`	Reformats ruleset tests.
`test/unit/lib/plugins/environments.test.js`	Reformats environment tests.
`test/unit/lib/mergeConfigs.test.js`	Tests hub configuration merging.
`test/unit/index.test.js`	Adds hub-handler mocking.
`TECH_ASSET_ENRICHMENT_GUIDE.md`	Documents enrichment approaches.
`safe-settings.log`	Adds a runtime log artifact.
`package.json`	Adds hub/UI dependencies.
`lib/utils.js`	No textual change supplied.
`lib/settings.js`	Integrates enrichment processing.
`lib/plugins/tech_asset_enrichment.js`	Implements enrichment plugin.
`lib/plugins/overrides.js`	Applies style cleanup.
`lib/plugins/environments.js`	Simplifies object construction.
`lib/mergeDeep.js`	Documents deep-merge behavior.
`lib/installationCache.js`	Adds installation caching.
`lib/env.js`	Adds hub environment configuration.
`lib/configManager.js`	Handles missing API responses.
`index.js`	Wires routes, cache, and hub events.
`examples/smart-merge-example.js`	Demonstrates smart merging.
`examples/repo-with-tech-asset.yml`	Provides enrichment example.
`examples/merge-configs-example.js`	Demonstrates merge modes.
`docs/tech-asset-enrichment.md`	Documents enrichment plugin.
`docs/hubSyncHandler/usecase-merge-behavior.md`	Starts merge-use-case documentation.
`docs/hubSyncHandler/README.md`	Documents hub synchronization.
`docs/hubSyncHandler/IMPLEMENTATION-SUMMARY.md`	Summarizes manifest implementation.
`docs/hubSyncHandler/BASE_PATH.md`	Documents URL prefixes.
`docs/hubSyncHandler/architecture-sequence.md`	Documents synchronization flows.
`docs/hubSyncHandler/ADR-reimport_control.md`	Proposes reimport controls.
`docs/hubSyncHandler/ADR-manifest-control.md`	Records manifest decisions.
`docs/dry-runs.md`	Documents dry-run behavior.
`.gitignore`	Ignores log files.
`.github/workflows/enrich-tech-assets.yml`	Adds enrichment workflow.
`.github/workflows/advanced-codeql.yml`	Cleans workflow formatting.
`.github/scripts/enrich-tech-assets.js`	Implements workflow enrichment.
`.env.example`	Documents hub environment variables.

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

  let appSlug = 'safe-settings'
+
+  // Initialize all routes (static UI + API) via centralized module
+  setupRoutes(robot, getRouter)


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Remove redundant initCache call when using getRouter.

Updated comments to clarify the purpose of mocking startup requests.

Copilot

Copilot review overview

🟡 Changes recommended

The UI configuration has a syntax error, Settings imports a missing plugin, and several routing and test regressions remain.

Review effort: Balanced
Findings: 5 · 2

Open (7)

Eager plugin require causes startup MODULE_NOT_FOUND · New
Test file violates StandardJS semicolon rules · New
Align UI and Express prefix mapping for unset and empty values
Pair organization and file predicates within manifest rules
Dashboard API routes lack authentication and authorization
Make sortable headers keyboard accessible
Resolve top-level enrichment configuration and custom properties

Resolved since last review (6)

Prevent repository YAML from redirecting deployment secrets
PR-controlled enrichment script can exfiltrate service secrets
Normalize property_name aliases before filtering
Normalize property_name before prefix checks
Normalize custom properties before enrichment
Preserve empty hub URL prefix

Previously missed (3)

In code that hasn't changed since last review

Hub repository guard omits organization check

index.js:781

Unlike the other new hub-repository guards, this one checks only the repository name. A PR opened in a same-named repository in any other organization will incorrectly create a Safe Settings validation check that the later check_run.created owner guard refuses to process. Include the configured hub organization in this predicate.

Reopened PR hub check omits organization validation

index.js:802

This reopened-PR path also identifies the hub by repository name alone. Reopening a PR in a same-named repository outside SAFE_SETTINGS_HUB_ORG therefore creates a validation check that cannot be completed by the owner-scoped handler. Apply the same organization check used for check_suite.requested.

Asset URLs ignore configured base path

ui/src/app/layout.jsx:14

These public-asset URLs are rooted at the host, so under the configured /safe-settings (or any custom) basePath they request /favicon.* outside the mounted router and return 404. Prefix metadata and the duplicate <head> icon links with the configured base path, or use a base-path-aware file metadata approach.

Removed tech asset enrichment plugin from settings and related code.

Copilot

Copilot review overview

🟡 Changes recommended

Build-breaking syntax, unauthenticated mutation routes, production packaging failures, and routing inconsistencies must be resolved.

Review effort: Balanced
Findings: 7 · 1

Open (8)

Include the built UI in the production container · New
Validate both repository owner and name for hub checks · New
Declare express as a production dependency · New
Await asynchronous route parameters · New
Align UI and Express prefix mapping for unset and empty values
Pair organization and file predicates within manifest rules
Dashboard API routes lack authentication and authorization
Make sortable headers keyboard accessible

Resolved since last review (3)

Test file violates StandardJS semicolon rules
Eager plugin require causes startup MODULE_NOT_FOUND
Resolve top-level enrichment configuration and custom properties

Previously missed (10)

In code that hasn't changed since last review

Restore archived tests or update the test script

test/integration/transport/archived-repositories.test.js:1

Deleting this suite leaves the existing npm run test:archived script pointing to a nonexistent file, so that documented command now fails instead of running. It also silently removes these archived-repository regression cases from the integration-test glob; restore the suite or update the script and provide equivalent coverage.

Make the success test assert a valid response

test/unit/lib/routes.test.js:90

This success test accepts every plausible outcome, including HTTP 500, and only asserts that Express returned a body. Because cacheGetInstallations is not configured here, the route currently takes its error path and the test still passes. Mock a hub installation/authenticated content response and require 200 plus the expected tree; inject a distinct failure for the following error test.

Make file selection keyboard accessible

ui/src/app/components/Safe-settings-hubContent.jsx:132

File selection is implemented as an onClick handler on a non-focusable <div>, making files impossible to select from the keyboard. Render this row as a button/treeitem with keyboard support and selected state.

This issue also appears on line 143 of the same file.

Make content-table navigation keyboard accessible

ui/src/app/components/Safe-settings-hubContent.jsx:218

The content-table rows are clickable but not keyboard-focusable or operable, so this second navigation path is also inaccessible without a pointer. Put the item action in a link/button or add equivalent keyboard semantics.

Fix insufficient dark-theme navigation contrast

ui/src/app/globals.css:91

This !important rule forces dark-theme navigation text to #6c757d on the #22272e navigation background, which is below the WCAG 4.5:1 contrast requirement for normal text. It also overrides the higher-contrast component rule in TitleBar.css; use the dark theme's primary text color instead.

Update the link to the dry-run orchestration code

docs/dry-runs.md:48

This anchor currently points to the repository-renamed handler (index.js:639-700), not dry-run check orchestration, which is now around index.js:747-918. Update the link so readers reach the implementation being described.

Remove stale destination files during reimport

docs/hubSyncHandler/ADR-reimport_control.md:219

The implementation does not replace the entire destination tree during reimport. retrieveSettingsFromOrgs() creates a Git tree with base_tree and only overlays files currently found in the source, so destination files deleted from the organization remain in the hub. Either implement deletion of stale paths or document the additive/update behavior rather than promising full replacement.

Replace absolute filesystem paths with repository links

docs/hubSyncHandler/IMPLEMENTATION-SUMMARY.md:216

These machine-specific absolute paths are not usable repository references and expose a contributor's local filesystem layout. Replace them with repository-relative paths/links; the same issue recurs for the entries below.

Correct the documented default configuration path

docs/hubSyncHandler/README.md:279

The documented default incorrectly includes CONFIG_PATH. Runtime code constructs paths as CONFIG_PATH/SAFE_SETTINGS_HUB_PATH, and lib/env.js defaults this variable to safe-settings; configuring .github/safe-settings as shown would produce .github/.github/safe-settings.

Fix the nonexistent module in the usage example

examples/merge-configs-example.js:238

The printed usage imports a module that does not exist; this example itself imports mergeConfigs from lib/hubSyncHandler.js. Copying the summary snippet therefore fails with MODULE_NOT_FOUND.

+  if (typeof getRouter === 'function') {
+    setupRoutes(robot, getRouter)
+  }


Copilot

Copilot review overview

🟡 Changes recommended

The UI build is currently invalid, routing defaults conflict, privileged APIs lack access control, and test execution has guaranteed failures.

Review effort: Balanced
Findings: 5 · 2

Open (7)

Protect dashboard APIs with authentication and CSRF controls · New
Include the built UI in the production container
Align UI and Express prefix mapping for unset and empty values
Pair organization and file predicates within manifest rules
Dashboard API routes lack authentication and authorization
Validate hub repository owner in pull request events · New
Make sortable headers keyboard accessible

Resolved since last review (3)

Await asynchronous route parameters
Declare express as a production dependency
Validate both repository owner and name for hub checks

Previously missed (2)

In code that hasn't changed since last review

Ensure deepMerge lets explicit null override existing values

test/unit/lib/mergeConfigs.test.js:193

This new test cannot pass against the added deepMerge implementation: when the overlay value is null, deepMerge returns the existing target value, so the result is { a: 1, b: 2 }, not { a: 1, b: null }. Update the merge logic so an explicit YAML null overrides the prior value, or change the documented/tested contract consistently.

Prefix public asset URLs with the configured base path

ui/src/app/layout.jsx:14

These root-relative public-asset URLs bypass the configured Next.js basePath. Under the default /safe-settings deployment they request /favicon.* from the server root, while static assets are mounted below the prefix, so the icons return 404. Prefix all metadata and manual icon URLs with the same normalized base path.

+  if (typeof getRouter === 'function') {
+    setupRoutes(robot, getRouter)
+  }


Copilot

Copilot review overview

🟡 Changes recommended

The UI configuration does not parse, privileged routes lack authorization, and several synchronization and test regressions remain unresolved.

Review effort: Balanced
Findings: 6 · 5 · 2

Open (13)

Skip content validation for deleted YAML files · New
Protect dashboard APIs with authentication and CSRF controls
Include the built UI in the production container
Align UI and Express prefix mapping for unset and empty values
Pair organization and file predicates within manifest rules
Dashboard API routes lack authentication and authorization
Require successful hub content response in the test · New
Increase dark-theme error text contrast · New
Increase light-theme warning text contrast · New
Make icon URLs respect the configured Next.js basePath · New
Make sortable headers keyboard accessible
Implement or document mergeStrategy behavior · New
Make file tree nodes keyboard selectable · New

Resolved since last review (1)

Validate hub repository owner in pull request events

+    const hubPath = `${env.CONFIG_PATH}/${env.SAFE_SETTINGS_HUB_PATH}`.replace(/\/+/g, '/')
+    const globalsPattern = new RegExp(`^${hubPath}/globals/.*\\.ya?ml$`)
+    const orgsPattern = new RegExp(`^${hubPath}/organizations/([^/]+)/.*\\.ya?ml$`)
+    const hubSyncFilesChanged = files.filter(f => globalsPattern.test(f) || orgsPattern.test(f))


+  it('should return hub content', async () => {
+    const res = await request(app).get('/api/safe-settings/hub/content')
+    expect([200, 404, 500]).toContain(res.statusCode)
+    expect(res.body).toBeDefined()
+  })


+.log-error {
+  color: #c00 !important;
+}


+.log-warn {
+  color: #b8860b !important;
+}


+        <link rel="icon" type="image/svg+xml" href="/favicon.svg" />
+        <link rel="icon" href="/favicon.ico" sizes="any" />


+      # mergeStrategy: merge | overwrite | preserve
+      # --------------------------------------------
+      # merge     = use a PR to sync files 
+      # overwrite = sync all files to the target ORG(s) (no PR)
+      mergeStrategy: merge


+        <div key={node.path} className={`d-flex align-items-center py-1 ${selected ? 'theme-bg-secondary' : ''}`} style={{ paddingLeft: depth * 12, cursor: 'pointer' }} onClick={() => setSelectedPath(node.path)}>
+          <FileIcon size={12} className="me-2" />
+          <span className="small text-truncate">{node.name}</span>
+        </div>


jefeish and others added 24 commits August 27, 2025 21:21

adding hub-sync feature code

ea663c2

adjusting ui

6457790

hub improvements

498bbbd

Add ui screen

dad3fe8

updated README

5188801

improved ui

a5ef531

merge ydhav-issue-fix

6938bf2

added base_url support

3312795

updated docs

bfbd874

fixed sync log page

4cb5e10

added architecture diagram

e4498f1

hub-sync ui update

79a8d43

Merge remote-tracking branch 'origin/decyjphr-fix-suborg-targeting-re…

50fef03

…moval' into feature/hub-sync

mergeConfig added

4b9c44e

added yml merge-sync

03b94b9

added PR Yaml validation

16ce389

added ADR

342f9f3

added manifest docs

ba1ff24

added manifest docs

6af5abc

enhanced hub-sync import

3eb521c

feat: add tech asset enrichment plugin and GitHub Actions workflow

bba1268

chore: merge recent fixes into hub sync

3a1e0df

🔀 - Generated by Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

test: remove obsolete axios route mock

7efec11

✅ - Generated by Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge branch 'yadhav/fix-recent-issues' into feature/hub-sync

59cfde8

decyjphr requested a balanced review from Copilot September 30, 2026 21:34

Copilot started reviewing on behalf of decyjphr September 30, 2026 21:35 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Refactor and update various components and configs

55a9539

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

jefeish requested a balanced review from Copilot September 30, 2026 23:48

Copilot started reviewing on behalf of jefeish September 30, 2026 23:49 View session

jefeish added 9 commits September 30, 2026 21:33

Refactor initCache call in index.js

f7d75b0

Remove redundant initCache call when using getRouter.

Refactor comments in loadInstance function

c6e6373

Updated comments to clarify the purpose of mocking startup requests.

Delete .github/workflows/enrich-tech-assets.yml

ab1020a

Delete docs/tech-asset-enrichment.md

a68b557

Delete examples/repo-with-tech-asset.yml

55ccaf4

Delete .github/scripts/enrich-tech-assets.js

0afe4c2

Delete test/integration/transport/archived-repositories.test.js

acd42ac

Delete lib/plugins/tech_asset_enrichment.js

f16a252

Delete TECH_ASSET_ENRICHMENT_GUIDE.md

84e898c

jefeish requested a balanced review from Copilot October 2, 2026 21:16

Copilot started reviewing on behalf of jefeish October 2, 2026 21:17 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread lib/settings.js Outdated

Comment thread test/unit/lib/routes.test.js Outdated

jefeish added 2 commits October 2, 2026 17:39

Remove tech asset enrichment functionality

a798108

Removed tech asset enrichment plugin from settings and related code.

Refactor test file for consistency and readability

99226eb

jefeish requested a balanced review from Copilot October 2, 2026 21:46

Copilot started reviewing on behalf of jefeish October 2, 2026 21:46 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread index.js

Comment on lines +30 to +32

if (typeof getRouter === 'function') {

setupRoutes(robot, getRouter)

}

Comment thread index.js Outdated

Comment thread package.json Outdated

Comment thread ui/src/app/[slug]/route.js Outdated

jefeish added 4 commits October 2, 2026 17:57

Update express dependency version in package.json

c4f0cb5

Update express version in package-lock.json

c63b9db

Update hubMasterRepo condition to include owner check

ef05e4a

Refactor slug extraction from context parameters

24a3619

jefeish requested a balanced review from Copilot October 2, 2026 22:22

Copilot started reviewing on behalf of jefeish October 2, 2026 22:23 View session

Refine hubMasterRepo condition to include owner check

39acc89

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread index.js

Comment on lines +30 to +32

if (typeof getRouter === 'function') {

setupRoutes(robot, getRouter)

}

Comment thread index.js Outdated

jefeish added 2 commits October 5, 2026 14:41

Delete docs/dry-runs.md

f380f9a

Delete lib/utils.js

02fc29e

jefeish requested a balanced review from Copilot October 6, 2026 22:59

Copilot started reviewing on behalf of jefeish October 6, 2026 23:00 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Navigation Menu

Repository navigation

Feature/hub sync - #1089

Feature/hub sync#1089
jefeish wants to merge 46 commits into
github-community-projects:yadhav/fix-recent-issuesfrom
jefeish:feature/hub-sync

jefeish commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
		<link rel="icon" href="/favicon.ico" sizes="any" />

Conversation

jefeish commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants