Repository navigation
feat(extensions): replace hardcoded workspace ULID with shell-published store (#8 P1-1 part 2) - #53
Merged
Merged
Conversation
…ed store (#8 P1-1 part 2) Second of two staged PRs closing #8 P1-1. Part 1 (#50) built the foundation: shell-side store, useWorkspaceContext composable, extensionElementContext injection. This part removes every `?? DEFAULT_WORKSPACE_ID` extension fallback and lifts the hardcoded ULID out of source code. Pattern - ext_issues/types.ts + ext_epics/types.ts: replace `export const DEFAULT_WORKSPACE_ID = "ws_..."` with `export function defaultWorkspaceId(): string` that reads `activeWorkspaceId()` from the shell store at call time. Empty-string fallback covers the brief boot-race window before `App.vue::loadShellSummary` runs; production callers gate on `whenWorkspaceReady()` first. - Per-component WORKSPACE_URI / DEFAULT_WORKSPACE_ID consts removed from PullsQueue, PullsOverview, PullsYourWork, PullsDetail, HomeYourWork. Each `?? DEFAULT_WORKSPACE_ID` → `?? defaultWorkspaceId()`. Module-scope command bindings (the iteration-3 review snag the abort comment called out) - bindPrCommands, bindIssueCommands, bindEpicCommands now defer their initial sync until `whenWorkspaceReady()` resolves, then subscribe to the topics scoped to that workspace. Race-free: a cleanup flag ensures the deferred bind doesn't fire after the consumer has unsubscribed. Acceptance grep - `grep -rn ws_01HV0K4XAVE2H6R5M8KJZ8Q1A3 frontend extensions/first-party/*/ui/src` returns 0. - Remaining matches are start.sh (smoke-test curls against the seed fixture's actual workspace id — legitimate data references, not hardcoded constants) and the regenerated extension asset bundles (which inline the seed workspace via the bootstrap, not from app code). Verification - All 4 first-party extensions rebuilt via `extensions/bundler/build-extension.sh`. - `cd frontend && bun test` — 73/73 green. - `cd frontend && bun run typecheck` — clean. Closes #8 P1-1.
There was a problem hiding this comment.
Pull request overview
This PR is the second stage of #8 P1-1, aiming to remove first-party extension reliance on a hardcoded “dogfood” workspace ULID and instead source workspace identity from the shell-published workspace store.
Changes:
- Replaces
?? DEFAULT_WORKSPACE_IDfallbacks acrossext_issues,ext_epics, andext_pull_requestswith store-sourced workspace resolution. - Refactors extension module-scope command bindings (
bind*Commands) to defer initial sync + event subscriptions untilwhenWorkspaceReady()resolves. - Rebuilds extension UI bundles and updates
manifest.jsonentryIntegrityhashes accordingly.
Reviewed changes
Copilot reviewed 22 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/packages/sdk-vue/src/use-workspace-context.ts | Updates docstring wording to remove the literal ULID reference. |
| extensions/first-party/ext_workspace_home/ui/src/HomeYourWork.vue | Removes hardcoded workspace URI constant (but currently leaves an unresolved reference). |
| extensions/first-party/ext_workspace_home/ui/manifest.json | Updates entryIntegrity for rebuilt UI bundle. |
| extensions/first-party/ext_workspace_home/assets/index.js | Rebuilt bundle output (includes workspace/SDK runtime code changes). |
| extensions/first-party/ext_pull_requests/ui/src/PullsYourWork.vue | Replaces workspace fallback logic to call defaultWorkspaceId() (currently missing definition/import). |
| extensions/first-party/ext_pull_requests/ui/src/PullsQueue.vue | Same fallback replacement in queue view (currently missing definition/import). |
| extensions/first-party/ext_pull_requests/ui/src/PullsOverview.vue | Same fallback replacement in overview view (currently missing definition/import). |
| extensions/first-party/ext_pull_requests/ui/src/PullsDetail.vue | Same fallback replacement for markdown rendering context (currently missing definition/import). |
| extensions/first-party/ext_pull_requests/ui/src/pr-commands.ts | Defers command binding and live-event subscriptions until whenWorkspaceReady(). |
| extensions/first-party/ext_pull_requests/ui/manifest.json | Updates entryIntegrity for rebuilt UI bundle. |
| extensions/first-party/ext_issues/ui/src/types.ts | Replaces exported hardcoded constant with defaultWorkspaceId() backed by activeWorkspaceId(). |
| extensions/first-party/ext_issues/ui/src/register.ts | Swaps call sites from DEFAULT_WORKSPACE_ID to defaultWorkspaceId(). |
| extensions/first-party/ext_issues/ui/src/IssuesList.vue | Changes default prop workspaceId to call defaultWorkspaceId() (risk of locking empty id). |
| extensions/first-party/ext_issues/ui/src/IssueDetail.vue | Swaps workspace fallback to defaultWorkspaceId(). |
| extensions/first-party/ext_issues/ui/src/issue-commands.ts | Defers binding/subscriptions until whenWorkspaceReady() resolves. |
| extensions/first-party/ext_issues/ui/manifest.json | Updates entryIntegrity for rebuilt UI bundle. |
| extensions/first-party/ext_epics/ui/src/types.ts | Replaces exported hardcoded constant with defaultWorkspaceId() backed by activeWorkspaceId(). |
| extensions/first-party/ext_epics/ui/src/register.ts | Swaps call sites from DEFAULT_WORKSPACE_ID to defaultWorkspaceId(). |
| extensions/first-party/ext_epics/ui/src/EpicsList.vue | Changes default prop workspaceId to call defaultWorkspaceId() (risk of locking empty id). |
| extensions/first-party/ext_epics/ui/src/EpicDetail.vue | Swaps workspace fallback to defaultWorkspaceId(). |
| extensions/first-party/ext_epics/ui/src/epic-commands.ts | Defers binding/subscriptions until whenWorkspaceReady() resolves. |
| extensions/first-party/ext_epics/ui/manifest.json | Updates entryIntegrity for rebuilt UI bundle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
2
to
6
| import { computed, onMounted, onUnmounted, ref, watch } from "vue"; | ||
| import { invokeOp, subscribeLiveEvents } from "@comtrya/sdk-core"; | ||
| import { loadYourWork } from "./api"; | ||
| import type { ComtryaGraphQLClient, LoadState, WorkItem } from "./types"; | ||
|
|
Comment on lines
27
to
32
| const loadState = ref<LoadState>("idle"); | ||
|
|
||
| const workspaceId = computed( | ||
| () => props.workspaceId ?? props.host?.workspaceId ?? DEFAULT_WORKSPACE_ID, | ||
| () => props.workspaceId ?? props.host?.workspaceId ?? defaultWorkspaceId(), | ||
| ); | ||
| const viewerRef = computed(() => props.viewerRef ?? props.host?.viewerRef ?? null); |
| @@ -53,7 +52,7 @@ const error = ref<string | null>(null); | |||
| const focusedIndex = ref(0); | |||
|
|
|||
Comment on lines
25
to
30
| const pulls = ref<PullRequest[]>([]); | ||
| const loadState = ref<LoadState>("idle"); | ||
|
|
||
| const workspaceId = computed( | ||
| () => props.workspaceId ?? props.host?.workspaceId ?? DEFAULT_WORKSPACE_ID, | ||
| () => props.workspaceId ?? props.host?.workspaceId ?? defaultWorkspaceId(), | ||
| ); |
Comment on lines
167
to
173
| // Hardcoded default workspace matches the rest of ext_pull_requests | ||
| // (PullsQueue, PullsOverview, PullsYourWork); swap to a real | ||
| // resolver when the multi-workspace surface lands. | ||
| const DEFAULT_WORKSPACE_ID = "ws_01HV0K4XAVE2H6R5M8KJZ8Q1A3"; | ||
| const renderedBody = computed(() => | ||
| pull.value?.bodyMarkdown | ||
| ? renderMarkdown(pull.value.bodyMarkdown, { workspaceId: DEFAULT_WORKSPACE_ID }) | ||
| ? renderMarkdown(pull.value.bodyMarkdown, { workspaceId: defaultWorkspaceId() }) | ||
| : "", |
Comment on lines
40
to
46
| projectName?: string; | ||
| /** Active repo's label catalog (see IssueDetail for details). */ | ||
| labelCatalog?: LabelCatalog | null; | ||
| }>(), { | ||
| workspaceId: DEFAULT_WORKSPACE_ID, | ||
| workspaceId: defaultWorkspaceId(), | ||
| repositoryId: null, | ||
| state: null, |
Comment on lines
26
to
33
| state?: string | null; | ||
| title?: string; | ||
| showNewLink?: boolean; | ||
| /** Scope listing to this Project; new-epic link stamps it on create. */ | ||
| projectName?: string; | ||
| }>(), { | ||
| workspaceId: DEFAULT_WORKSPACE_ID, | ||
| workspaceId: defaultWorkspaceId(), | ||
| state: null, |
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of two staged PRs closing #8 P1-1. Part 1 (#50) built the foundation: workspace store, `useWorkspaceContext` composable, and `extensionElementContext()` injection. This PR removes every `?? DEFAULT_WORKSPACE_ID` extension fallback.
Pattern
`types.ts` in ext_issues + ext_epics:
```ts
import { activeWorkspaceId } from "@comtrya/sdk-core";
export function defaultWorkspaceId(): string {
return activeWorkspaceId() ?? "";
}
```
Empty-string covers the brief boot-race window; production callers gate on `whenWorkspaceReady()`.
Per-component `WORKSPACE_URI` / `DEFAULT_WORKSPACE_ID` consts removed from PullsQueue, PullsOverview, PullsYourWork, PullsDetail, HomeYourWork. Each `?? DEFAULT_WORKSPACE_ID` → `?? defaultWorkspaceId()`.
Module-scope command bindings (the abort-comment race fix)
`bindPrCommands`, `bindIssueCommands`, `bindEpicCommands` are called from `setup(host)` BEFORE any element mounts. Previously they captured the hardcoded ULID at bind time; now they defer the initial sync until `whenWorkspaceReady()` resolves and subscribe scoped to that workspace. A `cancelled` flag makes teardown race-safe.
Acceptance grep
`grep -rn ws_01HV0K4XAVE2H6R5M8KJZ8Q1A3 frontend extensions/first-party/*/ui/src` → 0. Remaining hits are start.sh smoke probes (legit data refs) and generated `assets/index.js` bundles (which no longer inline the literal — any occurrence is from bootstrap data).
Verification
Closes #8 P1-1.