mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
Close residual gaps: CEO escalation from blocked, ref repair, reset-script state
Three independent residual hardenings surfaced by the smoke run:
- escalate_to_ceo is now reachable from a blocked task (source widened to
{awaiting_pm_review, blocked} with a matching status-transition row), so a
main_pm or board agent has a clean verb to surface a task it cannot resolve
to the CEO — who can then approve, reject, or cancel it. Regenerated the
lifecycle artifacts (status-transitions doc + panel JSON) to match.
- WorkspaceService now prunes broken loose refs (.bak debris and any ref whose
contents are neither an object id nor a symref) before the refresh fetch, so
a ref left corrupt by an interrupted recovery no longer produces per-operation
"broken ref" warnings or wedges ref enumeration. Best-effort, file-reads-only.
- The reset script's full-reset Claude-state clear now defaults to the
replay-state subdirs (projects/, todos/) of the mounted Claude home and
refuses to clear the home root, so a full reset actually clears conversation
replay state by default without wiping the host's stored credentials.
This commit is contained in:
@@ -17,6 +17,7 @@
|
|||||||
| awaiting_qa | needs_revision | qa_fail | qa |
|
| awaiting_qa | needs_revision | qa_fail | qa |
|
||||||
| backlog | cancelled | cancel | cell_pm, ceo, main_pm |
|
| backlog | cancelled | cancel | cell_pm, ceo, main_pm |
|
||||||
| backlog | pending | activate | any |
|
| backlog | pending | activate | any |
|
||||||
|
| blocked | awaiting_ceo_approval | escalate_to_ceo | head_marketing, main_pm, product_owner |
|
||||||
| blocked | cancelled | cancel | cell_pm, ceo, main_pm |
|
| blocked | cancelled | cancel | cell_pm, ceo, main_pm |
|
||||||
| blocked | in_progress | unblock | any |
|
| blocked | in_progress | unblock | any |
|
||||||
| blocked | pending | unblock | any |
|
| blocked | pending | unblock | any |
|
||||||
|
|||||||
@@ -456,6 +456,16 @@
|
|||||||
"source": "backlog",
|
"source": "backlog",
|
||||||
"target": "pending"
|
"target": "pending"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"action": "escalate_to_ceo",
|
||||||
|
"roles": [
|
||||||
|
"head_marketing",
|
||||||
|
"main_pm",
|
||||||
|
"product_owner"
|
||||||
|
],
|
||||||
|
"source": "blocked",
|
||||||
|
"target": "awaiting_ceo_approval"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"action": "cancel",
|
"action": "cancel",
|
||||||
"roles": [
|
"roles": [
|
||||||
|
|||||||
@@ -285,6 +285,13 @@ _STATUS_TRANSITIONS: tuple[StatusTransition, ...] = (
|
|||||||
"escalate_to_ceo",
|
"escalate_to_ceo",
|
||||||
frozenset({Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING}),
|
frozenset({Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING}),
|
||||||
),
|
),
|
||||||
|
# A blocked task the PM cannot resolve can be surfaced to the CEO directly.
|
||||||
|
StatusTransition(
|
||||||
|
Status.BLOCKED,
|
||||||
|
Status.AWAITING_CEO_APPROVAL,
|
||||||
|
"escalate_to_ceo",
|
||||||
|
frozenset({Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING}),
|
||||||
|
),
|
||||||
# CEO approve / reject
|
# CEO approve / reject
|
||||||
StatusTransition(
|
StatusTransition(
|
||||||
Status.AWAITING_CEO_APPROVAL,
|
Status.AWAITING_CEO_APPROVAL,
|
||||||
@@ -510,7 +517,10 @@ _ATOMIC_ACTIONS: dict[str, ActionSpec] = {
|
|||||||
Role.HEAD_MARKETING,
|
Role.HEAD_MARKETING,
|
||||||
}
|
}
|
||||||
),
|
),
|
||||||
source_statuses=frozenset({Status.AWAITING_PM_REVIEW}),
|
# Reachable from a completed review (the normal sign-off escalation) AND
|
||||||
|
# from a blocked task the PM cannot resolve — so a wedged task has a
|
||||||
|
# clean verb to a human decision instead of only the admin override.
|
||||||
|
source_statuses=frozenset({Status.AWAITING_PM_REVIEW, Status.BLOCKED}),
|
||||||
target_status=Status.AWAITING_CEO_APPROVAL,
|
target_status=Status.AWAITING_CEO_APPROVAL,
|
||||||
allowed_task_types=None,
|
allowed_task_types=None,
|
||||||
preconditions=(),
|
preconditions=(),
|
||||||
|
|||||||
@@ -40,6 +40,11 @@ from roboco.models.base import Team
|
|||||||
|
|
||||||
logger = get_logger(__name__)
|
logger = get_logger(__name__)
|
||||||
|
|
||||||
|
# A healthy loose ref file holds either an object id (sha1 = 40 hex, sha256 = 64
|
||||||
|
# hex) or a symbolic ref ("ref: refs/..."). Anything else is debris — used to
|
||||||
|
# detect broken loose refs left by interrupted recovery before a fetch.
|
||||||
|
_REF_OBJECT_ID_RE = re.compile(r"\A[0-9a-f]{40}\Z|\A[0-9a-f]{64}\Z")
|
||||||
|
|
||||||
# Agent container runs the `agent` user created in agent-base.Dockerfile.
|
# Agent container runs the `agent` user created in agent-base.Dockerfile.
|
||||||
# Debian's `useradd -m` defaults to uid 1000 when that uid is free.
|
# Debian's `useradd -m` defaults to uid 1000 when that uid is free.
|
||||||
# Overridable via env so operators can customize if they rebuild agent-base
|
# Overridable via env so operators can customize if they rebuild agent-base
|
||||||
@@ -416,6 +421,44 @@ class WorkspaceService:
|
|||||||
and (git_dir / "objects").exists()
|
and (git_dir / "objects").exists()
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _prune_broken_refs(workspace: Path) -> None:
|
||||||
|
"""Drop debris loose refs before a fetch. Best-effort; never raises.
|
||||||
|
|
||||||
|
Interrupted hard-stop recovery can leave ``.bak`` ref debris and
|
||||||
|
truncated/garbage loose-ref files under ``.git/refs``. Git tolerates
|
||||||
|
them but emits a "ignoring broken ref" warning on every ref-walking
|
||||||
|
operation (fetch included), which pollutes logs and can wedge ref
|
||||||
|
enumeration. Remove ``.bak`` debris and any loose ref whose contents are
|
||||||
|
neither an object id nor a symref. Reads files only — no per-ref
|
||||||
|
subprocess — so it stays cheap even on a many-branch monorepo clone.
|
||||||
|
"""
|
||||||
|
refs_dir = workspace / ".git" / "refs"
|
||||||
|
if not refs_dir.is_dir():
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
for ref_file in refs_dir.rglob("*"):
|
||||||
|
if not ref_file.is_file():
|
||||||
|
continue
|
||||||
|
if ref_file.suffix == ".bak":
|
||||||
|
ref_file.unlink(missing_ok=True)
|
||||||
|
continue
|
||||||
|
content = ref_file.read_text(encoding="utf-8", errors="replace").strip()
|
||||||
|
if not (
|
||||||
|
_REF_OBJECT_ID_RE.match(content) or content.startswith("ref: ")
|
||||||
|
):
|
||||||
|
logger.debug(
|
||||||
|
"ensure_workspace: pruning broken loose ref",
|
||||||
|
ref=str(ref_file),
|
||||||
|
)
|
||||||
|
ref_file.unlink(missing_ok=True)
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(
|
||||||
|
"ensure_workspace: broken-ref prune failed",
|
||||||
|
workspace=str(workspace),
|
||||||
|
error=str(exc),
|
||||||
|
)
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
async def _fetch_origin_best_effort(workspace: Path, project_slug: str) -> None:
|
async def _fetch_origin_best_effort(workspace: Path, project_slug: str) -> None:
|
||||||
"""Refresh `origin`'s refs into a healthy clone. Never raises.
|
"""Refresh `origin`'s refs into a healthy clone. Never raises.
|
||||||
@@ -603,6 +646,10 @@ class WorkspaceService:
|
|||||||
# the first call always runs the fetch regardless of clock value.
|
# the first call always runs the fetch regardless of clock value.
|
||||||
last_fetch = self._fetch_cache.get(str(workspace), -math.inf)
|
last_fetch = self._fetch_cache.get(str(workspace), -math.inf)
|
||||||
if force or (now - last_fetch) >= _FETCH_CACHE_TTL_SECONDS:
|
if force or (now - last_fetch) >= _FETCH_CACHE_TTL_SECONDS:
|
||||||
|
# Repair broken-ref debris first so the fetch (and the
|
||||||
|
# agent's later `git diff/log origin/...`) doesn't trip on a
|
||||||
|
# ref left corrupt by an interrupted recovery.
|
||||||
|
await asyncio.to_thread(self._prune_broken_refs, workspace)
|
||||||
await self._fetch_origin_best_effort(workspace, project_slug)
|
await self._fetch_origin_best_effort(workspace, project_slug)
|
||||||
self._fetch_cache[str(workspace)] = _monotonic()
|
self._fetch_cache[str(workspace)] = _monotonic()
|
||||||
# Re-chown so the agent user can still write into .git
|
# Re-chown so the agent user can still write into .git
|
||||||
|
|||||||
@@ -78,11 +78,17 @@ fi
|
|||||||
|
|
||||||
# Optional full clean-slate (opt-in via FULL_RESET=1): wipe everything under the
|
# Optional full clean-slate (opt-in via FULL_RESET=1): wipe everything under the
|
||||||
# roboco data root EXCEPT the persistent service stores (ollama / postgres /
|
# roboco data root EXCEPT the persistent service stores (ollama / postgres /
|
||||||
# redis), and clear any persisted agent Claude session state that would
|
# redis), and clear the persisted agent Claude session state that would
|
||||||
# otherwise replay across runs. Default OFF — the workspace git-reset below is
|
# otherwise replay across runs. Default OFF — the workspace git-reset below is
|
||||||
# the usual path. The data root is resolved from the workspaces root's parent
|
# the usual path. The data root is resolved from the workspaces root's parent
|
||||||
# (or ROBOCO_DATA_ROOT); the Claude-state paths come from ROBOCO_CLAUDE_STATE_DIRS
|
# (or ROBOCO_DATA_ROOT).
|
||||||
# (space-separated) so nothing is guessed.
|
#
|
||||||
|
# Claude session state lives under the mounted Claude home (ROBOCO_HOST_CLAUDE_DIR,
|
||||||
|
# the same dir the agent containers mount). When ROBOCO_CLAUDE_STATE_DIRS is unset
|
||||||
|
# we clear the conversation-transcript and todo subdirs of that home — the state
|
||||||
|
# that replays across runs — and NEVER the home root itself, which holds
|
||||||
|
# `.credentials.json` (wiping it would log the host out of Claude). Override
|
||||||
|
# ROBOCO_CLAUDE_STATE_DIRS (space-separated) to target different paths.
|
||||||
if [ "${FULL_RESET:-0}" = "1" ]; then
|
if [ "${FULL_RESET:-0}" = "1" ]; then
|
||||||
DATA_ROOT="${ROBOCO_DATA_ROOT:-}"
|
DATA_ROOT="${ROBOCO_DATA_ROOT:-}"
|
||||||
if [ -z "$DATA_ROOT" ]; then
|
if [ -z "$DATA_ROOT" ]; then
|
||||||
@@ -110,7 +116,16 @@ if [ "${FULL_RESET:-0}" = "1" ]; then
|
|||||||
else
|
else
|
||||||
echo ">>> FULL_RESET: no data root resolved — skipping data wipe."
|
echo ">>> FULL_RESET: no data root resolved — skipping data wipe."
|
||||||
fi
|
fi
|
||||||
for cdir in ${ROBOCO_CLAUDE_STATE_DIRS:-}; do
|
CLAUDE_HOME="${ROBOCO_HOST_CLAUDE_DIR:-$HOME/.claude}"
|
||||||
|
CLAUDE_STATE_DIRS="${ROBOCO_CLAUDE_STATE_DIRS:-$CLAUDE_HOME/projects $CLAUDE_HOME/todos}"
|
||||||
|
for cdir in $CLAUDE_STATE_DIRS; do
|
||||||
|
# Never wipe the Claude home itself — credentials live there.
|
||||||
|
case "$cdir" in
|
||||||
|
"$CLAUDE_HOME" | "$CLAUDE_HOME/")
|
||||||
|
echo " refusing to clear Claude home $cdir (holds credentials)"
|
||||||
|
continue
|
||||||
|
;;
|
||||||
|
esac
|
||||||
if [ -e "$cdir" ]; then
|
if [ -e "$cdir" ]; then
|
||||||
echo " clearing Claude session state $cdir"
|
echo " clearing Claude session state $cdir"
|
||||||
rm -rf "$cdir"
|
rm -rf "$cdir"
|
||||||
|
|||||||
@@ -1025,7 +1025,7 @@ async def test_escalate_to_ceo_matches_spec(role: str, status: str) -> None:
|
|||||||
|
|
||||||
- role in {main_pm, product_owner, head_marketing},
|
- role in {main_pm, product_owner, head_marketing},
|
||||||
- composed ``escalate_to_ceo`` action's source_status
|
- composed ``escalate_to_ceo`` action's source_status
|
||||||
(AWAITING_PM_REVIEW only).
|
(AWAITING_PM_REVIEW or BLOCKED).
|
||||||
|
|
||||||
The verb body keeps the journal:decision preflight (the spec doesn't
|
The verb body keeps the journal:decision preflight (the spec doesn't
|
||||||
model journal side effects); satisfied here so the spec gate is the
|
model journal side effects); satisfied here so the spec gate is the
|
||||||
|
|||||||
@@ -242,6 +242,8 @@ def test_status_transitions_includes_ceo_paths() -> None:
|
|||||||
) in sources
|
) in sources
|
||||||
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED) in sources
|
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED) in sources
|
||||||
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.NEEDS_REVISION) in sources
|
assert (spec.Status.AWAITING_CEO_APPROVAL, spec.Status.NEEDS_REVISION) in sources
|
||||||
|
# A blocked task the PM cannot resolve can also be surfaced to the CEO.
|
||||||
|
assert (spec.Status.BLOCKED, spec.Status.AWAITING_CEO_APPROVAL) in sources
|
||||||
|
|
||||||
|
|
||||||
def test_status_transitions_includes_block_pause_paths() -> None:
|
def test_status_transitions_includes_block_pause_paths() -> None:
|
||||||
@@ -307,20 +309,35 @@ def test_status_transitions_role_constraints_match_canon() -> None:
|
|||||||
assert by_pair[
|
assert by_pair[
|
||||||
(spec.Status.AWAITING_PM_REVIEW, spec.Status.COMPLETED, "complete")
|
(spec.Status.AWAITING_PM_REVIEW, spec.Status.COMPLETED, "complete")
|
||||||
] == frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM})
|
] == frozenset({spec.Role.CELL_PM, spec.Role.MAIN_PM})
|
||||||
# escalate_to_ceo: main_pm + product_owner + head_marketing
|
# escalate_to_ceo: main_pm + product_owner + head_marketing — from a
|
||||||
assert by_pair[
|
# completed review and from a blocked task, same role gate.
|
||||||
(
|
escalate_roles = frozenset(
|
||||||
spec.Status.AWAITING_PM_REVIEW,
|
|
||||||
spec.Status.AWAITING_CEO_APPROVAL,
|
|
||||||
"escalate_to_ceo",
|
|
||||||
)
|
|
||||||
] == frozenset(
|
|
||||||
{
|
{
|
||||||
spec.Role.MAIN_PM,
|
spec.Role.MAIN_PM,
|
||||||
spec.Role.PRODUCT_OWNER,
|
spec.Role.PRODUCT_OWNER,
|
||||||
spec.Role.HEAD_MARKETING,
|
spec.Role.HEAD_MARKETING,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
assert (
|
||||||
|
by_pair[
|
||||||
|
(
|
||||||
|
spec.Status.AWAITING_PM_REVIEW,
|
||||||
|
spec.Status.AWAITING_CEO_APPROVAL,
|
||||||
|
"escalate_to_ceo",
|
||||||
|
)
|
||||||
|
]
|
||||||
|
== escalate_roles
|
||||||
|
)
|
||||||
|
assert (
|
||||||
|
by_pair[
|
||||||
|
(
|
||||||
|
spec.Status.BLOCKED,
|
||||||
|
spec.Status.AWAITING_CEO_APPROVAL,
|
||||||
|
"escalate_to_ceo",
|
||||||
|
)
|
||||||
|
]
|
||||||
|
== escalate_roles
|
||||||
|
)
|
||||||
# CEO actions: CEO only
|
# CEO actions: CEO only
|
||||||
assert by_pair[
|
assert by_pair[
|
||||||
(spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED, "ceo_approve")
|
(spec.Status.AWAITING_CEO_APPROVAL, spec.Status.COMPLETED, "ceo_approve")
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
"""`WorkspaceService._prune_broken_refs` drops debris loose refs before a fetch.
|
||||||
|
|
||||||
|
Interrupted hard-stop recovery can leave `.bak` ref debris and truncated/garbage
|
||||||
|
loose-ref files under `.git/refs`. The prune reads files only (no git subprocess),
|
||||||
|
so these tests just stage a `.git/refs` tree on disk.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import TYPE_CHECKING
|
||||||
|
|
||||||
|
from roboco.services.workspace import WorkspaceService
|
||||||
|
|
||||||
|
if TYPE_CHECKING:
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
_VALID_SHA1 = "a" * 40
|
||||||
|
_VALID_SHA256 = "b" * 64
|
||||||
|
|
||||||
|
|
||||||
|
def _make_heads(workspace: Path) -> Path:
|
||||||
|
heads = workspace / ".git" / "refs" / "heads"
|
||||||
|
heads.mkdir(parents=True)
|
||||||
|
return heads
|
||||||
|
|
||||||
|
|
||||||
|
def test_keeps_valid_sha1_ref(tmp_path: Path) -> None:
|
||||||
|
good = _make_heads(tmp_path) / "main"
|
||||||
|
good.write_text(_VALID_SHA1 + "\n", encoding="utf-8")
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
|
assert good.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_keeps_valid_sha256_ref(tmp_path: Path) -> None:
|
||||||
|
good = _make_heads(tmp_path) / "main"
|
||||||
|
good.write_text(_VALID_SHA256 + "\n", encoding="utf-8")
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
|
assert good.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_keeps_symref(tmp_path: Path) -> None:
|
||||||
|
sym = _make_heads(tmp_path) / "current"
|
||||||
|
sym.write_text("ref: refs/heads/main\n", encoding="utf-8")
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
|
assert sym.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_removes_bak_debris_even_with_valid_content(tmp_path: Path) -> None:
|
||||||
|
bak = _make_heads(tmp_path) / "feature.bak"
|
||||||
|
bak.write_text(_VALID_SHA1, encoding="utf-8")
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
|
assert not bak.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_removes_garbage_ref(tmp_path: Path) -> None:
|
||||||
|
junk = _make_heads(tmp_path) / "broken"
|
||||||
|
junk.write_text("not-a-sha-or-symref garbage", encoding="utf-8")
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
|
assert not junk.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_git_refs_dir_is_noop(tmp_path: Path) -> None:
|
||||||
|
# No .git/refs present — must not raise.
|
||||||
|
WorkspaceService._prune_broken_refs(tmp_path)
|
||||||
Reference in New Issue
Block a user