Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8bca349aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Using `_` in YAML numbers (`timeoutMs: 1_200_000`) | YAML doesn't support `_` separators | | ||
| | Workflow timeout under 30 min for complex workflows | Use `3600000` (1 hour) as default | | ||
| | Using `require()` in ESM projects | Check `package.json` for `"type": "module"` — use `import` if ESM | | ||
| | Wrapping in `async function main()` in ESM | ESM supports top-level `await` — no wrapper needed | |
There was a problem hiding this comment.
This guidance says wrapping execution in async function main() is a mistake and recommends top-level await, but the same skill’s failure-prevention section warns that agent-relay run can execute through a CJS tsx/esbuild path where top-level await fails. In environments that hit that path, following this row produces workflows that crash before any steps run, so the recommendation should be removed or explicitly constrained to runtimes guaranteed to support top-level await.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| ### 1. Do not use raw top-level `await` | ||
|
|
||
| Avoid `timeoutMs` on agents/steps unless you have a specific reason. The global `.timeout()` is the safety net. Per-agent timeouts cause premature kills on steps that legitimately need more time. | ||
| Executor-driven workflow files may be run through a `tsx`/`esbuild` path that behaves like CJS. Raw top-level `await` can fail with: | ||
|
|
||
| ## Agent Definition | ||
| - `Top-level await is currently not supported with the "cjs" output format` | ||
|
|
||
| ```typescript | ||
| .agent('name', { | ||
| cli: 'claude' | 'codex' | 'gemini' | 'aider' | 'goose' | 'opencode' | 'droid', | ||
| role?: string, // describes agent's purpose (used by pattern auto-selection) | ||
| preset?: 'lead' | 'worker' | 'reviewer' | 'analyst', // sets interactive mode + task guardrails | ||
| retries?: number, // default retry count for steps using this agent | ||
| model?: string, // model override | ||
| interactive?: boolean, // default: true. Set false for non-interactive subprocess mode | ||
| }) | ||
| ``` | ||
| Always wrap execution like this: | ||
|
|
||
| ## Step Definition | ||
| ```ts | ||
| async function runWorkflow() { | ||
| const result = await workflow('my-workflow') | ||
| // ... | ||
| .run({ cwd: process.cwd() }); | ||
|
|
||
| ### Agent Steps | ||
| console.log('Workflow status:', result.status); | ||
| } | ||
|
|
||
| ```typescript | ||
| .step('name', { | ||
| agent: string, // must match an .agent() name | ||
| task: string, // supports {{var}} and {{steps.NAME.output}} | ||
| dependsOn?: string[], // DAG edges | ||
| verification?: VerificationCheck, | ||
| retries?: number, // overrides agent-level retries | ||
| }) | ||
| runWorkflow().catch((error) => { | ||
| console.error(error); | ||
| process.exit(1); | ||
| }); | ||
| ``` | ||
|
|
||
| ### Deterministic Steps (Shell Commands) | ||
| Do not end workflow files with bare top-level `await workflow(...).run(...)`. |
There was a problem hiding this comment.
🟡 Contradictory guidance on top-level await within the same skill file
The Quick Reference (line 29) demonstrates bare top-level await: const result = await workflow('my-workflow')..., and Critical TypeScript rule #1 (line 59) says ESM projects should use top-level await. However, Failure Prevention Section 1 (line 146) explicitly states "Do not use raw top-level await" and "Always wrap execution" with an async function runWorkflow() wrapper. Additionally, the Common Mistakes table (line 775) says Wrapping in async function main() in ESM | ESM supports top-level await — no wrapper needed, further contradicting Failure Prevention. An agent following the Quick Reference template will produce code that the Failure Prevention section warns will fail. The same contradiction exists in the codex version at .agents/skills/writing-agent-relay-workflows/SKILL.md:29 and :112.
Prompt for agents
The Failure Prevention section 1 at line 146-169 says 'Do not use raw top-level await' and 'Always wrap execution', but three other places in the same file contradict this: (1) the Quick Reference at line 29 uses bare top-level await, (2) Critical TypeScript rule #1 at line 59 says ESM projects should use top-level await, and (3) the Common Mistakes table at line 775 says wrapping in async function main() in ESM is a mistake.
The Failure Prevention section should be qualified to only apply when the project uses CJS or when the executor runs through a tsx/esbuild CJS path. Change the absolute 'Always wrap' and 'Do not use raw top-level await' language to conditional language like 'If your project uses CJS or the executor may run through a CJS path, wrap in an async function'. Alternatively, update the Quick Reference to show the wrapped pattern.
The same fix needs to be applied to .agents/skills/writing-agent-relay-workflows/SKILL.md lines 112-127, which has the same contradiction.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🟡 Stale prpm.lock entry for trail-snippet at version 1.1.0 while installed content is 1.1.2
The lockfile retains a @agent-workforce/trail-snippet#agents.md:AGENTS.md entry at version 1.1.0 (line 5-21) with integrity sha256-8d8824b..., but the actual AGENTS.md content now uses the @1.1.2 snippet markers. Two new entries (#codex:AGENTS.md at line 114 and #claude:AGENTS.md at line 149) correctly reference version 1.1.2 with integrity sha256-36f049.... The stale 1.1.0 entry was not removed or updated during the lockfile regeneration, creating a version conflict for the same target file.
(Refers to lines 5-21)
Prompt for agents
The prpm.lock file has three entries for trail-snippet targeting AGENTS.md: one old entry at #agents.md format with version 1.1.0 (lines 5-21), and two new entries at #codex and #claude formats with version 1.1.2 (lines 114-133 and 149-168). The 1.1.0 entry appears stale since AGENTS.md now contains the 1.1.2 snippet content. Either remove the stale #agents.md:AGENTS.md entry at lines 5-21, or update it to version 1.1.2 with the matching integrity hash sha256-36f0490e6afffb8e6793f06119d4dd2058d7b0e23d65ead2adce9a8010da0bf0. Running prpm install or prpm update should reconcile this automatically.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Uh oh!
There was an error while loading. Please reload this page.