Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/JOURNAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,31 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of:

---

## 2026-07-23 — workflow parser rejected a bare-int access index #mistake #fix #repro
**Context:** issue #225 — `_normalize_access` in `src/trinity/fugu/workflow.py` validates one
`access_list` entry of a Conductor workflow proposal ("which earlier steps may this step read").
**Expected:** `access_list=[[], 0]` parses like the semantically identical `[[], "0"]` / `[[], [0]]`.
**Actual:** the two spellings disagreed purely on notation — the bare int dropped the **whole**
workflow:
```python
parse_workflow('model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], 0]', 3) # (None, False)
parse_workflow('model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], "0"]', 3) # parses fine
```
**Root cause:** the function handled a scalar **string** digit (`"0"` -> `[0]`), `None`/`""` (-> `[]`),
and **lists** of indices, but a bare scalar **int** matched no branch and fell through to
`return None` — and `None` from `_normalize_access` rejects the entire proposal, not just that entry.
**Fix / decision:** add a bare-`int` branch returning `[acc]` when `0 <= acc < step_index`, matching
the already-accepted `"0"`/`[0]` forms, and document the form in the docstring. `bool` is rejected
**first**, since it subclasses `int` and is never a step index. A bare integer is the most natural
thing a model emits for "read step N", and the parser's stated design goal is to avoid false
negatives; rejecting it depressed parse-rate on otherwise-valid workflows, and a rejected workflow
means `parsed_ok=False`, `training_reward=0.0`, `is_correct=0`. Added
`tests/test_fugu_workflow_bare_int_access.py` (offline): the three equivalent forms produce an
identical workflow, plus guards that the rejections that matter still hold — `True`/`False`
(bool-before-int ordering), negatives, forward references (`>= step_index`), and self-reference at
step 0.
**Follow-up:** none. Note #226 proposes the same one-line fix independently.

## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix #security #decision
**Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main.
**Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main).
Expand Down
15 changes: 12 additions & 3 deletions src/trinity/fugu/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ def _list_candidates(text: str, names: tuple[str, ...]) -> list[tuple[int, list]
def _normalize_access(acc: object, step_index: int) -> object | None:
"""Validate/normalize one access entry. Returns ``None`` if invalid.

Valid forms: ``"all"``, ``[]``/``None`` (query only), or a list of integer
indices that strictly precede ``step_index`` (a forward reference is an
invalid DAG and rejects the whole workflow).
Valid forms: ``"all"``, ``[]``/``None`` (query only), a bare integer index,
or a list of integer indices that strictly precede ``step_index`` (a forward
reference is an invalid DAG and rejects the whole workflow).
"""
if acc is None:
return []
Expand All @@ -208,6 +208,15 @@ def _normalize_access(acc: object, step_index: int) -> object | None:
j = int(s)
return [j] if j < step_index else None
return None
if isinstance(acc, bool):
# `bool` subclasses `int`, but True/False is never a step index.
return None
if isinstance(acc, int):
# A bare int is the most natural way to say "read step N", and is
# semantically identical to the already-accepted "0" / [0] forms.
# Falling through to `return None` here rejected the whole workflow,
# depressing parse-rate (and so training reward) on valid proposals.
return [acc] if 0 <= acc < step_index else None
if isinstance(acc, (list, tuple)):
if len(acc) == 1 and isinstance(acc[0], str) and acc[0].strip().lower() == "all":
return "all"
Expand Down
85 changes: 85 additions & 0 deletions tests/test_fugu_workflow_bare_int_access.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
"""Offline tests for bare-int access indices in the fugu workflow parser (#225).

`_normalize_access` accepted a scalar string digit ("0" -> [0]) and lists of
indices, but a bare scalar int (0) fell through every branch to `None`, which
rejects the *whole* workflow — so a semantically identical proposal scored
`parsed_ok=False` / `training_reward=0.0` purely on notation. These tests pin
that the three equivalent forms agree, and that the rejections that matter
(bool, negative, forward reference, self reference) still hold. No GPU/network.
"""
from __future__ import annotations

import pytest

from trinity.fugu.workflow import _normalize_access, parse_workflow

_BASE = 'model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], {}]'


# --------------------------------------------------------------------------- #
# The bug: bare int was rejected end-to-end
# --------------------------------------------------------------------------- #
@pytest.mark.parametrize("form", ["0", '"0"', "[0]"])
def test_equivalent_access_forms_all_parse(form):
_, parsed_ok = parse_workflow(_BASE.format(form), 3)
assert parsed_ok is True


def test_equivalent_access_forms_produce_identical_workflow():
bare, _ = parse_workflow(_BASE.format("0"), 3)
string, _ = parse_workflow(_BASE.format('"0"'), 3)
listed, _ = parse_workflow(_BASE.format("[0]"), 3)
assert bare == string == listed


# --------------------------------------------------------------------------- #
# Helper: accepted bare ints
# --------------------------------------------------------------------------- #
@pytest.mark.parametrize("idx", [0, 1, 2])
def test_bare_int_normalizes_like_its_list_form(idx):
assert _normalize_access(idx, 3) == [idx]
# ...and matches the already-supported string / list spellings.
assert _normalize_access(idx, 3) == _normalize_access(str(idx), 3)
assert _normalize_access(idx, 3) == _normalize_access([idx], 3)


# --------------------------------------------------------------------------- #
# Guards: what must still be rejected
# --------------------------------------------------------------------------- #
@pytest.mark.parametrize("bad", [True, False])
def test_bool_is_not_a_step_index(bad):
# bool subclasses int, so it must be rejected before the int branch.
assert _normalize_access(bad, 3) is None


@pytest.mark.parametrize("bad", [-1, -5])
def test_negative_index_rejected(bad):
assert _normalize_access(bad, 3) is None


@pytest.mark.parametrize("bad", [3, 4, 99])
def test_forward_reference_rejected(bad):
# An index at/after the current step is an invalid DAG edge.
assert _normalize_access(bad, 3) is None


def test_self_reference_at_first_step_rejected():
assert _normalize_access(0, 0) is None


def test_bare_int_forward_reference_rejects_whole_workflow():
# Step 1 referencing step 5 is still invalid, bare-int notation or not.
_, parsed_ok = parse_workflow(_BASE.format("5"), 3)
assert parsed_ok is False


# --------------------------------------------------------------------------- #
# Unchanged behaviour
# --------------------------------------------------------------------------- #
def test_existing_forms_unchanged():
assert _normalize_access(None, 3) == []
assert _normalize_access("all", 3) == "all"
assert _normalize_access("", 3) == []
assert _normalize_access("query", 3) == []
assert _normalize_access([0, 1], 3) == [0, 1]
assert _normalize_access("not-an-index", 3) is None
Loading