Address GHCP review: stash race, signal ordering, a11y, outcome schema, error handling

- extension.mjs: serialize stash read-modify-write behind a per-desk mutex;
  order signals by explicit ISO timestamp (clone-safe) not mtime; keep desks
  with malformed latest signal as 'awaiting'; gate calibration badge on a real
  gap value; add prefers-reduced-motion CSS; toast aria-live region; rename
  open_desk -> get_desk_path (returns path, no session); handle server.listen errors.
- signal-write/SKILL.md: document timestamp + run_id, ordering guidance, and the
  outcome (calibration) signal schema.
- workshop-ta.agent.md: a desk is a persistent workstream picked up by independent
  sessions, not one long-running process.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 26faf13e-639c-4a21-ac05-c0dc2bff7c62
This commit is contained in:
jennyf19
2026-07-19 19:24:09 -07:00
parent 47ad74c5bd
commit 670c5f0419
3 changed files with 112 additions and 18 deletions
+3 -1
View File
@@ -16,7 +16,9 @@ room. When the operator asks "what's everyone working on?" or
## What a workshop is ## What a workshop is
A **workshop** is a named directory containing desks that share a A **workshop** is a named directory containing desks that share a
workspace. Each desk is a long-running Copilot CLI session with: workspace. Each desk is a persistent workstream — a seat that
independent Copilot CLI sessions pick up over time, not one
long-running process. Each desk has:
- **A journal** (`journal.md`) — persistent memory across sessions. - **A journal** (`journal.md`) — persistent memory across sessions.
Every desk reads its own journal at the start and writes to it Every desk reads its own journal at the start and writes to it
+70 -17
View File
@@ -11,6 +11,18 @@ import { joinSession, createCanvas } from "@github/copilot-sdk/extension";
const servers = new Map(); const servers = new Map();
const STASH_TTL_MS = 48 * 60 * 60 * 1000; const STASH_TTL_MS = 48 * 60 * 60 * 1000;
// Serialize stash read-modify-write per workshop. The UI fires stash/restore
// POSTs without awaiting each other, so two overlapping mutations could both
// read the same array and the last write would silently drop the other. Each
// workshop gets a promise chain so its mutations run one at a time.
const stashLocks = new Map();
function withStashLock(workshopDir, fn) {
const prev = stashLocks.get(workshopDir) || Promise.resolve();
const run = prev.then(fn, fn);
stashLocks.set(workshopDir, run.then(() => {}, () => {}));
return run;
}
// Desk names are single path segments (folder names under desks/ or classroom/). // Desk names are single path segments (folder names under desks/ or classroom/).
// Reject anything that could escape the workshop dir via path traversal. // Reject anything that could escape the workshop dir via path traversal.
function isValidDeskName(name) { function isValidDeskName(name) {
@@ -34,6 +46,19 @@ function toCount(v) {
return Math.floor(n); return Math.floor(n);
} }
// Prefer an explicit, persisted timestamp over filesystem mtime. A git
// clone/checkout resets mtimes (often to a single instant), which would
// otherwise scramble "latest" ordering and outcome pairing. Signals may carry
// an ISO-8601 `timestamp` (or `emitted_at`); fall back to mtime when absent.
function signalTime(parsed, mtimeMs) {
const explicit = parsed && (parsed.timestamp || parsed.emitted_at);
if (explicit) {
const t = Date.parse(explicit);
if (Number.isFinite(t)) return t;
}
return mtimeMs;
}
// Reject cross-site POSTs to the state-changing /api/* routes (CSRF). The panel // Reject cross-site POSTs to the state-changing /api/* routes (CSRF). The panel
// loads as a top-level loopback document, so its own fetches are same-origin // loads as a top-level loopback document, so its own fetches are same-origin
// (Origin === our loopback origin) and header-less / non-web-scheme callers fall // (Origin === our loopback origin) and header-less / non-web-scheme callers fall
@@ -70,18 +95,22 @@ async function writeStash(workshopDir, entries) {
} }
async function stashDesk(workshopDir, deskName) { async function stashDesk(workshopDir, deskName) {
const stash = await readStash(workshopDir); return withStashLock(workshopDir, async () => {
if (stash.some(e => e.name === deskName)) return stash; const stash = await readStash(workshopDir);
stash.push({ name: deskName, stashedAt: new Date().toISOString() }); if (stash.some(e => e.name === deskName)) return stash;
await writeStash(workshopDir, stash); stash.push({ name: deskName, stashedAt: new Date().toISOString() });
return stash; await writeStash(workshopDir, stash);
return stash;
});
} }
async function restoreDesk(workshopDir, deskName) { async function restoreDesk(workshopDir, deskName) {
let stash = await readStash(workshopDir); return withStashLock(workshopDir, async () => {
stash = stash.filter(e => e.name !== deskName); let stash = await readStash(workshopDir);
await writeStash(workshopDir, stash); stash = stash.filter(e => e.name !== deskName);
return stash; await writeStash(workshopDir, stash);
return stash;
});
} }
// --- Signal reading --- // --- Signal reading ---
@@ -131,14 +160,27 @@ async function scanSignals(workshopDir) {
const s = await stat(fp); const s = await stat(fp);
const raw = await readFile(fp, "utf-8"); const raw = await readFile(fp, "utf-8");
const parsed = JSON.parse(raw); const parsed = JSON.parse(raw);
allSignals.push({ parsed, mtimeMs: s.mtimeMs, path: fp }); const emittedMs = signalTime(parsed, s.mtimeMs);
allSignals.push({ parsed, mtimeMs: emittedMs, path: fp });
// Latest non-outcome signal (execution, partnership, escalation) // Latest non-outcome signal (execution, partnership, escalation)
if ((parsed.signal_type || "execution") !== "outcome" && s.mtimeMs > latestTime) { if ((parsed.signal_type || "execution") !== "outcome" && emittedMs > latestTime) {
latestTime = s.mtimeMs; latest = { parsed, mtimeMs: s.mtimeMs }; latestTime = emittedMs; latest = { parsed, mtimeMs: emittedMs };
} }
} catch {} } catch {}
} }
if (!latest) continue; if (!latest) {
// Files exist but none parsed into a usable non-outcome signal
// (malformed JSON, or outcome-only). Keep the desk visible as
// "awaiting" instead of silently dropping it from the board.
results.push({
deskName: entry.name, signalType: "none", agentName: entry.name,
confidence: 0, accuracy: 0, completeness: 0, intent: 0,
whatWorked: "", whatWasHard: "", skillGap: "",
escalationReason: null, escalationBlocked: null, recommendation: null,
emittedAt: null, signalCount: 0, tokensIn: 0, tokensOut: 0, model: null,
});
continue;
}
try { try {
const sig = latest.parsed; const sig = latest.parsed;
const intentRaw = sig.intent || sig.self_assessment?.intent || null; const intentRaw = sig.intent || sig.self_assessment?.intent || null;
@@ -303,7 +345,7 @@ function renderSummaryBar(activeSignals) {
? `<span style="font-size:11px;color:#475569;">🪙 ${formatTokens(totalTokens)}</span>` ? `<span style="font-size:11px;color:#475569;">🪙 ${formatTokens(totalTokens)}</span>`
: ""; : "";
const calibrationBadge = withOutcomes > 0 const calibrationBadge = avgGap !== null
? `<span style="font-size:11px;color:${avgGap <= 1 ? '#22c55e' : avgGap <= 2 ? '#eab308' : '#ef4444'};" title="${withOutcomes} outcome signal${withOutcomes > 1 ? 's' : ''}, avg gap: ${avgGap}">🔍 gap ${avgGap}</span>` ? `<span style="font-size:11px;color:${avgGap <= 1 ? '#22c55e' : avgGap <= 2 ? '#eab308' : '#ef4444'};" title="${withOutcomes} outcome signal${withOutcomes > 1 ? 's' : ''}, avg gap: ${avgGap}">🔍 gap ${avgGap}</span>`
: ""; : "";
@@ -544,6 +586,9 @@ function renderDashboard(signals, stashed) {
50% { border-color: #7f1d1d; } 50% { border-color: #7f1d1d; }
} }
#content { transition: opacity .15s; } #content { transition: opacity .15s; }
@media (prefers-reduced-motion: reduce) {
* { animation: none !important; transition: none !important; }
}
</style> </style>
</head> </head>
<body> <body>
@@ -568,6 +613,8 @@ function renderDashboard(signals, stashed) {
} }
function showToast(title, detail) { function showToast(title, detail) {
const toast = document.createElement('div'); const toast = document.createElement('div');
toast.setAttribute('role', 'status');
toast.setAttribute('aria-live', 'polite');
toast.style.cssText = 'position:fixed;bottom:20px;left:50%;transform:translateX(-50%);' + toast.style.cssText = 'position:fixed;bottom:20px;left:50%;transform:translateX(-50%);' +
'background:#1e3a5f;color:#7dd3fc;padding:10px 20px;border-radius:8px;font-size:13px;' + 'background:#1e3a5f;color:#7dd3fc;padding:10px 20px;border-radius:8px;font-size:13px;' +
'border:1px solid #3b82f6;z-index:999;max-width:90%;text-align:center;'; 'border:1px solid #3b82f6;z-index:999;max-width:90%;text-align:center;';
@@ -722,7 +769,13 @@ async function startServer(instanceId, workshopDir) {
} }
} }
}); });
await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); await new Promise((resolve, reject) => {
const onError = (err) => { server.removeListener("listening", onListening); reject(err); };
const onListening = () => { server.removeListener("error", onError); resolve(); };
server.once("error", onError);
server.once("listening", onListening);
server.listen(0, "127.0.0.1");
});
const address = server.address(); const address = server.address();
const port = typeof address === "object" && address ? address.port : 0; const port = typeof address === "object" && address ? address.port : 0;
return { server, url: `http://127.0.0.1:${port}/` }; return { server, url: `http://127.0.0.1:${port}/` };
@@ -788,8 +841,8 @@ const session = await joinSession({
}, },
}, },
{ {
name: "open_desk", name: "get_desk_path",
description: "Open a desk as a new session in the GHCP app. Returns the desk path so the agent can create_session or navigate to it.", description: "Resolve a desk name to its filesystem path. Does not open a session — returns the path so the caller can create_session or navigate to it.",
inputSchema: { inputSchema: {
type: "object", type: "object",
properties: { deskName: { type: "string", description: "Name of the desk to open" } }, properties: { deskName: { type: "string", description: "Name of the desk to open" } },
+39
View File
@@ -52,6 +52,8 @@ Create `desks/<desk-name>/.signals/<timestamp>.json`:
{ {
"signal_type": "execution", "signal_type": "execution",
"subtype": "checkpoint", "subtype": "checkpoint",
"timestamp": "2026-07-19T21:30:00Z",
"run_id": "<optional; set to pair this with an outcome signal>",
"agent_name": "<desk-name>", "agent_name": "<desk-name>",
"self_assessment": { "self_assessment": {
"intent": 4, "intent": 4,
@@ -91,6 +93,11 @@ dashboard consumers. `signal_type` controls sort priority
> consuming signals in your own tooling, prefer `subtype` for the > consuming signals in your own tooling, prefer `subtype` for the
> specific state. > specific state.
> **Ordering:** include a `timestamp` (ISO 8601 UTC). The dashboard
> orders signals by it and falls back to file mtime only when it's
> absent — a git clone/checkout resets mtimes, so mtime alone is not a
> dependable clock.
### 2. Note the signal in the journal ### 2. Note the signal in the journal
Also append a short marker to the desk's journal for persistence: Also append a short marker to the desk's journal for persistence:
@@ -103,6 +110,38 @@ Also append a short marker to the desk's journal for persistence:
The journal note is the trail marker. The JSON file is the The journal note is the trail marker. The JSON file is the
machine-readable signal. machine-readable signal.
## Outcome signals (calibration)
The signals-dashboard can pair a desk's self-assessment with an
*outcome* — an independent rating of the realized result — and show
the **honesty gap** (how far the desk's confidence was from the
delivered quality). Outcome signals are optional and are usually
emitted by a reviewer/evaluator, not the desk itself.
Write them to the **same** `.signals/` directory:
```json
{
"signal_type": "outcome",
"run_id": "<same run_id as the signal it rates>",
"agent_name": "<reviewer name>",
"quality_rating": 4,
"effort_to_merge": "minimal",
"issues_found": ["optional short strings"],
"timestamp": "2026-07-19T22:00:00Z"
}
```
- **`run_id`** correlates an outcome with the execution/partnership
signal it rates — set the same `run_id` on both. If it's absent, the
dashboard falls back to the nearest outcome emitted shortly after the
latest signal.
- **`quality_rating`** (05) is the realized quality; the dashboard
compares it to the desk's self-assessed `confidence` to compute the
honesty gap.
- **`effort_to_merge`** — `"minimal"`, `"moderate"`, or `"significant"`.
- **`issues_found`** — optional array of short strings.
## Principles ## Principles
- Signals are structured, not chatty. Short, factual, actionable. - Signals are structured, not chatty. Short, factual, actionable.