Skip to content

Commit f907ad1

Browse files
committed
refactor: improve progress detail handling and logging in orchestrator
- Shorten console output for long details in progress notifications. - Introduce a new function to merge and truncate subprocess output for better clarity in logs. - Update tests to cover new behavior for subprocess output handling. Made-with: Cursor
1 parent 64d1354 commit f907ad1

8 files changed

Lines changed: 222 additions & 33 deletions

paperclip

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit 14ffbe30a0969df009dd776c66e02f3ad7d926ef

skills/issue-fix-workflow.md

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,11 @@ If no repo is given, assume the current workspace root or ask once for `owner/re
2222

2323
## Run status (supervising agents)
2424

25-
Long Iynx runs (`python run.py`) stream work through Docker and Cursor. A supervising agent or host process should **not** assume instant completion.
25+
For the **full** host pipeline—**discovery**, Docker, all Cursor phases, PR creation—read **`skills/iynx-run-workflow.md`** in the Iynx repo. It is the single runbook for `python run.py`.
2626

27-
**Cadence:** Use a **scheduler (or timer) that checks run status every 60 seconds** until the run finishes or the user stops it. Sixty seconds is the default interval unless the user specifies another.
27+
**Short reminder:** Long runs stream through Docker and Cursor; **do not** assume instant completion.
2828

29-
**What to check**
30-
31-
- **Progress file:** `.iynx-run-progress.jsonl` at the Iynx project root (or the path in `IYNX_PROGRESS_JSONL` if set). Each line is one JSON event: `phase`, `status`, `repo`, `issue`, `detail`, `exit_code`.
32-
- **Done:** The run is finished when you see `phase` `run_complete` (read `detail` for `pr_created` vs `no_pr`, and `exit_code`).
33-
- **Between polls:** You can still read the latest line or tail new lines since the last check; avoid tight loops.
34-
35-
**Implementation note:** The host may later ship a built-in scheduler that performs this 60s poll; until then, follow the same cadence when implementing supervision yourself.
29+
**Cadence:** Poll **`.iynx-run-progress.jsonl`** at the Iynx project root about **every 60 seconds** until **`phase` `run_complete`** (see **`iynx-run-workflow.md`** for phases and exit codes).
3630

3731
## Quick Checklist
3832

skills/iynx-run-workflow.md

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# Iynx run workflow
2+
3+
How to **start** the full Iynx pipeline (actually execute `python run.py`), then **supervise** it and **explain** it in plain language (progress, watch commands, failures like **`no_pr`**).
4+
5+
For **manual** contribution work (no orchestrator), use **`issue-fix-workflow.md`**. The orchestrator injects that file into the target repo; it does **not** inject this one.
6+
7+
---
8+
9+
## 1. When the user wants to *run* the workflow — execute it
10+
11+
If they say **run Iynx**, **start the workflow**, **run discovery**, **run `python run.py`**, or equivalent, you must **run the process**, not only read `.iynx-run-progress.jsonl` or describe what would happen.
12+
13+
1. **`cd`** to the **Iynx repo root** (directory that contains **`run.py`**, **`Dockerfile`**, **`skills/`**).
14+
2. **Check prerequisites** before starting:
15+
- **`CURSOR_API_KEY`** set (in `.env` beside `run.py` or in the environment). Without it the host exits **1** immediately.
16+
- **`GITHUB_TOKEN`** set for real discovery and PR work (strongly recommended).
17+
- Docker daemon running; image **`iynx-agent:latest`** built (`docker build -t iynx-agent:latest .` from that root if missing).
18+
3. **Run** the entrypoint:
19+
- Full pipeline **with discovery:** `python run.py`
20+
- **One repo** (no discovery): `python run.py owner/repo` or `python run.py owner/repo 849`
21+
4. **Long runs:** This can take **tens of minutes to an hour+** (Docker + Cursor phases). If your environment allows **background** execution for long commands, start it in the **background** and tell the user it is running; otherwise run foreground and warn that the session will stay busy. **Do not** skip starting the workflow because it is slow—start it, then use §§3–6 to report status.
22+
5. After it starts, confirm in your reply: **what ran** (command + discovery vs explicit target), and **how to watch** (§4).
23+
24+
You can also pass the same targets via **`IYNX_TARGET_REPO`** and optional **`IYNX_TARGET_ISSUE`** instead of argv. Default `python run.py` runs **discovery** (GitHub issue search → filters → **random** repo), then the Docker/Cursor/PR pipeline.
25+
26+
---
27+
28+
## 2. What actually runs (for the user-facing sentence)
29+
30+
The Iynx agent workflow is **`python run.py`** from that root. Details and env setup: **README → Usage**.
31+
32+
When you tell the user what ran, say it plainly—e.g. *“The Iynx agent workflow is `python run.py` (see README under Usage).”* Note **foreground** vs **background** if you know.
33+
34+
---
35+
36+
## 3. “Progress so far” — how to describe it
37+
38+
Read **`.iynx-run-progress.jsonl`** at the **Iynx project root** (gitignored). Each line is one JSON object: `phase`, `status`, `repo`, `issue`, `detail`, `run_id`, `exit_code`, etc. Use **`IYNX_PROGRESS_JSONL`** only if the user overrode the path.
39+
40+
Build the narrative in this order (skip lines the user’s run has not reached yet):
41+
42+
1. **Discovery** (only when there was no explicit `owner/repo`): Find the row with `phase` **`discovery`**. If `status` is **`completed`**, `detail` is usually the **number of repos** after filters (string). Say e.g. *“Discovery: 29 repos after filters.”* If `status` is **`skipped`** and `detail` is **`no_repos`**, the run stopped early—**no clone**—and will hit **`run_complete`** with **`no_pr`**.
43+
2. **Selected repo:** The host logs something like *“Selected owner/repo (random of N qualifying)”*; the JSONL rows for **`clone`** onward carry **`repo`**: **`owner/repo`**. Say e.g. *“Selected repo: judgemind/judgemind (random pick).”*
44+
3. **Preflight:** Row **`preflight`**. **`completed`***“Preflight: passed (open issues exist)”* or, with an issue override, that the issue was validated. **`failed`** → read `detail` (e.g. no open issues, bad issue number) and say the run **did not clone**.
45+
4. **Clone:** **`clone`** **`completed`***“Clone: finished (repo cloned in Docker).”*
46+
5. **Bootstrap:** **`bootstrap`** **`completed`** → fold into *“Clone / bootstrap: done”* if you want a short status.
47+
6. **Cursor phases** — use **friendly names** in the user-facing summary; tie them to JSONL `phase` values:
48+
49+
| Say this | JSONL `phase` |
50+
|----------|----------------|
51+
| Phase 1 (context) | `phase1_context` |
52+
| Phase 2 (issue pick) | `phase2_issue_pick` |
53+
| Phase 3 (implement) | `phase3_implement` |
54+
| Phase 4 (PR draft) | `phase4_pr_draft` |
55+
| PR create | `pr_create` |
56+
57+
For each: **`started`** / **`completed`** / **`failed`**. If **`phase3_implement`** is **`started`** or only **`phase1`/`phase2`** are **`completed`**, say clearly what is **still in progress** (e.g. *“Phase 3 (implement) in progress — issue #1545* when `issue` is present on the row).
58+
59+
7. **Still running:** If there is **no** row with `phase` **`run_complete`** yet, the run is **not** finished. Mention that **phase4**, **pr_create**, and **run_complete** are still to come, or whichever is next.
60+
61+
8. **Duration:** Long runs are normal. Per Docker step, the default timeout is **large** (on the order of **3600s** unless changed—see **`IYNX_DOCKER_RUN_TIMEOUT`** in §8).
62+
63+
---
64+
65+
## 4. “How to watch it” — tell the user
66+
67+
- **Console:** Same session’s log stream; lines tagged **`[docker]`** and **`[iynx]`** mirror progress.
68+
- **Structured file:** Full path to **`.iynx-run-progress.jsonl`** (project root); each line is JSON; **final** lifecycle row is **`run_complete`** when the process is done.
69+
- **PowerShell:** `Get-Content .iynx-run-progress.jsonl -Wait`
70+
- **Unix:** `tail -f .iynx-run-progress.jsonl`
71+
72+
Optional: mention **`run_id`** from a recent JSON line so multiple runs are distinguishable.
73+
74+
---
75+
76+
## 5. Status updates while running
77+
78+
A **table** is easy to scan:
79+
80+
| Step | Status |
81+
|------|--------|
82+
| Discovery | Done — N repos *(or skipped / not applicable if explicit target)* |
83+
| Preflight | Done — `owner/repo` |
84+
| Clone / bootstrap | Done |
85+
| Phase 1 (context) | Done |
86+
| Phase 2 (issue pick) | Done |
87+
| Phase 3 (implement) | In progress — issue #N *(or Done)* |
88+
89+
If **`phase4_pr_draft`**, **`pr_create`**, or **`run_complete`** are **missing**, say the agent **has not** finished implementation or **opened a PR** yet.
90+
91+
Mention **process** if known: e.g. still running, approximate elapsed time from logs.
92+
93+
---
94+
95+
## 6. When the run finishes
96+
97+
Read the last **`run_complete`** row:
98+
99+
- **`detail`:** **`pr_created`** vs **`no_pr`**
100+
- **`exit_code`:** aligns with the process (**0** = PR created, **2** = finished without PR, **1** = fatal host config such as missing **`CURSOR_API_KEY`**)
101+
102+
**If the user sees exit 2 and `no_pr` but phases mostly succeeded**, do **not** stop at “no PR.” Scan JSONL (and stderr/log) for the **last failing step**, often **`pr_create`** with `status` **`failed`** and `exit_code` on that row.
103+
104+
### Example: PR not created because of a label
105+
106+
Orchestrator passes **`IYNX_PR_LABEL`** into `gh pr create`. If that label **does not exist** on the **upstream** repo, `gh` fails with something like *`could not add label: 'lynx' not found`*.
107+
108+
**What to tell the user:**
109+
110+
- What **succeeded** (e.g. repo, issue, phases through implement/draft, branch pushed to fork—if logs show that).
111+
- **Why no PR:** `pr_create` failed; quote or paraphrase the **`gh`** error.
112+
- **What they can do:** Create the label on the upstream repo (if they control it), **or** unset / change **`IYNX_PR_LABEL`** in `.env` and re-run, **or** open the PR manually from GitHub’s “compare / new PR” flow for the pushed branch.
113+
114+
Always tie **actionable** steps to the **actual** error text you see.
115+
116+
---
117+
118+
## 7. Polling (supervising agents)
119+
120+
Do **not** assume `python run.py` finishes quickly. If you are checking on a run, **poll** **`.iynx-run-progress.jsonl`** on the order of **every 60 seconds** (or as the user asks), reading **new** lines since the last check—not a tight loop.
121+
122+
---
123+
124+
## 8. Short reference (implementation details)
125+
126+
Use this when you need exact behavior, not when writing a user-facing status blurb.
127+
128+
**Discovery (default `python run.py`):** GitHub **issue search** per language (**JavaScript, TypeScript, Python**), pool capped by **`DISCOVERY_POOL_SIZE`** (see `src/orchestrator.py`). Filters: **CONTRIBUTING** expected; optionally **skip repos where the token’s user already has PRs**. One repo chosen with **`random.choice`**. Tuning is via **constants** in **`orchestrator.py`**, not env vars.
129+
130+
**Explicit target:** **`IYNX_TARGET_REPO`** or `python run.py owner/repo [issue]`**discovery skipped**; **`target_resolve`** in JSONL.
131+
132+
**Pipeline order:** `preflight``clone``bootstrap``phase1_context``phase2_issue_pick` (skipped when issue fixed upfront) → `phase3_implement``verify_tests` (often **skipped**; **`VERIFY_TESTS_AFTER_FIX`** default **false** in code) → `phase4_pr_draft``pr_create`**`run_complete`**.
133+
134+
**Exit codes:** **0** = PR created; **1** = fatal host misconfiguration; **2** = ended without PR.
135+
136+
**Env (common):** `CURSOR_API_KEY` (required), `GITHUB_TOKEN`, `IYNX_TARGET_REPO`, `IYNX_TARGET_ISSUE`, `IYNX_PROGRESS_JSONL`, `IYNX_DOCKER_RUN_TIMEOUT`, `IYNX_PR_LABEL`, `IYNX_DOCKER_TRACE`, `IYNX_CURSOR_MODEL`, … — full list in **`README`** / **`src/orchestrator.py`**.
137+
138+
**Clones:** Under **`workspace/`** at the Iynx root.
139+
140+
**Follow-up on an existing PR:** **`issue-fix-workflow.md`** (PR review section) and **`pr_review.py`**.
141+
142+
**Source files:** `run.py`, `src/orchestrator.py`, `src/discovery.py`.

src/orchestrator.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,12 @@ def _notify_progress(
195195
if issue is not None:
196196
parts.append(f"issue={issue}")
197197
if detail:
198-
parts.append(detail)
198+
# Full `detail` is stored in JSONL; keep console `[iynx]` lines short.
199+
if len(detail) > 500:
200+
one_line = detail[:500].replace("\n", " ").strip()
201+
parts.append(f"{one_line} ... [detail {len(detail)} chars; see JSONL]")
202+
else:
203+
parts.append(detail)
199204
if exit_code is not None:
200205
parts.append(f"exit_code={exit_code}")
201206
logger.info("[iynx] %s", " ".join(parts))
@@ -210,6 +215,31 @@ def _notify_progress(
210215
)
211216

212217

218+
# Cap for progress JSONL `detail` on subprocess failures (e.g. gh pr create stderr).
219+
_PROGRESS_SUBPROCESS_DETAIL_MAX = 4000
220+
221+
222+
def _progress_detail_from_subprocess(
223+
result: subprocess.CompletedProcess[str],
224+
*,
225+
max_chars: int | None = None,
226+
) -> str:
227+
"""Merge stderr/stdout from a failed CompletedProcess for progress JSONL."""
228+
limit = max_chars if max_chars is not None else _PROGRESS_SUBPROCESS_DETAIL_MAX
229+
err = (result.stderr or "").strip()
230+
out = (result.stdout or "").strip()
231+
if err and out and err not in out:
232+
combined = f"{err}\n---\n{out}"
233+
else:
234+
combined = err or out
235+
combined = combined.replace("\r\n", "\n").strip()
236+
if not combined:
237+
return "no_output"
238+
if len(combined) <= limit:
239+
return combined
240+
return f"...(truncated, {len(combined)} chars total)\n" + combined[-limit:]
241+
242+
213243
def _parse_owner_repo_string(raw: str) -> tuple[str, str] | None:
214244
s = raw.strip().rstrip("/")
215245
if not s:
@@ -977,13 +1007,15 @@ def run_one_repo(
9771007
workdir="/home/dev/workspace",
9781008
)
9791009
if r5.returncode != 0:
980-
logger.error("PR creation failed: %s", r5.stderr or r5.stdout)
1010+
pr_fail_detail = _progress_detail_from_subprocess(r5)
1011+
logger.error("PR creation failed: %s", pr_fail_detail)
9811012
_notify_progress(
9821013
progress,
9831014
repo.full_name,
9841015
"pr_create",
9851016
"failed",
9861017
issue=issue_num,
1018+
detail=pr_fail_detail,
9871019
exit_code=r5.returncode,
9881020
)
9891021
return False

src/pr_stats.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,7 @@ def compute_stats(
293293

294294
max_fetchable = min(open_total, 1000) + min(closed_total, 1000)
295295
user_capped = (
296-
max_items is not None
297-
and fetched_total >= max_items
298-
and fetched_total < max_fetchable
296+
max_items is not None and fetched_total >= max_items and fetched_total < max_fetchable
299297
)
300298

301299
kept: list[tuple[str, dict[str, Any]]] = []
@@ -601,8 +599,7 @@ def run(argv: list[str] | None = None) -> int:
601599
lim = result.limits
602600
if lim.get("search_truncated"):
603601
print(
604-
"warning: GitHub Search returned more than 1,000 matches; "
605-
"counts may omit older PRs.",
602+
"warning: GitHub Search returned more than 1,000 matches; counts may omit older PRs.",
606603
file=sys.stderr,
607604
)
608605
if lim.get("user_capped"):

tests/test_orchestrator_unit.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,29 @@ def test_read_json_file_os_error(tmp_path: Path) -> None:
3838
assert orchestrator._read_json_file(p) is None
3939

4040

41+
def test_progress_detail_from_subprocess_no_output() -> None:
42+
p = subprocess.CompletedProcess(["x"], 1, stdout="", stderr="")
43+
assert orchestrator._progress_detail_from_subprocess(p) == "no_output"
44+
45+
46+
def test_progress_detail_from_subprocess_prefers_stderr_when_distinct() -> None:
47+
p = subprocess.CompletedProcess(["x"], 1, stdout="out", stderr="gh: label not found")
48+
assert orchestrator._progress_detail_from_subprocess(p) == "gh: label not found\n---\nout"
49+
50+
51+
def test_progress_detail_from_subprocess_stdout_only() -> None:
52+
p = subprocess.CompletedProcess(["x"], 1, stdout="docker log line\n", stderr="")
53+
assert orchestrator._progress_detail_from_subprocess(p) == "docker log line"
54+
55+
56+
def test_progress_detail_from_subprocess_truncates() -> None:
57+
long_out = "a" * 100
58+
p = subprocess.CompletedProcess(["x"], 1, stdout=long_out, stderr="")
59+
d = orchestrator._progress_detail_from_subprocess(p, max_chars=20)
60+
assert d.startswith("...(truncated, 100 chars total)")
61+
assert d.endswith("a" * 20)
62+
63+
4164
def test_load_pr_draft_malformed_json_uses_defaults(tmp_path: Path) -> None:
4265
iynx = tmp_path / ".iynx"
4366
iynx.mkdir()

tests/test_pr_review_followup.py

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212

1313

1414
def test_parse_pr_ref_url() -> None:
15-
v, o, r, n = prf.parse_pr_ref(
16-
"https://github.com/foo/bar/pull/99", None, None
17-
)
15+
v, o, r, n = prf.parse_pr_ref("https://github.com/foo/bar/pull/99", None, None)
1816
assert v == "https://github.com/foo/bar/pull/99"
1917
assert o == "foo" and r == "bar" and n == 99
2018

@@ -42,9 +40,7 @@ def test_parse_pr_ref_invalid() -> None:
4240

4341

4442
def test_owner_repo_from_pr_json() -> None:
45-
o, r = prf.owner_repo_from_pr_json(
46-
{"baseRepository": {"nameWithOwner": "cli/cli"}}
47-
)
43+
o, r = prf.owner_repo_from_pr_json({"baseRepository": {"nameWithOwner": "cli/cli"}})
4844
assert o == "cli" and r == "cli"
4945

5046

@@ -86,7 +82,9 @@ def test_resolve_output_with_override(tmp_path: Path) -> None:
8682
assert p.is_file()
8783

8884

89-
def test_resolve_output_default_requires_gitignore(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
85+
def test_resolve_output_default_requires_gitignore(
86+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
87+
) -> None:
9088
monkeypatch.chdir(tmp_path)
9189
subprocess_run = __import__("subprocess").run
9290
import subprocess
@@ -112,9 +110,7 @@ def fake_run(cmd: list, **kw): # type: ignore[no-untyped-def]
112110
return subprocess.CompletedProcess(cmd, 1, "", "err")
113111

114112
with patch("pr_review_followup.subprocess.run", side_effect=fake_run):
115-
p, _ = prf.resolve_output_path(
116-
output_cli=None, env_path=None, repo_root=tmp_path
117-
)
113+
p, _ = prf.resolve_output_path(output_cli=None, env_path=None, repo_root=tmp_path)
118114
assert p.name == "pr-review-feedback.md"
119115
assert ".iynx" in str(p)
120116

tests/test_pr_stats.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,20 @@ def test_build_search_q_without_label() -> None:
3030

3131

3232
def test_repo_from_repository_url() -> None:
33-
assert pr_stats._repo_from_repository_url(
34-
"https://api.github.com/repos/foo/bar"
35-
) == ("foo", "bar")
33+
assert pr_stats._repo_from_repository_url("https://api.github.com/repos/foo/bar") == (
34+
"foo",
35+
"bar",
36+
)
3637

3738

3839
def test_repo_from_issue_item_fallbacks() -> None:
3940
assert pr_stats._repo_from_issue_item(
4041
{"number": 1, "html_url": "https://github.com/foo/bar/pull/99"}
4142
) == ("foo", "bar")
42-
assert pr_stats._repo_from_issue_item(
43-
{"number": 1, "repository": {"full_name": "a/b"}}
44-
) == ("a", "b")
43+
assert pr_stats._repo_from_issue_item({"number": 1, "repository": {"full_name": "a/b"}}) == (
44+
"a",
45+
"b",
46+
)
4547
assert pr_stats._repo_from_issue_item({"number": 1}) is None
4648

4749

@@ -163,7 +165,9 @@ def test_result_to_json_omits_empty_by_repo() -> None:
163165
assert "by_repo" not in d
164166

165167

166-
def test_json_output_schema(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None:
168+
def test_json_output_schema(
169+
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
170+
) -> None:
167171
monkeypatch.setenv("GITHUB_TOKEN", "tok")
168172
monkeypatch.setenv("NO_COLOR", "1")
169173

0 commit comments

Comments
 (0)