feat: add Instance tab for small-team sharing flows#294
Conversation
github-actions
Bot
added
the
needs linked issue
📝 WalkthroughWalkthroughAdds a new client-side Next.js Instance management page with multi-view join/create flows and an admin dashboard (polling pending requests, approve/reject), new frontend API types/helpers, a navbar entry, and two package.json edits. ChangesInstance Management Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Context Summary
Suggested issue links
Use |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@frontend/src/app/instance/page.tsx`:
- Around line 203-205: The disconnect button's hover color (className on the
button that calls setCurrentView("no-instance-view")) uses hover:text-zinc-200
which becomes low-contrast in light mode; update the className to use
theme-specific hover classes such as hover:text-zinc-700
dark:hover:text-zinc-200 (or equivalent light/dark-friendly tokens) so the label
remains readable in both themes while keeping the existing layout and onClick
behavior.
🪄 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
Run ID: 0f6a44ac-416c-421f-a0bd-be2b0bd07782
📒 Files selected for processing (1)
frontend/src/app/instance/page.tsx
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| )} | ||
|
|
||
| {currentView === "error" && ( | ||
| <div className="p-4 border rounded-xl border-red-200 bg-red-50 text-red-900 dark:border-red-950/20 dark:text-red-200 dark:border-red-900/50 text-sm"> |
There was a problem hiding this comment.
🟡 Medium instance/page.tsx:59
The error state div on line 59 uses dark:border-red-950/20 twice instead of dark:bg-red-950/20 for the background. In dark mode, the error box renders with no background color, making the text unreadable against the dark page background. Change the first dark:border-red-950/20 to dark:bg-red-950/20 to match the warning box pattern on line 42.
- <div className="p-4 border rounded-xl border-red-200 bg-red-50 text-red-900 dark:border-red-950/20 dark:text-red-200 dark:border-red-900/50 text-sm">
+ <div className="p-4 border rounded-xl border-red-200 bg-red-50 text-red-900 dark:bg-red-950/20 dark:text-red-200 dark:border-red-900/50 text-sm">🤖 Copy this AI Prompt to have your agent fix this:
In file frontend/src/app/instance/page.tsx around line 59:
The error state div on line 59 uses `dark:border-red-950/20` twice instead of `dark:bg-red-950/20` for the background. In dark mode, the error box renders with no background color, making the text unreadable against the dark page background. Change the first `dark:border-red-950/20` to `dark:bg-red-950/20` to match the warning box pattern on line 42.
Evidence trail:
frontend/src/app/instance/page.tsx line 59 at REVIEWED_COMMIT: dark-mode classes are `dark:border-red-950/20 dark:text-red-200 dark:border-red-900/50` — two `dark:border-*` and no `dark:bg-*`. Compare line 42: `dark:bg-amber-950/20 dark:text-amber-200 dark:border-amber-900/50` — correct pattern with `dark:bg-*`.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR adds a new Instance Management feature page with multiple user-facing workflows (joining instances, creating admin instances, approval dashboard). New feature additions introducing new components and workflows warrant human review regardless of current implementation scope. You can customize Macroscope's approvability policy. Learn more. |
Abhash-Chakraborty
changed the title
Abhash-Chakraborty
added
do-not-merge
ChatGPT Codex Connector
Abhash-Chakraborty
left a comment
Abhash-Chakraborty
left a comment
There was a problem hiding this comment.
This is a useful start, but it does not complete issue #263 yet.
Main gaps:
- The page is static/local state only. Issue #263 asks for create/join/admin flows using the shared API client/base URL pattern.
- Approve/reject and active users are mock arrays, so nothing persists or talks to the backend.
- The page is not wired into the main navigation, so users cannot discover the Instance tab from the app.
- The PR body should use
Fixes #263orCloses #263once it actually completes the issue. - Please avoid wording like “cluster”, “gateway”, and “pipeline” in the UI. This should feel like a simple small-team sharing screen for normal users.
Keeping do-not-merge for now. The next version should connect to the actual auth/instance endpoints or clearly narrow the scope if this is only meant to be a UI mock.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/src/components/NavBar.tsx`:
- Line 14: The frontend change in NavBar.tsx requires running the package-local
validation and build, so from the repo root run the frontend package checks and
build and capture logs: execute "pnpm -C frontend check" and then "pnpm -C
frontend build" (or the equivalent scripts used by CI) and attach the console
output/CI logs; ensure you run these before merging and paste the full
stdout/stderr so reviewers can confirm the checks passed for the change that
touches NavBar.tsx (the object entry { href: "/instance", label: "Instance" }).
In `@frontend/src/lib/api.ts`:
- Around line 721-726: The URL for processInstanceRequest currently interpolates
an unconstrained requestId directly into the path; encode requestId as a path
segment to avoid broken routes for reserved characters. Update
processInstanceRequest to call encodeURIComponent(requestId) when composing the
endpoint (the template string passed to api.post) so the POST target becomes
`/api/instance/requests/${encodeURIComponent(requestId)}/process`.
🪄 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
Run ID: 02ac123a-18e9-4db8-90bf-05b158b494c4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/package.jsonfrontend/src/app/instance/page.tsxfrontend/src/components/NavBar.tsxfrontend/src/lib/api.tspackage.json
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/instance/page.tsx
| { href: "/gallery", label: "Gallery" }, | ||
| { href: "/vault", label: "Vault" }, | ||
| { href: "/search", label: "Search" }, | ||
| { href: "/instance", label: "Instance" }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pnpm check
pnpm buildRepository: Abhash-Chakraborty/Find
Length of output: 371
Run required frontend validation before merge
pnpm check isn’t available when run from the repo root (pnpm reports “Command "check" not found”). For this frontend/src change, run the required checks inside the frontend package (e.g., pnpm -C frontend check && pnpm -C frontend build) and share the output/CI logs.
🤖 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 `@frontend/src/components/NavBar.tsx` at line 14, The frontend change in
NavBar.tsx requires running the package-local validation and build, so from the
repo root run the frontend package checks and build and capture logs: execute
"pnpm -C frontend check" and then "pnpm -C frontend build" (or the equivalent
scripts used by CI) and attach the console output/CI logs; ensure you run these
before merging and paste the full stdout/stderr so reviewers can confirm the
checks passed for the change that touches NavBar.tsx (the object entry { href:
"/instance", label: "Instance" }).
Source: Coding guidelines
| export const processInstanceRequest = async ( | ||
| requestId: string, | ||
| action: "approve" | "reject" | ||
| ): Promise<{ success: boolean }> => { | ||
| const response = await api.post<{ success: boolean }>(`/api/instance/requests/${requestId}/process`, { | ||
| action, |
There was a problem hiding this comment.
Encode requestId before composing the process URL.
requestId is an unconstrained string; reserved characters can alter the path and break approve/reject calls. Encode it as a path segment.
Suggested patch
export const processInstanceRequest = async (
requestId: string,
action: "approve" | "reject"
): Promise<{ success: boolean }> => {
- const response = await api.post<{ success: boolean }>(`/api/instance/requests/${requestId}/process`, {
+ const encodedRequestId = encodeURIComponent(requestId);
+ const response = await api.post<{ success: boolean }>(`/api/instance/requests/${encodedRequestId}/process`, {
action,
});
return response.data;
};📝 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.
| export const processInstanceRequest = async ( | |
| requestId: string, | |
| action: "approve" | "reject" | |
| ): Promise<{ success: boolean }> => { | |
| const response = await api.post<{ success: boolean }>(`/api/instance/requests/${requestId}/process`, { | |
| action, | |
| export const processInstanceRequest = async ( | |
| requestId: string, | |
| action: "approve" | "reject" | |
| ): Promise<{ success: boolean }> => { | |
| const encodedRequestId = encodeURIComponent(requestId); | |
| const response = await api.post<{ success: boolean }>(`/api/instance/requests/${encodedRequestId}/process`, { | |
| action, | |
| }); | |
| return response.data; | |
| }; |
🤖 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 `@frontend/src/lib/api.ts` around lines 721 - 726, The URL for
processInstanceRequest currently interpolates an unconstrained requestId
directly into the path; encode requestId as a path segment to avoid broken
routes for reserved characters. Update processInstanceRequest to call
encodeURIComponent(requestId) when composing the endpoint (the template string
passed to api.post) so the POST target becomes
`/api/instance/requests/${encodeURIComponent(requestId)}/process`.
2 participants
Hi @Abhash-Chakraborty,
I have added the frontend UI for the opt-in shared instance management tab as requested in issue #263.
What was done:
instancepage route underfrontend/src/app/instance/page.tsx.Please review it! Thanks!
Summary by CodeRabbit
New Features
Chores