diff --git a/project/TICKETS.md b/project/TICKETS.md index 293e4bb..dde6e9a 100644 --- a/project/TICKETS.md +++ b/project/TICKETS.md @@ -101,4 +101,5 @@ | **ticket-110** | [`README.md`](./ticket-110/README.md) | - | - | - | - | - | | **ticket-111** | [`README.md`](./ticket-111/README.md) | - | - | - | - | - | | **ticket-112** | [`README.md`](./ticket-112/README.md) | - | - | - | - | - | +| **ticket-113** | [`README.md`](./ticket-113/README.md) | - | - | - | - | - | diff --git a/project/ticket-113/README.md b/project/ticket-113/README.md new file mode 100644 index 0000000..6629a0f --- /dev/null +++ b/project/ticket-113/README.md @@ -0,0 +1,27 @@ +# Ticket 113: Agent process visibility + +- **ID**: ticket-113 +- **Owner**: agent:codex +- **Status**: IN_PROGRESS +- **Workflow state**: PUBLICATION +- **Created**: 2026-10-11 + +## Goal and scope + +Read identity metadata for every observed process, then resolve working directories only for recognized/reported agents and their descendants. Unrelated services with restricted cwd links must not falsely invalidate agent attribution. Missing metadata or denied cwd in an agent tree remains an incomplete observation. Preserve the existing recognized-agent policy, privacy boundary and protected admission gate. + +SESSION_EXECUTION_AUTHORIZATION: user explicitly approved repairing Monag as part of autonomous Willman multi-project execution and protected merging. Invoke local OneDev and independent Validator for publication; this note is not merge approval. + +## Acceptance criteria + +- [x] AC-01: Unrelated cwd denial does not discard readable identity data or falsely block agent observation. +- [x] AC-02: Recognized and reported agents, child processes, and inaccessible metadata remain fail-closed; grouping and attribution stay correct. +- [ ] AC-03: Regression/full tests and managed governance pass; a real host scan and independent protected publication verify the change. + +## Bounds + +maxActiveMinutes: 120; checkpointMinutes: 30. Only monitor.py and its tests are implementation scope; no privilege or admission policy changes. + +Validation: 498 passed, 1 skipped, 42 subtests passed; governance 0 errors/0 warnings. Host scan detected 19 agents with zero inaccessible relevant processes. Protected publication remains pending. + +Scope reconciliation: OneDev exact-head verification exposed an existing MCP CLI test dependency on ~/github in an isolated CI home. Include only tests/test_mcp.py to provide an explicit temporary root; product MCP behavior is unchanged. Existing session execution/publication authorization applies. No competing active owner was observed for that path. diff --git a/project/ticket-113/intent.json b/project/ticket-113/intent.json new file mode 100644 index 0000000..0238d75 --- /dev/null +++ b/project/ticket-113/intent.json @@ -0,0 +1,92 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-113", + "summary": "Read process working directories only for identified agent trees", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "requested" + }, + "allowedPaths": [ + "project/ticket-113/**", + "TODO.md", + "project/TICKETS.md", + "src/monag/monitor.py", + "tests/test_monitor.py", + "tests/test_mcp.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [ + "python" + ], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "d1ad4ec5ed07d488dbcd02088480949aca556b17", + "targetBranch": "main", + "outcome": "Observe cwd only for recognized and reported agent trees while retaining fail-closed metadata coverage; publish through OneDev and independent Validator.", + "nonGoals": [ + "No unscoped queue activation", + "No direct merge, approval synthesis or change to protected reviewer policy", + "No edits or cleanup of unrelated worktrees" + ], + "complexity": "S", + "estimatedMinutes": 45, + "budgets": { + "maxImplementationFiles": 3, + "maxAffectedComponents": 2, + "maxPublicInterfaceChanges": 0, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Build complete identity/ancestry inventory before reading cwd for recognized or reported agents and their descendants.", + "components": [ + { + "name": "process-observation", + "paths": [ + "src/monag/monitor.py", + "tests/test_monitor.py" + ] + }, + { + "name": "mcp-cli-test-fixture", + "paths": [ + "tests/test_mcp.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert the monitor change and retain incomplete observation failures." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "PYTHONPATH=src python3 -m pytest -q tests/test_monitor.py tests/test_activity.py" + ], + "evidence": "Scope and denial regression tests." + }, + { + "criterion": "AC-03", + "commands": [ + "bash project/governance-check.sh", + "PYTHONPATH=src python3 -m pytest -q" + ], + "evidence": "498 passed, 1 skipped; governance passed; exact-head protected publication pending." + } + ] + } +} diff --git a/src/monag/monitor.py b/src/monag/monitor.py index b16caaf..22c2a4b 100644 --- a/src/monag/monitor.py +++ b/src/monag/monitor.py @@ -83,17 +83,21 @@ def agent_name(argv): return identify(argv)[0] -def process_record(path, registry=None): +def process_cwd(path): + try: + return str((path / 'cwd').resolve(strict=True)) + except FileNotFoundError: + return None + + +def process_record(path, registry=None, *, read_cwd=True): fields = (path / 'stat').read_text().rsplit(')', 1)[1].split() argv = (path / 'cmdline').read_bytes().decode('utf-8', 'replace').strip('\0').split('\0') if len(argv) == 1 and ' ' in argv[0] and not Path(argv[0]).exists(): # Electron and npm rewrite their title into one space-separated string. argv = argv[0].split(' ') kind, launcher = identify(argv, registry) - try: - cwd = str((path / 'cwd').resolve(strict=True)) - except FileNotFoundError: - cwd = None + cwd = process_cwd(path) if read_cwd else None return {'pid': int(path.name), 'ppid': int(fields[1]), 'state': fields[0], 'start': fields[19], 'kind': kind, 'launcher': launcher, 'executable': Path(argv[0]).name, 'cwd': cwd, @@ -111,7 +115,10 @@ def processes(root, proc=Path('/proc'), registry=None, machine=False, all_users= uid = path.stat().st_uid if not all_users and uid != os.getuid(): continue - row = process_record(path, registry) + # Identity and ancestry must be visible even for unrelated services. + # Their cwd is optional: Linux may protect it independently (e.g. + # ssh-agent or a container with different supplementary groups). + row = process_record(path, registry, read_cwd=False) row['uid'] = uid records[row['pid']] = row except PermissionError: @@ -123,6 +130,30 @@ def processes(root, proc=Path('/proc'), registry=None, machine=False, all_users= if row and row['start'] == task['start'] and task['status'] == 'running': row['kind'] = row['kind'] or task.get('agent_kind', 'reported') row['reported_task'] = task['task'] + # Resolve attribution only after identifying all agents, including reported + # agents and descendants that appear before their parent in /proc. Never + # drop a denied child: a missing checkout in an agent tree is still a gap. + for row in records.values(): + pid, visited = row['pid'], set() + while pid in records and pid not in visited: + visited.add(pid) + if records[pid]['kind']: + try: + row['cwd'] = process_cwd(proc / str(row['pid'])) + # Attribution is a second pass: bind it to the identity we + # inventoried, rather than a new process reusing this PID. + current = (proc / str(row['pid']) / 'stat').read_text().rsplit(')', 1)[1].split() + if current[19] != row['start'] or int(current[1]) != row['ppid']: + row['cwd'] = None + denied += 1 + except FileNotFoundError: + row['cwd'] = None # Process exited during the observation. + except PermissionError: + denied += 1 + except (OSError, ValueError, IndexError): + denied += 1 + break + pid = records[pid]['ppid'] # Only collapse a runtime launcher and its direct same-kind child. # Separate native child agents retain their own identities. grouped = {} diff --git a/tests/test_mcp.py b/tests/test_mcp.py index b1c56df..7824402 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -228,11 +228,14 @@ def test_diagnose_server(self): def test_cli_mcp_health_and_diagnose(self): from monag import cli + workspace = tempfile.TemporaryDirectory() + self.addCleanup(workspace.cleanup) + root_args = ['--root', workspace.name, '--state-dir', str(Path(workspace.name) / 'state')] # Test mcp --health out_buf = StringIO() with patch('sys.stdout', out_buf): - ret = cli.main(['mcp', '--health', '--json']) + ret = cli.main([*root_args, 'mcp', '--health', '--json']) self.assertEqual(ret, 0) health_data = json.loads(out_buf.getvalue()) self.assertEqual(health_data['schema'], 'monag.mcp/health/v1') @@ -240,7 +243,7 @@ def test_cli_mcp_health_and_diagnose(self): # Test mcp --diagnose out_buf_diag = StringIO() with patch('sys.stdout', out_buf_diag): - ret_diag = cli.main(['mcp', '--diagnose', '--json']) + ret_diag = cli.main([*root_args, 'mcp', '--diagnose', '--json']) self.assertEqual(ret_diag, 0) diag_data = json.loads(out_buf_diag.getvalue()) self.assertTrue(diag_data['ok']) diff --git a/tests/test_monitor.py b/tests/test_monitor.py index d533b95..127fa36 100644 --- a/tests/test_monitor.py +++ b/tests/test_monitor.py @@ -74,6 +74,81 @@ def test_git_scan_includes_hidden_linked_checkout_and_changed_files(self): self.assertEqual(row['recent_committed_files'], ['file.txt']) self.assertFalse(errors) + def test_unrelated_service_cwd_is_not_required_for_agent_visibility(self): + proc = self.root / 'proc' + self.fake_process(proc, 10, 1, ['ssh-agent'], self.root) + self.fake_process(proc, 20, 10, ['codex'], self.root) + from monag.monitor import process_cwd + + def restricted(path): + if path.name == '10': + raise PermissionError('protected service cwd') + return process_cwd(path) + + with patch('monag.monitor.process_cwd', side_effect=restricted) as read: + rows, denied = processes(self.root, proc, machine=True) + self.assertEqual(denied, 0) + self.assertEqual([r['pid'] for r in rows], [20]) + self.assertEqual([c.args[0].name for c in read.call_args_list], ['20']) + + def test_agent_and_descendant_cwd_denials_remain_visible(self): + proc = self.root / 'proc' + # Child precedes its parent; an intermediate non-agent must not hide it. + self.fake_process(proc, 30, 20, ['git'], self.root) + self.fake_process(proc, 20, 10, ['bash'], self.root) + self.fake_process(proc, 10, 1, ['codex'], self.root) + from monag.monitor import process_cwd + for denied_pid in ('10', '20', '30'): + with self.subTest(pid=denied_pid): + def restricted(path): + if path.name == denied_pid: + raise PermissionError('protected cwd') + return process_cwd(path) + with patch('monag.monitor.process_cwd', side_effect=restricted): + rows, denied = processes(self.root, proc, machine=True) + self.assertEqual(denied, 1) + self.assertEqual([r['pid'] for r in rows], [10]) + self.assertEqual({r['pid'] for r in rows[0]['descendants']}, {20, 30}) + + def test_reported_agent_requires_cwd_only_for_matching_live_identity(self): + proc = self.root / 'proc' + self.fake_process(proc, 10, 1, ['custom-worker'], self.root) + for start, status, expected in [('123', 'running', 1), ('old', 'running', 0), + ('123', 'finished', 0)]: + with self.subTest(start=start, status=status), patch( + 'monag.monitor.process_cwd', side_effect=PermissionError): + rows, denied = processes(self.root, proc, machine=True, reported=[ + dict(pid=10, start=start, status=status, task='governed job')]) + self.assertEqual(denied, expected) + self.assertEqual(len(rows), expected) + + def test_unreadable_metadata_still_invalidates_observation(self): + proc = self.root / 'proc' + self.fake_process(proc, 10, 1, ['ssh-agent'], self.root) + for field in ('stat', 'cmdline'): + method = 'read_text' if field == 'stat' else 'read_bytes' + original = getattr(Path, method) + def restricted(path, *args, **kwargs): + if path.name == field: + raise PermissionError('protected metadata') + return original(path, *args, **kwargs) + with self.subTest(field=field), patch.object(Path, method, restricted): + rows, denied = processes(self.root, proc, machine=True) + self.assertEqual(rows, []) + self.assertEqual(denied, 1) + + def test_pid_reuse_during_deferred_cwd_read_invalidates_attribution(self): + proc = self.root / 'proc' + self.fake_process(proc, 10, 1, ['codex'], self.root) + def reused(path): + stat = path / 'stat' + stat.write_text(stat.read_text().replace('123', '456')) + return str(self.root) + with patch('monag.monitor.process_cwd', side_effect=reused): + rows, denied = processes(self.root, proc, machine=True) + self.assertEqual(denied, 1) + self.assertIsNone(rows[0]['cwd']) + def test_invalid_git_marker_does_not_hide_descendant_repositories(self): repo = self.repo() (self.root / '.git').mkdir()