mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(cli-registry): address review on GitHub Copilot run mode
- Drop GH_TOKEN / GITHUB_TOKEN from env.allowedKeys. The allowlist is one global list with no mode context, so they were settable on every session. COPILOT_GITHUB_TOKEN is already admitted by the COPILOT_ prefix. - Declare capabilities.mcpConfig (copilot-json, COPILOT_HOME relocation) so Settings does not list Copilot as unsupported while the sync-only target also lists it. The sync-only row is skipped while the run mode is enabled. - Require a leading alphanumeric on the resume id (path-segment in the registry, matching regex in the schema): --resume's value is optional, so `--resume --yolo` made --yolo its own flag and bypassed the allowAll clamp. Golden cases added; "ids only" wording corrected. - Remove the unread overlays.credStore line and document signing in inside a Docker container. - Add the zh-CN "Run GitHub Copilot" string, pin the new declarations in tests and update the MCP registry expectations.
This commit is contained in:
1 parent
c5de8383e7
commit
dc87ef724c
11 files changed
+108
-42
No files matched your search
@@ -1684,13 +1684,15 @@ const COPILOT: CliEntry = {
|
||||
// `--yolo` (allow every tool, path and URL) is opt-in through `allowAll`, which the Run button
|
||||
// sends like the other agent CLIs' bypass switches; an absent config spawns a bare `copilot`
|
||||
// in its own Manual Approval mode. `--model` and `--resume=<id>` are Copilot's own flags
|
||||
// (`copilot --help`, 1.0.94). `--resume` takes an id, id prefix or session NAME, so the id
|
||||
// pattern below is what keeps a name or a path out of it.
|
||||
// (`copilot --help`, 1.0.94). `--resume` takes an id, id prefix or session NAME and its value
|
||||
// is optional, so the token must be a single plain word that cannot start with `-`: a value
|
||||
// like `--yolo` would otherwise be parsed as its own flag and get around the `allowAll` clamp.
|
||||
launch: {
|
||||
params: {
|
||||
allowAll: { type: 'bool' },
|
||||
model: { type: 'token', pattern: 'model' },
|
||||
resumeId: { type: 'token', pattern: 'id-dotted' },
|
||||
// `path-segment`, not `id-dotted`: the leading alphanumeric keeps a flag-shaped value out.
|
||||
resumeId: { type: 'token', pattern: 'path-segment' },
|
||||
continueSession: { type: 'bool' },
|
||||
// The tab's name, so the CLI's own session list reads the same as the tab.
|
||||
sessionName: { type: 'engine', source: 'sessionName' },
|
||||
@@ -1736,12 +1738,14 @@ const COPILOT: CliEntry = {
|
||||
env: {
|
||||
exports: [{ name: 'COLORTERM', value: 'truecolor' }],
|
||||
unset: ['NO_COLOR'],
|
||||
// Sign-in is the CLI's own (`copilot login`, kept under ~/.copilot); a token in the
|
||||
// environment (COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN) is what a headless host uses.
|
||||
// Sign-in is the CLI's own (`copilot login`, kept under ~/.copilot). A headless host sets
|
||||
// COPILOT_GITHUB_TOKEN, which the COPILOT_ prefix already admits. GH_TOKEN / GITHUB_TOKEN are
|
||||
// deliberately NOT allowlisted: the env allowlist is one global list with no mode context, so
|
||||
// admitting them would make them settable on every session (see allowedEnvPrefixes()).
|
||||
tmuxSetenvKeys: [],
|
||||
dockerExecEnvNames: [],
|
||||
allowedPrefixes: ['COPILOT_'],
|
||||
allowedKeys: ['GH_TOKEN', 'GITHUB_TOKEN'],
|
||||
allowedKeys: [],
|
||||
},
|
||||
capabilities: {
|
||||
...agentDefaults(),
|
||||
@@ -1764,6 +1768,13 @@ const COPILOT: CliEntry = {
|
||||
screenLine: String.raw`(?:tab next tab|\? help|esc edit prompt|github-mcp-server) {3,}([A-Za-z0-9][\w.:/@ -]{1,60}?) *(?:\n|$)`,
|
||||
screenLines: 2,
|
||||
},
|
||||
// `copilot mcp add` writes `~/.copilot/mcp-config.json`; COPILOT_HOME replaces ~/.copilot
|
||||
// (checked: `COPILOT_HOME=<dir> copilot mcp list` reads <dir>).
|
||||
mcpConfig: {
|
||||
path: '.copilot/mcp-config.json',
|
||||
format: 'copilot-json',
|
||||
relocation: { envVar: 'COPILOT_HOME', path: 'mcp-config.json' },
|
||||
},
|
||||
// COPILOT_HOME can restate permissions and COPILOT_ALLOW_ALL turns every one on; both
|
||||
// already match the COPILOT_ allowedPrefix, so a non-granted multi-user owner must not
|
||||
// be able to set them through envOverrides.
|
||||
@@ -1794,7 +1805,9 @@ const COPILOT: CliEntry = {
|
||||
},
|
||||
},
|
||||
overlays: {
|
||||
credStore: { rel: '.copilot', seedFiles: ['config.json', 'mcp-config.json'] },
|
||||
// No credStore: Docker seeding is CRED_STORES in docker-hosts.ts, which has no `.copilot` row,
|
||||
// and the sign-in token sits in plain text in config.json on a host without a keyring, so it
|
||||
// is not copied into a container by default. Sign in inside the container instead.
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -4,9 +4,12 @@
|
||||
* `mcpSyncTargets()` (routes/mcp-sync-routes.ts) takes the registry's enabled CLIs that declare an
|
||||
* `mcpConfig`. Some tools read an MCP server list worth keeping in step with the others but are not
|
||||
* something Codeman launches, so they have no registry entry (and no id to branch on): GitHub
|
||||
* Copilot CLI is the first. They are plain data here, take part only when installed or when their
|
||||
* config file already exists (an absent tool is reported `absent`, never created), and sort after
|
||||
* the registry CLIs, so when two definitions of a name differ the registry CLI's is the one copied.
|
||||
* Copilot CLI is the first. It is also a run mode that declares its own `mcpConfig`, so while that
|
||||
* entry is enabled this row is skipped (`mcpSyncOnlyTargets` drops ids the registry already
|
||||
* supplies); it applies when the run mode is disabled but the tool is still on the machine. They are
|
||||
* plain data here, take part only when installed or when their config file already exists (an
|
||||
* absent tool is reported `absent`, never created), and sort after the registry CLIs, so when two
|
||||
* definitions of a name differ the registry CLI's is the one copied.
|
||||
*
|
||||
* @module mcp-sync-targets
|
||||
*/
|
||||
|
||||
@@ -254,6 +254,7 @@
|
||||
'Run Grok': '运行 Grok',
|
||||
'Run DeepSeek': '运行 DeepSeek',
|
||||
'Run OMP': '运行 OMP',
|
||||
'Run GitHub Copilot': '运行 GitHub Copilot',
|
||||
'Run Shell': '运行 Shell',
|
||||
'More tools': '更多工具',
|
||||
'Select AI backend': '选择 AI 后端',
|
||||
|
||||
@@ -10,8 +10,8 @@
|
||||
* only, never env values, headers or file content (a parse failure is reported by position).
|
||||
*
|
||||
* A CLI takes part when it is ENABLED in the registry, declares an `mcpConfig`, and is installed
|
||||
* or already has its config file (Copilot CLI, which is not a registry CLI, takes part when installed
|
||||
* or when its config file exists); one that is enabled but absent from the machine is reported
|
||||
* or already has its config file (Copilot CLI declares its own `mcpConfig`; with its run mode disabled it still
|
||||
* takes part as a sync-only tool when installed or when its config file exists); one that is enabled but absent from the machine is reported
|
||||
* `absent` and never created. Its file is located with this process's env (the env the CLIs
|
||||
* Codeman spawns inherit), so a relocation var such as `CODEX_HOME` is followed.
|
||||
*/
|
||||
@@ -39,8 +39,8 @@ export async function readMcpSyncEnabled(): Promise<boolean> {
|
||||
|
||||
/**
|
||||
* Enabled CLIs that declare an MCP config file, in registry order (first definition wins), then the
|
||||
* sync-only tools (src/mcp-sync-targets.ts: Copilot CLI), which come last so a registry CLI's
|
||||
* definition wins a same-name difference.
|
||||
* sync-only tools (src/mcp-sync-targets.ts: Copilot CLI while its run mode is disabled), which come
|
||||
* last so a registry CLI's definition wins a same-name difference.
|
||||
*/
|
||||
export function mcpSyncTargets(availability: Record<string, boolean>): McpSyncTarget[] {
|
||||
const registry = enabledClis()
|
||||
|
||||
+4
-3
@@ -448,12 +448,13 @@ const CopilotConfigSchema = z
|
||||
.optional(),
|
||||
allowAll: z.boolean().optional(),
|
||||
continueSession: z.boolean().optional(),
|
||||
// Ids only: --resume also matches session NAMES (arbitrary user strings), which this
|
||||
// regex deliberately cannot express.
|
||||
// A single plain word with a leading alphanumeric: --resume also matches session NAMES, so
|
||||
// `my-feature` is accepted, but a value like `--yolo` must not be, because --resume's value is
|
||||
// optional and the next token would be read as its own flag (bypassing the allowAll clamp).
|
||||
resumeSessionId: z
|
||||
.string()
|
||||
.max(100)
|
||||
.regex(/^[a-zA-Z0-9._-]+$/)
|
||||
.regex(/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/)
|
||||
.optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
Reference in new issue
Block a user