[agents] Add GitHub Copilot CLI installer [v8/v8 : main]
Issack John (Gerrit)
Message from Issack John![]( "Open in Gerrit")
Set Ready For Review
Related details
- Camillo Bruni
- Michael Achenbach
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Issack John (Gerrit)
Issack John added 1 comment![]( "Open in Gerrit")
Related details
- Camillo Bruni
- Michael Achenbach
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Achenbach (Gerrit)
Michael Achenbach added 1 comment![]( "Open in Gerrit")
Do you have a tracker bug for this addition? Was this already discussed anywhere?
Related details
- Camillo Bruni
- Issack John
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
Camillo Bruni added 1 comment![]( "Open in Gerrit")
sorry got sidetracked today, will have a detailed look tomorrow. generally in favour of adding more diversity here 😊
Related details
- Issack John
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Issack John (Gerrit)
Issack John added 2 comments![]( "Open in Gerrit")
Do you have a tracker bug for this addition? Was this already discussed anywhere?
I don't have a tracker bug, but I will create one now and we can discuss there.
sorry got sidetracked today, will have a detailed look tomorrow. generally in favour of adding more diversity here 😊
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Issack John (Gerrit)
Issack John added 1 comment![]( "Open in Gerrit")
Issack JohnDo you have a tracker bug for this addition? Was this already discussed anywhere?
I don't have a tracker bug, but I will create one now and we can discuss there.
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
Camillo Bruni added 10 comments![]( "Open in Gerrit")
Adding marco who's one of the main owners of the agents code.
wheel: <
name: "infra/python/wheels/colorama-py2_py3"
version: "version:0.4.1"
>is this needed for the new code or was this already broken?
COPILOT_INCOMPATIBLE_RULES = {
# This rule requires choosing between Jetski and Gemini CLI and hard-codes
# their adapter paths.
"framework.md",
}
COPILOT_INCOMPATIBLE_SKILLS = {let's add tests that these skills and files actually exist.
"v8-setup",this one is also mentioning additional tools and setup?
def _frontmatter(self, markdown_file: Path) -> dict[str, str]:
"""Extract top-level scalar values used by V8 rule frontmatter."""
try:
lines = markdown_file.read_text(encoding="utf-8").splitlines()
except UnicodeDecodeError:
return {}
if not lines or lines[0].strip() != "---":
return {}
try:
end = next(i for i, line in enumerate(lines[1:], start=1)
if line.strip() == "---")
except StopIteration:
return {}
metadata = {}
for line in lines[1:end]:
if not line or line[0].isspace() or ":" not in line:
continue
key, value = line.split(":", 1)
metadata[key.strip()] = value.strip().strip("\"'")
return metadatanot sure if much better, but might be more portable if we want to share this in scripts helper.py or so
```
FRONTMATTER_PATTERN = re.compile(r"^---\s*\n(.*?)\n---\s*\n", re.DOTALL)
def extract_v8_frontmatter(markdown_file: Path) -> dict[str, str]:
"""Extract top-level scalar values from frontmatter, returning {} if missing."""
content = markdown_file.read_text(encoding="utf-8")
match = FRONTMATTER_PATTERN.match(content)
if not match:
return {}
data = yaml.safe_load(match.group(1)) or {} return {
str(k): str(v)
for k, v in data.items()
if not isinstance(v, (dict, list))
}
``` def _frontmatter_block(self, markdown_file: Path) -> str:Could we use _frontmatter instead here?
if GENERATED_MARKER not in existing:
print(f"Skipping {path}: existing file is not generated")
return Falsecould we simplify this by having a v8 dedicate skill subfolder e.g. .github/skills/v8-generated/ (I'm not fully sure if that works)
# V8 GitHub Copilot Instructions
The canonical V8 agent knowledge lives under `agents/`. Update those checked-in
files rather than this generated adapter.
## Canonical context
For V8 development tasks, read and follow the relevant checked-in files:
- `agents/rules/` for repository rules.
- `agents/skills/` for on-demand workflows.
Prefer V8-native commands over Chromium browser defaults:
- Build: `tools/dev/gm.py quiet <config> <target>`.
- Test: `tools/run-tests.py --progress dots --outdir=out/<config> <test>`.
- Format: `git cl format`.
this feels like something we should use from an existing generic skill ... maybe we need to split the v8 ones so we do not have the jetski / gemini speciif part in there?
On Windows, run Python scripts through Python if executable shims are not on
`PATH`, for example:
`python tools\\dev\\gm.py quiet x64.optdebug d8`.
vpython3 is usually the safer choice? is this an issue on windows?
except OSError as exc:I'd prefer removing most of these defensive OSError handlers. I think it's fine if these errors just bubble up given that they should be very rare.
Probably handling symlinks is the only exception since that's a bit tricky on windows?
Related details
- Issack John
- Marco Vitale
- Michael Achenbach
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Issack John (Gerrit)
Issack John added 2 comments![]( "Open in Gerrit")
Adding marco who's one of the main owners of the agents code.
Good to know, thanks!
if GENERATED_MARKER not in existing:
print(f"Skipping {path}: existing file is not generated")
return Falsecould we simplify this by having a v8 dedicate skill subfolder e.g. .github/skills/v8-generated/ (I'm not fully sure if that works)
I left a comment on the linked bug but. What do you think about moving the canonical skills from `agents/skills/` to `.agents/skills/`? That is a documented cross-client location supported by GitHub Copilot CLI and Gemini CLI, and Jetski already consumes that layout. This would remove the skill-adapter portion of this change.
Related details
- Camillo Bruni
- Marco Vitale
- Michael Achenbach
Issack John (Gerrit)
Issack John added 8 comments![]( "Open in Gerrit")
wheel: <
name: "infra/python/wheels/colorama-py2_py3"
version: "version:0.4.1"
>is this needed for the new code or was this already broken?
This was already broken.
COPILOT_INCOMPATIBLE_RULES = {
# This rule requires choosing between Jetski and Gemini CLI and hard-codes
# their adapter paths.
"framework.md",
}
COPILOT_INCOMPATIBLE_SKILLS = {let's add tests that these skills and files actually exist.
Done
"v8-setup",this one is also mentioning additional tools and setup?
Good catch, the wording was too broad. I updated the comment to explain that these are excluded and their reasons.
def _frontmatter(self, markdown_file: Path) -> dict[str, str]:
"""Extract top-level scalar values used by V8 rule frontmatter."""
try:
lines = markdown_file.read_text(encoding="utf-8").splitlines()
except UnicodeDecodeError:
return {}
if not lines or lines[0].strip() != "---":
return {}
try:
end = next(i for i, line in enumerate(lines[1:], start=1)
if line.strip() == "---")
except StopIteration:
return {}
metadata = {}
for line in lines[1:end]:
if not line or line[0].isspace() or ":" not in line:
continue
key, value = line.split(":", 1)
metadata[key.strip()] = value.strip().strip("\"'")
return metadatanot sure if much better, but might be more portable if we want to share this in scripts helper.py or so
```
FRONTMATTER_PATTERN = re.compile(r"^---\s*\n(.*?)\n---\s*\n", re.DOTALL)
def extract_v8_frontmatter(markdown_file: Path) -> dict[str, str]:
"""Extract top-level scalar values from frontmatter, returning {} if missing."""
content = markdown_file.read_text(encoding="utf-8")
match = FRONTMATTER_PATTERN.match(content)
if not match:
return {}data = yaml.safe_load(match.group(1)) or {}return {
str(k): str(v)
for k, v in data.items()
if not isinstance(v, (dict, list))
}
```
Done
def _frontmatter_block(self, markdown_file: Path) -> str:Could we use _frontmatter instead here?
Done
# V8 GitHub Copilot Instructions
The canonical V8 agent knowledge lives under `agents/`. Update those checked-in
files rather than this generated adapter.
## Canonical context
For V8 development tasks, read and follow the relevant checked-in files:
- `agents/rules/` for repository rules.
- `agents/skills/` for on-demand workflows.
Prefer V8-native commands over Chromium browser defaults:
- Build: `tools/dev/gm.py quiet <config> <target>`.
- Test: `tools/run-tests.py --progress dots --outdir=out/<config> <test>`.
- Format: `git cl format`.
this feels like something we should use from an existing generic skill ... maybe we need to split the v8 ones so we do not have the jetski / gemini speciif part in there?
yeah, I removed the duplicated command guidance and left those details with the canonical V8 skills and rules.
On Windows, run Python scripts through Python if executable shims are not on
`PATH`, for example:
`python tools\\dev\\gm.py quiet x64.optdebug d8`.
vpython3 is usually the safer choice? is this an issue on windows?
Ah no, I missed this. Done.
except OSError as exc:I'd prefer removing most of these defensive OSError handlers. I think it's fine if these errors just bubble up given that they should be very rare.
Probably handling symlinks is the only exception since that's a bit tricky on windows?
Agreed. I removed the general OSError catches and kept handling only for symlink behavior.
Related details
- Camillo Bruni
- Marco Vitale
- Michael Achenbach
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Issack John (Gerrit)
Issack John added 4 comments![]( "Open in Gerrit")
wheel: <
name: "infra/python/wheels/colorama-py2_py3"
version: "version:0.4.1"
>Issack Johnis this needed for the new code or was this already broken?
This was already broken.
More precisely, the new documented Windows invocation exposes a pre-existing gap: the already-pinned Click 8.0.3 explicitly declares Colorama as a Windows dependency. Click 8.5.0 removes that dependency, but it is currently unreleased, so Colorama is still required for the pinned version.
"v8-setup",Issack Johnthis one is also mentioning additional tools and setup?
Good catch, the wording was too broad. I updated the comment to explain that these are excluded and their reasons.
Done
# V8 GitHub Copilot Instructions
The canonical V8 agent knowledge lives under `agents/`. Update those checked-in
files rather than this generated adapter.
## Canonical context
For V8 development tasks, read and follow the relevant checked-in files:
- `agents/rules/` for repository rules.
- `agents/skills/` for on-demand workflows.
Prefer V8-native commands over Chromium browser defaults:
- Build: `tools/dev/gm.py quiet <config> <target>`.
- Test: `tools/run-tests.py --progress dots --outdir=out/<config> <test>`.
- Format: `git cl format`.
Issack Johnthis feels like something we should use from an existing generic skill ... maybe we need to split the v8 ones so we do not have the jetski / gemini speciif part in there?
yeah, I removed the duplicated command guidance and left those details with the canonical V8 skills and rules.
The remaining mixed or client-specific skills are excluded rather than partially adapted. I think splitting or making those skills client-neutral is a good idea.
except OSError as exc:Issack JohnI'd prefer removing most of these defensive OSError handlers. I think it's fine if these errors just bubble up given that they should be very rare.
Probably handling symlinks is the only exception since that's a bit tricky on windows?
Agreed. I removed the general OSError catches and kept handling only for symlink behavior.
Done