merge: resolve changelog conflicts with main
Keep both Unreleased bullets: the config-wipe Settings fix and the stale merged-PR Git panel fix from main. Co-authored-by: serkraser <serkraser@gmail.com>
This commit is contained in:
@@ -75,8 +75,9 @@
|
||||
- It resolves those remotes into GitHub repos.
|
||||
- It expands each repo through `parent` and `source` so PRs in upstream repos can still be found.
|
||||
- It skips PR lookup when the current branch matches that repo's default branch.
|
||||
- It first searches for PRs by likely source owner plus exact head branch.
|
||||
- If that fails, it falls back to broader GitHub search for the branch name.
|
||||
- It first searches for **open** PRs by likely source owner plus exact head branch.
|
||||
- If that fails, it falls back to broader GitHub search for open PRs on the branch name.
|
||||
- Closed/merged PRs are intentionally not associated with branch status; historical PR browsing stays on explicit list/detail endpoints.
|
||||
- `403` and `404` during repo lookups are treated as expected gaps, not hard errors.
|
||||
|
||||
## Shared client state model
|
||||
@@ -108,11 +109,16 @@
|
||||
- Open PR with pending checks -> refresh about every `1m`.
|
||||
- Open PR with non-pending checks -> refresh about every `5m`.
|
||||
- Open PR without a stable checks signal -> refresh about every `2m`.
|
||||
- Closed or merged PR -> stop regular polling.
|
||||
- Closed or merged PR -> discovery refresh every `5m` (do not permanently stop polling).
|
||||
- Hidden tab -> skip polling.
|
||||
- Non-forced refreshes use a `90s` TTL.
|
||||
- Failed non-forced attempts also observe the `90s` TTL so transient server or rate-limit failures cannot retry on every sidebar update. Forced user/action refreshes bypass this guard.
|
||||
|
||||
## Persistence notes for terminal PRs
|
||||
|
||||
- Closed/merged branch-status entries are not written to local storage.
|
||||
- Legacy persisted terminal entries are stripped on hydrate (`pr: null`) and marked unresolved until the next refresh.
|
||||
|
||||
## Background tracking rules
|
||||
|
||||
- Track up to `50` likely directories.
|
||||
|
||||
@@ -440,61 +440,62 @@ const searchFallbackPr = async ({ octokit, branch, repoNames }) => {
|
||||
|
||||
const normalizedRepoNames = new Set(repoNames.map((name) => normalizeLower(name)).filter(Boolean));
|
||||
|
||||
for (const state of ['open', 'closed']) {
|
||||
let response;
|
||||
// Branch status only discovers open PRs. Closed/merged history belongs to
|
||||
// explicit PR list/detail workflows, not automatic branch association.
|
||||
let response;
|
||||
try {
|
||||
response = await octokit.rest.search.issuesAndPullRequests({
|
||||
q: `is:pr state:open head:${branch}`,
|
||||
per_page: 20,
|
||||
});
|
||||
// If we get here, search API works for this repo — clear the disabled flag
|
||||
_searchApiDisabledRepos.delete(repoKey);
|
||||
} catch (error) {
|
||||
noteIfGitHubRateLimit(error);
|
||||
if (error?.status === 403) {
|
||||
_searchApiDisabledRepos.set(repoKey, Date.now());
|
||||
return null;
|
||||
}
|
||||
if (error?.status === 404) {
|
||||
rememberSearchMiss(missKey);
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
const items = Array.isArray(response?.data?.items) ? response.data.items : [];
|
||||
for (const item of items) {
|
||||
const repo = parseRepoFromApiUrl(item?.repository_url);
|
||||
if (!repo) {
|
||||
continue;
|
||||
}
|
||||
if (normalizedRepoNames.size > 0 && !normalizedRepoNames.has(normalizeLower(repo.repo))) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
response = await octokit.rest.search.issuesAndPullRequests({
|
||||
q: `is:pr state:${state} head:${branch}`,
|
||||
per_page: 20,
|
||||
const prResponse = await octokit.rest.pulls.get({
|
||||
owner: repo.owner,
|
||||
repo: repo.repo,
|
||||
pull_number: item.number,
|
||||
});
|
||||
// If we get here, search API works for this repo — clear the disabled flag
|
||||
_searchApiDisabledRepos.delete(repoKey);
|
||||
} catch (error) {
|
||||
noteIfGitHubRateLimit(error);
|
||||
if (error?.status === 403) {
|
||||
_searchApiDisabledRepos.set(repoKey, Date.now());
|
||||
return null;
|
||||
const pr = prResponse?.data;
|
||||
if (!pr || normalizeText(pr.head?.ref) !== branch) {
|
||||
continue;
|
||||
}
|
||||
if (error?.status === 404) {
|
||||
return {
|
||||
repo: {
|
||||
owner: repo.owner,
|
||||
repo: repo.repo,
|
||||
url: `https://github.com/${repo.owner}/${repo.repo}`,
|
||||
},
|
||||
pr,
|
||||
};
|
||||
} catch (error) {
|
||||
if (error?.status === 403 || error?.status === 404) {
|
||||
continue;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
const items = Array.isArray(response?.data?.items) ? response.data.items : [];
|
||||
for (const item of items) {
|
||||
const repo = parseRepoFromApiUrl(item?.repository_url);
|
||||
if (!repo) {
|
||||
continue;
|
||||
}
|
||||
if (normalizedRepoNames.size > 0 && !normalizedRepoNames.has(normalizeLower(repo.repo))) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
const prResponse = await octokit.rest.pulls.get({
|
||||
owner: repo.owner,
|
||||
repo: repo.repo,
|
||||
pull_number: item.number,
|
||||
});
|
||||
const pr = prResponse?.data;
|
||||
if (!pr || normalizeText(pr.head?.ref) !== branch) {
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
repo: {
|
||||
owner: repo.owner,
|
||||
repo: repo.repo,
|
||||
url: `https://github.com/${repo.owner}/${repo.repo}`,
|
||||
},
|
||||
pr,
|
||||
};
|
||||
} catch (error) {
|
||||
if (error?.status === 403 || error?.status === 404) {
|
||||
continue;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
rememberSearchMiss(missKey);
|
||||
@@ -511,24 +512,22 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates,
|
||||
.filter((pr) => matcher.matches(pr, target.repo.repo))
|
||||
.sort((left, right) => matcher.compare(left, right, target.repo.repo))[0] ?? null;
|
||||
|
||||
for (const state of ['open', 'closed']) {
|
||||
// Shared per-repo list first: one pulls.list answers every branch of the
|
||||
// repo within the TTL. A miss in a complete list is authoritative — skip
|
||||
// the per-branch query fan entirely.
|
||||
let listWasComplete = false;
|
||||
try {
|
||||
const listEntry = await getRepoPulls(octokit, target.repo, state, { force });
|
||||
const fromList = pickPreferred(listEntry.prs);
|
||||
if (fromList) {
|
||||
return fromList;
|
||||
}
|
||||
listWasComplete = listEntry.complete;
|
||||
} catch {
|
||||
// fall through to the precise per-branch queries
|
||||
}
|
||||
if (listWasComplete) {
|
||||
continue;
|
||||
// Branch status associates the current head with an open PR only. Returning a
|
||||
// closed/merged PR here made the client cache a terminal status that could not
|
||||
// self-heal until a manual forced refresh. A miss also lets the next repo
|
||||
// target run, so an open upstream PR wins over a merged fork PR.
|
||||
let listWasComplete = false;
|
||||
try {
|
||||
const listEntry = await getRepoPulls(octokit, target.repo, 'open', { force });
|
||||
const fromList = pickPreferred(listEntry.prs);
|
||||
if (fromList) {
|
||||
return fromList;
|
||||
}
|
||||
listWasComplete = listEntry.complete;
|
||||
} catch {
|
||||
// fall through to the precise per-branch queries
|
||||
}
|
||||
if (!listWasComplete) {
|
||||
if (coverage) {
|
||||
coverage.authoritative = false;
|
||||
}
|
||||
@@ -537,7 +536,7 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates,
|
||||
const directCandidates = await safeListPulls(octokit, {
|
||||
owner: target.repo.owner,
|
||||
repo: target.repo.repo,
|
||||
state,
|
||||
state: 'open',
|
||||
head: `${owner}:${branch}`,
|
||||
per_page: 100,
|
||||
});
|
||||
@@ -551,6 +550,9 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates,
|
||||
return null;
|
||||
};
|
||||
|
||||
// Exported for focused unit tests of open-only branch matching.
|
||||
export { findFirstMatchingPr };
|
||||
|
||||
export async function resolveGitHubPrStatus({ octokit, directory, branch, remoteName, force = false }) {
|
||||
// A deleted worktree can still have a session in the sidebar that keeps
|
||||
// requesting its PR status. Bail before touching git or GitHub for a
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
import { beforeEach, describe, expect, mock, test } from 'bun:test';
|
||||
|
||||
const listMock = mock(async () => ({ data: [] }));
|
||||
|
||||
mock.module('../git/index.js', () => ({
|
||||
getRemotes: async () => [],
|
||||
getStatus: async () => null,
|
||||
}));
|
||||
|
||||
mock.module('./repo/index.js', () => ({
|
||||
resolveGitHubRepoFromDirectory: async () => null,
|
||||
}));
|
||||
|
||||
mock.module('./rate-limit.js', () => ({
|
||||
noteIfGitHubRateLimit: () => {},
|
||||
}));
|
||||
|
||||
const { findFirstMatchingPr, invalidateRepoPullsCache } = await import('./pr-status.js');
|
||||
|
||||
const openPr = {
|
||||
number: 15,
|
||||
state: 'open',
|
||||
head: {
|
||||
ref: 'feature',
|
||||
label: 'acme:feature',
|
||||
user: { login: 'acme' },
|
||||
repo: { owner: { login: 'acme' }, name: 'app' },
|
||||
},
|
||||
};
|
||||
|
||||
const closedPr = {
|
||||
number: 12,
|
||||
state: 'closed',
|
||||
merged_at: '2026-01-01T00:00:00Z',
|
||||
head: {
|
||||
ref: 'feature',
|
||||
label: 'acme:feature',
|
||||
user: { login: 'acme' },
|
||||
repo: { owner: { login: 'acme' }, name: 'app' },
|
||||
},
|
||||
};
|
||||
|
||||
describe('findFirstMatchingPr open-only branch status', () => {
|
||||
beforeEach(() => {
|
||||
listMock.mockReset();
|
||||
invalidateRepoPullsCache('acme', 'app');
|
||||
});
|
||||
|
||||
test('returns a matching open PR', async () => {
|
||||
listMock.mockImplementation(async ({ state }) => {
|
||||
if (state === 'open') {
|
||||
return { data: [openPr] };
|
||||
}
|
||||
return { data: [closedPr] };
|
||||
});
|
||||
|
||||
const pr = await findFirstMatchingPr({
|
||||
octokit: { rest: { pulls: { list: listMock } } },
|
||||
target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' },
|
||||
branch: 'feature',
|
||||
sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }],
|
||||
force: true,
|
||||
});
|
||||
|
||||
expect(pr?.number).toBe(15);
|
||||
expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true);
|
||||
});
|
||||
|
||||
test('returns null when only a closed/merged PR exists for the head branch', async () => {
|
||||
listMock.mockImplementation(async ({ state }) => {
|
||||
if (state === 'open') {
|
||||
return { data: [] };
|
||||
}
|
||||
return { data: [closedPr] };
|
||||
});
|
||||
|
||||
const pr = await findFirstMatchingPr({
|
||||
octokit: { rest: { pulls: { list: listMock } } },
|
||||
target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' },
|
||||
branch: 'feature',
|
||||
sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }],
|
||||
force: true,
|
||||
});
|
||||
|
||||
expect(pr).toBeNull();
|
||||
expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true);
|
||||
expect(listMock.mock.calls.some((call) => call[0]?.state === 'closed')).toBe(false);
|
||||
});
|
||||
|
||||
test('does not query closed PRs when the open list is complete and empty', async () => {
|
||||
listMock.mockImplementation(async () => ({ data: [] }));
|
||||
|
||||
const pr = await findFirstMatchingPr({
|
||||
octokit: { rest: { pulls: { list: listMock } } },
|
||||
target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' },
|
||||
branch: 'feature',
|
||||
sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }],
|
||||
force: true,
|
||||
});
|
||||
|
||||
expect(pr).toBeNull();
|
||||
expect(listMock.mock.calls).toHaveLength(1);
|
||||
expect(listMock.mock.calls[0]?.[0]?.state).toBe('open');
|
||||
});
|
||||
});
|
||||
@@ -375,6 +375,9 @@ export const createSettingsHelpers = (dependencies) => {
|
||||
const normalizedDays = Math.max(1, Math.min(365, Math.round(candidate.autoDeleteAfterDays)));
|
||||
result.autoDeleteAfterDays = normalizedDays;
|
||||
}
|
||||
if (candidate.sessionRetentionAction === 'archive' || candidate.sessionRetentionAction === 'delete') {
|
||||
result.sessionRetentionAction = candidate.sessionRetentionAction;
|
||||
}
|
||||
if (candidate.tunnelBootstrapTtlMs === null) {
|
||||
result.tunnelBootstrapTtlMs = null;
|
||||
} else if (typeof candidate.tunnelBootstrapTtlMs === 'number' && Number.isFinite(candidate.tunnelBootstrapTtlMs)) {
|
||||
|
||||
@@ -466,4 +466,39 @@ describe('settings helpers', () => {
|
||||
expect(sanitized.recentModels).toEqual(payload.recentModels);
|
||||
});
|
||||
});
|
||||
|
||||
describe('session retention settings persistence', () => {
|
||||
it('round-trips sessionRetentionAction archive and delete through the sanitizer', () => {
|
||||
const helpers = createTestHelpersWithRealSanitizers();
|
||||
|
||||
expect(helpers.sanitizeSettingsUpdate({ sessionRetentionAction: 'archive' })).toEqual({
|
||||
sessionRetentionAction: 'archive',
|
||||
});
|
||||
expect(helpers.sanitizeSettingsUpdate({ sessionRetentionAction: 'delete' })).toEqual({
|
||||
sessionRetentionAction: 'delete',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects invalid sessionRetentionAction values', () => {
|
||||
const helpers = createTestHelpersWithRealSanitizers();
|
||||
|
||||
expect(helpers.sanitizeSettingsUpdate({ sessionRetentionAction: 'remove' })).toEqual({});
|
||||
expect(helpers.sanitizeSettingsUpdate({ sessionRetentionAction: true })).toEqual({});
|
||||
});
|
||||
|
||||
it('survives a full settings payload containing sessionRetentionAction (regression)', () => {
|
||||
const helpers = createTestHelpersWithRealSanitizers();
|
||||
const payload = {
|
||||
autoDeleteEnabled: true,
|
||||
autoDeleteAfterDays: 60,
|
||||
sessionRetentionAction: 'delete',
|
||||
};
|
||||
|
||||
const sanitized = helpers.sanitizeSettingsUpdate(payload);
|
||||
|
||||
expect(sanitized.autoDeleteEnabled).toBe(true);
|
||||
expect(sanitized.autoDeleteAfterDays).toBe(60);
|
||||
expect(sanitized.sessionRetentionAction).toBe('delete');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user