Repository navigation
chore: release develop to main - #362
Conversation
- Added `pr_monitor.rs` to handle the lifecycle states of pull requests (PRs) including conflicts, CI status, review states, and comments. - Introduced `PrMonitorState` enum to represent various PR states and methods to classify PRs based on their current status. - Created `ReworkDirective` struct to encapsulate the information sent to FORGE for addressing PRs. - Enhanced `VesselConfig` to include a flag for enabling/disabling `/address_review` directives. - Updated `VesselOutcome` to track when a review directive is dispatched. - Added new actions and keys in the state management for handling address review dispatch and rearming. - Extended GitHub REST client with methods to list PR reviews and comments. - Documented the `/address_review` command and its usage in the agent documentation. - Updated various agent skills to handle the new review directive and its implications for PR management.
- Added logic to resolve workspace branch based on pending PRs in agent-nexus. - Enhanced vessel agent to dispatch /ci_fix directives into existing forge chat sessions for CI failures. - Updated Terraform configuration to include branch parameter for workspace provisioning. - Improved error handling in GitHub API client to fall back to Checks API when legacy combined-status API is inaccessible. - Created documentation for the new /ci_fix command and its usage in the context of CI failures. - Introduced skills for handling /ci_fix directives in both FORGE and VESSEL agents. - Added tests for workspace branch resolution and API error handling. - Documented the Forge-Sentinel pair architecture and workflow in a new blog post.
…and comments GitHub returns `user` as an object on review and review-comment responses, but PrReview/ReviewComment expected a plain string, so a normal response failed to deserialize and VESSEL treated even approved PRs as having no reviews. Add a custom deserializer that extracts the login from either the nested user object or a plain string. Addresses review comment on crates/github/src/rest.rs.
…dispatch - Timeout path: a CI timeout can no longer bypass outstanding review feedback (ChangesRequested/Comments) and merge; it now routes back to FORGE via /address_review like the CI-Success path. - Review dispatch fallback: when no live FORGE chat exists, persist the /address_review directive (with the actual review feedback) for NEXUS to deliver, instead of routing to the conflict handler which drops the feedback. - Review wait: the NeedsReview gating wait now runs before the rework-attempt cap, so a PR merely awaiting SENTINEL approval is not escalated to human after the cap is exhausted. Addresses review comments on crates/agent-vessel/src/node.rs.
…ict PR - openflows-harness review_submit now rejects non-SENTINEL roles, so FORGE cannot submit an approve for its own ticket and bypass the independent reviewer. - SENTINEL resolve_pr_number verifies a supplied --pr against the ticket's recorded PR; a mismatched --pr falls back to the recorded PR instead of posting APPROVE/REQUEST_CHANGES to another PR. Addresses review comments on crates/openflows-harness/src/store.rs and crates/agent-sentinel/src/lib.rs.
…PR reentry - The persisted rework directive (/ci_fix / /address_review) is now cleared only after the replacement chat is successfully created, so a failed chat creation no longer loses the rework request. - A PR whose ticket is AwaitingHuman is no longer re-added to automatic polling even when a stale /address_review dispatch marker is still present. Addresses review comments on crates/agent-nexus/src/lib.rs.
- The requested branch is validated against a strict git-ref-safe charset before interpolation into the startup bash script, so a malicious branch cannot inject shell command substitution. - Rework checkout now creates a local tracking branch (git checkout -B) instead of checking out origin/<branch> directly, which left HEAD detached and broke the plain `git push` used by the CI-fix / /address_review flow. Addresses review comments on crates/coder-client/templates/openflows-forge/main.tf.
…branch - The branch whitelist now accepts all git-valid, shell-safe characters (including `+`, `=`, `@`) so valid branches like `feat/foo+bar` are no longer rejected, while still blocking shell injection. - The rework checkout prefers an existing local branch (preserving in-flight work), then creates a tracking branch from origin, and only falls back to the default branch when the PR branch is absent locally and on origin (e.g. fork-backed PR) — it no longer fabricates a PR-named branch at origin/HEAD, which would start rework without the PR's commits and could reset local work. Addresses review comments on crates/coder-client/templates/openflows-forge/main.tf.
When a reused workspace has the target branch locally but uncommitted work or an unresolved merge blocks the switch, the previous bare `git checkout` aborted the whole startup script via `set -e`, so the heartbeat and agent never started and FORGE could not perform the rework. The switch is now guarded so a failure logs a warning and startup continues on the current branch. Addresses review comment on crates/coder-client/templates/openflows-forge/main.tf.
When the startup script cannot switch to the intended PR branch (uncommitted work or an unresolved merge, or a fork-backed branch absent from origin), the agent stays available but the workspace would otherwise sit on a different branch and the /ci_fix flow's plain `git push` could publish to the wrong branch while the PR stays unchanged. Add a warn_branch_mismatch helper that logs the actual-vs-intended branch prominently and writes a workspace-root REWORK_BRANCH_MISMATCH.md marker telling the agent not to push on the current branch, and call it from both failure paths. Addresses review comment on crates/coder-client/templates/openflows-forge/main.tf.
The REWORK_BRANCH_MISMATCH.md marker write can fail if a reused workspace is not writable by `coder`. Under `set -e` that would abort startup before the heartbeat and agent start, leaving FORGE unavailable to resolve the mismatch. Guard the write with `|| true` so it is best-effort: the log warning already carries the message, and FORGE stays available. Addresses review comment on crates/coder-client/templates/openflows-forge/main.tf.
…orge chat liveness - Detect tickets with a persisted rework directive by scanning the ticket list instead of pending_prs, so a directive for an InProgress ticket (whose PR was removed from pending_prs and is not rediscovered) is still found and a FORGE is provisioned to deliver it. - Verify a stored forge chat is actually live (get_chat_opt) before treating it as able to carry the rework; clear the binding and provision a replacement when the chat no longer exists, instead of skipping provisioning on a stale id. - Bail out of workspace config provisioning immediately when load_registry fails, rather than burning the retry loop on futile SSH waits, since the registry is deterministic and provision_role cannot run without it.
…rovisioning
- Skip tickets marked AwaitingHuman in the rework-provisioning scan so a
lingering rework directive can no longer let NEXUS grab the released idle
FORGE slot and flip the ticket back to Assigned, undoing the human escalation.
- Restore the rework PR to pending_prs (re-fetching open PRs from GitHub) when
its ticket has a persisted directive but the PR is not tracked there. VESSEL
removes the PR from pending_prs when it persists the directive, and the
replacement workspace's branch is resolved from pending_prs — without the
restore it would be provisioned on the default {worker}/{ticket} branch and
start without the PR's commits.
…ch directive PR A persisted rework directive with no available FORGE worker previously had its PR restored to pending_prs without assigning a handler, re-routing it back to VESSEL where another failed /address_review dispatch consumed the attempt limit and could escalate a ticket merely waiting for a free FORGE. Now the worker is resolved first and only provisionable tickets restore their PR / get provisioned; the rest wait for a worker without burning attempts. Also identify the PR to restore from the directive's embedded pr: number instead of the first open PR matching the ticket_id, so multiple open PRs referencing the same ticket cannot select the wrong one for rework.
When a rework directive locates the PR by number but GitHub cannot extract
the ticket id from that PR's title/body/branch, storing the PR's extracted
id made resolve_workspace_branch (which matches the rework ticket id) miss
the restored PR and fall back to {worker}/{ticket}, starting FORGE without
the PR's commits. Associate the restored PR with the rework ticket's id so
provisioning always checks out the PR's real head branch.
feat: vessel lifecycle - spawn merge agent as controllable coder workspace
…esses - Updated the FORGE planning skill to clarify the planning lifecycle and emphasize the importance of grounding plans before implementation. - Enhanced the structure and content of PLAN.md to ensure comprehensive planning, including explicit segment breakdowns and risk assessments. - Revised the SENTINEL review skill to focus on actionable feedback and the verification process, ensuring clear communication of requirements and expectations. - Improved the shared harness protocol documentation to outline authoritative state management and lifecycle transitions more clearly. - Modified build scripts to support cross-compilation for Linux targets, ensuring compatibility with both Intel and Apple Silicon macOS hosts. - Added checks to prevent syncing non-ELF binaries that could disrupt the Coder workspace.
…approval handling
…ments fix(lifecycle): enforce state machine review and merge gates
feat(docs): add comprehensive state machine whitepaper detailing life…
The docker-publish workflow switched from the workflow_run trigger to a direct dispatch from release.yml (passing the version explicitly), so resolve-version.sh is no longer used by any workflow. Remove it and its regression test, and drop the now-obsolete shell-tests CI job.
Only route PRs whose ticket lifecycle is in submit to VESSEL for merge. PRs whose tickets are still in an earlier phase (building, testing, etc.) are held via a new lifecycle Block event instead of stalling the whole queue, so the next assignable ticket can proceed. - config: add Block lifecycle event (records reason + feedback, enters blocked) - config: add KEY_MERGE_READY_PRS shared-store key - nexus: compute merge-ready PRs in pending_prs_ready_for_vessel and persist them - vessel: prefer the merge-ready handoff over the raw pending queue - forge template: install and auth gh CLI in the workspace for PR creation - docker-compose: correct REDIS_URL service host for the controller
When the coder CLI is missing from PATH, push_template reported only 'Failed to run coder templates push', masking the real cause and bootstrap conflated it with missing template-management permissions. Handle a NotFound spawn error with an actionable install hint, and return the actual CLI stderr/exit code on non-zero template push.
…-push-error # Conflicts: # crates/coder-client/src/lib.rs
Replace the optional gh CLI setup with a required, verified credential inheritance: workspace startup now fails loudly if the controller-spawner's inherited GitHub credential is missing, gh cannot be installed or authed, or the authed identity does not match the inherited token. Sets git author identity and records the resolved login (never the token).
…error fix(coder-client): surface real stderr on coder templates push failure
…solver chore(ci): remove unused docker version resolver
…ating fix(merge): gate VESSEL on merge-ready PRs and hold non-submit PRs
| fn prohibited_operation(argv: &[String]) -> Option<&'static str> { | ||
| let program = argv.first()?.rsplit('/').next()?; | ||
| let args = &argv[1..]; |
There was a problem hiding this comment.
Interpreter bypasses command policy. Verification now permits
sh -c because the policy checks the executable name rather than the script it runs. A SENTINEL request can run a prohibited operation, such as deleting FORGE workspace files or invoking git push, with FORGE’s filesystem and credential access. How this was verified: The relay accepts the literal sh argument while the executor passes it to exec in FORGE’s login environment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/a2a-protocol/src/lib.rs
Line: 31-33
Comment:
**Interpreter bypasses command policy.** Verification now permits `sh -c` because the policy checks the executable name rather than the script it runs. A SENTINEL request can run a prohibited operation, such as deleting FORGE workspace files or invoking `git push`, with FORGE’s filesystem and credential access. **How this was verified:** The relay accepts the literal `sh` argument while the executor passes it to `exec` in FORGE’s login environment.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| state | ||
| .relay | ||
| .verify_pair_token(pair_id, pair_token(params)?) | ||
| .await?; |
There was a problem hiding this comment.
Task progress remains unprotected. The new pair-token check protects progress writes, but
tasks/resubscribe and the SSE endpoint still read progress using only a task ID. A caller who obtains another pair’s task ID can read its buffered or live stdout and stderr. How this was verified: Both progress readers replay task events without checking a pair token or task ownership.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/agent-nexus/src/a2a/http_server.rs
Line: 471-474
Comment:
**Task progress remains unprotected.** The new pair-token check protects progress writes, but `tasks/resubscribe` and the SSE endpoint still read progress using only a task ID. A caller who obtains another pair’s task ID can read its buffered or live stdout and stderr. **How this was verified:** Both progress readers replay task events without checking a pair token or task ownership.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| fn checkout_head() -> Result<String> { | ||
| let dirty = std::process::Command::new("git") | ||
| .args(["status", "--porcelain"]) | ||
| .output()?; | ||
| anyhow::ensure!( | ||
| dirty.status.success() && dirty.stdout.is_empty(), | ||
| "Commit all work before testing; checkout must be clean" |
There was a problem hiding this comment.
Provisioned files block testing. Normal FORGE provisioning writes
AGENTS.md, a persona, and .agents/skills files into the checkout, but those paths are not ignored. The new git status --porcelain check treats them as untracked changes, so status set testing fails before the ticket can enter testing. Verification checkout creation applies the same clean-checkout requirement.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/openflows-harness/src/store.rs
Line: 163-169
Comment:
**Provisioned files block testing.** Normal FORGE provisioning writes `AGENTS.md`, a persona, and `.agents/skills` files into the checkout, but those paths are not ignored. The new `git status --porcelain` check treats them as untracked changes, so `status set testing` fails before the ticket can enter testing. Verification checkout creation applies the same clean-checkout requirement.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| /// Construct a GitHub REST client for SENTINEL's review submission from the | ||
| /// environment (external-auth token, same source VESSEL uses). | ||
| fn github_client_from_env() -> Option<github::GithubRestClient> { | ||
| let token = config::GithubConfig::init_from_env() | ||
| .ok() | ||
| .and_then(|g| g.resolve_token()) | ||
| .filter(|t| !t.is_empty())?; | ||
| Some(github::GithubRestClient::new(token)) |
There was a problem hiding this comment.
PR author cannot approve. SENTINEL submits its GitHub
APPROVE review with the controller’s external-auth token, while FORGE is configured to open PRs as that same workspace owner. GitHub rejects approval by a PR’s author. The failed delivery remains pending, so merge_ready stays false even after the lifecycle approvals are recorded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/agent-sentinel/src/lib.rs
Line: 150-157
Comment:
**PR author cannot approve.** SENTINEL submits its GitHub `APPROVE` review with the controller’s external-auth token, while FORGE is configured to open PRs as that same workspace owner. GitHub rejects approval by a PR’s author. The failed delivery remains pending, so `merge_ready` stays false even after the lifecycle approvals are recorded.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if approved && phase == Phase::Testing && actor == "sentinel" { | ||
| // The verified testing verdict and permission to publish | ||
| // commit atomically, matching plan approval. | ||
| next.phase = Phase::Submit; | ||
| next.review_round += 1; | ||
| next.pr_delivery = None; | ||
| next.feedback = None; |
There was a problem hiding this comment.
Testing skips human approval. A SENTINEL testing approval immediately moves the ticket to
submit without checking test_human. A human can no longer record a testing decision afterward because decisions must match the current phase. This skips the testing human-approval gate required by the added lifecycle design.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/config/src/lifecycle.rs
Line: 369-375
Comment:
**Testing skips human approval.** A SENTINEL testing approval immediately moves the ticket to `submit` without checking `test_human`. A human can no longer record a testing decision afterward because decisions must match the current phase. This skips the testing human-approval gate required by the added lifecycle design.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let url = format!( | ||
| "{}/repos/{}/{}/pulls/{}/reviews?per_page=100", | ||
| self.api_base, owner, repo, pr_number | ||
| ); | ||
| self.get_json(&url).await |
There was a problem hiding this comment.
Review queries miss later pages. PR reviews and inline comments are fetched with
per_page=100, but neither query fetches another page. On a PR with more than 100 reviews, the latest change request may be missing when VESSEL computes the current verdict; excess inline feedback is missed as well. Please paginate both queries so long-running reviews are assessed from the full record.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/github/src/rest.rs
Line: 1077-1081
Comment:
**Review queries miss later pages.** PR reviews and inline comments are fetched with `per_page=100`, but neither query fetches another page. On a PR with more than 100 reviews, the latest change request may be missing when VESSEL computes the current verdict; excess inline feedback is missed as well. Please paginate both queries so long-running reviews are assessed from the full record.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| fn same_sources(before: &Path, after: &Path) -> Result<bool> { | ||
| for entry in std::fs::read_dir(before)? { | ||
| let before = entry?.path(); | ||
| let after = after.join(before.file_name().unwrap()); | ||
| let old = std::fs::symlink_metadata(&before)?; | ||
| let Ok(new) = std::fs::symlink_metadata(&after) else { |
There was a problem hiding this comment.
New source files escape comparison. The source comparison visits only files present in the baseline checkout. A verification command can add a source or test file to its temporary checkout and still receive clean-head evidence for the original commit. Generated outputs may need an allowance, but without checking new source files the gate can treat a different tree as the candidate it verified.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/openflows-harness/src/sandbox.rs
Line: 185-190
Comment:
**New source files escape comparison.** The source comparison visits only files present in the baseline checkout. A verification command can add a source or test file to its temporary checkout and still receive clean-head evidence for the original commit. Generated outputs may need an allowance, but without checking new source files the gate can treat a different tree as the candidate it verified.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let sandbox = crate::sandbox::Sandbox::create(&std::env::current_dir()?, argv)?; | ||
| let before_head = Some(sandbox.head.clone()); | ||
| let start = Instant::now(); |
There was a problem hiding this comment.
Checkout preparation escapes timeout. The requested
timeout_secs starts only after synchronous checkout preparation. On a slow or large repository, the two clones can outlast the verification deadline; the initial Git status and HEAD commands have no setup deadline at all. SENTINEL can stop waiting while FORGE is still preparing the checkout, with no terminal result for that request.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/openflows-harness/src/executor.rs
Line: 54-56
Comment:
**Checkout preparation escapes timeout.** The requested `timeout_secs` starts only after synchronous checkout preparation. On a slow or large repository, the two clones can outlast the verification deadline; the initial Git status and HEAD commands have no setup deadline at all. SENTINEL can stop waiting while FORGE is still preparing the checkout, with no terminal result for that request.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // Moving on supersedes queued notification of an older review. | ||
| // Its report remains in history; GitHub availability cannot | ||
| // prevent rework from reaching a new testing round. | ||
| next.pr_delivery = None; | ||
| next.feedback = None; | ||
| next.phase = phase; |
There was a problem hiding this comment.
Blocked reason is discarded.
status set blocked uses Event::Move, which clears feedback; the separate reason-bearing Event::Block is not used by that command. A worker reporting an external blocker therefore leaves no durable reason in the lifecycle, and NEXUS can send only generic fallback text rather than the question a human needs to resolve.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/config/src/lifecycle.rs
Line: 281-286
Comment:
**Blocked reason is discarded.** `status set blocked` uses `Event::Move`, which clears `feedback`; the separate reason-bearing `Event::Block` is not used by that command. A worker reporting an external blocker therefore leaves no durable reason in the lifecycle, and NEXUS can send only generic fallback text rather than the question a human needs to resolve.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| openflows-harness gate decide --phase plan_ready --revision 1 --round 1 \ | ||
| --verdict approve --report review.md | ||
| ``` | ||
|
|
||
| Reject uses the same command with `--verdict reject`. FORGE reads the feedback, | ||
| returns to planning, revises/uploads, and resubmits. After approval FORGE enters |
There was a problem hiding this comment.
Plan example uses rejected path. The new worker-command example still uploads
PLAN.md, but the planning hook accepts only the current chat-specific absolute plan path. Following the example produces a policy denial at the first plan upload. Please use the path supplied by Coder or the startup hook in this example.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/architecture/state-machine.md
Line: 42-47
Comment:
**Plan example uses rejected path.** The new worker-command example still uploads `PLAN.md`, but the planning hook accepts only the current chat-specific absolute plan path. Following the example produces a policy denial at the first plan upload. Please use the path supplied by Coder or the startup hook in this example.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
New release from develop to main.
Changes
/cc @Christiantyemele
The PR is not safe to merge while normal verification and GitHub review delivery are blocked and the testing approval boundary is incomplete.
Fix with agent prompt
Summary
This release introduces a versioned ticket lifecycle, pair-token-authenticated A2A verification, PR-review delivery, and current-head CI/merge checks. It also changes worker provisioning, registry defaults, and cross-build tooling.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR P["FORGE: plan"] --> G["SENTINEL: plan decision"] G --> B["FORGE: build and clean candidate"] B --> T["A2A verification and testing decision"] T --> S["Submit PR"] S --> H["Human PR decision"] H --> D["SENTINEL GitHub review delivery"] D --> M["VESSEL: current-head CI and merge"] M --> X["Done"]Reviews (1) · Last reviewed commit: "Merge pull request #359 from The-Agentic..."