Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
79 changes: 79 additions & 0 deletions .github/scripts/__tests__/keepalive-loop.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,49 @@ test('updateKeepaliveLoopSummary migrates legacy state to the selected App write
assert.equal(github.actions.some((action) => action.commentId === 44), false);
});

test('updateKeepaliveLoopSummary reuses a summary written by the selected PAT user', async () => {
const existingState = [
'<!-- keepalive-loop-summary -->',
formatStateComment({
trace: 'pat-summary-trace',
iteration: 1,
max_iterations: 5,
failure_threshold: 3,
}),
].join('\n');
const github = buildGithubStub({
comments: [{
id: 46,
body: existingState,
html_url: 'https://example.com/46',
user: { login: 'stranske-automation-bot', type: 'User' },
}],
});

await updateKeepaliveLoopSummary({
github,
context: buildContext(123),
core: buildCore(),
inputs: {
prNumber: 123,
action: 'run',
runResult: 'success',
gateConclusion: 'success',
tasksTotal: 2,
tasksUnchecked: 1,
keepaliveEnabled: true,
iteration: 1,
maxIterations: 5,
failureThreshold: 3,
trace: 'pat-summary-trace',
trusted_summary_author: 'stranske-automation-bot',
},
});

assert.equal(github.actions[0].type, 'update');
assert.equal(github.actions[0].commentId, 46);
});

test('updateKeepaliveLoopSummary recovers trusted state past an untrusted summary writer', async () => {
const trustedState = [
'<!-- keepalive-loop-summary -->',
Expand Down Expand Up @@ -4930,6 +4973,42 @@ test('markAgentRunning migrates legacy state to the selected App writer', async
assert.equal(persistedState.running, true);
});

test('markAgentRunning reuses a running summary written by the selected PAT user', async () => {
const existingStateBody = formatStateComment({
trace: 'running-pat-trace',
iteration: 2,
tasks: { total: 4, unchecked: 3 },
});
const github = buildGithubStub({
comments: [{
id: 202,
body: `<!-- keepalive-loop-summary -->\n## Summary\n${existingStateBody}`,
html_url: 'https://example.com/202',
user: { login: 'stranske-automation-bot', type: 'User' },
}],
});

await markAgentRunning({
github,
context: { repo: { owner: 'test', repo: 'repo' } },
core: buildCore(),
inputs: {
pr_number: 44,
agent_type: 'codex',
iteration: 2,
max_iterations: 5,
tasks_total: 4,
tasks_unchecked: 3,
trace: 'running-pat-trace',
trusted_summary_author: 'stranske-automation-bot',
},
});

assert.equal(github.actions.length, 1);
assert.equal(github.actions[0].type, 'update');
assert.equal(github.actions[0].commentId, 202);
});

test('markAgentRunning creates comment when none exists', async () => {
const github = buildGithubStub({ comments: [] });
const inputs = {
Expand Down
25 changes: 25 additions & 0 deletions .github/scripts/__tests__/sync_pr_merge_contract.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ test('maint71 run writes reports and records a no-PR result with fake action cli
CLEANUP_BRANCHES_INPUT: process.env.CLEANUP_BRANCHES_INPUT,
DRY_RUN_INPUT: process.env.DRY_RUN_INPUT,
AUTO_MERGE_INPUT: process.env.AUTO_MERGE_INPUT,
OWNER_PR_PAT: process.env.OWNER_PR_PAT,
SYNC_PR_MERGE_REPORT_JSON: process.env.SYNC_PR_MERGE_REPORT_JSON,
};
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'maint71-run-'));
Expand Down Expand Up @@ -298,6 +299,7 @@ test('maint71 run writes reports and records a no-PR result with fake action cli
process.env.CLEANUP_BRANCHES_INPUT = 'false';
process.env.DRY_RUN_INPUT = 'true';
process.env.AUTO_MERGE_INPUT = 'false';
process.env.OWNER_PR_PAT = 'test-owner-token';
process.env.SYNC_PR_MERGE_REPORT_JSON = reportPath;

await run({
Expand Down Expand Up @@ -330,6 +332,27 @@ test('maint71 run writes reports and records a no-PR result with fake action cli
}
});

test('maint71 fails closed before cross-repository API calls without OWNER_PR_PAT', async () => {
const originalOwnerPat = process.env.OWNER_PR_PAT;
try {
delete process.env.OWNER_PR_PAT;
await assert.rejects(
run({
github: {},
core: { warning: () => {} },
context: {
repo: { owner: 'stranske', repo: 'Workflows' },
payload: {},
},
}),
/Maint 71 requires OWNER_PR_PAT/,
);
} finally {
if (originalOwnerPat === undefined) delete process.env.OWNER_PR_PAT;
else process.env.OWNER_PR_PAT = originalOwnerPat;
}
});

test('maint71 recovers exact-head evidence from an already-merged candidate PR', async () => {
const originalCwd = process.cwd();
const envKeys = [
Expand All @@ -339,6 +362,7 @@ test('maint71 recovers exact-head evidence from an already-merged candidate PR',
'AUTO_MERGE_INPUT',
'EVIDENCE_ONLY_INPUT',
'ACTIVE_SYNC_HASH_INPUT',
'OWNER_PR_PAT',
'CONSUMER_SYNC_CANARIES_PATH',
'TRUSTED_SYNC_ACTORS',
'SYNC_PR_MERGE_REPORT_JSON',
Expand Down Expand Up @@ -452,6 +476,7 @@ test('maint71 recovers exact-head evidence from an already-merged candidate PR',
process.env.DRY_RUN_INPUT = 'true';
process.env.AUTO_MERGE_INPUT = 'false';
process.env.ACTIVE_SYNC_HASH_INPUT = 'candidate';
process.env.OWNER_PR_PAT = 'test-owner-token';
process.env.EVIDENCE_ONLY_INPUT = 'true';
process.env.CONSUMER_SYNC_CANARIES_PATH = canaryConfigPath;
process.env.TRUSTED_SYNC_ACTORS = 'stranske';
Expand Down
20 changes: 16 additions & 4 deletions .github/scripts/keepalive_loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ function normalise(value) {
return String(value ?? '').trim();
}

function expectedTrustedSummaryAuthorType(author) {
return normalise(author).toLowerCase().endsWith('[bot]') ? 'bot' : 'user';
}

function buildAuthorityChallengeEvidence({
agentSummary,
summaryReason,
Expand Down Expand Up @@ -3217,14 +3221,18 @@ async function updateKeepaliveLoopSummary({ github: rawGithub, context, core, in
).toLowerCase();
const existingSummaryAuthor = normalise(commentAuthorLogin).toLowerCase();
const existingSummaryAuthorType = normalise(commentAuthorType).toLowerCase();
const trustedSummaryAuthorType = expectedTrustedSummaryAuthorType(trustedSummaryAuthor);
const migrateSummaryWriter = Boolean(
commentId &&
trustedSummaryAuthor &&
(existingSummaryAuthor !== trustedSummaryAuthor || existingSummaryAuthorType !== 'bot'),
(
existingSummaryAuthor !== trustedSummaryAuthor ||
existingSummaryAuthorType !== trustedSummaryAuthorType
),
);
if (migrateSummaryWriter) {
core?.info?.(
`Creating a trusted App-owned keepalive summary; existing writer ` +
`Creating a trusted keepalive summary; existing writer ` +
`${existingSummaryAuthor || 'unknown'} is not ${trustedSummaryAuthor}; migrating known state.`,
);
}
Expand Down Expand Up @@ -4795,14 +4803,18 @@ async function markAgentRunning({ github: rawGithub, context, core, inputs }) {
).toLowerCase();
const existingSummaryAuthor = normalise(commentAuthorLogin).toLowerCase();
const existingSummaryAuthorType = normalise(commentAuthorType).toLowerCase();
const trustedSummaryAuthorType = expectedTrustedSummaryAuthorType(trustedSummaryAuthor);
const migrateSummaryWriter = Boolean(
commentId &&
trustedSummaryAuthor &&
(existingSummaryAuthor !== trustedSummaryAuthor || existingSummaryAuthorType !== 'bot'),
(
existingSummaryAuthor !== trustedSummaryAuthor ||
existingSummaryAuthorType !== trustedSummaryAuthorType
),
);
if (migrateSummaryWriter) {
core?.info?.(
`Creating a trusted App-owned running summary; existing writer ` +
`Creating a trusted running summary; existing writer ` +
`${existingSummaryAuthor || 'unknown'} is not ${trustedSummaryAuthor}; migrating known state.`,
);
}
Expand Down
29 changes: 14 additions & 15 deletions .github/scripts/maint71_merge_sync_prs.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,21 +344,20 @@ async function run({ github, context, core }) {
paginateWithRetry: (githubInstance, method, params) =>
githubInstance.paginate(method, params),
};
const { createTokenAwareRetry } = retryHelpers;
const { withRetry } = createTokenAwareRetry
? await createTokenAwareRetry({
github,
core,
env: process.env,
task: 'maint-71-merge-sync-prs',
capabilities: [
'pull-requests:read',
'pull-requests:write',
'checks:read',
'contents:write',
],
})
: { github, withRetry: (fn) => fn(github) };
if (!String(process.env.OWNER_PR_PAT || '').trim()) {
throw new Error(
'Maint 71 requires OWNER_PR_PAT for fail-closed cross-repository PR mutations',
);
Comment thread
stranske marked this conversation as resolved.
Resolved
}
// The actions/github-script client is authenticated with OWNER_PR_PAT by
// the workflow. Keep retries on that exact client so credential or quota
// degradation cannot rotate a cross-repository mutation to a token with a
// smaller repository installation scope.
const withRetry = (fn, options = {}) => retryHelpers.withRetry(fn, {
github,
core,
...options,
});
async function getRequiredContexts({ owner, repo, branch }) {
try {
const { data: protection } = await withRetry((client) =>
Expand Down
20 changes: 16 additions & 4 deletions templates/consumer-repo/.github/scripts/keepalive_loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ function normalise(value) {
return String(value ?? '').trim();
}

function expectedTrustedSummaryAuthorType(author) {
return normalise(author).toLowerCase().endsWith('[bot]') ? 'bot' : 'user';
}

function buildAuthorityChallengeEvidence({
agentSummary,
summaryReason,
Expand Down Expand Up @@ -3217,14 +3221,18 @@ async function updateKeepaliveLoopSummary({ github: rawGithub, context, core, in
).toLowerCase();
const existingSummaryAuthor = normalise(commentAuthorLogin).toLowerCase();
const existingSummaryAuthorType = normalise(commentAuthorType).toLowerCase();
const trustedSummaryAuthorType = expectedTrustedSummaryAuthorType(trustedSummaryAuthor);
const migrateSummaryWriter = Boolean(
commentId &&
trustedSummaryAuthor &&
(existingSummaryAuthor !== trustedSummaryAuthor || existingSummaryAuthorType !== 'bot'),
(
existingSummaryAuthor !== trustedSummaryAuthor ||
existingSummaryAuthorType !== trustedSummaryAuthorType
),
);
if (migrateSummaryWriter) {
core?.info?.(
`Creating a trusted App-owned keepalive summary; existing writer ` +
`Creating a trusted keepalive summary; existing writer ` +
`${existingSummaryAuthor || 'unknown'} is not ${trustedSummaryAuthor}; migrating known state.`,
);
}
Expand Down Expand Up @@ -4795,14 +4803,18 @@ async function markAgentRunning({ github: rawGithub, context, core, inputs }) {
).toLowerCase();
const existingSummaryAuthor = normalise(commentAuthorLogin).toLowerCase();
const existingSummaryAuthorType = normalise(commentAuthorType).toLowerCase();
const trustedSummaryAuthorType = expectedTrustedSummaryAuthorType(trustedSummaryAuthor);
const migrateSummaryWriter = Boolean(
commentId &&
trustedSummaryAuthor &&
(existingSummaryAuthor !== trustedSummaryAuthor || existingSummaryAuthorType !== 'bot'),
(
existingSummaryAuthor !== trustedSummaryAuthor ||
existingSummaryAuthorType !== trustedSummaryAuthorType
),
);
if (migrateSummaryWriter) {
core?.info?.(
`Creating a trusted App-owned running summary; existing writer ` +
`Creating a trusted running summary; existing writer ` +
`${existingSummaryAuthor || 'unknown'} is not ${trustedSummaryAuthor}; migrating known state.`,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,9 @@ jobs:
run: |
set -euo pipefail
case "$TOKEN_SOURCE:$TOKEN_LOGIN" in
'ACTIONS_BOT_PAT:stranske'|'SERVICE_BOT_PAT:stranske-automation-bot') ;;
'ACTIONS_BOT_PAT:stranske'|\
'ACTIONS_BOT_PAT:stranske-automation-bot'|\
'SERVICE_BOT_PAT:stranske-automation-bot') ;;
*)
echo "::error::$TOKEN_SOURCE has unexpected identity '$TOKEN_LOGIN'."
exit 1 ;;
Expand Down Expand Up @@ -617,9 +619,8 @@ jobs:
'stranske-keepalive[bot]' ||
steps.running_workflows_app_token.outputs.token != '' &&
'agents-workflows-bot[bot]' ||
steps.running_token_writer.outputs.source == 'ACTIONS_BOT_PAT' &&
'stranske' ||
'stranske-automation-bot'
steps.running_pat_identity.outputs.login ||
''
}}
with:
github-token: >-
Expand Down
3 changes: 3 additions & 0 deletions tests/workflows/test_sync_delivery_liveness.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ def test_maint71_has_proof_bound_review_resolution_and_exact_evidence_promotion(
assert "canary_evidence_json: JSON.stringify(evidence)" in workflow
assert "cancel-in-progress: false" in workflow
assert "EXCLUDED_REPOS_INPUT: stranske/Collab-Admin" in workflow
assert "Maint 71 requires OWNER_PR_PAT" in executor
assert "retryHelpers.withRetry(fn" in executor
assert "createTokenAwareRetry" not in executor


def test_sync_lifecycle_chains_and_has_event_plus_timer_fallbacks():
Expand Down
4 changes: 2 additions & 2 deletions tests/workflows/test_workflow_agents_consolidation.py
Original file line number Diff line number Diff line change
Expand Up @@ -840,14 +840,14 @@ def test_consumer_mark_running_supports_identity_checked_pat():
assert "github.rest.users.getAuthenticated()" in mark_running
assert "withRetry(() => github.rest.users.getAuthenticated())" in mark_running
assert "'ACTIONS_BOT_PAT:stranske'" in mark_running
assert "'ACTIONS_BOT_PAT:stranske-automation-bot'" in mark_running
assert "'SERVICE_BOT_PAT:stranske-automation-bot'" in mark_running
update = mark_running[mark_running.index("- name: Update summary with running status") :]
assert "steps.running_keepalive_app_token.outputs.token ||" in update
assert "steps.running_workflows_app_token.outputs.token ||" in update
assert "secrets.ACTIONS_BOT_PAT ||" in update
assert "secrets.SERVICE_BOT_PAT" in update
assert "steps.running_token_writer.outputs.source == 'ACTIONS_BOT_PAT'" in update
assert "'stranske-automation-bot'" in update
assert "steps.running_pat_identity.outputs.login ||" in update
assert "secrets.GITHUB_TOKEN" not in update
assert "github.token" not in update

Expand Down
Loading