Skip to content

fix(cli): report effective workflow timeouts #884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
moncher-dev merged 2 commits into from
Sep 5, 2026
Merged

fix(cli): report effective workflow timeouts #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc43c5b
fix(cli): report effective workflow timeouts
Sep 5, 2026
2a51e85
fix(cli): stabilize timeout validation report
Sep 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-pandas-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@gh-symphony/cli": patch

@hojinzs hojinzs Sep 5, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

P3 — patch understates a removal of released --json keys.

I agree with your call on the codex bot's alias request: summary.codex.*TimeoutMs could report values the runtime never used, so aliasing them forward would preserve exactly the failure #882 exists to kill. Removing them is right.

The severity is the separate question that thread didn't cover. summary.codex.{readTimeoutMs,stallTimeoutMs,turnTimeoutMs} were in released workflow validate --json output and are now absent — I confirmed against the built CLI at this head that summary.codex carries only approvalPolicy, threadSandbox, turnSandboxPolicy. Automation reading those keys gets undefined, not a wrong number, which is the better failure but still a break.

A minor bump would put the removal in the changelog where a consumer pinning ~ actually sees it before it bites. The one-line body is otherwise good and does mention the new object.

Your call — if CLI --json output isn't covered by the package's compat surface, say so and this drops.

Generated by Claude Code

---

Report the effective runtime timeout values and their configuration source from `workflow validate`, including a stable three-field JSON timeout object (#882).
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ gh-symphony workflow init --non-interactive --project PVT_xxx --output WORKFLOW.
gh-symphony workflow init --non-interactive --project PVT_xxx --dry-run
```

`gh-symphony workflow validate` parses the target file, strictly renders the prompt body and continuation guidance with canonical sample variables, and prints a compact runtime/lifecycle summary.
`gh-symphony workflow validate` parses the target file, strictly renders the prompt body and continuation guidance with canonical sample variables, and prints a compact runtime/lifecycle summary. Its `runtime.timeouts.*` values are the effective runtime settings: `runtime.timeouts` takes precedence over the legacy `codex.*_timeout_ms` fields, with documented defaults used when neither is configured.

`gh-symphony workflow preview --issue owner/repo#123` is the fastest validation step after `workflow init`: it resolves the active managed project (or `--project-id`) and renders the exact worker prompt from the live GitHub Project issue. Linear workflows can preview a single issue with `gh-symphony workflow preview ENG-123`, which routes through the configured Linear tracker adapter and `LINEAR_API_KEY`. Keep `--sample <path-to-json>` for fixture-based debugging, and use `--attempt <n>` to inspect retry prompts before changing policy files.

Expand Down
5 changes: 5 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ negative, that threshold is disabled, but the orchestrator still applies a hard
30-minute elapsed-run fallback to prevent an indefinitely stuck worker. This is
an intentional implementation-defined safety limit.

`runtime.timeouts` is the preferred timeout configuration and takes precedence
over the legacy `codex.*_timeout_ms` fields. `gh-symphony workflow validate`
prints the effective values under `runtime.timeouts.*`, matching the values the
orchestrator injects into a worker.

Hooks are opt-in repository-local extensions, not shell snippets. Each hook
value must be a path to an executable script; shell syntax and inline commands
are rejected. Set `SYMPHONY_ALLOW_WORKFLOW_HOOKS=1` (or `true`) in the host
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,16 @@ unanswerable request. To migrate an existing workflow, change either value to
maximum silence interval for a Codex app-server turn. Every app-server output
resets it; it is not a total turn-duration cap.

`gh-symphony workflow validate` reports the effective values under
`runtime.timeouts.*`. An explicit `runtime.timeouts` block takes precedence over
the legacy `codex.*_timeout_ms` fields; documented defaults apply when neither
location provides a value.

In JSON output, effective timeout values are exposed as
`summary.runtimeTimeouts.{readTimeoutMs,stallTimeoutMs,turnTimeoutMs}`. These
replace the former `summary.codex.*TimeoutMs` fields, which could report values
that the runtime did not use; no compatibility aliases are emitted.

Lifecycle generation enables blocker checks for the first configured active
state (`Todo` with built-in defaults) while leaving planning states disabled.
An explicit `tracker.provider.blocker_check_states: []` disables blocker gating; this is
Expand Down
109 changes: 109 additions & 0 deletions packages/cli/src/commands/workflow.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,115 @@ describe("workflow command handler", () => {
expect(stdout.output()).toContain("active_states=Ready, In progress");
});

it.each([
{
name: "runtime timeout precedence",
frontMatter: `runtime:
kind: codex-app-server
command: codex
args: [app-server]
timeouts:
read_timeout_ms: 30000
stall_timeout_ms: 900000
turn_timeout_ms: 1800000
codex:
read_timeout_ms: 7000
stall_timeout_ms: 60000
turn_timeout_ms: 120000`,
expected: [30000, 900000, 1800000],
expectedSource: "runtime.timeouts",
},
{
name: "legacy codex timeout fallback",
frontMatter: `codex:
read_timeout_ms: 7000
stall_timeout_ms: 60000
turn_timeout_ms: 120000`,
expected: [7000, 60000, 120000],
expectedSource: "codex/defaults",
},
{
name: "runtime defaults over legacy codex timeouts",
frontMatter: `runtime:
kind: codex-app-server
command: codex
args: [app-server]
codex:
read_timeout_ms: 7000
stall_timeout_ms: 60000
turn_timeout_ms: 120000`,
expected: [5000, 300000, 3600000],
expectedSource: "runtime.timeouts",
},
{
name: "documented timeout defaults",
frontMatter: "codex:\n command: codex app-server",
expected: [5000, 300000, 3600000],
Comment thread
moncher-dev marked this conversation as resolved.
expectedSource: "codex/defaults",
},
])(
"reports $name as effective runtime timeouts",
async ({ frontMatter, expected, expectedSource }) => {
const root = await mkdtemp(join(tmpdir(), "workflow-validate-timeouts-"));
const workflowPath = join(root, "WORKFLOW.md");
const stdout = captureWrites(process.stdout);

await writeFile(
workflowPath,
`---\ntracker:\n kind: github-project\n${frontMatter}\n---\nPrompt {{ issue.identifier }}\n`,
"utf8"
);

try {
await workflowCommand(["validate", "--file", workflowPath], {
configDir: root,
verbose: false,
json: false,
noColor: false,
});
} finally {
stdout.restore();
}

expect(stdout.output()).toContain(
`runtime.timeouts.read_timeout_ms=${expected[0]}`
);
expect(stdout.output()).toContain(
`runtime.timeouts.stall_timeout_ms=${expected[1]}`
);
expect(stdout.output()).toContain(
`runtime.timeouts.turn_timeout_ms=${expected[2]}`
);
expect(stdout.output()).not.toContain("codex.read_timeout_ms=");
expect(stdout.output()).toContain(`(source: ${expectedSource})`);

const jsonStdout = captureWrites(process.stdout);
try {
await workflowCommand(["validate", "--file", workflowPath], {
configDir: root,
verbose: false,
json: true,
noColor: false,
});
} finally {
jsonStdout.restore();
}

const report = JSON.parse(jsonStdout.output()) as {
summary: {
runtimeTimeouts: Record<string, number>;
runtimeTimeoutSource: string;
};
};
expect(report.summary.runtimeTimeouts).toEqual({
readTimeoutMs: expected[0],
stallTimeoutMs: expected[1],
turnTimeoutMs: expected[2],
});
expect(report.summary.runtimeTimeoutSource).toBe(expectedSource);
}
);

it("prints a typed error when a removed flat tracker key is configured", async () => {
const root = await mkdtemp(join(tmpdir(), "workflow-validate-priority-"));
const workflowPath = join(root, "WORKFLOW.md");
Expand Down
22 changes: 16 additions & 6 deletions packages/cli/src/commands/workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
WorkflowValidationError,
renderPrompt,
resolveWorkflowExecutionPhase,
resolveWorkflowRuntimeTimeouts,
type TrackedIssue,
} from "@gh-symphony/core";
import {
Expand Down Expand Up @@ -105,10 +106,13 @@ type WorkflowValidationReport = {
approvalPolicy: string | null;
threadSandbox: string | null;
turnSandboxPolicy: string | null;
};
runtimeTimeouts: {
readTimeoutMs: number;
stallTimeoutMs: number;
turnTimeoutMs: number;
};
runtimeTimeoutSource: "runtime.timeouts" | "codex/defaults";
hooks: {
afterCreate: string | null;
beforeRun: string | null;
Expand Down Expand Up @@ -843,6 +847,7 @@ function validateWorkflow(
return "pass" as const;
})()
: ("skip" as const);
const effectiveTimeouts = resolveWorkflowRuntimeTimeouts(workflow);

return {
ok: true,
Expand Down Expand Up @@ -878,10 +883,15 @@ function validateWorkflow(
approvalPolicy: workflow.codex.approvalPolicy,
threadSandbox: workflow.codex.threadSandbox,
turnSandboxPolicy: workflow.codex.turnSandboxPolicy,
readTimeoutMs: workflow.codex.readTimeoutMs,
stallTimeoutMs: workflow.codex.stallTimeoutMs,
turnTimeoutMs: workflow.codex.turnTimeoutMs,
},
runtimeTimeouts: {
readTimeoutMs: effectiveTimeouts.readTimeoutMs,
stallTimeoutMs: effectiveTimeouts.stallTimeoutMs,
turnTimeoutMs: effectiveTimeouts.turnTimeoutMs,
},
runtimeTimeoutSource: workflow.runtime
? "runtime.timeouts"
: "codex/defaults",
Comment on lines +892 to +894

@hojinzs hojinzs Sep 5, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

P3 — the source is re-derived here instead of coming from the resolver, so it can silently drift when #888 lands.

resolveWorkflowRuntimeTimeouts branches on workflow.runtime?.timeouts; this branches on workflow.runtime. They agree today, and not by luck — WorkflowRuntimeConfig.timeouts is a required field (config.ts:131) and the parser always materializes it (parser.ts:1143-1151), so runtime truthy ⟺ runtime.timeouts present. (Which also means the ?? workflow.codex arm of the resolver is reachable only when runtime is absent entirely — the "runtime block, no timeouts sub-block" shape never falls back, exactly as your fourth test case pins.)

The drift risk is that this is a duplicated invariant rather than a derived one. #888 is about changing precisely that precedence; when it changes the resolver, nothing here fails to compile and no test in this file necessarily fails — the label just starts lying again, in the same class of way #882 was filed for.

Cheap insurance, and it makes the follow-up in the sibling comment fall out for free: have the resolver return the provenance alongside the values (or add a thin resolveWorkflowRuntimeTimeoutSource next to it in core), so the label and the number are read from one place.

Non-blocking — the current output is correct and verified.

Generated by Claude Code

hooks: {
afterCreate: workflow.hooks.afterCreate,
beforeRun: workflow.hooks.beforeRun,
Expand Down Expand Up @@ -920,9 +930,9 @@ Runtime
codex.approval_policy=${report.summary.codex.approvalPolicy ?? "unset"}
codex.thread_sandbox=${report.summary.codex.threadSandbox ?? "unset"}
codex.turn_sandbox_policy=${report.summary.codex.turnSandboxPolicy ?? "unset"}
codex.read_timeout_ms=${report.summary.codex.readTimeoutMs}
codex.stall_timeout_ms=${report.summary.codex.stallTimeoutMs}
codex.turn_timeout_ms=${report.summary.codex.turnTimeoutMs}
runtime.timeouts.read_timeout_ms=${report.summary.runtimeTimeouts.readTimeoutMs} (source: ${report.summary.runtimeTimeoutSource})
runtime.timeouts.stall_timeout_ms=${report.summary.runtimeTimeouts.stallTimeoutMs} (source: ${report.summary.runtimeTimeoutSource})
runtime.timeouts.turn_timeout_ms=${report.summary.runtimeTimeouts.turnTimeoutMs} (source: ${report.summary.runtimeTimeoutSource})
Comment on lines +933 to +935

@hojinzs hojinzs Sep 5, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

P3 — codex/defaults conflates two different answers, and per-report granularity can't express a partially-declared block.

The source label is one value for all three lines, so it can only say "not runtime.timeouts". Verified against the built CLI at this head with a codex: block declaring only read_timeout_ms:

codex:
  read_timeout_ms: 7777
runtime.timeouts.read_timeout_ms=7777 (source: codex/defaults)
runtime.timeouts.stall_timeout_ms=300000 (source: codex/defaults)
runtime.timeouts.turn_timeout_ms=3600000 (source: codex/defaults)

7777 came from codex:; 300000 and 3600000 are defaults. Three lines, one label, two genuinely different provenances. The operator who greps WORKFLOW.md for stall_timeout_ms after reading "codex" finds nothing — the smaller version of the loop #882 describes.

This is a real improvement over the previous head and I'm not blocking on it, but the criterion asked for the source to be unambiguous, and per-field provenance is what actually delivers that: resolve each field and label it codex or default individually. Worth a follow-up rather than another round here.

nit, same lines: (source: …) repeated identically three times is noise while the label stays per-report — a single runtime.timeouts source=… line above the three values reads better.

Generated by Claude Code


Hooks
after_create=${report.summary.hooks.afterCreate ?? "unset"}
Expand Down
Loading