feat(python/robomp): implemented has_authorized_impl_event in the

- Implement `has_authorized_impl_event` in the database to retrieve historical authorization state.
- Update `_enforce_impl_authorization` to permit actions if prior events on the issue provided implementation authorization.
- Normalize maintainer logins by stripping `[bot]` suffixes and allow match-regex to ignore them.

python/robomp/.env.example

@@ -46,8 +46,8 @@ ROBOMP_GIT_AUTHOR_EMAIL=
 ROBOMP_REPO_ALLOWLIST=
 
 # Optional comma-separated GitHub logins whose @ROBOMP_BOT_LOGIN comments may
-# authorize implementation work in addition to repo OWNER. `@` and case do not
-# matter; use the webhook actor login, not a display name.
+# authorize implementation work in addition to repo OWNER. Prefer lowercase
+# bare logins; startup ignores leading `@`, case, whitespace, and `[bot]`.
 ROBOMP_MAINTAINER_LOGINS=
 
 

python/robomp/AGENTS.md

@@ -109,7 +109,7 @@ Lint + format: TypeScript via Biome (config in `biome.json`), Python via Ruff (c
 - **Package manager**: `pip` only. No poetry / uv / pdm files; don't introduce one.
 - **Task runner**: `bun` (root `package.json` `scripts`). Always reach for an existing `bun run` recipe before invoking `docker compose` or `pytest` directly.
 - **Container runtime**: Docker Compose v2. The image embeds Bun 1.3.14 + a rustup launcher and exposes `omp` via a `/usr/local/bin/omp` shim; `ROBOMP_OMP_COMMAND=omp` should not need changing.
-- **Required env** (set in `.env`, see `.env.example`): `GITHUB_WEBHOOK_SECRET`, `ROBOMP_BOT_LOGIN`, `ROBOMP_GIT_AUTHOR_NAME`, `ROBOMP_GIT_AUTHOR_EMAIL`, `ROBOMP_REPO_ALLOWLIST`, plus model knobs (`ROBOMP_MODEL`, `ROBOMP_THINKING`, optional `ROBOMP_PROVIDER`) and rate-limit / concurrency / timeout overrides. Set `ROBOMP_BOT_LOGIN` to the lowercase mention handle (`roboomp` in production, no leading `@` or `[bot]`; config normalizes common variants). `ROBOMP_MAINTAINER_LOGINS` is optional comma-separated logins (`@` optional, case-insensitive) for non-owner implementation authorizers. **GitHub auth is mode-exclusive**: either set `ROBOMP_GH_PROXY_URL` + `ROBOMP_GH_PROXY_HMAC_KEY` (gh-proxy mode; PAT lives only in the sidecar container — the bundled compose default), or set `GITHUB_TOKEN` directly (single-process PAT mode). `Settings._validate_proxy_or_pat` rejects a `.env` that sets both.
+- **Required env** (set in `.env`, see `.env.example`): `GITHUB_WEBHOOK_SECRET`, `ROBOMP_BOT_LOGIN`, `ROBOMP_GIT_AUTHOR_NAME`, `ROBOMP_GIT_AUTHOR_EMAIL`, `ROBOMP_REPO_ALLOWLIST`, plus model knobs (`ROBOMP_MODEL`, `ROBOMP_THINKING`, optional `ROBOMP_PROVIDER`) and rate-limit / concurrency / timeout overrides. Set `ROBOMP_BOT_LOGIN` to the lowercase mention handle (`roboomp` in production, no leading `@` or `[bot]`; config normalizes common variants). `ROBOMP_MAINTAINER_LOGINS` is optional comma-separated bare logins (`@`/`[bot]` optional, case-insensitive) for non-owner implementation authorizers. **GitHub auth is mode-exclusive**: either set `ROBOMP_GH_PROXY_URL` + `ROBOMP_GH_PROXY_HMAC_KEY` (gh-proxy mode; PAT lives only in the sidecar container — the bundled compose default), or set `GITHUB_TOKEN` directly (single-process PAT mode). `Settings._validate_proxy_or_pat` rejects a `.env` that sets both.
 - **PI_ROOT resolution**: roboomp lives inside the oh-my-pi monorepo at `python/robomp/`. `bun run pi:image` builds the parent monorepo (`../..`) as its docker build context to produce `oh-my-pi/pi:dev`; `docker-compose.yml` extends that image via `PI_BASE` and mounts the same parent path read-only at `/work/pi` for the orchestrator to see live source. Override `PI_ROOT` only when pointing the build/mount at a different oh-my-pi checkout. Inside the container the path is always `/work/pi`. Build invalidation stays bounded: Python-only edits in roboomp never trigger a natives recompile.
 - **Forbidden**: no docker-in-docker, no extra service containers, no new background workers outside `WorkerPool`. The container itself is the isolation boundary; per-issue isolation is the git worktree.
 

python/robomp/src/config.py

@@ -305,7 +305,10 @@ class Settings(BaseSettings):
 
     @property
     def maintainer_logins(self) -> frozenset[str]:
-        items = [piece.strip().lstrip("@").lower() for piece in self.maintainer_logins_raw.split(",")]
+        items = [
+            piece.strip().lstrip("@").lower().removesuffix("[bot]")
+            for piece in self.maintainer_logins_raw.split(",")
+        ]
         return frozenset(item for item in items if item)
 
     def allows(self, full_name: str) -> bool:

python/robomp/src/db.py

@@ -607,6 +607,26 @@ class Database:
             last_error=row["last_error"],
         )
 
+    def has_authorized_impl_event(self, issue_key: str) -> bool:
+        """Return whether a non-skipped event on this issue carried implementation authorization."""
+        with self._lock:
+            rows = self._conn.execute(
+                """
+                SELECT payload_json
+                FROM events
+                WHERE issue_key = ?
+                  AND state <> 'skipped'
+                ORDER BY received_at DESC
+                """,
+                (issue_key,),
+            ).fetchall()
+        for row in rows:
+            payload = json.loads(row["payload_json"])
+            directive = payload.get("_robomp_directive")
+            if isinstance(directive, dict) and directive.get("authorizes_impl") is True:
+                return True
+        return False
+
     def requeue_event(
         self,
         delivery_id: str,

python/robomp/src/github_events.py

@@ -89,16 +89,11 @@ def _login_matches_personal_repo_owner(
             raw_type = owner.get("type")
             if isinstance(raw_type, str) and raw_type:
                 owner_type = raw_type
-    if owner_type is not None and owner_type.lower() == "organization":
+    if owner_type is None or owner_type.lower() != "user":
+        return False
+    if not owner_login:
         return False
-    if owner_login:
     return login.lower() == owner_login.lower()
-    if not isinstance(repo, str):
-        return False
-    owner, sep, _name = repo.partition("/")
-    if not sep or not owner:
-        return False
-    return login.lower() == owner.lower()
 
 
 def _effective_association(
@@ -158,7 +153,7 @@ def extract_mention(body: str | None, bot_login: str) -> str | None:
     if not login:
         return None
     pattern = re.compile(
-        rf"(?<![A-Za-z0-9_-])@{re.escape(login)}(?![A-Za-z0-9_-])",
+        rf"(?<![A-Za-z0-9_-])@{re.escape(login)}(?:\[bot\])?(?![A-Za-z0-9_-])",
         re.IGNORECASE,
     )
     if not pattern.search(body):

python/robomp/src/host_tools.py

@@ -1121,6 +1121,8 @@ def _enforce_impl_authorization(
     """Refuse first publish on issue classes that require maintainer authorization."""
     if bindings.impl_authorized:
         return
+    if bindings.db.has_authorized_impl_event(bindings.issue_key):
+        return
     row = bindings.db.get_issue(bindings.issue_key)
     if row is not None:
         if row.pr_number is not None:

python/robomp/tests/test_config.py

@@ -119,7 +119,7 @@ def test_maintainer_logins_normalize_csv_entries(
     monkeypatch.setenv("ROBOMP_MAINTAINER_LOGINS", " can1357, @ROBOOMP , @Alice[bot] ,, ")
     reset_settings_cache()
     cfg = Settings()  # type: ignore[call-arg]
-    assert cfg.maintainer_logins == frozenset({"can1357", "roboomp", "alice[bot]"})
+    assert cfg.maintainer_logins == frozenset({"can1357", "roboomp", "alice"})
 
 
 @pytest.mark.parametrize(
@@ -128,9 +128,9 @@ def test_maintainer_logins_normalize_csv_entries(
         ("roboomp", "roboomp"),
         (" @roboomp ", "roboomp"),
         (" @ROBOOMP ", "roboomp"),
-        ("roboomp[bot]", "roboomp[bot]"),
-        ("@roboomp[bot]", "roboomp[bot]"),
-        (" @ROBOOMP[BOT] ", "roboomp[bot]"),
+        ("roboomp[bot]", "roboomp"),
+        ("@roboomp[bot]", "roboomp"),
+        (" @ROBOOMP[BOT] ", "roboomp"),
     ],
 )
 def test_maintainer_logins_common_entry_forms(

python/robomp/tests/test_github_events.py

@@ -534,6 +534,10 @@ def test_extract_mention_accepts_prefixed_or_app_bot_login(configured_login: str
     assert extract_mention("@roboomp go ahead", configured_login) == "go ahead"
 
 
+def test_extract_mention_strips_literal_app_suffix_from_body() -> None:
+    assert extract_mention("@roboomp[bot] go ahead", "roboomp[bot]") == "go ahead"
+
+
 def test_extract_mention_returns_none_without_mention() -> None:
     assert extract_mention("hello there", "robomp-bot") is None
     assert extract_mention(None, "robomp-bot") is None

python/robomp/tests/test_host_tools.py

@@ -1181,8 +1181,9 @@ def test_review_mode_rejects_push_and_open_pr_before_repo_commands(db: Database,
     assert calls == []
 
 
-def test_impl_gate_rejects_unauthorized_proposal_before_repo_commands(
-    db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+@pytest.mark.parametrize("classification", ["enhancement", "proposal"])
+def test_impl_gate_rejects_unauthorized_non_auto_classification_before_repo_commands(
+    db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, classification: str
 ) -> None:
     calls: list[list[str] | tuple[str, ...]] = []
 
@@ -1192,7 +1193,7 @@ def test_impl_gate_rejects_unauthorized_proposal_before_repo_commands(
         raise AssertionError("repo command must not run before implementation authorization")
 
     bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
-    db.set_issue_classification(bindings.issue_key, "proposal")
+    db.set_issue_classification(bindings.issue_key, classification)
     monkeypatch.setattr(host_tools, "_run_repo_command", record_repo_command)
     try:
         push = next(x for x in build(bindings) if x.name == "gh_push_branch")
@@ -1205,7 +1206,7 @@ def test_impl_gate_rejects_unauthorized_proposal_before_repo_commands(
         _stop_loop(loop, t)
 
     for msg in (str(push_exc.value), str(pr_exc.value)):
-        assert "classified `proposal`" in msg
+        assert f"classified `{classification}`" in msg
         assert "OWNER or allowlisted maintainer" in msg
         assert "gh_post_comment" in msg
     assert calls == []
@@ -1213,14 +1214,17 @@ def test_impl_gate_rejects_unauthorized_proposal_before_repo_commands(
         "SELECT tool, error FROM tool_calls WHERE tool IN ('gh_push_branch', 'gh_open_pr') ORDER BY id"
     ).fetchall()
     assert [row["tool"] for row in rows] == ["gh_push_branch", "gh_open_pr"]
-    assert all("classified `proposal`" in row["error"] for row in rows)
+    assert all(f"classified `{classification}`" in row["error"] for row in rows)
 
 
-def test_impl_gate_allows_authorized_proposal_to_reach_pr_validation(db: Database, tmp_path: Path) -> None:
+@pytest.mark.parametrize("classification", ["enhancement", "proposal"])
+def test_impl_gate_allows_authorized_non_auto_classification_to_reach_pr_validation(
+    db: Database, tmp_path: Path, classification: str
+) -> None:
     from dataclasses import replace
 
     bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
-    db.set_issue_classification(bindings.issue_key, "proposal")
+    db.set_issue_classification(bindings.issue_key, classification)
     bindings = replace(bindings, impl_authorized=True)
     try:
         tool = next(x for x in build(bindings) if x.name == "gh_open_pr")
@@ -1234,8 +1238,9 @@ def test_impl_gate_allows_authorized_proposal_to_reach_pr_validation(db: Databas
     assert "OWNER or allowlisted maintainer" not in msg
 
 
-def test_impl_gate_allows_authorized_proposal_push_to_reach_repo_commands(
-    db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+@pytest.mark.parametrize("classification", ["enhancement", "proposal"])
+def test_impl_gate_allows_authorized_non_auto_classification_push_to_reach_repo_commands(
+    db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, classification: str
 ) -> None:
     from dataclasses import replace
 
@@ -1244,15 +1249,15 @@ def test_impl_gate_allows_authorized_proposal_push_to_reach_repo_commands(
     def record_repo_command(_bindings: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None):
         del timeout
         calls.append(cmd)
-        raise RuntimeError("authorized proposal reached gh_push_branch repo command")
+        raise RuntimeError("authorized non-auto issue reached gh_push_branch repo command")
 
     bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
-    db.set_issue_classification(bindings.issue_key, "proposal")
+    db.set_issue_classification(bindings.issue_key, classification)
     bindings = replace(bindings, impl_authorized=True)
     monkeypatch.setattr(host_tools, "_run_repo_command", record_repo_command)
     try:
         tool = next(x for x in build(bindings) if x.name == "gh_push_branch")
-        with pytest.raises(RuntimeError, match="authorized proposal reached gh_push_branch repo command"):
+        with pytest.raises(RuntimeError, match="authorized non-auto issue reached gh_push_branch repo command"):
             tool.execute({}, _ctx())
     finally:
         _stop_loop(loop, t)
@@ -1260,6 +1265,80 @@ def test_impl_gate_allows_authorized_proposal_push_to_reach_repo_commands(
     assert calls
 
 
+def test_impl_gate_allows_later_authorized_event_to_reach_repo_commands(
+    db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    calls: list[list[str] | tuple[str, ...]] = []
+
+    def record_repo_command(_bindings: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None):
+        del timeout
+        calls.append(cmd)
+        raise RuntimeError("later authorized event reached gh_push_branch repo command")
+
+    bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
+    db.set_issue_classification(bindings.issue_key, "enhancement")
+    db.record_event(
+        delivery_id="auth-event",
+        event_type="issue_comment",
+        repo=bindings.issue.repo,
+        issue_key=bindings.issue_key,
+        payload={
+            "_robomp_directive": {
+                "body": "go ahead",
+                "author": "can1357",
+                "pragmas": [],
+                "authorizes_impl": True,
+            }
+        },
+    )
+    monkeypatch.setattr(host_tools, "_run_repo_command", record_repo_command)
+    try:
+        tool = next(x for x in build(bindings) if x.name == "gh_push_branch")
+        with pytest.raises(RuntimeError, match="later authorized event reached gh_push_branch repo command"):
+            tool.execute({}, _ctx())
+    finally:
+        _stop_loop(loop, t)
+
+    assert calls
+
+
+def test_impl_gate_ignores_skipped_authorized_event(db: Database, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    calls: list[list[str] | tuple[str, ...]] = []
+
+    def record_repo_command(_bindings: ToolBindings, cmd: list[str] | tuple[str, ...], *, timeout: float | None = None):
+        del timeout
+        calls.append(cmd)
+        raise AssertionError("skipped authorization must not reach repo command")
+
+    bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
+    db.set_issue_classification(bindings.issue_key, "enhancement")
+    db.record_event(
+        delivery_id="skipped-auth-event",
+        event_type="issue_comment",
+        repo=bindings.issue.repo,
+        issue_key=bindings.issue_key,
+        payload={
+            "_robomp_directive": {
+                "body": "go ahead",
+                "author": "can1357",
+                "pragmas": [],
+                "authorizes_impl": True,
+            }
+        },
+        state="skipped",
+    )
+    monkeypatch.setattr(host_tools, "_run_repo_command", record_repo_command)
+    try:
+        tool = next(x for x in build(bindings) if x.name == "gh_push_branch")
+        with pytest.raises(RpcCommandError) as exc:
+            tool.execute({}, _ctx())
+    finally:
+        _stop_loop(loop, t)
+
+    assert "OWNER or allowlisted maintainer" in str(exc.value)
+    assert calls == []
+
+
 def test_impl_gate_allows_bug_without_directive_to_reach_pr_validation(db: Database, tmp_path: Path) -> None:
     bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda _r: httpx.Response(500)))
     db.set_issue_classification(bindings.issue_key, "bug")

python/robomp/tests/test_server.py

@@ -1598,8 +1598,16 @@ def test_webhook_maintainer_bypasses_rate_limit(
             )
             assert resp.status_code == 202
             states.append(resp.json()["state"])
+        directive_event = get_database(cfg.sqlite_path).get_event("m-3")
     close_database()
     assert states == ["queued"] * 4, states
+    assert directive_event is not None
+    assert directive_event.payload.get("_robomp_directive") == {
+        "body": "do X",
+        "author": "can1357",
+        "pragmas": [],
+        "authorizes_impl": True,
+    }
 
 
 # -------- handler-level: bootstrap + reopen ----------------------------