Skip to content

Add 仓库透视 Repo X-Ray (repo-xray) - #30

Merged
MyPrototypeWhat merged 4 commits into
MiniMax-AI:mainfrom
yujiachen-y:add-repo-xray
Oct 20, 2026
Merged

MyPrototypeWhat merged 4 commits into
MiniMax-AI:mainfrom
yujiachen-y:add-repo-xray

Conversation

@yujiachen-y

@yujiachen-y yujiachen-y commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

仓库透视 Repo X-Ray (repo-xray)

A Mini App that explains an unfamiliar codebase from its Git history before you read any code. Add a GitHub or GitLab repository, or pick one on your machine, and it turns the commit log into a code map you can drill into (coloured by change heat, bug-fix share, owner, last change, knowledge loss, or file type), a short list of high-risk files that are both change hotspots and bug magnets, owners and knowledge loss, commit rhythm and firefighting, and the tech stack. ⌘K search, a Markdown export in the codebase-recon-skill report format, light and dark themes. Interface in Chinese.

preview

Data & access

  • Writes only inside context.dataDir: config.json (repository list and settings, including any access token pasted in Settings, in plain text), reports/<id>.json, and repos/<id>.git (bare, blob-less clones of remote repositories). JSON files are written with mode 0600. Local repositories are read in place with read-only git commands and never modified.
  • Network: only git, over HTTPS, to the host of each remote repository the user adds (clone, fetch, ls-remote), through a proxy if one is set (Settings, then the HTTPS_PROXY-style variables, then the macOS system proxy). The page loads only its own same-origin files and /api/ routes. The local server answers 403 unless Host is a loopback name and Origin, when sent, is the page itself. No telemetry.
  • Processes, all started with an argument array and never through a shell: git (inherited GIT_DIR-style variables and GIT_TRACE* removed); gh auth token (for github.com and hosts in gh's hosts.yml) and glab config get token to reuse an existing login when those CLIs are installed; /usr/sbin/scutil --proxy on macOS. Addresses and paths the user enters are validated (host names included) and passed as separate arguments, never in an option position.
  • Tokens stay in the Node process and reach git as an http.extraHeader through GIT_CONFIG_* environment variables, never in arguments, repository config, the page, or logs. While a token is attached, git does not follow redirects (http.followRedirects=false), so the token only goes to the host the user added. Credential helpers and interactive prompts are turned off.
  • No Agent turns, no agentAccess, no MCP endpoints. "发到对话…" and the "让 Agent …" actions use window.mcode.miniapp.message.append, so the Host asks for confirmation and the user sends the text. Declares surface.window (1360×900).
  • localStorage keeps only the theme, the last opened repository, and the identity merge preference.

Tested environment

  • MiniMax Code 3.1.3-preview.4675 on macOS 27 with Git 2.54. Installed through the Host publish flow (also re-installed directly from this repository directory), opened in the right panel.
  • Exercised: a public GitHub repository, a self-hosted GitLab repository using the glab login, and local repositories, including re-analysis; every tab; every code-map colouring, size, and time option, drilling in and out with the mouse and the keyboard; the ⌘K search; the Markdown export; light and dark themes. No console errors or failed requests.
  • Not exercised: the pop-out window (surface.window); the append into the conversation input, which asks the Host for confirmation; prefers-reduced-motion; Windows and Linux.

Checks


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@MyPrototypeWhat MyPrototypeWhat left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the submission — this is a genuinely useful tool, and clearly a lot of care went into security. I ran npm run check on the merged tree (Node 22: repo-xray: OK, 69/69 pass), then set up two local HTTPS git servers (git http-backend) for end-to-end tests. Confirmed OK: every git call uses an argument array and remote URLs are always https://…; inputs like --upload-pack=, ext::, file::, -oProxyCommand are either rejected or never land in an option position; clones are pinned to dataDir/repos/<hash>.git; the token is not in argv, the clone's config, FETCH_HEAD, reports, /api/state, SSE, logs, or error messages; local repositories only see read-only commands. A malicious repo with HTML in file names, authors, commit messages, and README, walked through every view and ⌘K, produced no XSS.

Required changes

  1. After a redirect, the token is sent to another host. I stored a token for 127.0.0.1:18443 and added a repo URL that 302s to 127.0.0.2:18444. The clone succeeded, and both POST …/git-upload-pack requests the second host received carried Authorization: Basic base64(oauth2:<token>). curl strips the auth header only on the first hop; after that git switches to the new base URL, and http.extraHeader applies to every request. I also tried the URL-scoped http.https://<host>/.extraHeader form — it still leaks (git matches on the original URL). Only http.followRedirects=false stops it. Please add http.followRedirects=false in netEnv(); if you need to support rename redirects, probe the redirect target without the token first and only attach the token once it's confirmed to be the same host. Once fixed, the README line "tokens stay in the Node process" will hold.
  2. config.json, which holds the plaintext token, is mode 0644. writeJsonAtomic writes with the default mode; measured config.json is 644. We don't control where dataDir lives or its permissions, so please at least write config.json with { mode: 0o600 } (create the temp file with that mode; rename keeps it).
  3. The local server doesn't validate Host and Origin (repo standard). Without the Host credential layer, Host: attacker.example can read /api/state and full reports, and a cross-site text/plain POST can change the proxy or add token hosts. Since 3.1.2 the Host's loopback credential already blocks these, so the risk is low, but the repo requires an app-level check too: Host must be a loopback name, and Origin, when present, must be http://<loopback>:<listen.port>; otherwise return 403. cq-viewer's implementation is a good reference.

Suggestions (optional)

  • git() inherits GIT_DIR and similar variables from process.env. I tested it: if the Host process has GIT_DIR set, config remote.origin.fetch is written into that repository instead of your own clone. Suggest stripping these variables from the env.
  • Host names in git@ and ssh:// forms aren't validated: git@github.com@evil.example:a/b is detected as GitHub but actually connects to evil.example. Tokens are matched by exact host name, so I couldn't reproduce a leak, but suggest validating with the same host-name regex as the settings page, and only giving the gh token to github.com or hosts gh is logged in to.
  • readBlobs output is unbounded; consider filtering oids by size first, then reading with --batch.
  • When deleting a repo, consider waiting for any running job to actually finish before rm-ing the clone directory.
  • This PR and #27 will conflict on one line of the root README; you already said you'd rebase — thanks.

const pairs = [];
if (token) {
const user = target.provider === 'github' ? 'x-access-token' : 'oauth2';
pairs.push(['http.extraHeader', `Authorization: Basic ${Buffer.from(`${user}:${token}`).toString('base64')}`]);

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

An unscoped http.extraHeader plus git's default followRedirects=initial: in my test, after a 302 to another host the token shows up in the follow-up git-upload-pack POSTs. Switching to http.<url>.extraHeader doesn't stop it either (git matches on the original URL). Please add ['http.followRedirects', 'false'] here.

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.

已在 9c6e3a4 修复:带令牌时 netEnv() 加上 http.followRedirects=false。

按你的方式复现过:127.0.0.1 302 到 localhost 上的 git http-backend(不同主机),用这里的 git() + netEnv() 跑 ls-remote。修复前,第二台主机收到 GET /info/refs(无认证头)和 POST /git-upload-pack(带令牌);修复后,第二台主机收不到任何请求,界面提示"服务器要求跳转到另一个地址…请用新地址重新添加"。

没有做自动跟随改名:带令牌时不去探测跳转目标,改名或迁移的仓库需要用新地址重新添加,README 写明了。不带令牌时(公开仓库、未登录)仍是 git 默认的 initial,因为没有凭据可泄露。

export async function writeJsonAtomic(file, value) {
await mkdir(dirname(file), { recursive: true });
const tmp = `${file}.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2)}.tmp`;
await writeFile(tmp, JSON.stringify(value));

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

config.json holds the plaintext token, and the file written here is mode 0644 in my test. Suggest writeFile(tmp, …, { mode: 0o600 }), at least for config.json.

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.

已修复(9c6e3a4):writeJsonAtomic 用 { mode: 0o600 } 创建临时文件,rename 后保留,config.json 和 reports/*.json 都生效。启动时还会对已有的 config.json 做一次 chmod 600,老版本写出的 644 文件也会收紧。

实测:预先写成 644 → 启动后 600 → 保存设置后仍是 600。

}

await resolveProxy();
const server = createServer((req, res) => {

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Per the repo standard, please check before handle that Host is a loopback name and that Origin (if present) equals http://<loopback>:${context.listen.port}. Without the Host credential layer, Host: attacker.example can read /api/state and reports, and a cross-site text/plain POST /api/settings can change the proxy.

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.

已修复(9c6e3a4):handle 最前面加了 isLoopbackRequest,照 cq-viewer 的写法:Host 必须是 127.0.0.1 / localhost / ::1;带 Origin 时必须是 http://<loopback>:<listen.port>(null 也拒绝)。否则返回 403。

实测:Host: attacker.example 读 /api/state 和 /dashboard 都是 403;Origin: http://evil.example 和 Origin: null 的 text/plain POST /api/settings 是 403;本页面的请求(含 Origin: http://localhost:<port>)照常 200。真实宿主里重新同步了两个远程仓库,没有出现 4xx。

export function git(args, opts = {}) {
return new Promise((resolvePromise, reject) => {
const env = {
...process.env,

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: ...process.env carries GIT_DIR and similar variables. In my test, with GIT_DIR set, config remote.origin.fetch was written into a different repository. Suggest stripping GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_CONFIG_PARAMETERS, etc.

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.

已采纳(9c6e3a4):git() 不再整份继承 process.env。去掉的变量:

  • git rev-parse --local-env-vars 列出的全部变量(GIT_DIR、GIT_WORK_TREE、GIT_INDEX_FILE、GIT_OBJECT_DIRECTORY、GIT_ALTERNATE_OBJECT_DIRECTORIES、GIT_CONFIG_PARAMETERS、GIT_CONFIG_COUNT、GIT_COMMON_DIR 等);
  • 编号的 GIT_CONFIG_KEY_n / GIT_CONFIG_VALUE_n;
  • GIT_TRACE*,避免请求被打印出去。

PATH、HOME、代理和 CA 相关变量照常传递。实测:设置 GIT_DIR 后,修复前 config 写进了 GIT_DIR 指向的仓库,修复后写回自己的克隆。

let host;
let path;
let m;
if ((m = input.match(/^git@([^:/\s]+):(.+)$/))) {

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: [^:/\s]+ allows @ and #, so git@github.com@evil.example:a/b is detected as provider=github but actually connects to evil.example. Suggest validating host names consistently with the regex at server.mjs:552.

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.

已采纳(9c6e3a4):parseTarget 对所有形式统一用设置页那条主机名正则。它提到 git.mjs 里导出为 HOST_RE,设置页也改用它。git@github.com@evil.example:a/b 和 ssh://git@github.com@evil.example/a/b 现在都会被拒绝;修复前两者都被识别成 provider=github,实际连到 evil.example。

gh auth token 也只用于 github.com 和 gh 的 hosts.yml 里登录过的主机。否则,如果环境里设置了 GH_ENTERPRISE_TOKEN,gh 会把它交给任意非 github.com 主机。

env: { GIT_NO_LAZY_FETCH: '1' },
signal,
timeout: 120_000,
onStdout: (c) => chunks.push(c),

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: stdout collected here has no cap (not bound by maxBuffer), so a huge blob gets read fully into memory. You could filter by size with --batch-check first.

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.

已采纳(9c6e3a4):先用 cat-file --batch-check="%(objectname) %(objecttype) %(objectsize)" 拿到类型和大小,只把小于 512KB 的 blob 交给 --batch。

实测一个 40MB 的 blob 和一个小清单文件:修复前读取时峰值多占约 80MB,修复后约 0MB,返回结果相同(只有小文件)。

- `gh auth token --hostname <host>` for GitHub hosts and `glab config get token --host <host>` for
hosts in `glab`'s config, only when those CLIs are installed, to reuse your existing login.
- `/usr/sbin/scutil --proxy` on macOS, only when no proxy is set in Settings or the environment.
- Credentials: tokens from `gh`, `glab`, or Settings stay in the Node process. They reach git as an

@MyPrototypeWhat MyPrototypeWhat Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This paragraph only holds once followRedirects is fixed. You could also state that config.json is saved with 0600 permissions.

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.

已更新中英文 README(9c6e3a4):

  • Credentials:带令牌时 git 不跟随 HTTP 跳转(http.followRedirects=false),令牌只会发给你添加的那台主机;会跳转的仓库需要用新地址重新添加。
  • Files written:JSON 文件的权限是 0600。
  • 补充了本地服务的 Host/Origin 校验(否则 403)、被去掉的 git 环境变量、读取 gh 的 hosts.yml 主机名,以及 gh auth token 的适用主机。

@yujiachen-y

Copy link
Copy Markdown
Contributor Author

@MyPrototypeWhat 谢谢这么细致的 review,三个必改项和四个可选项都已在 9c6e3a4 处理,每条的复现对照写在对应的行内回复里。

  • 必改:带令牌时 http.followRedirects=false;JSON 文件以 0600 写入,启动时收紧已有的 config.json;每个路由都校验 Host 和 Origin(照 cq-viewer)。
  • 可选:去掉继承的 GIT_DIR 等仓库变量和 GIT_TRACE*;所有地址形式统一校验主机名,gh 令牌只用于 github.com 和 hosts.yml 里的主机;readBlobs 先按大小过滤;删除仓库时先等正在跑的任务结束(git 退出、最后一次写文件完成),再删克隆和报告。最后这一条的竞态在本地没能稳定复现,属于按构造修复;实测分析进行中删除仓库,返回很快,也没有残留文件。
  • 验证:
    • 在修复前的提交上,用同一个脚本先复现了重定向泄露令牌、Host/Origin、0644、GIT_DIR、主机名这几项,修复后全部转为预期结果。
    • npm run check(Node 22.22.2):repo-xray: OK,69/69 通过。
    • 在 MiniMax Code 里重新同步了一个公开 GitHub 仓库(带 gh 令牌)和一个自建 GitLab 仓库(带 glab 令牌):都成功,没有 4xx。
  • 另外 ec4cbd6 是个和 review 无关的界面修复:欢迎页的 icon 之前会随栏宽一直放大到 880px,现在固定为 56px。
  • 和 Add 心屿 Xinyu Island (xinyu-daily) #27 的根 README 冲突:哪个先合,我就 rebase 另一个。

@MyPrototypeWhat MyPrototypeWhat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the thorough follow-up. I re-ran my repros against 9c6e3a4:

  • Redirect with a token: fixed. A 302 from host A to host B now ends with the new redirect error, and B receives no requests at all. Direct clones still authenticate normally.
  • File modes: fixed. A pre-existing 0644 config.json becomes 0600 on start, and new config.json / reports/*.json writes are 0600.
  • Host/Origin: fixed. A foreign Host gets 403 on /api/state, /dashboard and the report route. Cross-site, null and wrong-port Origins get 403 on POST/DELETE. Same-page requests still get 200.
  • Optional items: also confirmed. Inherited GIT_DIR/GIT_CONFIG_*/GIT_TRACE* no longer reach git. git@github.com@evil.example:a/b and the ssh:// variant are rejected. gh auth token is only called for github.com and hosts in hosts.yml, even with GH_ENTERPRISE_TOKEN set. readBlobs never streams the 40 MB blob.
  • npm run check passes on Node 22 (69/69).

One thing left, from the delete change. DELETE /api/repos/:id now awaits job.done. Aborting kills only the direct git child, but git-remote-https inherits its stdout/stderr pipes, so close doesn't fire until the helper exits. Against a remote that accepts the connection and never answers, DELETE hadn't returned after 20 s, the clone directory was still there, and an orphaned git-remote-https kept running. Before this commit, DELETE returned immediately. Could you make the abort reach the whole process tree (e.g. spawn(..., { detached: true }) + process.kill(-child.pid, 'SIGKILL') on POSIX), or settle on exit after a kill instead of close, and/or cap the wait (e.g. Promise.race with a few seconds)? The same kill path is used by dispose().

Minor: config is saved only after the wait, so a crash during the wait brings the deleted repo back. Saving before awaiting would avoid that.

The root README rows still conflict with #27. Whichever lands second will need a rebase, as you said.

// Let a running job's git exit and the job write its last file before deleting them.
const job = jobs.get(repo.id);
job?.ac.abort();
await job?.done;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can now block indefinitely: against a remote that accepts the TLS connection and never responds, DELETE hadn't returned after 20 s. ac.abort() SIGKILLs only git, while git-remote-https keeps the stdout/stderr pipes open, so job.done waits for the network. Please bound this wait (e.g. await Promise.race([job.done, sleep(3000)])), or make the kill reach the helper (see git.mjs:67). Also consider calling saveConfig() before the wait.

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.

已修复(2396f55):

  • 进程组的 kill 让 job.done 在中止后马上结束。
  • saveConfig() 挪到等待之前,等待中崩溃也不会把删掉的仓库带回来。
  • 等待再加 5 秒上限:Promise.race([job.done, sleep(5000)])。上限只兜底那几个不接收 signal 的步骤(gh、glab、scutil 查询,各自有 10 秒超时)。

同一个"卡住的远端"复现:修复前,DELETE 10 秒没有返回,克隆目录和 git-remote-https 都还在,dispose() 之后 helper 仍残留;修复后,DELETE 18 ms 返回 200,克隆目录已删,helper 为 0。正常的本地分析不受影响,MiniMax Code 里重新同步 GitHub 仓库也正常。

}, opts.timeout ?? 120_000);
const onAbort = () => {
killedFor = 'aborted';
child.kill('SIGKILL');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Killing only the direct child leaves git-remote-https (and git-upload-pack/ssh) running and holding the pipes, so close at line 90 doesn't fire until the remote answers. With the new await job.done in DELETE, that becomes a hang. Suggest detached: true at spawn and process.kill(-child.pid, 'SIGKILL') (POSIX), with the current child.kill as the Windows fallback.

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.

已修复(rebase 后的提交是 2396f55):按你的建议,POSIX 上 spawn(..., { detached: true }),abort、timeout、overflow 三条路径和 dispose() 都改用 killTree(),即 process.kill(-child.pid, "SIGKILL");Windows 和非进程组长的子进程(gh、glab、scutil)退回 child.kill。另外 signal 已经 aborted 时 git() 直接 reject、不再启动进程,这样中止后恢复执行的任务不会再起新的 git。

复现方式:一个只接受连接、从不回话的代理,让 git-remote-https 卡在 CONNECT。修复前,abort 后 8 秒仍未结束,helper 一直在;修复后,abort 后约 20–40 ms reject,helper 为 0。ps 显示 git clone、git remote-https、git-remote-https 同属一个 pgid,一次就全部杀掉。

Understand a codebase from its Git history: a code map you can drill into,
hotspots and bug magnets, high-risk files, owners and knowledge loss, commit
rhythm and firefighting, and the tech stack, for GitHub, GitLab, and local
repositories.

Analysis runs locally with the user's git: blob-less bare clones inside
context.dataDir, network only to the repository hosts the user adds, tokens
kept in the Node process, no Agent turns.
The welcome page put the icon in a .brand-mark <span> with no display
value. Inside the sidebar's flex row that span is blockified, so its
size applies; on the welcome page it stayed inline, its width was
ignored, and the image's width: 100% filled the whole column (up to
880px). .brand-mark is now inline-block, and the welcome icon has a
fixed 56px size in CSS instead of an inline style.

Also shorten the input placeholder so it fits the right panel next to
the sidebar (the focused input clipped the last characters), and add
text-overflow: ellipsis for when the input is not focused.
…/Origin check

- netEnv(): with a token attached, set http.followRedirects=false. The
  unscoped http.extraHeader went with every request, and after a
  redirect git kept talking to the new host, so a 302 to another host
  received the token on POST git-upload-pack. A redirect now fails with
  a message asking for the repository's new address.
- writeJsonAtomic() creates files with mode 0600 (config.json holds
  pasted tokens); start() also tightens an existing config.json.
- Every route answers 403 unless Host is a loopback name and Origin,
  when sent, is this listener (same check as cq-viewer).
- git() drops inherited GIT_DIR and the rest of
  `git rev-parse --local-env-vars`, GIT_CONFIG_KEY_n/VALUE_n, and
  GIT_TRACE*.
- parseTarget() validates the host with the Settings host rule for every
  form, so "git@github.com@evil.example:a/b" is rejected; gh tokens are
  only used for github.com and hosts in gh's hosts.yml.
- readBlobs() checks sizes first and reads only blobs under 512KB.
- Removing a repository waits for its running job to finish before
  deleting the clone and report.
- README (en, zh-CN): Data & access updated to match.
…wait

Aborting killed only the direct git child. git-remote-https shares its
stdout/stderr pipes, so 'close' waited until the remote answered: with
a remote that accepts the connection and never responds, DELETE (which
now awaits the job) hung and the helper was left running, also after
dispose().

- git() spawns git in its own process group on POSIX (detached) and the
  abort, timeout, and overflow paths kill the group (killTree); Windows
  keeps the plain kill. dispose() uses the same kill.
- git() rejects at once when its signal is already aborted, so a job
  that resumes after an abort never starts another git command.
- DELETE saves the config before waiting, and waits at most 5 s: the
  cap bounds the gh / glab / scutil lookups, which do not take the
  signal.
@yujiachen-y

Copy link
Copy Markdown
Contributor Author

@MyPrototypeWhat 谢谢复测。删除路径的问题已修复,并且已经 rebase 到最新 main(1e37335):

  • 2396f55:git 在 POSIX 上自成进程组,abort、timeout、overflow 和 dispose() 都改为杀整个进程组。signal 已中止时不再启动 git。DELETE 先保存配置再等待,等待最多 5 秒。复现对照写在两条行内回复里。
  • Rebase:Add 心屿 Xinyu Island (xinyu-daily) #27 等 PR 合入后,根 README 表格有冲突。解决方式是上游的行全部保留,Repo X-Ray 放在两个表格的最后一行;git diff origin/main 里每个 README 只多这一行。
  • npm ci && npm run check(Node 22.22.2):repo-xray: OK,69/69 通过。剩下的 warning 属于其他包。

@MyPrototypeWhat MyPrototypeWhat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the quick follow-up and the detailed repro notes. I re-checked the two items from my previous review (5566642142) on 2396f55:

  • Process group kill — git() now spawns with detached: true on POSIX, and the abort, timeout and overflow paths plus dispose() all go through killTree() (process.kill(-pid, 'SIGKILL'), falling back to child.kill on Windows / non-group children). This addresses the lingering git-remote-https holding the pipes.
  • No spawn after abort — git() rejects immediately when opts.signal is already aborted, so a resumed job can't start a new git.
  • DELETE — saveConfig() now runs before the wait, and the wait is bounded with Promise.race([job.done, sleep(5000)]), so DELETE can no longer hang.

npm run check passes locally (69/69, repo-xray: OK). The earlier fixes (followRedirects, 0600, Host/Origin check, env stripping, host regex, blob size filter) are still in place.

Two optional follow-ups, not blocking:

  1. If the 5 s cap ever expires while a job is still finishing, that job can still write reports/<id>.json after the DELETE removed it, leaving an orphan report for a repo that no longer exists. A cheap guard is to skip the report write when ac.signal.aborted (or when the repo id is no longer in config.repos).
  2. Because the git children are now detached, they won't be killed with the runtime's process group if the runtime dies abruptly without running dispose(). That's a reasonable trade-off; a one-line comment near the spawn would help future readers.

// the cap only bounds steps that do not take the signal (the gh / glab / scutil lookups).
const job = jobs.get(repo.id);
job?.ac.abort();
if (job?.done) await Promise.race([job.done, sleep(5000)]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: after this cap times out, the job may still reach writeJsonAtomic(reportFile(...)) and recreate the report you delete two lines below. Consider checking ac.signal.aborted / repo existence before writing the report.

LC_ALL: 'C',
...opts.env,
};
const child = spawn('git', args, { cwd: opts.cwd, env, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true, detached: process.platform !== 'win32' });

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice. Optional: a short comment that detached children survive an abrupt runtime crash (only dispose() reaps them) would document the trade-off.

@MyPrototypeWhat
MyPrototypeWhat merged commit bb4c2f5 into MiniMax-AI:main Oct 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants