-
Notifications
You must be signed in to change notification settings - Fork 191
adding hub-sync feature code #873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main-enterprise
Are you sure you want to change the base?
Conversation
const directPush = (env.SAFE_SETTINGS_HUB_DIRECT_PUSH === 'true' || env.SAFE_SETTINGS_HUB_DIRECT_PUSH === '1'); | ||
|
||
// Find installation for destination org to auth | ||
const installs = await getInstallations(robot) |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
the enclosing function
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the issue, simply terminate the statement const installs = await getInstallations(robot)
on line 95 with an explicit semicolon. This improves readability and consistency within the function, reduces the risk of accidents if the code is refactored, and aligns with the project's apparent style, which is to use semicolons after statements. Specifically, edit line 95 in lib/hubSyncHandler.js to add a semicolon.
-
Copy modified line R95
@@ -92,7 +92,7 @@ | ||
const directPush = (env.SAFE_SETTINGS_HUB_DIRECT_PUSH === 'true' || env.SAFE_SETTINGS_HUB_DIRECT_PUSH === '1'); | ||
|
||
// Find installation for destination org to auth | ||
const installs = await getInstallations(robot) | ||
const installs = await getInstallations(robot); | ||
const install = installs.find(i => i.account && i.account.type === 'Organization' && i.account.login.toLowerCase() === destOwner.toLowerCase()); | ||
if (!install) { | ||
robot.log.warn(`Installation for destination org ${destOwner} not found; cannot sync`); |
async function run() { | ||
if (i >= items.length) return | ||
const idx = i++ | ||
const p = Promise.resolve(mapper(items[idx], idx)).then(r => { out[idx] = r; running.delete(p) }) |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
anonymous function
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the issue, remove the superfluous second argument (idx
) from the call to mapper
within mapWithLimit
(on line 283). The function signature being used as a callback only takes a single parameter and does not utilize further arguments, and there is no indication elsewhere in this snippet that the index is needed (nor any evidence of use of the arguments
object). Thus, update line 283 to only pass items[idx]
to mapper
, eliminating the unused idx
argument. No further code or import changes are necessary since functionality remains the same and no new features are introduced.
-
Copy modified line R283
@@ -280,7 +280,7 @@ | ||
async function run() { | ||
if (i >= items.length) return | ||
const idx = i++ | ||
const p = Promise.resolve(mapper(items[idx], idx)).then(r => { out[idx] = r; running.delete(p) }) | ||
const p = Promise.resolve(mapper(items[idx])).then(r => { out[idx] = r; running.delete(p) }) | ||
running.add(p) | ||
if (running.size >= MAX_DIR_CONCURRENCY) await Promise.race(running) | ||
return run() |
|
||
const copyToClipboard = (text) => { | ||
try { navigator.clipboard.writeText(text); } catch(_) {} | ||
} |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
the enclosing function
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the problem and make semicolon usage completely explicit and consistent, we should add a semicolon after the closing brace of the copyToClipboard
function at line 89, making it clear that this is the end of a function expression assigned to a variable. This also matches the style used for other functions in the file (such as fetchData
, cycleSort
, etc.), and prevents any confusion arising from relying on implicit semicolon insertion. No further imports or changes are required.
-
Copy modified line R89
@@ -86,7 +86,7 @@ | ||
|
||
const copyToClipboard = (text) => { | ||
try { navigator.clipboard.writeText(text); } catch(_) {} | ||
} | ||
}; | ||
|
||
return ( | ||
<div className="ui-table"> |
import { useHydrated } from '../hooks/useHydrated'; | ||
|
||
// Mock organizations used when /api/organizations returns 404 | ||
const MOCK_ORGS = [ |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
The correct way to fix this is simply to remove the unused variable declaration (lines 8–12 defining MOCK_ORGS
). This does not impact any existing functionality, as the variable isn't referenced anywhere else in the code or used for actual rendering or fallback logic. You should remove those lines from the file ui/src/app/components/OrganizationsTable.jsx. No additional imports, method definitions, or code changes are needed.
@@ -5,11 +5,6 @@ | ||
import { useHydrated } from '../hooks/useHydrated'; | ||
|
||
// Mock organizations used when /api/organizations returns 404 | ||
const MOCK_ORGS = [ | ||
{ id: 1, name: 'mock-org-one', lastSyncDate: new Date(Date.now() - 3600 * 1000).toISOString(), lastSyncMessage: 'Initial mock sync', lastSyncSha: 'abcdef1', ageSeconds: 3600 }, | ||
{ id: 2, name: 'example-inc', lastSyncDate: new Date(Date.now() - 7200 * 1000).toISOString(), lastSyncMessage: 'Second mock sync', lastSyncSha: 'abcdef2', ageSeconds: 7200 }, | ||
{ id: 3, name: 'demo-labs', lastSyncDate: null, lastSyncMessage: null, lastSyncSha: null, ageSeconds: null, na: true } | ||
]; | ||
|
||
const OrganizationsTable = ({ organizations: propOrganizations = [] }) => { | ||
const [searchTerm, setSearchTerm] = useState(''); |
import { useHydrated } from '../hooks/useHydrated'; | ||
|
||
// Simple mock tree used when API returns 404 (dev convenience) | ||
const MOCK_TREE = { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To resolve the problem, simply remove the declaration and definition of the unused variable MOCK_TREE
(lines 7–46). This consists of a single contiguous block representing the mock object. No other part of the shown file refers to MOCK_TREE
, so its removal will not alter existing functionality. No imports or additional definitions are needed.
@@ -4,46 +4,6 @@ | ||
import { useHydrated } from '../hooks/useHydrated'; | ||
|
||
// Simple mock tree used when API returns 404 (dev convenience) | ||
const MOCK_TREE = { | ||
name: '.github', | ||
path: '.github', | ||
type: 'dir', | ||
lastCommitAt: new Date(Date.now() - 3600 * 1000).toISOString(), | ||
entries: [ | ||
{ | ||
name: 'settings.yml', | ||
path: '.github/settings.yml', | ||
type: 'file', | ||
lastCommitAt: new Date(Date.now() - 1800 * 1000).toISOString(), | ||
lastCommitMessage: 'chore: mock settings', | ||
lastCommitSha: 'mock123' | ||
}, | ||
{ | ||
name: 'CODEOWNERS', | ||
path: '.github/CODEOWNERS', | ||
type: 'file', | ||
lastCommitAt: new Date(Date.now() - 7200 * 1000).toISOString(), | ||
lastCommitMessage: 'feat: add mock CODEOWNERS', | ||
lastCommitSha: 'mock456' | ||
}, | ||
{ | ||
name: 'workflows', | ||
path: '.github/workflows', | ||
type: 'dir', | ||
lastCommitAt: new Date(Date.now() - 5400 * 1000).toISOString(), | ||
entries: [ | ||
{ | ||
name: 'ci.yml', | ||
path: '.github/workflows/ci.yml', | ||
type: 'file', | ||
lastCommitAt: new Date(Date.now() - 2500 * 1000).toISOString(), | ||
lastCommitMessage: 'ci: mock workflow', | ||
lastCommitSha: 'mock789' | ||
} | ||
] | ||
} | ||
] | ||
}; | ||
|
||
export default function SafeSettingsHubContent() { | ||
const hydrated = useHydrated(); |
}); | ||
}, [search, displayTree]); | ||
|
||
const flatList = useMemo(() => { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To resolve this problem, simply delete the flatList
assignment using useMemo
since its result is never subsequently used in the component. This means deleting lines 179–186 as currently written. No other code modifications are required, as its removal will not affect component functionality (because it was unused).
Only remove the block that defines flatList
and its memoization logic, without affecting adjacent logic or hooks. No new imports, methods, or other code changes are required.
-
Copy modified line R180
@@ -176,15 +176,8 @@ | ||
}); | ||
}, [search, displayTree]); | ||
|
||
const flatList = useMemo(() => { | ||
if (!displayTree) return []; | ||
// If display root is a directory, list its children instead of the directory itself (hide intermediate root) | ||
if (displayTree.type === 'dir') { | ||
return displayTree.entries.flatMap(child => flattenNodes(child, [], 0)); | ||
} | ||
return flattenNodes(displayTree, [], 0); | ||
}, [displayTree, flattenNodes]); | ||
|
||
|
||
// Build hierarchical visible list honoring expandedPaths and optional sorting | ||
const sortedFlatList = useMemo(() => { | ||
if (!displayTree) return []; |
import { useTheme } from './ThemeContext'; | ||
|
||
export default function ThemeToggle() { | ||
const { theme, toggleTheme, isDark } = useTheme(); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
To fix the problem, we should remove the unused theme
variable from the destructuring assignment in the ThemeToggle
component. This means modifying the line
const { theme, toggleTheme, isDark } = useTheme();
to exclude theme
:
const { toggleTheme, isDark } = useTheme();
This change should be applied only in ui/src/app/components/ThemeToggle.jsx
. No other code or imports are affected by this change.
-
Copy modified line R6
@@ -3,7 +3,7 @@ | ||
import { useTheme } from './ThemeContext'; | ||
|
||
export default function ThemeToggle() { | ||
const { theme, toggleTheme, isDark } = useTheme(); | ||
const { toggleTheme, isDark } = useTheme(); | ||
|
||
return ( | ||
<button |
This pull request introduces a new "Safe Settings Hub Sync" feature for centralized configuration management across multiple organizations.
The main changes include: