Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 17 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -800,33 +800,29 @@ and therefore cannot be performed as the app today. Under `"app"` they refuse
rather than writing as the operator, so an explicit app identity never produces
a human-attributed write:

| Write | Refuses under `"app"` | Missing connected capability |
| Write | Built-in CLI identity gate | Missing connected capability |
|---|---|---|
| Guarded squash merge (`mergePolicy: "on-green-with-review"`) | the merge is declined and logged; nothing is merged | `mergePullRequest` |
| Notion intake issue create | the run is blocked with the reason, before any durable claim is taken | `createIssue` |
| Notion intake issue edit | the run is blocked with the reason, before any durable claim is taken | `updateIssue` |
| Guarded squash merge (`mergePolicy: "on-green-with-review"`) | `"app"` declines and logs; nothing is merged | `mergePullRequest` |
| Notion intake issue create | only exact `"user"`; `"auto"` and `"app"` block before any durable claim | `createIssue` |
| Notion intake issue edit | only exact `"user"`; `"auto"` and `"app"` block before any CLI access | `updateIssue` |

Both refusals name the missing capability and the recovery path: set
`github.identity` to `"user"` or `"auto"` to deliberately accept local-user
attribution for that operation. Under `"auto"` and `"user"` both paths behave
exactly as they always have. Neither refusal is reachable in the default cloud
deployment, which runs `mergePolicy: "never"` and does not run Notion intake.
The merge refusal names `"user"` or `"auto"` as its local-user recovery path.
The built-in Notion publisher is stricter: its local-host opt-in is exact
`github.identity: "user"`, because the production Factory container contains
no `gh` binary and the App surface has no issue-create or source-marker query.

Notion intake is a separate surface from the Factory lifecycle writeback and
still requires local `gh` authentication when enabled. Its CLI entry point
resolves `github.identity` from the selected contract — including a split
`workspaceConfig`/`nodeConfig` contract, where the node half wins — so `"app"`
refuses while `"user"` and `"auto"` proceed. An absent contract resolves to
`"auto"`, matching the schema's own synthesis of an unset `github` block; a
contract that exists but cannot be parsed is an error rather than a silent
downgrade to the permissive value.

Only the mutations refuse. Reconciliation of an already-dispatched task
performs no GitHub write, so it continues to work under `"app"`: the refusal is
raised immediately before the issue create or the issue edit, and in the create
path before the durable delivery claim is taken, so a refused run never
consumes the exactly-once claim and can be retried under a permitted
identity.
`workspaceConfig`/`nodeConfig` contract, where the node half wins. Only exact
`"user"` enables the built-in CLI publisher. An absent contract resolves to
`"auto"` and therefore blocks repository intake explicitly; a contract that
exists but cannot be parsed is an error rather than a silent downgrade.

The gate covers the built-in publisher's visibility, label, reconciliation,
create, and edit operations. `"auto"` and `"app"` fail before spawning `gh` and
before reserving a durable claim. Project-only intake and injected custom
publishers do not use this adapter and remain available.

The standalone babysitter reads complete PR metadata from the authenticated
mounted projection and reports `source: 'mount'`; it does not fall back to
Expand Down
76 changes: 69 additions & 7 deletions src/cli/fleet.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, expect, it, vi } from 'vitest'
import type { CloudSession } from '@agent-relay/cloud'
import { existsSync } from 'node:fs'
import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'
import { chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'

Expand Down Expand Up @@ -855,6 +855,68 @@ describe('fleet CLI runtime', () => {
}
})

it('blocks default-auto Notion repository intake before invoking local gh or reserving a claim', async () => {
const root = await mkdtemp(join(tmpdir(), 'factory-cli-notion-no-gh-'))
try {
const mountedPage = join(root, 'notion', 'pages', '3b36800c-1c90-801d-b1cf-c8f2e1cff7cf')
await mkdir(mountedPage, { recursive: true })
await writeFile(join(mountedPage, 'content.md'), [
'# Chief Spec',
'Status: ready',
'Title: Verify explicit local identity',
'Summary: Default auto must not reach the CLI adapter.',
'Recipe: single',
'Repos: AgentWorkforce/cloud',
].join('\n'))
const manifestPath = join(root, 'notion.json')
await writeFile(manifestPath, JSON.stringify({
version: 1,
mountRoot: './notion',
statePath: './state.json',
tasks: [{ page: '3b36800c1c90801db1cfc8f2e1cff7cf' }],
}))
const ghLogPath = join(root, 'gh.log')
const ghPath = join(root, 'gh')
await writeFile(ghPath, [
'#!/bin/sh',
'printf \'%s\\n\' "$*" >> "$FACTORY_TEST_GH_LOG"',
'if [ "$1 $2" = "issue list" ]; then printf \'[]\'; fi',
'if [ "$1 $2" = "repo view" ]; then printf \'private\'; fi',
].join('\n'))
await chmod(ghPath, 0o755)
vi.stubEnv('PATH', root)
vi.stubEnv('FACTORY_TEST_GH_LOG', ghLogPath)
const notionClaims = {
get: vi.fn(async () => undefined),
findBySourcePrefix: vi.fn(async () => []),
claim: vi.fn(async () => { throw new Error('claim must not be reserved') }),
dispose: vi.fn(async () => undefined),
}
const output = buffer()

const code = await runFleetCli(['intake', 'notion', manifestPath], {
fleet: new FakeFleetClient(),
notionClaims,
stdout: output,
stderr: buffer(),
})

expect(code).toBe(1)
expect(JSON.parse(output.text())).toMatchObject({
ok: false,
results: [{
status: 'blocked',
reason: expect.stringMatching(/GitHub identity "auto".*Notion intake.*local gh.*identity.*"user"/iu),
}],
})
await expect(readFile(ghLogPath, 'utf8')).rejects.toMatchObject({ code: 'ENOENT' })
expect(notionClaims.claim).not.toHaveBeenCalled()
} finally {
vi.unstubAllEnvs()
await rm(root, { recursive: true, force: true })
}
})

it('honours github.identity "app" for Notion intake instead of writing as the local gh user', async () => {
// The gate in GhCliIssuePublisher is worthless if this call site hardcodes
// an identity: this is the only production caller of runNotionIntake, so
Expand Down Expand Up @@ -903,16 +965,16 @@ describe('fleet CLI runtime', () => {
const resolved: string[] = []
const notionGithub = (identity: string) => {
resolved.push(identity)
const publisher = new GhCliIssuePublisher(
identity as 'app' | 'user' | 'auto',
async () => { throw new Error('gh must not be invoked in this test') },
)
return Object.assign(publisher, {
return {
assertWritable: () => {
if (identity === 'app') throw new Error('GitHub identity "app" refuses injected publisher writes')
},
repositoryVisibility: async () => 'private' as const,
missingLabels: async () => [],
findBySource: async () => undefined,
createIssue: async () => ({ number: 42, url: 'https://github.test/issues/42' }),
})
updateIssue: async () => undefined,
}
}

const code = await runFleetCli(
Expand Down
33 changes: 18 additions & 15 deletions src/github/gh-identity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import { FactoryConfigSchema } from '../config/schema'
* `identity: "app"` they still attribute the write to whichever human is
* logged in locally.
*
* Each case below is a must-fire / must-not-fire pair: `app` must refuse
* WITHOUT spawning `gh`, and `auto`/`user` must behave exactly as they do
* today. A test that only asserted the refusal would pass against a change
* that broke every local run.
* Each case below is a must-fire / must-not-fire pair. Merge keeps its
* historical auto/user policy, while the Notion CLI adapter is a stricter
* explicit-user local-host capability because the production container has
* no `gh` binary. A test that only asserted refusal would pass against a
* change that broke every local run.
*/

const mergeInput = { repo: 'AgentWorkforce/example', number: 7, expectedHeadSha: 'a'.repeat(40) }
Expand Down Expand Up @@ -172,20 +173,22 @@ describe('local gh mutations under github.identity', () => {
expect(calls).toEqual([])
})

it('MUST NOT FIRE: assertWritable permits user and auto', () => {
for (const identity of ['user', 'auto'] as const) {
const { gh } = fakeGh()
expect(() => new GhCliIssuePublisher(identity, gh).assertWritable()).not.toThrow()
}
it('requires exact user identity before the Notion gh adapter can write', () => {
const user = fakeGh()
expect(() => new GhCliIssuePublisher('user', user.gh).assertWritable()).not.toThrow()

const automatic = fakeGh()
expect(() => new GhCliIssuePublisher('auto', automatic.gh).assertWritable())
.toThrow(/GitHub identity "auto".*Notion intake.*local gh.*identity.*"user"/iu)
expect(automatic.calls).toEqual([])
})

it('MUST NOT FIRE: identity "app" leaves Notion intake READS working', async () => {
// Reads carry no authorship, so gating them would break intake without
// removing any attribution.
it.each(['app', 'auto'] as const)('blocks Notion intake reads under %s before spawning gh', async (identity) => {
const { gh, calls } = fakeGh()
const publisher = new GhCliIssuePublisher('app', gh)
const publisher = new GhCliIssuePublisher(identity, gh)

await publisher.missingLabels('AgentWorkforce/example', ['factory'])
expect(calls.map((args) => args[0])).toEqual(['api'])
await expect(publisher.missingLabels('AgentWorkforce/example', ['factory']))
.rejects.toThrow(new RegExp(`GitHub identity "${identity}".*Notion intake.*local gh`, 'iu'))
expect(calls).toEqual([])
})
})
6 changes: 4 additions & 2 deletions src/github/gh-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ export type GithubWriteIdentity = 'app' | 'user' | 'auto'
/**
* Whether the configured identity permits mutating GitHub through local `gh`.
*
* Reads are always permitted: `gh pr view` leaks no authorship, so read
* provenance is not an identity concern (see `StandalonePullRequest.source`).
* This helper governs mutations only. Read provenance is not an authorship
* concern, although a capability-specific adapter may still reject reads when
* its runtime has no `gh` binary (for example Notion intake in the cloud
* container).
*/
export function localGhMutationAllowed(identity: GithubWriteIdentity): boolean {
return identity !== 'app'
Expand Down
38 changes: 21 additions & 17 deletions src/intake/notion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import lockfile from 'proper-lockfile'
import { z } from 'zod'

import { dispatchNotionPageIdentity } from '../dispatch/work-unit-identity'
import { assertLocalGhMutationAllowed, type GithubWriteIdentity } from '../github/gh-identity'
import type { GithubWriteIdentity } from '../github/gh-identity'

const INTAKE_LOCK_STALE_MS = 60_000

Expand Down Expand Up @@ -398,11 +398,11 @@ export class GhCliIssuePublisher implements GithubIssuePublisher {
* @param identity the GitHub write identity this publisher may use. Notion
* intake is a separate surface from the Factory lifecycle writeback and
* still creates and edits issues through the local `gh` CLI, so its
* issues are authored by the operator. That is a documented exception
* (see README), not a silent fallback: the caller must state the identity
* it is choosing, and exact `app` refuses rather than mislabelling the
* write, because the connected App surface exposes no issue-create
* operation to route it through.
* issues are authored by the operator. That is an explicit local-host
* mode, not a production fallback: the caller must select exact `user`.
* `auto` and `app` refuse every operation before spawning `gh`, because
* the production container has no binary and the connected App surface
* exposes neither issue creation nor source-marker reconciliation.
* @param gh the `gh` invoker. Injectable because every method here mutates
* or reads real GitHub: without a seam the only way to exercise this
* class is against the live API, which during development of #221
Expand All @@ -415,14 +415,11 @@ export class GhCliIssuePublisher implements GithubIssuePublisher {
}

assertWritable(): void {
assertLocalGhMutationAllowed(
this.#identity,
'creating or editing Notion intake lifecycle issues',
'createIssue/updateIssue',
)
this.#assertExplicitLocalUser('createIssue/updateIssue')
}

async repositoryVisibility(repo: string): Promise<'public' | 'private' | 'internal'> {
this.#assertExplicitLocalUser('repositoryVisibility')
const output = (await this.#gh(['repo', 'view', repo, '--json', 'visibility', '--jq', '.visibility'])).trim().toLowerCase()
if (output !== 'public' && output !== 'private' && output !== 'internal') {
throw new Error(`GitHub returned unknown visibility for ${repo}: ${output || '(empty)'}`)
Expand All @@ -431,12 +428,14 @@ export class GhCliIssuePublisher implements GithubIssuePublisher {
}

async missingLabels(repo: string, labels: readonly string[]): Promise<string[]> {
this.#assertExplicitLocalUser('missingLabels')
const output = await this.#gh(['api', '--paginate', `repos/${repo}/labels?per_page=100`, '--jq', '.[].name'])
const available = new Set(output.split('\n').map((label) => label.trim()).filter(Boolean))
return labels.filter((label) => !available.has(label))
}

async findBySource(repo: string, sourceKey: string): Promise<ExistingGithubIssue | undefined> {
this.#assertExplicitLocalUser('findBySource')
const output = await this.#gh([
'issue', 'list', '--repo', repo, '--state', 'all', '--limit', '100',
'--search', `"factory-source:${sourceKey}" in:body`,
Expand All @@ -448,7 +447,7 @@ export class GhCliIssuePublisher implements GithubIssuePublisher {
}

async createIssue(input: { repo: string; title: string; body: string; labels: readonly string[] }): Promise<{ number: number; url: string }> {
assertLocalGhMutationAllowed(this.#identity, `creating a GitHub issue in ${input.repo}`, 'createIssue')
this.#assertExplicitLocalUser('createIssue')
const args = ['issue', 'create', '--repo', input.repo, '--title', input.title, '--body-file', '-']
for (const label of input.labels) args.push('--label', label)
const url = (await this.#gh(args, input.body)).trim()
Expand All @@ -458,13 +457,18 @@ export class GhCliIssuePublisher implements GithubIssuePublisher {
}

async updateIssue(input: { repo: string; number: number; body: string }): Promise<void> {
assertLocalGhMutationAllowed(
this.#identity,
`editing the body of GitHub issue ${input.repo}#${input.number}`,
'updateIssue',
)
this.#assertExplicitLocalUser('updateIssue')
await this.#gh(['issue', 'edit', String(input.number), '--repo', input.repo, '--body-file', '-'], input.body)
}

#assertExplicitLocalUser(operation: string): void {
if (this.#identity === 'user') return
throw new Error(
`GitHub identity "${this.#identity}" refuses Notion intake ${operation} through local gh. ` +
'This adapter is local-only and requires explicit github.identity "user"; the production Factory container does not contain gh, ' +
'and the connected GitHub App surface does not expose issue creation or source-marker reconciliation.',
)
}
}

async function publishRepoTask(
Expand Down