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
107 changes: 102 additions & 5 deletions src/orchestrator/factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ const config = (overrides: FactoryConfigOverrides = {}): FactoryConfig => Factor
const issuePath = (n: number) => `/linear/issues/AR-${n}__uuid-${n}.json`
const readyAliasPath = (n: number) => `/linear/issues/by-state/ready-for-agent/AR-${n}.json`
const githubIssuePath = (owner: string, repo: string, number: number) => `/github/repos/${owner}/${repo}/issues/by-id/${number}.json`
const githubIssueCompactPath = (owner: string, repo: string, number: number) => `/github/repos/${owner}__${repo}/issues/by-id/${number}.json`
const githubIssueNestedMetaPath = (owner: string, repo: string, number: number) => `/github/repos/${owner}/${repo}/issues/${number}/meta.json`
const githubIssueCompactNestedMetaPath = (owner: string, repo: string, number: number) => `/github/repos/${owner}__${repo}/issues/${number}/meta.json`
const capturedReadyCanaryPath = '/linear/issues/AR-133__dac27fce-e8de-4910-bbf6-98ad436df3dd.json'
const capturedStaleDoneCanonicalPath = '/linear/issues/AR-173__40c7e780-59ad-47ee-8809-3a9b8434d8fb.json'
const capturedStaleReadyAliasPath = '/linear/issues/by-state/ready-for-agent/AR-173.json'
Expand Down Expand Up @@ -906,36 +908,59 @@ describe('FactoryLoop', () => {
})

it('extracts GitHub issue path parts from legacy and nested relayfile shapes', () => {
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/2174.json')).toEqual({
const expected = {
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: undefined,
})
}
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/2174.json')).toEqual(expected)
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud/issues/2174.json')).toEqual(expected)
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/2174/meta.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: undefined,
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud/issues/2174/meta.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: undefined,
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/2174__factory-path-regression/meta.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: 'factory-path-regression',
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud/issues/2174__factory-path-regression/meta.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: 'factory-path-regression',
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/2174/metadata.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
number: 2174,
slug: undefined,
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/by-id/2174.json')).toEqual({
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud/issues/2174/metadata.json')).toEqual(expected)
expect(githubIssuePathParts('/github/repos/AgentWorkforce/cloud/issues/by-id/2174.json')).toEqual(expected)
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud/issues/by-id/2174.json')).toEqual(expected)
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud__platform/issues/by-id/2174.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud',
repo: 'cloud__platform',
number: 2174,
slug: undefined,
})
expect(githubIssuePathParts('/github/repos/AgentWorkforce__cloud__platform/issues/2174__factory-path-regression/meta.json')).toEqual({
owner: 'AgentWorkforce',
repo: 'cloud__platform',
number: 2174,
slug: 'factory-path-regression',
})
})

it('LIVE_GITHUB_ISSUE_GLOB matches every supported relayfile issue shape under the real glob matcher', () => {
Expand All @@ -950,6 +975,14 @@ describe('FactoryLoop', () => {
'/github/repos/AgentWorkforce/cloud/issues/2174/metadata.json',
'/github/repos/AgentWorkforce/cloud/issues/2174__factory-path-regression/meta.json',
'/github/repos/AgentWorkforce/pear/issues/1126__directory-event',
'/github/repos/AgentWorkforce__cloud/issues/by-id/2174.json',
'/github/repos/AgentWorkforce__cloud/issues/2174.json',
'/github/repos/AgentWorkforce__cloud/issues/2174/meta.json',
'/github/repos/AgentWorkforce__cloud/issues/2174/metadata.json',
'/github/repos/AgentWorkforce__cloud/issues/2174__factory-path-regression/meta.json',
'/github/repos/AgentWorkforce__pear/issues/1126__directory-event',
'/github/repos/AgentWorkforce__cloud__platform/issues/by-id/2174.json',
'/github/repos/AgentWorkforce__cloud__platform/issues/1126__directory-event',
]
for (const path of supported) {
expect(globMatchesPath(LIVE_GITHUB_ISSUE_GLOB, path)).toBe(true)
Expand Down Expand Up @@ -1090,6 +1123,49 @@ describe('FactoryLoop', () => {
expect(factory.status().counters.githubIssueMirrorsCreated).toBe(1)
})

it('mirrors factory-labeled GitHub issues from compact owner__repo relayfile paths to the repo label', async () => {
const ghPath = githubIssueCompactPath('AgentWorkforce', 'relayfile-adapters', 222)
const mount = new FakeMountClient({
[ghPath]: githubIssueFile(222, {
owner: 'AgentWorkforce',
repo: 'relayfile-adapters',
title: 'Telegram helper parity',
body: 'Mirror the compact Relayfile GitHub issue path.',
labels: ['factory'],
}),
})
const factory = createFactory(config({
repos: {
byLabel: { 'relayfile-adapters': 'AgentWorkforce/relayfile-adapters' },
clonePaths: { 'AgentWorkforce/relayfile-adapters': '/work/relayfile-adapters' },
default: 'AgentWorkforce/relayfile-adapters',
},
safety: { requireTitlePrefix: '[factory]', requireTeamKey: 'AR' },
}), { mount, fleet: new FakeFleetClient(), triage: new StaticTriage() })

await factory.runOnce({ dryRun: false })

expect(githubIssuePathParts(ghPath)).toEqual({
owner: 'AgentWorkforce',
repo: 'relayfile-adapters',
number: 222,
slug: undefined,
})
expect(mount.writes).toHaveLength(1)
expect(mount.writes[0]?.content).toEqual(expect.objectContaining({
title: '[factory] Telegram helper parity',
labels: [{ name: 'relayfile-adapters' }],
source: expect.objectContaining({
provider: 'github',
owner: 'AgentWorkforce',
repo: 'relayfile-adapters',
number: 222,
path: ghPath,
}),
}))
expect(factory.status().counters.githubIssueMirrorsCreated).toBe(1)
})

it('mirrors GitHub issues with owner, repo, and number derived from nested relayfile paths', async () => {
const ghPath = '/github/repos/AgentWorkforce/pear/issues/1116__route-github-factory-issues/meta.json'
const mount = new FakeMountClient({
Expand Down Expand Up @@ -1129,7 +1205,7 @@ describe('FactoryLoop', () => {
// listTree surfaces the issue directory alongside its meta.json file; both
// resolve to the same metadata, so the backfill scan must collect only the
// file path and skip the directory to avoid processing the issue twice.
const dirPath = '/github/repos/AgentWorkforce/pear/issues/1116__route-github-factory-issues'
const dirPath = '/github/repos/AgentWorkforce__pear__tools/issues/1116__route-github-factory-issues'
const metaPath = `${dirPath}/meta.json`
const mount = new FakeMountClient({
[dirPath]: githubIssueFile(1116, { title: 'Directory and file both listed' }),
Expand Down Expand Up @@ -1516,6 +1592,27 @@ describe('FactoryLoop', () => {
path: '/github/repos/AgentWorkforce/pear/issues/1124/metadata.json',
number: 1124,
},
{
path: '/github/repos/AgentWorkforce__pear/issues/1125.json',
number: 1125,
},
{
path: '/github/repos/AgentWorkforce__pear/issues/by-id/1126.json',
number: 1126,
},
{
path: githubIssueCompactNestedMetaPath('AgentWorkforce', 'pear', 1127),
number: 1127,
},
{
path: '/github/repos/AgentWorkforce__pear/issues/1128__route-github-factory/meta.json',
number: 1128,
slug: 'route-github-factory',
},
{
path: '/github/repos/AgentWorkforce__pear/issues/1129/metadata.json',
number: 1129,
},
]
const mount = new FakeMountClient(Object.fromEntries(cases.map(({ path, number }) => [
path,
Expand Down
39 changes: 25 additions & 14 deletions src/orchestrator/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4074,21 +4074,27 @@ const repoMapFromConfig = (config: FactoryConfig) => {

export const githubIssuePathParts = (path: string): { owner: string; repo: string; number: number; slug?: string } | undefined => {
// Canonical GitHub relayfile issue entries are nested as
// /github/repos/<owner>/<repo>/issues/<number>__<slug>/meta.json. Keep the
// legacy flat and by-id forms for older mount state, and accept metadata.json
// as a read-only compatibility alias for historical local mount snapshots.
// /github/repos/<owner>/<repo>/issues/<number>__<slug>/meta.json. Some
// mounts compact the repo directory to <owner>__<repo>; accept both layouts.
// Keep the legacy flat and by-id forms for older mount state, and accept
// metadata.json as a read-only compatibility alias for historical snapshots.
const match = path.match(
/^\/github\/repos\/([^/]+)\/([^/]+)\/issues\/(?:(?:by-id\/)?(\d+)\.json|(\d+)(?:__([^/]+))?\/(?:meta|metadata)\.json)$/u,
/^\/github\/repos\/(?:([^/]+)\/([^/]+)|([A-Za-z0-9-]+)__([^/]+))\/issues\/(?:(?:by-id\/)?(\d+)\.json|(\d+)(?:__([^/]+))?\/(?:meta|metadata)\.json)$/u,
)
Comment on lines 4081 to 4083

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

GitHub usernames and organization names are restricted to alphanumeric characters and hyphens, meaning they can never contain underscores (_). However, GitHub repository names are allowed to contain underscores.

In the compact layout owner__repo, if a repository name contains underscores (e.g., my_awesome_repo), the greedy match ([^/]+)__([^/]+) will incorrectly split the path, greedily matching up to the last __ and attributing part of the repository name to the owner.

Using a non-greedy match ([^/]+?) for the compact owner group ensures the regex correctly splits at the first __ delimiter.

Suggested change
const match = path.match(
/^\/github\/repos\/([^/]+)\/([^/]+)\/issues\/(?:(?:by-id\/)?(\d+)\.json|(\d+)(?:__([^/]+))?\/(?:meta|metadata)\.json)$/u,
/^\/github\/repos\/(?:([^/]+)\/([^/]+)|([^/]+)__([^/]+))\/issues\/(?:(?:by-id\/)?(\d+)\.json|(\d+)(?:__([^/]+))?\/(?:meta|metadata)\.json)$/u,
)
const match = path.match(
/^\/github\/repos\/(?:([^/]+)\/([^/]+)|([^/]+?)__([^/]+))\/issues\/(?:(?:by-id\/)?(\d+)\.json|(\d+)(?:__([^/]+))?\/(?:meta|metadata)\.json)$/u,
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 786198e. The compact owner segment now uses GitHub owner characters ([A-Za-z0-9-]+) so repo names can contain underscores/double underscores without shifting into the owner. Added regression coverage for /github/repos/AgentWorkforce__cloud__platform/issues/by-id/2174.json and slugged meta paths.

if (!match) {
return undefined
}
const number = Number(match[3] ?? match[4])
const owner = match[1] ?? match[3]
const repo = match[2] ?? match[4]
if (!owner || !repo) {
return undefined
}
const number = Number(match[5] ?? match[6])
return {
owner: match[1]!,
repo: match[2]!,
owner,
repo,
number,
slug: match[5],
slug: match[7],
}
}

Expand All @@ -4114,15 +4120,20 @@ const githubIssueReadCandidatePaths = (path: string): string[] => {
}

const githubIssueDirectoryPathParts = (path: string): { owner: string; repo: string; number: number; slug?: string } | undefined => {
const match = path.match(/^\/github\/repos\/([^/]+)\/([^/]+)\/issues\/(\d+)(?:__([^/]+))?$/u)
const match = path.match(/^\/github\/repos\/(?:([^/]+)\/([^/]+)|([A-Za-z0-9-]+)__([^/]+))\/issues\/(\d+)(?:__([^/]+))?$/u)
if (!match) {
return undefined
}
const owner = match[1] ?? match[3]
const repo = match[2] ?? match[4]
if (!owner || !repo) {
return undefined
}
return {
owner: match[1]!,
repo: match[2]!,
number: Number(match[3]),
slug: match[4],
owner,
repo,
number: Number(match[5]),
slug: match[6],
}
}

Expand Down Expand Up @@ -4367,7 +4378,7 @@ const isGithubIssueFilePath = (path: string): boolean =>
githubIssuePathParts(path) !== undefined || githubIssueDirectoryPathParts(path) !== undefined

const isGithubIssueTreePath = (path: string): boolean =>
/^\/github\/repos\/[^/]+\/[^/]+\/issues\/.+/u.test(path)
/^\/github\/repos\/(?:[^/]+\/[^/]+|[^/]+__[^/]+)\/issues\/.+/u.test(path)

const githubPullPathParts = (path: string): { owner: string; repo: string; number: number } | undefined => {
// Tolerate every webhook-fed PR mount layout we've seen, across both the
Expand Down