Skip to content

Enable GitHub Agentic Workflows (gh-aw) support#3

Merged
friggeri merged 1 commit intomainfrom
frg/enable-gh-aw
Jan 14, 2026
Merged

Enable GitHub Agentic Workflows (gh-aw) support#3
friggeri merged 1 commit intomainfrom
frg/enable-gh-aw

Conversation

@friggeri
Copy link
Copy Markdown
Collaborator

Embracing 2026!

  • Add copilot-setup-steps workflow to install gh-aw extension
  • Add agent prompts for creating, debugging, and upgrading workflows
  • Add gh-aw schema and documentation
  • Configure VS Code MCP server for gh-aw
  • Add gitattributes for workflow lock files

- Add copilot-setup-steps workflow to install gh-aw extension
- Add agent prompts for creating, debugging, and upgrading workflows
- Add gh-aw schema and documentation
- Configure VS Code MCP server for gh-aw
- Add gitattributes for workflow lock files
@friggeri friggeri requested a review from a team as a code owner January 14, 2026 21:32
Copilot AI review requested due to automatic review settings January 14, 2026 21:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request enables GitHub Agentic Workflows (gh-aw) support by adding MCP server configuration, installation workflows, comprehensive JSON schema validation, documentation, and git configuration for workflow lock files.

Changes:

  • Adds VS Code MCP server configuration for gh-aw
  • Creates copilot-setup-steps workflow to install gh-aw extension
  • Includes comprehensive agentic workflow JSON schema (6070 lines)
  • Adds extensive documentation for GitHub Agentic Workflows (1654 lines)
  • Configures gitattributes for workflow lock files

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.vscode/mcp.json Configures MCP server for gh-aw in VS Code
.github/workflows/copilot-setup-steps.yml Workflow to install gh-aw extension via remote script
.github/aw/schemas/agentic-workflow.json Complete JSON schema for agentic workflow validation
.github/aw/logs/.gitignore Ignores downloaded workflow logs
.github/aw/github-agentic-workflows.md Comprehensive documentation for agentic workflows
.gitattributes Marks workflow lock files as generated with merge strategy

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/copilot-setup-steps.yml
@friggeri friggeri enabled auto-merge January 14, 2026 21:41
@friggeri friggeri added this pull request to the merge queue Jan 14, 2026
Merged via the queue into main with commit 7f90ac2 Jan 14, 2026
29 checks passed
jmoseley pushed a commit that referenced this pull request Jan 21, 2026
* Fix Node test startup on Windows

* Fix Go tests

* Fix Python tests

* Remove Python egg-info dir which shouldn't be tracked in Git

* Publish packages only from main

* Fix formatting

* Add Windows CI job. Simplify Python CI matrix to just 3.12 (latest documented supported version)

* Avoid duplicate CI jobs (push and pull_request)

* Avoid line endings issues in CI

* Clean up defaults

* Similarly don't run go fmt on Windows due to line endings differences

* Fix Python tests in Windows CI job
tclem added a commit that referenced this pull request Apr 30, 2026
Closes RFD-400 review finding #3 (JoinHandle::abort lands at any
await point in the event loop body) and the remaining bit of #5
(Session::event_loop tokio::sync::Mutex -> parking_lot::Mutex). See
cancel-safety review session db4b1ac8-... for the full report.

The previous design called `JoinHandle::abort()` in two places to
stop the event-loop task: explicitly in `Session::stop_event_loop`
and best-effort in `Drop for Session`. RFD 400's strongest single
recommendation about task aborts:

> With the abort(), the future returned by do_long_running_operation()
> can be cancelled at *any await point*. It is unlikely that arbitrary
> async Rust code is resilient to cancellations anywhere during
> control flow. ... task aborts should be avoided entirely.

The event loop body has many await points:

- The outer `select!` itself
- `handle_notification`'s nested awaits (acquire idle_waiter lock,
  send on broadcast, dispatch RPC callbacks for permission/tool/
  elicitation requests)
- `handle_request`'s inline awaits (deserialize, dispatch handler,
  build response, write back to client)

If `abort()` landed mid-handler — say partway through writing the
permission response — the CLI would keep that requestId in flight
forever. Same hazard for the idle_waiter oneshot: an abort between
"observed session.idle" and "send on waiter.tx" leaves the caller's
`send_and_wait` await blocked until the entire transport tears down.

Fix: cooperative cancellation via `Arc<tokio::sync::Notify>`. The
event-loop body adds a fourth select arm:

    tokio::select! {
        _ = shutdown.notified() => break,
        Some(notification) = notifications.recv() => { ... }
        Some(request) = requests.recv() => { ... }
        else => break,
    }

`Notify::notified()` is cancel-safe per RFD 400's framework: the
unselected branch losing a poll is fine — it's reconstructed on the
next iteration. `notify_one()` (NOT `notify_waiters`) is used because
it buffers a single signal: if the loop is currently inside
`handle_request(...).await` (no `.notified()` future registered), the
signal is held until the handler returns and the loop re-enters
select on the next iteration. `notify_waiters()` would silently lose
the signal in this case.

`stop_event_loop` calls `shutdown.notify_one()` then awaits the
JoinHandle. The loop drains its current iteration's handler before
observing the buffered signal and exiting cleanly — no mid-protocol
state, no orphaned RPC ids on the CLI side.

`Drop for Session` calls `notify_one()` and unregisters from the
router. The handle is left in `event_loop` for tokio to reap when
the loop's spawned task exits naturally; we intentionally don't
await it because Drop is sync. Spawned child tasks inside
`handle_notification` (permission/tool/elicitation callbacks)
intentionally outlive the parent loop — RFD 400's "spawn background
tasks to perform cancel-unsafe operations" pattern, documented in
the loop's comment.

Mutex conversion (folded in from finding #5):

- `Session::event_loop`: `tokio::sync::Mutex<Option<JoinHandle<()>>>` ->
  `parking_lot::Mutex<Option<JoinHandle<()>>>`. Lock is never held
  across `.await` and the conversion eliminates the `try_lock()`
  Drop footgun (silent no-op on contention with stop_event_loop).
  This is the last tokio::sync::Mutex on Session — the import is no
  longer needed.

Tests: two new regression tests in tests/session_test.rs

- `stop_event_loop_completes_in_flight_handler`: install a
  `SlowHandler` whose userInput.request callback sleeps 150ms. Send
  the request from the server side, then 20ms in (handler is mid-
  flight) call `stop_event_loop`. Verify the response with
  `answer: "completed"` lands on the wire BEFORE stop_event_loop
  returns — proves the handler ran to completion, not aborted
  mid-await.
- `drop_session_does_not_abort_handler`: same shape but via
  `drop(session)` instead of `stop_event_loop`. Tracks handler
  completion via an AtomicBool that the handler sets only after
  its sleep completes; verifies both that the response lands on
  the wire AND that the handler actually completed despite the
  Session being dropped.

Existing 213 tests continue to pass; total now 215.

Validation: cargo test --all-features, cargo doc -D warnings, cargo
+nightly fmt --check, cargo clippy -- -D warnings all clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants