Skip to content

Commit d463cda

Browse files
authored
H4b: trust issues before runner actions (#139)
* H4b: guard issue trust before runner actions * Honor trust changes across active runner flows * Revalidate trust after prompt reads * Close trust handoff races * Record prompt evidence at launch * Preserve trust retry identity * Track prompt evidence delivery * Bound runner trust status reads * Revalidate trust at publish boundaries * Order issue trust response merges * Replay saved runner actions during shutdown * Preserve refreshed issue trust state * Revalidate issue access after pagination * Bound issue reads and preserve trust errors
1 parent 6dd57c8 commit d463cda

38 files changed

Lines changed: 1990 additions & 171 deletions

‎docs/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ Dependencies point downward only. `core/` imports nothing that does input or out
168168
| Review service | `runner/review.ts` | Builds the review view: reads history, links it to the plan, computes approval states and merge blockers. Applies review commands through the store. | In use |
169169
| Ask | `runner/questions.ts`, `runner/question-*.ts` | Answers a question about one plan item with a read-only Claude agent (Codex is refused in every phase, #93). Runs lane D's setup in a worker thread so the server stays responsive. Ask and planning share one read-only container runner (`runReadOnlyAgent`, #117) but keep separate workers, owners and leftovers. Labels its Docker objects with a per-database Ask owner and recovers only its own leftovers, so reviews sharing one Docker daemon do not block each other (#95). | In use |
170170
| Merge coordinator | `runner/merge.ts` | Runs the full merge gate twice, re-reads the local generation, then merges the exact reviewed head. Tracks merge-queue attempts. For a runner task it inspects and merges the task's own published pull request; a configured `github.pullRequest` that differs is refused (#121). | In use |
171-
| Planning agent | `runner/planning.ts`, `web/planning.ts`, `runner/planning-provider.ts` | Runs plan suggestions and drafts with Claude in lane D's planning phase, on a read-only copy of the current head, in its own worker with its own leftovers ledger and owner token. The request and its timer share one 10-minute budget. Issue text is read with collaborators' comments only. | In use for a non-demo review with a `github` block |
171+
| Planning agent | `runner/planning.ts`, `web/planning.ts`, `runner/planning-provider.ts` | Runs plan suggestions and drafts with Claude in lane D's planning phase, on a read-only copy of the current head, in its own worker with its own leftovers ledger and owner token. The request and its timer share one 10-minute budget. Issue text includes current collaborators' comments, or every comment under explicit author-bound trust. | In use for a non-demo review with a `github` block |
172172
| Runner coordinator | `runner/coordinator.ts`, `runner/lifecycle.ts` | Attempt admission, concurrency slots, compare-and-swap on results, retries, shutdown order. A stop made inside a user action takes effect only after that action's transaction commits (`Store.afterCommit`, #96). | In use with the opt-in `runner` block (#91) |
173173
| Execution | `runner/execution.ts` | Runs plan items in order: fresh workspace, prompt, agent, post-run audit, then the runner's own commit and ledger entry. Before launch, D's tree check (`checkTaskTree`) must pass. Start and resume require current approvals, own the review version across the run, recheck approvals before later items, and verify that a completed prefix still ends at the current head (#107). Pauses in needs amendment on an out-of-scope edit. A safety violation is saved before its terminal write and remains durably owed when a human gate delays escalation; a failed run is audited too. | In use with the opt-in `runner` block; started by `start` and `resume` on `/api/runner` (#91, #107, PRs #105 and #130) |
174174
| Workspace | `runner/workspace.ts`, `runner/runner-repository.ts` | The real lane D workspace. A runner-owned bare repository (owner-only directories) fetches base commits by ID and takes in each verified commit bundle under a per-attempt ref. Review and Ask read a task with runner commits from there, at the head the Store recorded. | In use with the opt-in `runner` block (#91) |

‎docs/implementation/issue-prioritization.md‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,7 @@ The status line says one of:
108108
- "✕ Unavailable": no list has been retrieved yet, with the error;
109109
- "– Not configured": the review configuration has no `github.repository`.
110110

111-
A note says that trusting issues and queueing are not available yet. Demo mode
112-
shows fixture issues and never contacts GitHub.
111+
Demo mode shows fixture issues and never contacts GitHub.
113112

114113
**State holders.**
115114

@@ -130,3 +129,42 @@ unconfigured state. `test/browser/issues.spec.ts` covers ranked order, reasons,
130129
trust marks, `aria-current` navigation, review input kept across navigation,
131130
review shortcuts ignored on the Issues screen, the unavailable → current → stale
132131
sequence, a late refresh after leaving the screen, and the 1280px layout.
132+
133+
## H4b: Trust this issue
134+
135+
**Decision and scope.** A trust decision is bound to the lower-cased repository identity, issue number and the issue's
136+
current author login. It applies to planning and execute prompts. Revoking trust does not interrupt an invocation that
137+
already started; every later plan-item admission, planning read and publish evaluates the new decision. Publishing is
138+
guarded too, so a revoked decision cannot cross the next irreversible boundary.
139+
140+
**Durable state.** Schema v17 adds one `issue_trust` row per repository and issue, retaining who decided, when, the
141+
author that was observed, and a revocation time. Changing or deleting the GitHub author makes the row inapplicable.
142+
Trust and untrust requests carry UUID v4 action IDs and use the ordinary durable action replay before GitHub is read.
143+
Definite GitHub read failures are saved too and replay with their upstream-failure classification; shutdown remains
144+
resendable. Concurrent callers with one action ID all observe the first durable outcome.
145+
Each execute attempt also stores the SHA-256 digest and count of the exact comment strings put in its prompt. The
146+
evidence is durably `prepared` after every pre-launch check and before the launcher receives the prompt, then becomes
147+
`delivered` only after the launcher returns an owned handle; recovery can therefore distinguish either crash window.
148+
149+
**Admission.** Start, resume, continuation approval and every plan item fetch the issue author and the complete current
150+
collaborator list under a bounded GitHub read. A collaborator-authored issue passes without a local decision. Every
151+
other issue needs a live, unrevoked row for that exact repository, number and author. A prompt text read revalidates the
152+
admitted author and collaborator result against the issue and collaborator snapshot used for that text, closing the gap
153+
between authorization and prompt construction. When explicit trust widened the comments, its author-bound Store row is
154+
re-read immediately after the awaited text fetch so revocation cannot admit the stale all-comments result. The returned
155+
execute source carries the same synchronous guard into `prepareExecution`, and the planning description carries it
156+
into the recorded user action; each runs in the same turn immediately before its prompt is constructed. Malformed,
157+
partial, failed and over-limit reads fail closed. Action-time
158+
failures are saved under the action ID; per-item failures settle that attempt without admitting the next item. The task
159+
and review versions are still checked in the same transaction that admits an attempt, after the external read.
160+
161+
**Comments.** Without matching explicit trust, only current collaborators' comments enter planning and execute prompts.
162+
With matching trust, every bounded comment enters the same untrusted-data block, including comments from deleted
163+
accounts. The issue author is re-read with the comments, so a decision for an earlier author cannot widen the prompt.
164+
165+
**Screen and demo.** The Issues table offers `Trust this issue` for outside authors, `Trust all comments` for current
166+
collaborators, and `Remove trust` for either explicit decision. The collaborator's author-bound decision widens comment
167+
access without changing its already-eligible status. While a request is in flight, the focused control remains
168+
enabled for focus purposes, uses `aria-disabled`, and ignores repeat activation. Responses merge only their own row;
169+
overlapping refreshes and trust requests cannot overwrite a committed decision or reset another control. Demo fixtures
170+
use the same Store and API, but resolve author and collaborator state locally and never contact GitHub.

‎github/issues.ts‎

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,24 @@ export const ISSUE_PAGE_MAX_BYTES = 64 * 1024 * 1024;
1515
* to 0.75 s later: 12.75 s, below the 15-second serving request budget.
1616
*/
1717
export const ISSUE_KILL_GRACE_MS = 500, ISSUE_PIPE_GRACE_MS = 250;
18+
/** One sequential access-and-text operation, including the subprocess settlement tail after its active-work abort. */
19+
export const ISSUE_READ_TIMEOUT_MS = 30_000;
20+
export const ISSUE_READ_ACTIVE_MS = ISSUE_READ_TIMEOUT_MS - ISSUE_KILL_GRACE_MS - ISSUE_PIPE_GRACE_MS;
21+
22+
/** Shares one active-work deadline across every sequential stage and awaits the caller's work through settlement. */
23+
export async function withIssueReadDeadline<T>(signal: AbortSignal,
24+
read: (sharedSignal: AbortSignal, activeTimeoutMs: number) => Promise<T>): Promise<T> {
25+
signal.throwIfAborted();
26+
const deadline = new AbortController();
27+
const shared = AbortSignal.any([signal, deadline.signal]);
28+
const timer = setTimeout(() => deadline.abort(new Error('Issue retrieval timed out.')), ISSUE_READ_ACTIVE_MS);
29+
try {
30+
const value = await read(shared, ISSUE_READ_ACTIVE_MS);
31+
shared.throwIfAborted();
32+
return value;
33+
}
34+
finally { clearTimeout(timer); }
35+
}
1836

1937
export type IssueAuthorAssociation =
2038
| 'OWNER' | 'MEMBER' | 'COLLABORATOR' | 'CONTRIBUTOR'
@@ -44,12 +62,21 @@ export interface IssueSnapshot {
4462

4563
/** The issue text an execute prompt carries, as untrusted data. */
4664
export interface IssueText { readonly number: number; readonly title: string; readonly body: string; readonly comments: readonly string[] }
65+
export interface IssueAccess { readonly number: number; readonly authorLogin: string | null; readonly collaborator: boolean }
4766

4867
export interface IssueGateway {
4968
readonly repository: string;
5069
fetch(options?: { signal?: AbortSignal; timeoutMs?: number }): Promise<IssueSnapshot>;
5170
}
5271

72+
/** The extra current-issue reads used by trust actions and runner admission. */
73+
export interface IssueTrustGateway extends IssueGateway {
74+
issueAccess(number: number, options?: { signal?: AbortSignal; timeoutMs?: number }): Promise<IssueAccess>;
75+
issueText(number: number, options?: { signal?: AbortSignal; timeoutMs?: number; trustedAuthor?: string | null;
76+
/** Revalidate the admission read against the issue and collaborator snapshot used for this text read. */
77+
expectedAccess?: IssueAccess }): Promise<IssueText>;
78+
}
79+
5380
type RunGh = (args: readonly string[], options?: { signal?: AbortSignal }) => Promise<string>;
5481

5582
const associations = new Set<IssueAuthorAssociation>([
@@ -233,12 +260,39 @@ export class GhIssueGateway implements IssueGateway {
233260
}));
234261
}
235262

263+
async #loadIssueAuthor(number: number, signal: AbortSignal): Promise<string | null> {
264+
let decoded: unknown;
265+
const output = await this.run(['api', '--method', 'GET', '-H', 'Accept: application/vnd.github+json', `repos/${this.repository}/issues/${number}`], { signal });
266+
try { decoded = JSON.parse(output); }
267+
catch { throw new Error('GitHub returned invalid issue JSON.'); }
268+
const issue = object(decoded, 'GitHub returned a malformed issue.');
269+
if (issue.number !== number) throw new Error('GitHub returned a different issue.');
270+
if (Object.hasOwn(issue, 'pull_request')) throw new Error(`#${number} is a pull request, not an issue.`);
271+
return issue.user === null ? null : login(object(issue.user, 'GitHub returned an invalid issue author.').login, 'issue author');
272+
}
273+
274+
async #loadIssueAccess(number: number, signal: AbortSignal): Promise<{ access: IssueAccess; collaborators: ReadonlySet<string> }> {
275+
const authorLogin = await this.#loadIssueAuthor(number, signal);
276+
const collaborators = await this.#loadCollaborators(signal);
277+
const currentAuthor = await this.#loadIssueAuthor(number, signal);
278+
if (currentAuthor !== authorLogin) throw new Error(`Issue #${number}'s author changed during access verification.`);
279+
return { access: { number, authorLogin: currentAuthor,
280+
collaborator: currentAuthor !== null && collaborators.has(currentAuthor.toLocaleLowerCase('en-US')) }, collaborators };
281+
}
282+
283+
async issueAccess(number: number, options: { signal?: AbortSignal; timeoutMs?: number } = {}): Promise<IssueAccess> {
284+
if (!Number.isSafeInteger(number) || number < 1) throw new Error('Invalid issue number.');
285+
return this.#bounded(options, async signal => (await this.#loadIssueAccess(number, signal)).access);
286+
}
287+
236288
/**
237-
* One issue's text for an execute prompt (#91): its title, body and the comments of repository collaborators only
238-
* (design, "Which comments reach the agent"), oldest first. Everything stays untrusted data inside the prompt.
289+
* One issue's text for an execute prompt (#91): its title, body and the comments permitted by current collaborator
290+
* access or explicit author-bound trust (design, "Which comments reach the agent"), oldest first.
291+
* Everything stays untrusted data inside the prompt.
239292
* Fails closed on anything malformed, on a pull request, and past the comment page limit.
240293
*/
241-
async issueText(number: number, options: { signal?: AbortSignal; timeoutMs?: number } = {}): Promise<IssueText> {
294+
async issueText(number: number, options: { signal?: AbortSignal; timeoutMs?: number; trustedAuthor?: string | null;
295+
expectedAccess?: IssueAccess } = {}): Promise<IssueText> {
242296
if (!Number.isSafeInteger(number) || number < 1) throw new Error('Invalid issue number.');
243297
return this.#bounded(options, async signal => {
244298
let decoded: unknown;
@@ -249,19 +303,28 @@ export class GhIssueGateway implements IssueGateway {
249303
if (issue.number !== number) throw new Error('GitHub returned a different issue.');
250304
if (Object.hasOwn(issue, 'pull_request')) throw new Error(`#${number} is a pull request, not an issue.`);
251305
const title = boundedString(issue.title, 'title', 4096), body = boundedString(issue.body, 'body', MAX_BODY_LENGTH, true);
306+
const authorLogin = issue.user === null ? null : login(object(issue.user, 'GitHub returned an invalid issue author.').login, 'issue author');
307+
const includeEveryComment = options.trustedAuthor !== undefined && options.trustedAuthor === authorLogin;
252308
// The execute prompt carries the issue as one JSON data block of at most MAX_PROMPT_BYTES (dataJSON). A running byte
253-
// count stops reading early (a title and body already over it read no collaborator or comment page); the exact check
309+
// count stops reading early (a title and body already over it read no included comment page); the exact check
254310
// below uses the prompt's own serializer, so an issue accepted here is one the prompt can carry.
255-
const tooLong = () => new Error(`Issue #${number}'s title, body and collaborator comments are larger than the ${MAX_PROMPT_BYTES / 1024} KiB an execute prompt carries; codeboost does not cut an issue to fit.`);
311+
const tooLong = () => new Error(`Issue #${number}'s title, body and included comments are larger than the ${MAX_PROMPT_BYTES / 1024} KiB an execute prompt carries; codeboost does not cut an issue to fit.`);
256312
// Only the size refusal is reworded; any other (text with a NUL, for one) keeps its own reason.
257313
const carried = (text: IssueText) => {
258314
try { dataJSON(text, 'Issue data'); } catch (error) { throw /exceeds/.test((error as Error).message) ? tooLong() : error; }
259315
};
260316
// The title and body alone, as the prompt serializes them (escaping included): an issue that cannot fit reads nothing more.
261317
carried({ number, title, body, comments: [] });
262318
let total = Buffer.byteLength(title) + Buffer.byteLength(body);
263-
const collaborators = await this.#loadCollaborators(signal);
264-
const comments: string[] = [];
319+
const collaborators = includeEveryComment && !options.expectedAccess ? null : await this.#loadCollaborators(signal);
320+
if (options.expectedAccess) {
321+
const collaborator = authorLogin !== null && collaborators!.has(authorLogin.toLocaleLowerCase('en-US'));
322+
const currentAuthor = await this.#loadIssueAuthor(number, signal);
323+
if (currentAuthor !== authorLogin || options.expectedAccess.number !== number || options.expectedAccess.authorLogin !== currentAuthor
324+
|| options.expectedAccess.collaborator !== collaborator)
325+
throw new Error(`Issue #${number}'s author or collaborator access changed during admission.`);
326+
}
327+
const comments: string[] = [], includedCommentAuthors = new Set<string>();
265328
for (let page = 1; ; page++) {
266329
const listed = await this.run(['api', '--method', 'GET', '-H', 'Accept: application/vnd.github+json',
267330
`repos/${this.repository}/issues/${number}/comments`, '-f', `per_page=${PAGE_SIZE}`, '-f', `page=${page}`], { signal });
@@ -275,9 +338,13 @@ export class GhIssueGateway implements IssueGateway {
275338
const comment = object(value, 'GitHub returned a malformed comment.');
276339
// A deleted ("ghost") author is nobody's collaborator. Other people's comments are dropped before their body is
277340
// checked, so none of theirs can make the issue unreadable.
278-
if (comment.user === null) continue;
279-
const author = login(object(comment.user, 'GitHub returned an invalid comment author.').login, 'issue author');
280-
if (!collaborators.has(author.toLocaleLowerCase('en-US'))) continue;
341+
if (!includeEveryComment) {
342+
if (comment.user === null) continue;
343+
const author = login(object(comment.user, 'GitHub returned an invalid comment author.').login, 'issue author');
344+
const authorKey = author.toLocaleLowerCase('en-US');
345+
if (!collaborators!.has(authorKey)) continue;
346+
includedCommentAuthors.add(authorKey);
347+
}
281348
const text = boundedString(comment.body, 'comment', MAX_BODY_LENGTH, true);
282349
total += Buffer.byteLength(text);
283350
if (total > MAX_PROMPT_BYTES) throw tooLong();
@@ -287,6 +354,13 @@ export class GhIssueGateway implements IssueGateway {
287354
}
288355
const text = { number, title, body, comments };
289356
carried(text);
357+
if (options.expectedAccess) {
358+
const current = await this.#loadIssueAccess(number, signal);
359+
if (current.access.number !== options.expectedAccess.number || current.access.authorLogin !== options.expectedAccess.authorLogin
360+
|| current.access.collaborator !== options.expectedAccess.collaborator
361+
|| [...includedCommentAuthors].some(author => !current.collaborators.has(author)))
362+
throw new Error(`Issue #${number}'s author or collaborator access changed during admission.`);
363+
}
290364
return text;
291365
});
292366
}

0 commit comments

Comments
 (0)