Repository navigation
Conversation
| */ | ||
| function partitionByPresence(git, sha, files) { | ||
| if (files.length === 0) return { present: [], removed: [] } | ||
| const names = new Set(git(["ls-tree", "-r", "--name-only", sha, "--", ...files]).split("\n").filter(Boolean)) |
There was a problem hiding this comment.
WARNING: A renamed doc file leaves the old path in the surface branch
changedFiles comes from git diff --name-only origin/main...HEAD, and with default rename detection that collapses a rename to only the destination path (I verified locally: a historical rename shows the new path only, while --no-renames shows both old and new). Because partitionByPresence only ever sees that collapsed list, the old path is classified neither present nor removed, so git rm never runs for it. The rebuilt surface branch therefore keeps the stale file that the integration tree deleted, and merging the PR leaves the old doc in place - the rename is only half-applied.
Consider building the changed-file set with --no-renames (or otherwise unioning the deleted/old-path side of renames) so renamed-away paths are still passed to git rm.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // The legacy rolling PR is superseded: its branch content already went into | ||
| // the integration base above. Comment and close it (best effort) so exactly | ||
| // one PR per surface plus `other` remain. | ||
| if (legacy) { |
There was a problem hiding this comment.
WARNING: Legacy branch cleanup depends on an open legacy PR
The docs/auto-sync ref is deleted (and its stale remote-tracking ref cleared) only inside if (legacy) {. If the old rolling branch outlives its PR - for example the PR was closed or merged manually, the branch was left behind by an auto-delete-off setting, or the delete below failed on an earlier run - then legacy is null, no cleanup happens, and because a git ref may not be a path prefix of another ref, every push of docs/auto-sync/<surface> fails with a directory/file ref conflict. That silently disables all per-surface PRs on every subsequent run. Detecting and deleting the remote docs/auto-sync branch independently of an open legacy PR would make the migration idempotent.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } catch (err) { | ||
| // 404 = not a collaborator; skip and try the next candidate. | ||
| if (err?.status === 404) continue | ||
| throw err |
There was a problem hiding this comment.
WARNING: A non-404 permission error discards reviewers already found
The permission lookup rethrows any error that is not a 404, and the outer catch at the end of computeSurfaceReviewers replaces the whole result with reviewers: []. So if the first ranked candidate already had write access and was pushed, but the next candidate's permission call fails (403, secondary rate limit, or a 5xx after the retries in lib.mjs), the accumulated reviewer is thrown away and the PR is created with no assignee and no requested reviewer. Returning the reviewers found so far (and naming the failure in note) would degrade more gracefully.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (15 files)
Fix these issues in Kilo Cloud Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
|
Superseded by #14093, which now targets main directly. This branch was an independent re-implementation of the same docs-sync change, not an increment on #14085, so GitHub read it as CONFLICTING against its own base. Four merge-conflict fix attempts were spent on it (the cap) without resolving it, and the stack above it could not move. #14093 is a single branch off main that carries the whole feature — the surface map, per-surface PRs, the reviewer ranking, the model config, the lockfile change and the orphan patch removal — plus this branch's scope.test.mjs and the new cloud product surfaces. It merges into main with no conflict. |
Changelog for users
cli,vscode,jetbrains,gateway,web— plus oneother, in place of the single rolling PR.otherkeeps its fixed reviewer pair because it has no source paths to rank.other.kilo runnow uses maximum reasoning effort by default, overridable through repository variables.Changelog for maintainers
bun installprunespatches/ghostty-web@0.3.0.patch: no dependency is patched with it, so the orphan file is removed.trustedDependenciesandpatchedDependenciesorder inbun.lockis the package manager's;bun installrewrites those lists..github/docs-sync/surfaces.jsonis the surface map;surfaces.mjsassigns each doc path to exactly one surface by longest prefix match, withotherlast.reviewers.mjsranks contributors by recency with a 180-day half-life, skips bots and users below write permission, and returns no reviewers plus a named reason on failure instead of throwing. The singleDOCS_OWNERconstant is gone.upsert-pr.mjsrebuilds each surface branch in a throwaway worktree, carries existing PR rows forward, and isolates per-surface failures so one surface cannot lose another's changes or stall the watermark.prepare-branch.mjsmerges open surface branches intodocs/auto-sync, then comments on and closes the legacy rolling PR; because a git ref cannot be a path prefix of another, the durable integration tree is pushed todocs/auto-sync-integration.kilo/deepseek/deepseek-v4.1-flash, setsDOCS_SYNC_VARIANT: "max", passes--varianton everykilo run, and runs the.github/docs-sync/*.test.mjsunit tests in the selftest job.prepare-branch.mjsandupsert-pr.mjs, and theotherfallback insurfaces.json. Selftest cases 12–16 cover segmentation, failure isolation, prepare-branch, update, and deletion;scope.test.mjsguards that the ghostty-web patch is orphaned without pinning a blob hash.E2E proof
node .github/docs-sync/selftest.mjsagainst a fixture change set, with the GitHub API served by a local stub (no real pull request or review request is created):Owner request