Skip to content

feat(harness): add GitHub Copilot CLI harness bundle#295

Open
ptone wants to merge 1 commit into
mainfrom
scion/copilot-harness-dev
Open

feat(harness): add GitHub Copilot CLI harness bundle#295
ptone wants to merge 1 commit into
mainfrom
scion/copilot-harness-dev

Conversation

@ptone

@ptone ptone commented Jun 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds harnesses/copilot/ — a complete harness bundle for the GitHub Copilot CLI (copilot from github/copilot-cli)
  • Uses the container-script provisioner pattern matching existing codex/antigravity/opencode harnesses
  • Implements PAT-based auth via COPILOT_GITHUB_TOKEN environment variable (no credential file needed — Copilot reads auth from env vars directly)
  • Translates MCP servers to Copilot's native format (stdiolocal, sse/streamable-httphttp) in ~/.copilot/mcp-config.json
  • Projects agent instructions to .github/copilot-instructions.md with SCION_MANAGED_BEGIN/END markers to protect injected content
  • Installs Copilot CLI binary from GitHub releases (Node.js SEA — no npm/node dependency at runtime)

Files

File Purpose
config.yaml Harness config: command flags, auth types, MCP mapping, capabilities
provision.py Container-side provisioner: auth, instructions, MCP, settings
capture_auth.py Post-login credential capture for no-auth fallback
Dockerfile Installs copilot-linuxmusl binary on scion-base
cloudbuild.yaml Multi-platform Cloud Build pipeline
home/.bashrc Shell init with c alias
home/.copilot/settings.json Default settings (auto-update off)
README.md Bundle docs, auth guide, known limitations

Test plan

  • Verify python3 -c "import py_compile; py_compile.compile('provision.py')" passes
  • Build Docker image with docker build --build-arg BASE_IMAGE=scion-base:latest .
  • Run copilot --version inside container
  • Test provisioner with mock manifest for api-key auth path
  • Test provisioner with no auth candidates (no-auth fallback)
  • Test MCP server translation (stdio→local, sse→http)
  • Install bundle with scion harness-config install harnesses/copilot
  • End-to-end: scion start --harness copilot --env COPILOT_GITHUB_TOKEN=github_pat_...

Implements harnesses/copilot/ with config.yaml, provision.py,
capture_auth.py, Dockerfile, cloudbuild.yaml, and home/ directory
files. The harness uses the container-script provisioner pattern
with PAT-based auth via environment variable, MCP server translation
(stdio→local, sse/streamable-http→http), and instruction projection
to .github/copilot-instructions.md with managed block markers.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new harness bundle for the GitHub Copilot CLI, including Docker configuration, installation documentation, and scripts for provisioning and credential capture. The review feedback identified several areas where defensive programming should be improved, specifically by adding type validation for JSON data loaded from configuration files and ensuring safer string slicing when handling managed block markers in the instructions file.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +204 to +211
if MANAGED_BEGIN in existing and MANAGED_END in existing:
before = existing[: existing.index(MANAGED_BEGIN)]
after = existing[existing.index(MANAGED_END) + len(MANAGED_END) :]
content = before + managed_block + after
elif existing.strip():
content = managed_block + "\n\n" + existing
else:
content = managed_block + "\n"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If MANAGED_END appears before MANAGED_BEGIN in the file (e.g., due to manual user edits or corruption), using .index() directly without checking their relative order will result in incorrect slicing and scrambled content. We should check that begin_idx < end_idx before slicing.

Suggested change
if MANAGED_BEGIN in existing and MANAGED_END in existing:
before = existing[: existing.index(MANAGED_BEGIN)]
after = existing[existing.index(MANAGED_END) + len(MANAGED_END) :]
content = before + managed_block + after
elif existing.strip():
content = managed_block + "\n\n" + existing
else:
content = managed_block + "\n"
begin_idx = existing.find(MANAGED_BEGIN)
end_idx = existing.find(MANAGED_END)
if begin_idx != -1 and end_idx != -1 and begin_idx < end_idx:
before = existing[:begin_idx]
after = existing[end_idx + len(MANAGED_END):]
content = before + managed_block + after
elif existing.strip():
content = managed_block + "\\n\\n" + existing
else:
content = managed_block + "\\n"

Comment on lines +372 to +377
settings: dict[str, Any] = {}
if os.path.isfile(settings_path):
try:
settings = _load_json(settings_path) or {}
except (OSError, json.JSONDecodeError):
settings = {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If _load_json returns a non-dictionary (e.g., a JSON list, string, or boolean), settings will not be a dictionary. This will cause a TypeError later when iterating over defaults.items() or trying to assign keys to settings. We should defensively verify that the loaded JSON is a dictionary.

    settings: dict[str, Any] = {}
    if os.path.isfile(settings_path):
        try:
            loaded = _load_json(settings_path)
            if isinstance(loaded, dict):
                settings = loaded
        except (OSError, json.JSONDecodeError):
            pass

Comment on lines +405 to +414
candidates: dict[str, Any] = {}
if os.path.isfile(auth_candidates_path):
try:
candidates = _load_json(auth_candidates_path) or {}
except (OSError, json.JSONDecodeError) as exc:
print(
f"copilot provision: invalid auth-candidates.json: {exc}",
file=sys.stderr,
)
return EXIT_ERROR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If _load_json returns a non-dictionary (e.g., a JSON list or string), candidates will not be a dictionary. This will cause an AttributeError when calling candidates.get(...) later. We should defensively verify that the loaded JSON is a dictionary.

Suggested change
candidates: dict[str, Any] = {}
if os.path.isfile(auth_candidates_path):
try:
candidates = _load_json(auth_candidates_path) or {}
except (OSError, json.JSONDecodeError) as exc:
print(
f"copilot provision: invalid auth-candidates.json: {exc}",
file=sys.stderr,
)
return EXIT_ERROR
candidates: dict[str, Any] = {}
if os.path.isfile(auth_candidates_path):
try:
loaded = _load_json(auth_candidates_path)
if not isinstance(loaded, dict):
raise ValueError("JSON is not an object")
candidates = loaded
except (OSError, json.JSONDecodeError, ValueError) as exc:
print(
f"copilot provision: invalid auth-candidates.json: {exc}",
file=sys.stderr,
)
return EXIT_ERROR

Comment on lines +58 to +66
with open(config_path, "r", encoding="utf-8") as f:
try:
data = json.load(f)
except (json.JSONDecodeError, OSError):
return []
creds = data.get("credentials")
if not isinstance(creds, list):
return []
return creds

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If json.load returns a non-dictionary (e.g., a JSON list or string), data will not be a dictionary. This will cause an AttributeError when calling data.get("credentials"). We should defensively verify that data is a dictionary.

Suggested change
with open(config_path, "r", encoding="utf-8") as f:
try:
data = json.load(f)
except (json.JSONDecodeError, OSError):
return []
creds = data.get("credentials")
if not isinstance(creds, list):
return []
return creds
with open(config_path, "r", encoding="utf-8") as f:
try:
data = json.load(f)
except (json.JSONDecodeError, OSError):
return []
if not isinstance(data, dict):
return []
creds = data.get("credentials")
if not isinstance(creds, list):
return []
return creds

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.

1 participant