Skip to content
Closed
22 changes: 22 additions & 0 deletions skills/github-software-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,25 @@ Use `python3 main.py --token-env FACTORY_ROLE_GRANT` as the entrypoint, with the
actual name from `config.json` substituted. Naming the secret lets the SDK inject
it from the selected profile without shell expansion; the CLI rejects a name that
does not match the bundle. The same command works in local and Docker workspaces.

## Parallel developer lanes

Install one developer automation per lane, using the existing dispatcher to keep
at most one run active per automation. Set `developer_lanes` to the same positive
count in every developer bundle and `developer_lane` to a distinct zero-based
index. Lane ownership is `issue_number % developer_lanes`. Keep this assignment
fixed while work is active. Each lane permits one outstanding implementation PR;
other lanes can work while it awaits review. Local and Docker bundles are identical.

Use `Depends on: #1, #2` on an issue to block triage/development until those issues
are closed as completed. Dependency references are repository-local. Configure the
existing service concurrency and Docker memory/CPU limits separately; no extra
scheduler or distributed lock service is introduced.

An accepted PR whose base has advanced is refreshed with GitHub's native
update-branch endpoint. This produces a new head that must pass independent review
again. Stale expected heads are re-read on the next sweep. Merge conflicts receive
a visible PR comment and enter the existing canonical developer revision workflow
with current-base context. The revised head must pass independent review before
the next native update attempt. Missing dependency references block only their
issue; dependency reads are cached within each sweep.
81 changes: 77 additions & 4 deletions skills/github-software-factory/scripts/main.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""One scheduled software-factory role, executed in an isolated runtime."""

import argparse
from functools import cache
import base64
import io
import importlib.util
Expand Down Expand Up @@ -165,6 +166,7 @@ def triage():
i
for i in open_issues()
if "ready-for-dev" not in {label["name"] for label in i["labels"]}
and dependencies_complete(i)
]
if not issues:
return
Expand Down Expand Up @@ -304,13 +306,82 @@ def publish(issue, base, branch, existing, local_base):
)


@cache
def completed_dependency(number):
try:
dependency = gh("GET", f"/issues/{number}")
except HTTPError as exc:
if exc.code == 404:
return False
raise
return (
dependency["state"] == "closed"
and dependency.get("state_reason") == "completed"
)


def dependencies_complete(issue):
"""Honor explicit Depends on lines; unknown/incomplete issues remain blocked."""
for line in re.findall(
r"^Depends on:\s*(.+)$", issue.get("body") or "", re.M | re.I
):
for number in re.findall(r"#(\d+)", line):
if not completed_dependency(number):
return False
return True


def developer():
prs = gh("GET", "/pulls?state=open&per_page=100")
existing = None
lane = CONFIG.get("developer_lane", 0)
lanes = CONFIG.get("developer_lanes", 1)
if type(lane) is not int or type(lanes) is not int or not 0 <= lane < lanes:
raise ValueError("Developer lane must be an integer in [0, developer_lanes)")
issues = open_issues()
owned = {i["number"] for i in issues if i["number"] % lanes == lane}
prs = [
p
for p in gh("GET", "/pulls?state=open&per_page=100")
if (match := re.fullmatch(r"factory/issue-(\d+)", p["head"]["ref"]))
and int(match[1]) % lanes == lane
]
existing = None
update_conflict = None
if prs:
for pr in prs:
state = statuses(pr["head"]["sha"])
if state.get("software-factory/review") == "success":
main_sha = gh("GET", "/git/ref/heads/main")["object"]["sha"]
comparison = gh("GET", f"/compare/{main_sha}...{pr['head']['sha']}")
if comparison["status"] not in ("ahead", "identical"):
# GitHub merges the base; the new head gets independent review again.
try:
gh(
"POST",
"/factory/update-branch",
{"number": pr["number"], "sha": pr["head"]["sha"]},
)
except HTTPError as exc:
if exc.code == 409:
return # Head changed; re-read it on the next sweep.
if exc.code != 422:
raise
update_conflict = {
"base_sha": main_sha,
"instruction": "The native base update found merge conflicts. "
"Inspect the current main files and PR diff with gh api. "
"Reconcile the conflicting changes in this branch while "
"preserving both completed features and this issue's criteria. "
"Run tests after the revision. The coordinator will publish "
"and retry the base update after independent review.",
}
comment(
pr["number"],
"Automatic base update found conflicts; "
"the canonical developer workflow is revising this PR.",
)
existing = pr
break
return
# Wait for the independent review to publish its findings before
# revising, even if the earlier deterministic test phase failed.
if state.get("software-factory/review") in ("failure", "error"):
Expand All @@ -337,7 +408,9 @@ def developer():
ready = [
i
for i in issues
if "ready-for-dev" in {label["name"] for label in i["labels"]}
if i["number"] in owned
and "ready-for-dev" in {label["name"] for label in i["labels"]}
and dependencies_complete(i)
]
if not ready:
return
Expand All @@ -358,7 +431,7 @@ def developer():
local_base = shell(["git", "rev-list", "--max-parents=0", "HEAD"])
else:
base, local_base = clone(branch if existing else "main")
feedback = {}
feedback = {"base_update": update_conflict} if update_conflict else {}
if existing:
for name, endpoint in (
("discussion", f"/issues/{existing['number']}/comments"),
Expand Down
4 changes: 3 additions & 1 deletion skills/github-software-factory/scripts/scoped_gh.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ def repository_path(endpoint, repository):
if not endpoint.startswith(prefix):
raise ValueError("Endpoint must be inside the configured repository")
path = "/" + endpoint[len(prefix) :]
if any(c in path for c in ("%", "..", "#", "\\")):
if any(c in path for c in ("%", "#", "\\")) or any(
part in (".", "..") for part in urlsplit(path).path.split("/")
):
raise ValueError("Invalid endpoint")
return path

Expand Down
55 changes: 44 additions & 11 deletions skills/openhands-automation/scripts/github_factory_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ def permitted(role, method, path, body):
return True
if role in ("developer", "reviewer", "watchdog") and route == "/actions/runs":
return True
if role == "developer" and re.fullmatch(
r"/compare/[0-9a-f]{40}\.\.\.[0-9a-f]{40}", route
):
return True
if role in ("developer", "reviewer") and re.fullmatch(
r"/git/ref/heads/(?:main|factory/issue-\d+)", route
):
Expand Down Expand Up @@ -215,22 +219,37 @@ def permitted(role, method, path, body):
return False


def merge(number, sha):
def factory_pr(role, number, sha):
if number < 1 or not re.fullmatch(r"[0-9a-f]{40}", sha):
raise ValueError("Merge requires a PR number and exact commit SHA")
role = "watchdog"
raise ValueError("Operation requires a PR number and exact commit SHA")
pr = github(role, "GET", f"/pulls/{number}")
if not (
pr["state"] == "open"
and pr["head"]["sha"] == sha
and pr["base"]["ref"] == "main"
and re.fullmatch(r"factory/issue-\d+", pr["head"]["ref"])
):
raise ValueError("PR is not an open factory branch at the expected head")
return pr


def update_branch(number, sha):
factory_pr("developer", number, sha)
return github(
"developer", "PUT", f"/pulls/{number}/update-branch", {"expected_head_sha": sha}
)


def merge(number, sha):
role = "watchdog"
pr = factory_pr(role, number, sha)
statuses = latest_statuses(role, sha)
contexts = ("software-factory/tests", "software-factory/review")
ci_ok = ci_passed(role, sha)
# Refuse a stale base and incomplete pagination rather than overlooking CI.
comparison = github(role, "GET", f"/compare/{pr['base']['sha']}...{sha}")
eligible = (
pr["state"] == "open"
and not pr["draft"]
and pr["head"]["sha"] == sha
and pr["base"]["ref"] == "main"
and re.fullmatch(r"factory/issue-\d+", pr["head"]["ref"])
not pr["draft"]
and pr["mergeable"] is True
and comparison["status"] in ("ahead", "identical")
and all(statuses.get(c, {}).get("state") == "success" for c in contexts)
Expand Down Expand Up @@ -263,7 +282,17 @@ def do_POST(self):
payload = json.loads(self.rfile.read(length))
method, path = payload["method"], payload["path"]
body = payload.get("body")
if "%" in path or ".." in path or "#" in path or "\\" in path:
if (
"%" in path
or (
".." in path
and not re.fullmatch(
r"/compare/[0-9a-f]{40}\.\.\.[0-9a-f]{40}", path
)
)
or "#" in path
or "\\" in path
):
return self.reply(403, {"error": "Invalid path"})
if path == "/factory/archive" and role in ("developer", "reviewer"):
return self.reply(200, archive(role, body["sha"]))
Expand All @@ -288,9 +317,13 @@ def do_POST(self):
},
),
)
if path == "/factory/merge" and role == "watchdog":
operation = {
("/factory/merge", "watchdog"): merge,
("/factory/update-branch", "developer"): update_branch,
}.get((path, role))
if operation:
try:
result = merge(int(body["number"]), body["sha"])
result = operation(int(body["number"]), body["sha"])
except ValueError as exc:
return self.reply(409, {"error": str(exc)})
return self.reply(200, result)
Expand Down
8 changes: 8 additions & 0 deletions tests/test_factory_extension_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,11 @@ def test_coordinator_implementation_prompt_has_no_direct_publication_commands(tm
assert "gh pr create" not in prompt
assert "coordinator publishes" in prompt
assert str(tmp_path / "bin/gh") in prompt


def test_scoped_transport_accepts_commit_comparisons():
path = "/compare/" + "a" * 40 + "..." + "b" * 40
assert (
load("scoped_gh").repository_path("repos/owner/repo" + path, "owner/repo")
== path
)
60 changes: 60 additions & 0 deletions tests/test_github_factory_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,32 @@ def upstream(role, method, path, body=None):
assert all(method == "GET" for method, _ in calls)


def test_native_branch_update_checks_identity_and_uses_developer(broker, monkeypatch):
calls = []

def github(role, method, path, body=None):
calls.append((role, method, path, body))
if method == "GET":
return {
"state": "open",
"head": {"sha": "a" * 40, "ref": "factory/issue-1"},
"base": {"ref": "main"},
}
return {"message": "Updating branch"}

monkeypatch.setattr(broker, "github", github)
broker.update_branch(3, "a" * 40)
assert calls[-1] == (
"developer",
"PUT",
"/pulls/3/update-branch",
{"expected_head_sha": "a" * 40},
)
with pytest.raises(ValueError):
broker.update_branch(3, "b" * 40)
assert calls[-1][1] == "GET"


def test_review_rejection_explains_required_fields(broker, monkeypatch):
import io
import json
Expand All @@ -532,3 +558,37 @@ def test_review_rejection_explains_required_fields(broker, monkeypatch):
assert responses[0][0] == 403
assert "COMMENT" in responses[0][1]["error"]
assert "commit_id" in responses[0][1]["error"]


@pytest.mark.parametrize(
"path,expected",
[("/compare/" + "a" * 40 + "..." + "b" * 40, 200), ("/compare/../issues", 403)],
)
def test_handler_allows_exact_commit_comparison_without_path_traversal(
broker, monkeypatch, path, expected
):
import io
import json
from types import SimpleNamespace

monkeypatch.setattr(broker, "CONTROL", {role: role for role in broker.ROLES})
upstream = []

def github(*args):
upstream.append(args)
return {"status": "ahead"}

monkeypatch.setattr(broker, "github", github)
payload = json.dumps({"method": "GET", "path": path}).encode()
replies = []
request = SimpleNamespace(
headers={
"Authorization": "Bearer developer",
"Content-Length": str(len(payload)),
},
rfile=io.BytesIO(payload),
reply=lambda code, body: replies.append((code, body)),
)
broker.Handler.do_POST(request)
assert replies[0][0] == expected
assert bool(upstream) == (expected == 200)
Loading
Loading