Add CatPilot Canvas extension#2359
Open
tanure wants to merge 6 commits into
Open
Conversation
A visual command center canvas for CatPilot: tasks (list + board), journal, milestones, memos, learning, growth, projects, Copilot-generated reports, an activity timeline, interactive settings/migration, a markdown editor on every field, and light/dark themes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cc565b3b-2964-445e-bb79-88466b6a5494
github-actions
Bot
added
canvas-extension
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CatPilot as a GitHub Copilot Canvas extension with a local storage engine and interactive SPA.
Changes:
- Adds dashboards, task management, reports, timelines, and settings.
- Integrates CatPilot storage and Copilot agent actions.
- Adds extension metadata, documentation, and visual assets.
Reviewed changes
Copilot reviewed 8 out of 12 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
extensions/catpilot-canvas/extension.mjs |
Canvas server, APIs, and actions |
extensions/catpilot-canvas/catpilot-store.mjs |
CatPilot storage engine |
extensions/catpilot-canvas/ui/app.js |
Canvas application behavior |
extensions/catpilot-canvas/ui/index.html |
Application shell |
extensions/catpilot-canvas/ui/styles.css |
Layout and themes |
extensions/catpilot-canvas/ui/hero.svg |
Vector CatPilot artwork |
extensions/catpilot-canvas/ui/hero.png |
UI CatPilot artwork |
extensions/catpilot-canvas/assets/preview.png |
Marketplace preview |
extensions/catpilot-canvas/README.md |
Extension documentation |
extensions/catpilot-canvas/package.json |
Package metadata |
extensions/catpilot-canvas/package-lock.json |
Locked dependencies |
extensions/catpilot-canvas/.github/plugin/plugin.json |
Plugin manifest |
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
| "markdown-editor" | ||
| ], | ||
| "logo": "assets/preview.png", | ||
| "extensions": "." |
Comment on lines
+143
to
+147
| if (p.startsWith("/api/")) { | ||
| try { | ||
| const data = await handleApi(req, url); | ||
| if (data === null) return sendJson(res, 404, { error: "Not found" }); | ||
| return sendJson(res, 200, data); |
Comment on lines
+385
to
+386
| const p = path.join(dir, filename); | ||
| if (!path.resolve(p).startsWith(path.resolve(dir))) throw new Error("Invalid memo path"); |
Comment on lines
+462
to
+463
| const p = path.join(dir, filename); | ||
| if (!path.resolve(p).startsWith(path.resolve(dir))) throw new Error("Invalid note path"); |
| try { | ||
| const { report } = await api(`/api/reports/${encodeURIComponent(filename)}`); | ||
| let body; | ||
| if (report.format === "html") { body = el("iframe", { class: "report-iframe" }); body.srcdoc = report.content; } |
Comment on lines
+352
to
+355
| @media (max-width: 720px) { | ||
| .app { grid-template-columns: 1fr; } | ||
| .sidebar { display: none; } | ||
| } |
| table.tbl tbody tr { transition: background var(--sq); } | ||
| table.tbl tbody tr:hover { background: var(--panel-hover); } | ||
| .row-actions { display: flex; gap: 4px; opacity: 0; transition: opacity var(--sq); justify-content: flex-end; } | ||
| table.tbl tr:hover .row-actions { opacity: 1; } |
| if (plan.needsMigration && (migration === "move" || migration === "copy")) { | ||
| for (const item of plan.items) { | ||
| if (!item.willMove) continue; | ||
| try { migrated += moveOrCopyPath(item.from, item.to, item.isDir, migration); } catch { /* best-effort per item */ } |
| // mutating the active config. | ||
| function resolveDomainPaths(baseDir, root, partitioning) { | ||
| const storageRoot = path.resolve(baseDir, root); | ||
| const partition = getPartitionFolder(partitioning); |
| } | ||
|
|
||
| function resolveFilePath(type, config) { | ||
| const partition = getPartitionFolder(config.storage.partitioning); |
Address the 18 inline review findings on PR github#2359: Security & correctness - Block cross-origin/non-loopback API requests (Host, Origin and Sec-Fetch-Site guards) in extension.mjs. - Harden memo/note reads against path traversal via basename + parent-dir checks; return sanitized filenames. - Escape markdown table cells (pipes/newlines) on write and decode on read so task/milestone fields round-trip safely. - Migration never overwrites: moveOrCopyPath skips existing destinations and reports skipped items; applyConfigChange validates the whole plan and fails before persisting on any error. - Config changes now persist to the resolved config path. Cross-partition aggregation - Dashboard "last 3 days", the activity timeline and generated-report journal now aggregate across every storage partition (day/week/month), skipping dotfolders (.trash/.git/.obsidian). Editable views and counts stay scoped to the current partition to avoid ID collisions. - Add last-7 / last-30 report periods (enum + range handling). Accessibility & UI - Nav items are real <button> elements with aria-current and aria-hidden icons; focus-visible outline and button reset in CSS. - Fix double-escaping of markdown list items. - Sandbox the report preview iframe and give it a title. - Config modal: require a preview before apply, surface skipped items and destination conflicts. - Responsive top nav (horizontal scroll) instead of hiding the sidebar. - Table row actions reveal on focus-within for keyboard users.
Author
|
Thanks for the thorough automated review! I pushed a commit that addresses all of the inline findings. Summary: Security & correctness
Cross-partition aggregation
Accessibility & UI
I re-ran |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 11 comments.
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
Comments suppressed due to low confidence (2)
extensions/catpilot-canvas/catpilot-store.mjs:84
- Onboarding always writes the global config even when
CATPILOT_CONFIGorCATPILOT_ROOTselects another canonical path. If that selected file does not yet exist,/api/statusremains unconfigured immediately after this write andstartApp()fails. Write to the path returned byresolveConfigPath()so setup creates the config that subsequent loads will actually use.
fs.mkdirSync(GLOBAL_CONFIG_DIR, { recursive: true });
fs.writeFileSync(GLOBAL_CONFIG_PATH, JSON.stringify(config, null, 2), "utf8");
config.__baseDir = configBaseDir(GLOBAL_CONFIG_PATH);
config.__configPath = GLOBAL_CONFIG_PATH;
extensions/catpilot-canvas/ui/app.js:1051
- Editing a field while the preview request is in flight calls
invalidate(), but the older response later restorespreviewedBodyand enables applying the superseded values. Re-check that the form still matches the submitted body before accepting the response.
try { const plan = await api("/api/config/plan", { method: "POST", body: b }); previewedBody = b; renderPlan(plan); }
Comment on lines
+59
to
+60
| store.writeConfig(await readBody(req)); | ||
| return { ok: true, ...store.configStatus() }; |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. First-run onboarding has no previously-configured source to migrate from, so the Move/Copy choices were genuine no-ops. I removed the migration select from the onboarding form and writeConfig now always records migration mode 'adopt'. Move/Copy migration remains available from Settings, which uses the plan -> preview -> approve -> apply flow.
Comment on lines
+944
to
+949
| function resolveDomainPaths(baseDir, root, partitioning) { | ||
| const storageRoot = path.resolve(baseDir, root); | ||
| const partition = getPartitionFolder(partitioning); | ||
| const out = {}; | ||
| for (const [type, fileName] of Object.entries(DEFAULT_FILES)) { | ||
| out[type] = { path: path.join(storageRoot, partition, fileName), isDir: !/\.[a-z]+$/i.test(fileName) }; |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. resolveDomainPaths now takes an optional files argument and merges DEFAULT_FILES with config.storage.files. All three callers (getConfig and both planConfigChange resolves) plus the root-migration loop now pass config.storage.files, so a customized filename map is honored everywhere.
Comment on lines
+416
to
+418
| const filename = `${todayISO()}_${slugify(params.title)}.md`; | ||
| const p = path.join(dir, filename); | ||
| fs.writeFileSync(p, `# ${params.title}\n\n${params.content || "Add your memo content here."}`, "utf8"); |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. createMemo now routes the target path through a new uniquePath() helper that appends -2, -3... before the extension when a file already exists, and returns the actual basename. Two memos with the same title on the same day no longer overwrite each other.
Comment on lines
+492
to
+497
| const filename = `${todayISO()}_${slugify(params.title)}.md`; | ||
| const p = path.join(dir, filename); | ||
| if (!path.resolve(p).startsWith(path.resolve(dir))) throw new Error("Invalid note path"); | ||
| const frontmatter = { catpilot: DOMAIN_TAG[domain], title: params.title, date: todayISO(), ...(params.frontmatter || {}) }; | ||
| const body = params.body ? `\n${params.body}\n` : "\n"; | ||
| fs.writeFileSync(p, `${toFrontmatter(frontmatter)}\n\n# ${params.title}\n${body}`, "utf8"); |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. addNote (learning/growth/projects) uses the same uniquePath() helper and returns the real basename, so a same-title note on the same day is preserved instead of overwritten.
Comment on lines
+815
to
+818
| const filename = `report-${nowStamp()}${slug ? "-" + slug : ""}.${ext}`; | ||
| let content = body || ""; | ||
| if (ext === "md" && title && !/^#\s/m.test(content)) content = `# ${title}\n\n${content}`; | ||
| fs.writeFileSync(path.join(dir, filename), content, "utf8"); |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. saveReport (and therefore generateReport, which delegates to it) now allocates a unique filename via uniquePath() and returns the final basename + date, so two reports generated in the same minute no longer collide.
Comment on lines
+243
to
+247
| try { | ||
| await VIEWS[view](content); | ||
| } catch (err) { | ||
| content.innerHTML = ""; | ||
| content.append(errorBox(err)); |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. go() now increments a navToken per navigation and builds the view into detached staging nodes (content + view-actions), committing to the DOM only when the token is still current. A slow view that resolves after the user has navigated away is discarded instead of painting into the newer view.
| }; | ||
|
|
||
| // ---------------------------------------------------------------- reports | ||
| const PERIODS = [["this-week", "This week"], ["last-week", "Last week"], ["this-month", "This month"], ["last-month", "Last month"], ["last-7", "Last 7 days"], ["last-30", "Last 30 days"]]; |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. Added an 'All time' entry to the PERIODS array. The backend periodRange and the report action's period enum already accepted 'all'; the UI now exposes it.
Comment on lines
+55
to
+61
| const backdrop = el("div", { class: "modal-backdrop", onclick: (e) => { if (e.target === backdrop) closeModal(); } }); | ||
| const modal = el("div", { class: "modal" }); | ||
| if (width) modal.style.width = `min(${width}px, 100%)`; | ||
| modal.append( | ||
| el("div", { class: "modal-head" }, | ||
| el("h3", { text: title }), | ||
| el("button", { class: "icon-btn", text: "✕", onclick: closeModal })), |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. openModal now sets role=dialog, aria-modal=true and aria-labelledby (associated with the title), traps Tab within the dialog, moves focus to the first focusable element on open, and restores focus to the previously-focused element on close.
| function taskRow(t) { | ||
| const done = t.status.toLowerCase() === "done"; | ||
| const tr = el("tr", { class: done ? "done-row" : "", dataset: { taskTitle: t.title } }); | ||
| const check = el("input", { type: "checkbox", style: "width:auto", ...(done ? { checked: "checked" } : {}) }); |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. The task row checkbox now gets an aria-label (Complete <title> / Reopen <title>) so it has an accessible name.
Comment on lines
+195
to
+202
| async function ensureServer() { | ||
| if (sharedServer) return sharedServer; | ||
| const server = createServer((req, res) => { | ||
| requestListener(req, res).catch(() => { try { res.writeHead(500); res.end(); } catch { /* */ } }); | ||
| }); | ||
| await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); | ||
| const port = server.address().port; | ||
| sharedServer = { server, url: `http://127.0.0.1:${port}/` }; |
Author
There was a problem hiding this comment.
Fixed in cbc8f85. ensureServer() now caches its in-flight init promise; concurrent open() calls await the same promise instead of each binding a new loopback listener. The cached promise is cleared on failure and on server close so a later reopen re-inits cleanly.
- catpilot-store: add uniquePath() so creating a memo, note, or report never silently overwrites an existing file with the same title/timestamp; return the actual (possibly suffixed) basename to callers - catpilot-store: bound timeline milestone events to <= today so far-future target dates don't leak into "recent activity" - catpilot-store: resolveDomainPaths now honors config.storage.files instead of always using DEFAULT_FILES (config, plan preview, and migration loop) - catpilot-store: writeConfig always adopts on first-run onboarding (no prior source to migrate); migration stays available via Settings plan/apply - extension.mjs: cache ensureServer() init promise so concurrent opens share one loopback server instead of racing to bind two - ui: onboarding form drops the no-op migration select - ui/app.js: navigation token guard + detached staging so a slow view can't paint into a newer view; add "All time" report period; modal gets role=dialog/aria-modal/labelledby + focus trap and restore; task checkbox gets an accessible name
Author
|
Thanks for the thorough second pass. All 11 findings are addressed in cbc8f85: Data safety (no silent overwrites)
Concurrency
Onboarding / a11y
Each thread has a specific reply. Validated at runtime: duplicate memo/report creation yields suffixed filenames, |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
Comments suppressed due to low confidence (3)
extensions/catpilot-canvas/ui/app.js:268
- The navigation token protects
stage, butactionsHostremains shared mutable state. If a slow view (for example Tasks) resolves after a newer navigation starts, its post-awaitactionsHost.append(...)writes into the newer view's action stage before the stale token is checked, leaving controls from the wrong view in the top bar. PassactionStageinto each view instead of assigning it globally.
actionsHost = actionStage;
extensions/catpilot-canvas/catpilot-store.mjs:876
- The selected report period is not used when loading tasks or milestones: both paths resolve only the current partition, and all current tasks feed the KPIs. Consequently “last month” and “last week” reports can show the same current totals and omit records from the requested partition, contradicting the per-period reporting behavior. Load/filter records for
since..until, or label these figures explicitly as a current snapshot.
const tasks = parseTasksTable(readFileOrCreate(resolveFilePath("tasks", config)));
const milestones = parseMilestones(readFileOrCreate(resolveFilePath("milestones", config)));
const journal = collectJournal(config);
const { since, until, label } = periodRange(period);
extensions/catpilot-canvas/catpilot-store.mjs:1123
- Migration failures are collected only after earlier items have already been renamed. If a later item fails, the config remains on the old root while successfully moved files exist only under the new root, making that data disappear from CatPilot until a successful retry. This contradicts the claimed non-destructive/consistent migration; preflight and roll back moved items, or use a staged transaction before changing either source or config.
const r = moveOrCopyPath(item.from, item.to, item.isDir, migration);
moved += r.moved;
skipped += r.skipped;
} catch (err) {
errors.push({ type: item.type, from: item.from, to: item.to, error: String(err && err.message || err) });
| const { prompt, timeout } = await readBody(req); | ||
| if (!prompt || !String(prompt).trim()) throw new Error("prompt is required"); | ||
| if (!sessionRef?.sendAndWait) return { ok: false, error: "No active session." }; | ||
| const ev = await sessionRef.sendAndWait(String(prompt), Number(timeout) || 120000); |
Comment on lines
+84
to
+87
| fs.mkdirSync(GLOBAL_CONFIG_DIR, { recursive: true }); | ||
| fs.writeFileSync(GLOBAL_CONFIG_PATH, JSON.stringify(config, null, 2), "utf8"); | ||
| config.__baseDir = configBaseDir(GLOBAL_CONFIG_PATH); | ||
| config.__configPath = GLOBAL_CONFIG_PATH; |
| <!-- Modal host --> | ||
| <div id="modal-root"></div> | ||
| <!-- Toasts --> | ||
| <div id="toasts" class="toasts"></div> |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 8 comments.
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
Comments suppressed due to low confidence (7)
extensions/catpilot-canvas/ui/app.js:542
- Dropping between Overdue and To do is always a no-op: both branches only set
status: "Open", while column membership is derived fromdueDate. The card therefore returns to its original column after reload even though both columns advertise a valid drop target. Either disable those drops or update the due date according to clearly defined rescheduling semantics.
if (col.key === "done") await api(`/api/tasks/${id}/complete`, { method: "POST" });
else await api(`/api/tasks/${id}`, { method: "PUT", body: { status: "Open" } });
extensions/catpilot-canvas/ui/index.html:71
- Async success and error messages are inserted here without a live-region role, so screen-reader users are not notified when saves, deletes, generation, or migration operations finish. Expose the toast host as a polite status region.
<div id="toasts" class="toasts"></div>
extensions/catpilot-canvas/catpilot-store.mjs:87
- Onboarding always writes the global config even when
CATPILOT_CONFIGorCATPILOT_ROOTselected a different missing config path. In that environment setup reports success, but the nextloadConfig()still resolves the missing environment path and the app fails to start. Write to the path returned byresolveConfigPath()so setup and subsequent reads agree.
fs.mkdirSync(GLOBAL_CONFIG_DIR, { recursive: true });
fs.writeFileSync(GLOBAL_CONFIG_PATH, JSON.stringify(config, null, 2), "utf8");
config.__baseDir = configBaseDir(GLOBAL_CONFIG_PATH);
config.__configPath = GLOBAL_CONFIG_PATH;
extensions/catpilot-canvas/catpilot-store.mjs:1131
- A move migration can partially relocate files before a later item fails; this then leaves the old config active while successfully moved data is no longer at the configured paths. Contrary to the comment above, config and data do drift until the user retries the exact migration. Stage/rollback moves or otherwise make the operation transactional before reporting failure.
if (errors.length) {
// Config is left untouched; already-moved files stay where they landed
// but the source→config mapping is unchanged, so a retry is safe.
throw Object.assign(new Error(`Migration failed for ${errors.length} item(s); configuration was not changed.`), {
extensions/catpilot-canvas/catpilot-store.mjs:1142
- All data moves complete before the config is persisted. If this write fails (permissions, disk full, or an I/O error), the function exits with the old config still active while the files have already moved to the new paths. Include config persistence in the migration transaction or roll the moves back on failure.
const configPath = persistConfig(config);
extensions/catpilot-canvas/catpilot-store.mjs:544
- This recursively treats every matching basename under the configured root as CatPilot data instead of restricting matches to valid partition paths. With the suggested Obsidian-vault root, an unrelated
journal.md,tasks.md, ormemos/elsewhere in the vault is included in timelines/reports and may be moved during a root migration. Enumerate only the supported day/week/month partition layouts before collecting or migrating files.
if (e.name === targetName && ((isDir && e.isDirectory()) || (!isDir && e.isFile()))) {
acc.push(full);
} else if (e.isDirectory() && !e.name.startsWith(".")) {
// Skip dotfolders (.trash, .git, .obsidian, …) — they are tooling
// state, not CatPilot partitions.
extensions/catpilot-canvas/catpilot-store.mjs:876
- The selected reporting period is not applied to tasks: this always loads the current partition and computes all task/completion KPIs from it. For example, a “Last month” report generated this month reports this month's tasks as though they were tracked last month. Load the relevant partition(s) and filter by task lifecycle dates, or stop presenting these task metrics as period-specific until the format records those dates.
const tasks = parseTasksTable(readFileOrCreate(resolveFilePath("tasks", config)));
const milestones = parseMilestones(readFileOrCreate(resolveFilePath("milestones", config)));
const journal = collectJournal(config);
const { since, until, label } = periodRange(period);
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
Comments suppressed due to low confidence (11)
extensions/catpilot-canvas/catpilot-store.mjs:87
- Onboarding always writes the global config, but config resolution gives
CATPILOT_CONFIGandCATPILOT_ROOTprecedence. If either variable points to a not-yet-created config, setup reports success while subsequent requests still resolve the missing env-selected path and the app immediately fails as unconfigured. Write to the path returned byresolveConfigPath()so onboarding creates the configuration it will actually load.
fs.mkdirSync(GLOBAL_CONFIG_DIR, { recursive: true });
fs.writeFileSync(GLOBAL_CONFIG_PATH, JSON.stringify(config, null, 2), "utf8");
config.__baseDir = configBaseDir(GLOBAL_CONFIG_PATH);
config.__configPath = GLOBAL_CONFIG_PATH;
extensions/catpilot-canvas/ui/app.js:543
- Dropping between Overdue and To do cannot change columns because both branches only set
status: "Open", while column membership is determined by the unchanged due date. For example, an overdue task dropped on To do immediately renders back under Overdue. Disable those invalid drop targets or update/clear the due date according to an explicit user choice.
const id = e.dataTransfer.getData("text/plain");
try {
if (col.key === "done") await api(`/api/tasks/${id}/complete`, { method: "POST" });
else await api(`/api/tasks/${id}`, { method: "PUT", body: { status: "Open" } });
await refreshSummary(); go("tasks");
extensions/catpilot-canvas/catpilot-store.mjs:1075
- Move mode relies exclusively on
renameSync, which throwsEXDEVwhen the new storage root is on another drive or filesystem—a common reason to change storage roots. The repository already handles this case by falling back to copy-and-remove ineng/materialize-plugins.mjs:27-42; use equivalent cross-device handling here so migration to an external volume works.
function moveOrCopyPath(src, dest, isDir, mode) {
if (!fs.existsSync(src)) return { moved: 0, skipped: 0 };
if (isDir) {
ensureDir(dest);
extensions/catpilot-canvas/ui/index.html:71
- Toast messages are the only feedback for many save, error, and agent operations, but this dynamically updated container is not a live region, so screen-reader users receive no status announcement. Mark it as a polite status region (and consider assertive announcements for errors).
<div id="toasts" class="toasts"></div>
extensions/catpilot-canvas/ui/app.js:694
- Each memo card is the only control that opens its content, but it is a non-focusable
divwith a click handler. Keyboard users cannot reach or activate memo details. Render this as a semantic button/link, or add button semantics, focusability, and Enter/Space handling with a visible focus style.
memos.forEach((m) => grid.append(el("div", { class: "card note-card hoverable", onclick: () => memoDetail(m.filename) },
el("h4", { text: m.title }),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: m.date || "" }), el("span", { class: "muted small", text: "📄 memo" })))));
extensions/catpilot-canvas/ui/app.js:742
- Learning, growth, and project cards are non-focusable
divelements whose details are available only throughonclick, leaving keyboard users unable to open the note body. Use a semantic interactive element or provide button role, tabindex, Enter/Space activation, and visible focus styling.
grid.append(el("div", { class: "card note-card hoverable", onclick: () => domainNoteDetail(domain, n.filename) },
el("h4", { text: fm.title || n.filename }),
domainChips(domain, fm),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: fm.date || "" }))));
extensions/catpilot-canvas/ui/app.js:1003
- Report cards expose the report reader only through a click handler on a non-focusable
div, so keyboard users cannot open generated reports. Make each card a semantic button/link, or add complete keyboard button behavior and a visible focus indicator.
reports.forEach((r) => grid.append(el("div", { class: "card report-card hoverable", onclick: () => reportDetail(r.filename) },
el("div", { class: "report-ic", text: r.format === "html" ? "🌐" : "📄" }),
el("h4", { text: r.title }),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: r.date || "" }), el("span", { class: "tag", text: r.format || "md" })))));
extensions/catpilot-canvas/catpilot-store.mjs:166
- This escaping is not compatible with the linked CatPilot storage reader:
tanure/cat-copilot/lib/cli-utils.jsparses task rows with plainline.split('|'), so a value written asfoo \| baris still split into two cells and shifts the due date, priority, and remaining columns when read by the CLI/MCP surfaces. Either reject/encode these characters in a representation understood by every CatPilot reader or update the shared parser before allowing them here.
function encodeCell(v) {
return String(v == null ? "" : v).replace(/\r?\n/g, "<br>").replace(/\|/g, "\\|");
extensions/catpilot-canvas/ui/app.js:268
- The navigation token does not isolate
actionsHost: everygo()overwrites this global before the awaited view fetch resolves. A stale Tasks/Timeline renderer can therefore append its switcher into a newer navigation'sactionStage, which then passes the token check and displays the wrong controls. Pass the localactionStageinto the view renderer instead of reading a shared mutable host.
actionsHost = actionStage;
extensions/catpilot-canvas/catpilot-store.mjs:1122
- Move mode mutates each source immediately, but failures are only checked after all items and the config is persisted later. If a later item or
persistConfigfails, already-renamed files remain solely under the new path while the active config still points to the old path, contradicting the non-destructive migration guarantee and making those records disappear from CatPilot until manual recovery. Stage/copy all data first and commit the config before deleting sources, or roll back every successful move on any failure.
try {
const r = moveOrCopyPath(item.from, item.to, item.isDir, migration);
moved += r.moved;
skipped += r.skipped;
} catch (err) {
extensions/catpilot-canvas/catpilot-store.mjs:824
listReports()extracts<title>/<h1>for HTML files, butreadReport()only searches for a Markdown heading. Opening an HTML report therefore replaces its displayed title with the filename even though the list showed the document title. Apply the same format-specific title extraction here.
let title = safe;
const h = content.match(/^#\s+(.+)$/m);
if (h) title = h[1].trim();
return { filename: safe, title, content, format: ext === "md" ? "markdown" : "html", date: reportDate(safe) };
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- extensions/catpilot-canvas/package-lock.json: Generated file
Comments suppressed due to low confidence (10)
extensions/catpilot-canvas/extension.mjs:132
sendAndWaitexpects the message options object as its first argument (the other extensions and SDK examples use{ prompt }). Passing the raw string means “Generate with Copilot” does not send the requested prompt correctly. Wrap the prompt and keep the timeout as the second argument.
const ev = await sessionRef.sendAndWait(String(prompt), Number(timeout) || 120000);
extensions/catpilot-canvas/ui/app.js:268
- The navigation token does not fully isolate view actions because
actionsHostremains shared mutable state. If Tasks starts loading, Timeline navigation replaces this variable, and Tasks resolves first, its switcher is appended to Timeline's staging node; the winning view then commits both controls. Pass this navigation'sactionStagedirectly to the renderer (or otherwise bind action writes totoken) instead of using the global host.
actionsHost = actionStage;
extensions/catpilot-canvas/catpilot-store.mjs:87
- Onboarding always writes the global config even when
CATPILOT_CONFIGorCATPILOT_ROOTselected a different missing config path. In that case/api/statuscontinues resolving the nonexistent environment-selected file, so setup reports success butstartApp()immediately fails withNOT_CONFIGURED. Write toresolveConfigPath().path; it already falls back to the global path when no override exists.
fs.mkdirSync(GLOBAL_CONFIG_DIR, { recursive: true });
fs.writeFileSync(GLOBAL_CONFIG_PATH, JSON.stringify(config, null, 2), "utf8");
config.__baseDir = configBaseDir(GLOBAL_CONFIG_PATH);
config.__configPath = GLOBAL_CONFIG_PATH;
extensions/catpilot-canvas/catpilot-store.mjs:1051
- When root and partitioning are changed together, this branch migrates only the current partition and then points the config at the new root. Historical journal/memo/note/report partitions remain under the old root, so timeline and cross-partition summaries can no longer discover them. Either reject combined changes or move every old-root partition while preserving its relative layout before applying the new partitioning mode.
} else {
// Partitioning change (re-bucketing history is ambiguous) or same root:
// operate on the current partition only.
for (const [type, meta] of Object.entries(from.paths)) {
items.push(mkItem(type, meta.isDir, meta.path, to.paths[type].path));
extensions/catpilot-canvas/catpilot-store.mjs:1082
renameSyncfails withEXDEVwhen the new storage root is on another filesystem or Windows drive, a normal use case for this settings wizard. Directory-backed domains therefore cannot be moved and may leave an already-partial migration behind. HandleEXDEVwith copy-then-delete (or use a cross-device move helper) while preserving the no-overwrite behavior.
else fs.renameSync(s, d);
extensions/catpilot-canvas/catpilot-store.mjs:1090
- This file-path branch also uses
renameSync, so moving tasks, journal, or milestones to a root on another filesystem deterministically fails withEXDEV. Use the same copy-then-delete fallback as the directory branch, without overwriting an existing destination.
else fs.renameSync(src, dest);
extensions/catpilot-canvas/ui/app.js:694
- Memo cards are clickable
divelements with no role, tab stop, or keyboard activation, and there is no other control for opening a memo. Keyboard users therefore cannot read memo contents. Render a button/link, or add button semantics plus Enter/Space handling.
memos.forEach((m) => grid.append(el("div", { class: "card note-card hoverable", onclick: () => memoDetail(m.filename) },
el("h4", { text: m.title }),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: m.date || "" }), el("span", { class: "muted small", text: "📄 memo" })))));
extensions/catpilot-canvas/ui/app.js:742
- These learning/growth/project cards are the only way to open note details, but clickable
divelements are not keyboard focusable or activatable. Use a semantic button/link or providerole="button",tabindex="0", and Enter/Space handling.
grid.append(el("div", { class: "card note-card hoverable", onclick: () => domainNoteDetail(domain, n.filename) },
el("h4", { text: fm.title || n.filename }),
domainChips(domain, fm),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: fm.date || "" }))));
extensions/catpilot-canvas/ui/app.js:1003
- Report cards can only be opened with a pointer because the clickable
divhas no keyboard semantics. Make each card a button/link or add a tab stop, button role, and Enter/Space activation so keyboard users can access saved reports.
reports.forEach((r) => grid.append(el("div", { class: "card report-card hoverable", onclick: () => reportDetail(r.filename) },
el("div", { class: "report-ic", text: r.format === "html" ? "🌐" : "📄" }),
el("h4", { text: r.title }),
el("div", { class: "note-foot" }, el("span", { class: "tag", text: r.date || "" }), el("span", { class: "tag", text: r.format || "md" })))));
extensions/catpilot-canvas/ui/index.html:71
- Toast messages are inserted dynamically but this host is not a live region, so screen readers do not announce success or failure feedback. Mark the container as a polite status/live region (errors can be made assertive separately if desired).
<div id="toasts" class="toasts"></div>
Comment on lines
+1012
to
+1015
| const del = el("button", { class: "btn btn-danger", text: "Delete", onclick: async () => { | ||
| try { await api(`/api/reports/${encodeURIComponent(filename)}`, { method: "DELETE" }); toast("Report deleted", "", "ok"); closeModal(); if (state.view === "reports") go("reports"); } | ||
| catch (e) { toast("Error", e.message, "err"); } | ||
| } }); |
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.
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.mainbranch for this pull request.Description
Type of Contribution
Additional Notes
Add CatPilot Canvas extension
This adds a canvas extension at
extensions/catpilot-canvas/.CatPilot Canvas is a visual command center for CatPilot — a beautiful, modern GitHub Copilot canvas that lets you manage everything CatPilot tracks without leaving the Copilot app. It reads and writes the same config-driven storage as the CatPilot CLI, agent, skills, and MCP server, so every surface stays in sync.
What you get
Checklist
extensions/catpilot-canvas/.github/plugin/plugin.jsonwithlogo: "assets/preview.png"andextensions: ".",namematches the folder, nox-awesome-copilot, nocanvas.json.assets/preview.pngprimary screenshot.npm run plugin:validatepasses (✅ extension catpilot-canvas is valid).By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.