Skip to content
Open
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
8 changes: 5 additions & 3 deletions .github/workflows/auto-author-assign.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ jobs:
steps:
- uses: toshimaru/auto-author-assign@v3.1.0
with:
# Falls back to the built-in token: GH_PAT is not set on this repo, and the job
# already has the pull-requests: write permission it needs.
repo-token: ${{ secrets.GH_PAT || secrets.GITHUB_TOKEN }}
# `github.token` rather than `secrets.GITHUB_TOKEN`: the secrets-context fallback
# added in 908f19a still resolved to an empty input, so the action failed with
# 'Input required and not supplied'. The github context is always populated, and
# the job already holds the pull-requests: write permission the action needs.
repo-token: ${{ github.token }}
59 changes: 51 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ Pulls `agent-ready` Jira tickets from the MAPCO project, implements them, and op
requests. Designed in [MAPCO-11374](https://mapcolonies.atlassian.net/browse/MAPCO-11374),
substrate decided in [MAPCO-11377](https://mapcolonies.atlassian.net/browse/MAPCO-11377).

**Current slice: MAPCO-11429.** It polls and reports. It claims nothing, writes nothing to
Jira, and touches no repository.
**Current slice: MAPCO-11431.** It walks the whole Jira state machine with nothing in the
middle — it claims a ticket and hands it straight back with a comment. It touches no
repository and writes no code yet.

## Shape

Expand Down Expand Up @@ -71,31 +72,73 @@ so refusal is the common path until the convention spreads.
| Variable | Default | Meaning |
|---|---|---|
| `MCP_ATLASSIAN_URL` | *required* | Address of the `atlassian-write` MCP server. Transport is picked from the path: `/sse` gets SSE, anything else Streamable HTTP |
| `JIRA_BOT_ACCOUNT` | *required* | Identifier written to a ticket's assignee field — an email or accountId |
| `JIRA_BOT_DISPLAY_NAME` | *required* | What `JIRA_BOT_ACCOUNT` reads back as, surname-first. The claim re-read compares against this |
| `POLL_INTERVAL_MS` | `300000` | How often a cycle runs |
| `MAX_TICKETS_PER_RUN` | `1` | Tickets one cycle may start |
| `MAX_CONCURRENT_TICKETS` | `1` | Tickets in flight at once |
| `GITHUB_TOKEN` | *optional* | Bearer token for repo lookups. A PAT locally; a short-lived App installation token in the cluster once MAPCO-11428 lands. Unauthenticated works at a lower rate limit |

Raise `MAX_TICKETS_PER_RUN` before ever raising `MAX_CONCURRENT_TICKETS`.

The two bot-identity variables look redundant and are not: Jira takes an *identifier* on
write and hands back a *display name* on read, and neither is derivable from the other in
this instance. Set them inconsistently and every claim reads as lost.

## Claiming and releasing

Jira is the only state store — no database, no files that outlive a run — so there is no
lock to take. Claiming is **optimistic**:

1. Look up the transition into `In Progress` *before* writing anything. A workflow with no
route in means the ticket can never be worked, and that is a refusal, not a half-claim.
The lookup matches a transition's **target status**, not its name — transition names are
verbs (`Start Progress`), so matching by name alone would refuse every ticket.
2. Write the assignee, then **read the issue back**. If the assignee is not the bot, a human
got there first: back off, write nothing further, and do not try to take it back off
them. Our write is already overwritten and is not ours to undo.
3. Transition to `In Progress`.

Releasing runs in the order comment → transition to `Open` → **unassign last**. That order
is deliberate and is the reverse of how it reads. Unassigning is what makes a ticket visible
to the poll query again (it filters `assignee is EMPTY`), so it goes last: if anything fails
part-way, the ticket is left held by the bot and `In Progress`, which the query skips and the
boot-time orphan sweep (MAPCO-11432) recovers. Unassigning first risks leaving a ticket
unassigned and `In Progress` — which polls straight back in, forever.

## Known gaps

- The worker knobs are read from the environment rather than `@map-colonies/config`, which
needs a schema published in `@map-colonies/schemas`. Telemetry still goes through the
library. Registering a real schema is follow-up work.
- **The Jira identity is the shared MCP service account.** It has no per-user attribution,
so the optimistic claim check (MAPCO-11431) cannot distinguish this worker from any other
session using the same MCP, and boot-time orphan release (MAPCO-11432) could release a
ticket someone else is working. A dedicated Jira account is the recommendation.
- **The Jira identity is configured, not discovered.** The MCP server runs under a shared
service account with no per-user attribution, so the worker cannot ask Jira who it is —
hence `JIRA_BOT_ACCOUNT` / `JIRA_BOT_DISPLAY_NAME`. The claim re-read can therefore tell
the bot apart from a *human*, but not from a second worker configured with the same
account. A dedicated Jira account per deployment is still the recommendation, and it is
what makes boot-time orphan release (MAPCO-11432) safe.
- **The real transition vocabulary is unverified.** `jira_get_transitions` and
`expand=transitions` are both rejected by the write-pilot MCP server, so the MAPCO
workflow's actual transition names and target statuses could not be read the way the poll
query was verified in MAPCO-11427. The lookup matches on target status with the
transition name as a fallback, which covers both shapes, and a `no-transition` refusal
logs the `offered` names — so the first real run reports the vocabulary rather than
refusing in silence. Confirm it from that log line before trusting a deployment.
- `helm lint` needs the private `mclabels` dependency and fails without registry access.

## Dry run

One cycle against the real MCP server, from a laptop. Read-only. Requires the corporate
VPN — the server is not reachable from outside it.
One cycle against the real MCP server, from a laptop. Requires the corporate VPN — the
server is not reachable from outside it.

**This writes to real tickets.** It claims the oldest `agent-ready` ticket and hands it
straight back, leaving a comment behind. That is the point: label a ticket `agent-ready` and
watch it get claimed and returned.

```sh
MCP_ATLASSIAN_URL="https://atlassian-mcp-write.mapcolonies.net/sse" \
JIRA_BOT_ACCOUNT="developer-agent@mapcolonies.net" \
JIRA_BOT_DISPLAY_NAME="AGENT DEVELOPER" \
GITHUB_TOKEN="$(gh auth token)" npm run dry-run
```

Expand Down
4 changes: 4 additions & 0 deletions helm/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ spec:
fieldPath: metadata.uid
- name: MCP_ATLASSIAN_URL
value: {{ .Values.worker.mcpUrl | quote }}
- name: JIRA_BOT_ACCOUNT
value: {{ .Values.worker.botAccount | quote }}
- name: JIRA_BOT_DISPLAY_NAME
value: {{ .Values.worker.botDisplayName | quote }}
- name: POLL_INTERVAL_MS
value: {{ .Values.worker.pollIntervalMs | quote }}
- name: MAX_TICKETS_PER_RUN
Expand Down
6 changes: 6 additions & 0 deletions helm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ worker:
# In-cluster address of the self-hosted atlassian-write MCP server. The worker holds no
# Jira credentials of its own — that server already has them.
mcpUrl: 'http://mcp-atlassian:8080/mcp'
# Who the worker claims tickets as. Two values because Jira takes an identifier on write
# and returns a display name on read, and display names here are surname-first — the
# optimistic claim's re-read compares against botDisplayName, so a mismatch between
# these two makes every claim look lost.
botAccount: ''
botDisplayName: ''
pollIntervalMs: 300000
maxTicketsPerRun: 1
maxConcurrentTickets: 1
Expand Down
11 changes: 11 additions & 0 deletions src/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ export const LABELS = {
attemptedPrefix: 'agent-attempted-',
} as const;

/**
* The two statuses the claim/release cycle moves a ticket between.
*
* Matched against a transition's *target status*, not against transition names — those are
* verbs on a real workflow (`Start Progress`).
*/
export const STATUS_NAMES = {
inProgress: 'In Progress',
open: 'Open',
} as const;

/**
* Status *names* that mean the ticket is finished.
*
Expand Down
25 changes: 20 additions & 5 deletions src/common/workerConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
* schema published in `@map-colonies/schemas`, and no schema exists for this service yet;
* the telemetry half still goes through it (see `common/config.ts`) because
* `commonBoilerplateV3` already covers logger and tracing. Registering a real schema for
* these knobs is follow-up work, not a blocker for the read-only slice.
* these knobs is follow-up work, not a blocker for the current slice.
*/

import type { BotIdentity } from '../tickets/claim';

interface WorkerConfig {
/** How often the internal scheduler runs a cycle. */
readonly pollIntervalMs: number;
Expand All @@ -17,6 +19,8 @@ interface WorkerConfig {
readonly maxConcurrentTickets: number;
/** Base URL of the in-cluster `atlassian-write` MCP server. */
readonly mcpUrl: string;
/** Who the worker claims tickets as. Both halves are required — see `BotIdentity`. */
readonly bot: BotIdentity;
}

const DEFAULT_POLL_INTERVAL_MS = 300_000;
Expand All @@ -42,17 +46,28 @@ function readInt(env: NodeJS.ProcessEnv, name: string, fallback: number): number
return parsed;
}

function loadWorkerConfig(env: NodeJS.ProcessEnv = process.env): WorkerConfig {
const mcpUrl = env.MCP_ATLASSIAN_URL;
if (mcpUrl === undefined || mcpUrl === '') {
throw new ConfigError('MCP_ATLASSIAN_URL must be set — the worker has no Jira credentials of its own');
function readRequired(env: NodeJS.ProcessEnv, name: string, why: string): string {
const raw = env[name];
if (raw === undefined || raw === '') {
throw new ConfigError(`${name} must be set — ${why}`);
}

return raw;
}

function loadWorkerConfig(env: NodeJS.ProcessEnv = process.env): WorkerConfig {
const mcpUrl = readRequired(env, 'MCP_ATLASSIAN_URL', 'the worker has no Jira credentials of its own');
const bot: BotIdentity = {
account: readRequired(env, 'JIRA_BOT_ACCOUNT', 'the worker has no way to put its own name on a ticket'),
displayName: readRequired(env, 'JIRA_BOT_DISPLAY_NAME', "the worker could not tell its own claims from a human's"),
};

return {
pollIntervalMs: readInt(env, 'POLL_INTERVAL_MS', DEFAULT_POLL_INTERVAL_MS),
maxTicketsPerRun: readInt(env, 'MAX_TICKETS_PER_RUN', 1),
maxConcurrentTickets: readInt(env, 'MAX_CONCURRENT_TICKETS', 1),
mcpUrl,
bot,
};
}

Expand Down
101 changes: 92 additions & 9 deletions src/cycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,113 @@ import type { Logger } from '@map-colonies/js-logger';
import type { WorkerConfig } from '@common/workerConfig';
import { buildPollQuery } from './jira/query';
import type { JiraPort, JiraTicket } from './jira/types';
import { claimTicket, releaseTicket } from './tickets/claim';

/** The attempt cap from MAPCO-11432. Enforced in the query so a capped ticket is never even seen. */
export const ATTEMPT_CAP = 2;
const ATTEMPT_CAP = 2;

export interface CycleDeps {
/**
* What the worker says on a ticket it hands straight back.
*
* Written to be read by whoever finds the ticket back in Open and wonders what touched it.
* It names what was tried, which in this slice is nothing at all.
*/
const HANDED_BACK_NOTE = [
'Picked this up automatically and handed it straight back.',
'',
'This build of the developer agent can claim a ticket and release it, but there is nothing in between yet (MAPCO-11431) — no branch, no changes, no pull request. Nothing was modified, and this does not count as an attempt.',
'',
'The ticket is available again for whoever wants it next.',
].join('\n');

interface CycleDeps {
readonly jira: JiraPort;
readonly logger: Logger;
readonly config: WorkerConfig;
}

export interface CycleResult {
interface CycleResult {
readonly found: number;
readonly started: number;
readonly skipped: number;
readonly more: boolean;
readonly outcome: 'ok' | 'failed';
}

/**
* Work through the run's tickets, no more than `maxConcurrentTickets` at a time.
*
* Both caps default to 1, which makes this sequential — the interesting case is a
* deployment that raises them, where the concurrency cap is what keeps the worker from
* claiming a whole page of tickets at once.
*/
async function handleTickets(tickets: JiraTicket[], deps: CycleDeps): Promise<number> {
const { maxConcurrentTickets } = deps.config;
let started = 0;

for (let index = 0; index < tickets.length; index += maxConcurrentTickets) {
const batch = tickets.slice(index, index + maxConcurrentTickets);
const outcomes = await Promise.all(batch.map(async (ticket) => handleTicket(ticket, deps)));

started += outcomes.filter((outcome) => outcome).length;
}

return started;
}

/**
* Claim one ticket and hand it back. Returns whether the worker actually held it.
*
* Nothing in here is allowed to end the run: one ticket the worker cannot take says
* nothing about the next one, and a run that dies on the first refusal would stall the
* whole queue behind it.
*/
async function handleTicket(ticket: JiraTicket, deps: CycleDeps): Promise<boolean> {
const { jira, logger, config } = deps;

try {
const claim = await claimTicket(ticket, jira, config.bot);

if (!claim.ok) {
// `saw` and `offered` are the diagnostic half of a refusal. A `lost-race` reporting
// the bot's own name means JIRA_BOT_DISPLAY_NAME is wrong, not that a human raced;
// `offered` names the transitions a workflow actually has.
logger.warn({ msg: 'not claimed', key: ticket.key, reason: claim.reason, saw: claim.saw, offered: claim.offered });

return false;
}

logger.info({ msg: 'claimed', key: ticket.key });

const release = await releaseTicket(ticket, HANDED_BACK_NOTE, jira);

if (!release.ok) {
// Still assigned to the bot and still In Progress, on purpose — see `releaseTicket`.
// The orphan sweep on boot (MAPCO-11432) is what gets it back.
logger.error({ msg: 'held ticket could not be released', key: ticket.key, reason: release.reason, offered: release.offered });
} else {
logger.info({ msg: 'released', key: ticket.key });
}

return true;
} catch (error) {
logger.error({ msg: 'ticket failed', key: ticket.key, err: error });

return false;
}
}

/**
* One full run of the worker, start to finish.
*
* This is the single seam the rest of the pipeline is built and tested through: the
* scheduler calls it, and so do the tests. Later slices add cases here rather than
* standing up harnesses of their own.
*
* In this slice it polls and reports. It starts nothing and writes nothing to Jira.
* In this slice it walks the whole Jira state machine with nothing in the middle: claim a
* ticket, then release it again (MAPCO-11431).
*/
export async function runCycle(deps: CycleDeps): Promise<CycleResult> {
async function runCycle(deps: CycleDeps): Promise<CycleResult> {
const { jira, logger, config } = deps;
const jql = buildPollQuery(ATTEMPT_CAP);

Expand All @@ -47,23 +126,27 @@ export async function runCycle(deps: CycleDeps): Promise<CycleResult> {
const more = tickets.length > config.maxTicketsPerRun;
const eligible = tickets.slice(0, config.maxTicketsPerRun);

const started = await handleTickets(eligible, deps);

const result: CycleResult = {
found: eligible.length,
started: 0,
skipped: eligible.length,
started,
skipped: eligible.length - started,
more,
outcome: 'ok',
};

// One structured line per run, shaped so Loki can ingest it later. A missing run line
// is the alarm — there is no alerting stack by decision (MAPCO-11437).
logger.info({
msg: 'poll complete',
msg: 'cycle complete',
...result,
reason: 'read-only slice: this worker cannot claim yet',
keys: eligible.map((ticket) => ticket.key),
tokensSpent: 0,
});

return result;
}

export { ATTEMPT_CAP, HANDED_BACK_NOTE, runCycle };
export type { CycleDeps, CycleResult };
3 changes: 2 additions & 1 deletion src/dryRun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* It exercises the same `runCycle` seam the deployed worker runs, so what it proves is
* about the worker and not about the harness.
*
* Read-only, like the slice it belongs to.
* Not read-only: this walks the same claim-and-release path the deployed worker walks, so
* it comments on, assigns and transitions a real ticket (MAPCO-11431).
*/
import 'reflect-metadata';
import { jsLogger } from '@map-colonies/js-logger';
Expand Down
Loading
Loading