Skip to content
Merged
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
2 changes: 1 addition & 1 deletion docs/reference/tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ No input properties.

**Core:** Yes

<pre v-pre style="white-space: pre-wrap; overflow-wrap: anywhere; font: inherit;">Converts natural language time to ISO datetime (e.g. &#x27;in 2 hours&#x27;, &#x27;tomorrow at 9am&#x27;, &#x27;next Friday at 3pm&#x27;). Uses bot timezone. For schedule_task&#x27;s run_at parameter.</pre>
<pre v-pre style="white-space: pre-wrap; overflow-wrap: anywhere; font: inherit;">Converts natural language time to ISO datetime (e.g. &#x27;in 2 hours&#x27;, &#x27;tomorrow at 9am&#x27;, &#x27;next Friday at 3pm&#x27;). Uses bot timezone unless the expression ends in a recognized timezone. IANA zones (for example, America/New_York), EST/ET, and New York time are supported; ambiguous or unknown explicit zones are rejected. For schedule_task&#x27;s run_at parameter.</pre>

<p v-pre><small>[affordances: cost=free risk=none latency=instant]</small></p>

Expand Down
33 changes: 32 additions & 1 deletion src/discord/native_tools/media.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io
import os
from collections.abc import Callable
from urllib.parse import urlsplit

import discord

Expand All @@ -21,6 +22,25 @@

# Hard cap on a URL-fetched image so a huge or hostile body can't exhaust memory.
_ANALYZE_IMAGE_MAX_BYTES = 25 * 1024 * 1024 # 25 MiB
_DISCORD_ATTACHMENT_HOSTS = frozenset({"cdn.discordapp.com", "media.discordapp.net"})


def _safe_discord_attachment_url(value: object) -> str | None:
"""Return a URL only when it points at Discord's HTTPS attachment CDN."""
if not isinstance(value, str) or len(value) > 4096:
return None
try:
parsed = urlsplit(value)
if (
parsed.scheme != "https"
or parsed.hostname not in _DISCORD_ATTACHMENT_HOSTS
or parsed.username is not None
or parsed.password is not None
):
return None
return value
except (TypeError, ValueError):
return None


class MediaTools:
Expand Down Expand Up @@ -313,7 +333,7 @@ async def _handle_generate_image(self, message, inp: dict):
}
try:
file = discord.File(io.BytesIO(result.data), filename="generated.png")
await message.channel.send(file=file)
posted_message = await message.channel.send(file=file)
except discord.HTTPException as e:
# Generation succeeded even though delivery failed — record both.
meta["delivery_status"] = "upload_failed"
Expand All @@ -325,6 +345,16 @@ async def _handle_generate_image(self, message, inp: dict):
)

meta["delivery_status"] = "posted"
attachment_url = None
attachments = getattr(posted_message, "attachments", ())
if attachments:
attachment_url = _safe_discord_attachment_url(
getattr(attachments[0], "url", None)
)
if attachment_url:
meta["attachment_url_available"] = True
else:
meta["attachment_url_available"] = False
log.info(
"image generated: backend=%s model=%s decoded=%dx%d route=%s",
result.backend, result.image_model, result.width, result.height, result.route,
Expand All @@ -333,6 +363,7 @@ async def _handle_generate_image(self, message, inp: dict):
output=(
f"Image generated ({result.width}x{result.height}, "
f"{len(result.data) / 1024:.1f} KB) and posted."
+ (f" Attachment URL: {attachment_url}" if attachment_url else "")
),
tool_name="generate_image",
audit_metadata=meta,
Expand Down
5 changes: 4 additions & 1 deletion src/tools/defs/media_scheduling.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,10 @@
"description": (
"Converts natural language time to ISO datetime "
"(e.g. 'in 2 hours', 'tomorrow at 9am', 'next Friday at 3pm'). "
"Uses bot timezone. For schedule_task's run_at parameter."
"Uses bot timezone unless the expression ends in a recognized timezone. "
"IANA zones (for example, America/New_York), EST/ET, and New York time "
"are supported; ambiguous or unknown explicit zones are rejected. "
"For schedule_task's run_at parameter."
),
"input_schema": {
"type": "object",
Expand Down
15 changes: 14 additions & 1 deletion src/tools/post_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,17 @@ def _default_compare_for(check_type: str) -> str:

# Validate the ERE first, then check journal visibility without -q; the data
# run uses -q to keep journalctl's own status lines out of the matches.
# Use cat output for the data run: unlike the default journalctl rendering it
# has no host/unit prefix, so the anchored filter identifies only Odin's own
# logger and tool-call message, not unrelated logs quoting the same text. Filter
# before the caller's regex, rather than trying to clean the matched output:
# a match only in the invocation must not prove presence or disprove absence.
_VALIDATION_INVOCATION_LINE = (
# Python logging's default asctime includes comma-separated milliseconds;
# accept older second-resolution journal entries as well.
r"^[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}(,[0-9]{3})? "
r"\[INFO\] odin\.discord: Tool call: validate_action\(\{"
)
_LOG_PROBE_SCRIPT = (
'p="$3"; '
'printf "" | grep -E -e "$p" >/dev/null 2>&1; '
Expand All @@ -320,7 +331,9 @@ def _default_compare_for(check_type: str) -> str:
'e=$(journalctl "$@" --no-pager -n 1 2>&1 >/dev/null); r=$?; '
'if [ "$r" -ne 0 ]; then echo "LOG_CHECK_ERROR journalctl exit $r"; '
'printf "%s\\n" "$e" | tail -n 1; exit 0; fi; '
'journalctl "$@" --no-pager -q 2>/dev/null | grep -E -e "$p" | head -n 20; '
'journalctl "$@" --no-pager -q -o cat 2>/dev/null | '
f'grep -v -E -e {shlex.quote(_VALIDATION_INVOCATION_LINE)} | '
'grep -E -e "$p" | head -n 20; '
'case "$e" in *"not seeing messages from"*|*"insufficient permissions"*'
'|*"No journal files were found"*) echo LOG_READ_PARTIAL;; *) echo LOG_READ_OK;; esac'
)
Expand Down
71 changes: 69 additions & 2 deletions src/tools/time_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,69 @@ def set_default_timezone(tz_name: str) -> None:
+ r")\b|may\s+\d)"
)

# Abbreviations are not reliable identifiers for time zones. EST/ET are
# explicitly treated as the colloquial US Eastern zone (including DST); other
# common abbreviations are rejected rather than guessed or silently ignored.
_ZONE_ALIASES = {"est": "America/New_York", "et": "America/New_York"}
_ZONE_ALIASES.update({"utc": "UTC", "gmt": "UTC"})
_AMBIGUOUS_ZONE_ABBREVIATIONS = {
"ast", "bst", "cst", "ist", "mst", "pst", "adt", "cdt", "edt", "mdt", "pdt"
}
_IANA_ZONE_SUFFIX = re.compile(
r"(?:\s+in)?\s+([a-z_+-]+(?:/[a-z0-9_+.-]+)+)$", re.IGNORECASE
)
_NAMED_ZONE_SUFFIX = re.compile(
r"\s+(?:in\s+)?(new\s+york|eastern)(?:\s+time)?$", re.IGNORECASE
)
_ABBREVIATION_SUFFIX = re.compile(r"\s+(?:in\s+)?([a-z]{2,5})$", re.IGNORECASE)
_EXPLICIT_ZONE_PHRASE = re.compile(r"\s+in\s+([a-z][a-z0-9_+./ -]*)$", re.IGNORECASE)


def _extract_explicit_timezone(expression: str) -> tuple[str, ZoneInfo | None]:
"""Remove a recognized trailing zone, rejecting explicit but unsafe zones."""
text = expression.strip()
match = _IANA_ZONE_SUFFIX.search(text)
if match:
zone_name = match.group(1)
try:
zone = ZoneInfo(zone_name)
except (KeyError, ValueError) as exc:
raise ValueError(f"Unknown timezone: {zone_name}") from exc
return text[: match.start()].strip(), zone

match = _NAMED_ZONE_SUFFIX.search(text)
if match:
return text[: match.start()].strip(), ZoneInfo("America/New_York")

match = _ABBREVIATION_SUFFIX.search(text)
if match:
abbreviation = match.group(1).lower()
if abbreviation in _ZONE_ALIASES:
return text[: match.start()].strip(), ZoneInfo(_ZONE_ALIASES[abbreviation])
# Only uppercase source tokens are presumed to be explicit abbreviations;
# ordinary prose tails retain the parser's historical behavior.
source_token = text[match.start(1) : match.end(1)]
if source_token.isupper() or abbreviation in _AMBIGUOUS_ZONE_ABBREVIATIONS:
raise ValueError(
f"Timezone abbreviation '{source_token}' is ambiguous or unsupported; "
"use an IANA zone such as America/New_York"
)

# An explicit "in <zone>" clause is not harmless trailing prose. If it
# was not one of the supported aliases or a valid IANA identifier, fail
# closed instead of silently scheduling in the configured default zone.
# Conventional time-of-day prose isn't a zone request: let the clock
# parser give its normal actionable time-format error instead.
if re.search(r"\s+in\s+the\s+(?:morning|afternoon|evening|night)$", text, re.IGNORECASE):
return text, None
match = _EXPLICIT_ZONE_PHRASE.search(text)
if match:
raise ValueError(
f"Unrecognized timezone '{match.group(1).strip()}'; use an IANA zone "
"such as America/New_York"
)
return text, None


def _split_time_of_day(text: str) -> tuple[tuple[int, int], str] | None:
"""Return a leading clock time and the unconsumed text."""
Expand Down Expand Up @@ -168,10 +231,14 @@ def _next_weekday(now: datetime, target_weekday: int) -> datetime:

def parse_time(expression: str, now: datetime | None = None) -> str:
"""Parse a natural language time expression into an ISO datetime string."""
expression, explicit_tz = _extract_explicit_timezone(expression)
tz = explicit_tz or _default_tz
if now is None:
now = datetime.now(_default_tz)
now = datetime.now(tz)
elif now.tzinfo is None:
now = now.replace(tzinfo=_default_tz)
now = now.replace(tzinfo=tz)
elif explicit_tz is not None:
now = now.astimezone(explicit_tz)
text = expression.strip().lower()

m = re.match(r"in\s+(\d+)\s+(\w+)", text)
Expand Down
3 changes: 2 additions & 1 deletion tests/characterization/test_tool_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@
# Updated with schedule_task: shared trigger shape, report_format clearing (B2).
"update_schedule": "ba5b0ca8c5b1f96a",
"delete_schedule": "01e54d37b70471a8",
"parse_time": "6ae3f4c04138a2cd",
# Explicit-zone support changes the user-facing description, not input shape.
"parse_time": "b732643e002f9fad",
"search_history": "72aaa6b1024b0fc0",
"memory_manage": "f7aa460db948c1d5",
"search_audit": "6fcb11f91a34bcb6",
Expand Down
52 changes: 52 additions & 0 deletions tests/test_campaign_a_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,58 @@ def test_readable_journal_still_matches(tmp_path, ctype, expected):
assert _shell_check(Check(type=ctype, target="FIXTUREERROR"), env=env)[0] == expected


@pytest.mark.parametrize("unit", ["", "unit=odin:"])
@pytest.mark.parametrize("pattern", ["BATTERY-NONEXISTENT-9f2c", "Startup diagnostics"])
def test_log_probe_ignores_own_invocation_for_absent_and_present(tmp_path, unit, pattern):
# Actual journalctl -u odin -q -o cat output uses comma milliseconds:
# 2026-09-27 23:21:56,515 [INFO] odin.discord: Tool call: validate_action({'checks': ...
# Without filtering, it proves presence and disproves absence.
invocation = (
"2026-09-27 23:21:56,515 [INFO] odin.discord: "
"Tool call: validate_action({'checks': [{'type': 'log_absent', "
f"'target': 'unit=odin:{pattern}'}}]}})"
)
env = _fake_journal(tmp_path, body=invocation)
assert _shell_check(Check(type="log_absent", target=f"{unit}{pattern}"), env=env)[0] == "pass"
assert _shell_check(Check(type="log_present", target=f"{unit}{pattern}"), env=env)[0] == "fail"


@pytest.mark.parametrize("unit", ["", "unit=odin:"])
def test_log_probe_preserves_genuine_messages_even_with_invocation_text(tmp_path, unit):
invocation = (
"2026-09-28 12:00:00 [INFO] odin.discord: "
"Tool call: validate_action({'target': 'unit=odin:Startup diagnostics'})"
)
real_message = "2026-09-28 12:00:01 [INFO] odin.discord: Startup diagnostics completed"
quoted_message = (
"2026-09-28 12:00:02 [INFO] odin.service: "
"User wrote Tool call: validate_action(Startup diagnostics)"
)
env = _fake_journal(tmp_path, body="\n".join((invocation, real_message, quoted_message)))
check = Check(type="log_present", target=f"{unit}Startup diagnostics")
assert _shell_check(check, env=env)[0] == "pass"
check = Check(type="log_absent", target=f"{unit}Startup diagnostics")
assert _shell_check(check, env=env)[0] == "fail"

# Quoting the invocation is still a real log message, not an Odin call.
env = _fake_journal(tmp_path, body=quoted_message)
assert _shell_check(check, env=env)[0] == "fail"


def test_log_probe_filters_before_limiting_matches(tmp_path):
invocations = "\n".join(
"2026-09-28 12:00:00,123 [INFO] odin.discord: "
f"Tool call: validate_action({{'target': 'unit=odin:Startup diagnostics', "
f"'attempt': {i}}})"
for i in range(25)
)
env = _fake_journal(tmp_path, body=f"{invocations}\nStartup diagnostics completed")
check = Check(type="log_present", target="Startup diagnostics")
assert _shell_check(check, env=env)[0] == "pass"
check = Check(type="log_absent", target="Startup diagnostics")
assert _shell_check(check, env=env)[0] == "fail"


@pytest.mark.parametrize(
"ctype,output,expected",
[
Expand Down
15 changes: 12 additions & 3 deletions tests/test_campaign_discord_final_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,9 @@ async def start_mcp_stub(received):


@pytest.mark.asyncio
async def test_application_startup_tolerates_quota_check_failure(monkeypatch):
"""The optional quota poller is isolated from required app startup."""
@pytest.mark.parametrize("diagnostics_fail", [False, True])
async def test_application_startup_tolerates_quota_check_failure(monkeypatch, diagnostics_fail):
"""Optional quota and diagnostic failures cannot prevent required startup."""
import src.discord.client as client_module

bot = object.__new__(OdinBot)
Expand All @@ -235,14 +236,22 @@ async def test_application_startup_tolerates_quota_check_failure(monkeypatch):
)
bot.load_extension = AsyncMock()
bot.computer = SimpleNamespace(start=AsyncMock())
bot._run_startup_diagnostics = lambda *, yaml_config: SimpleNamespace(results=[])

def diagnostics(*, yaml_config):
if diagnostics_fail:
raise RuntimeError("diagnostics unavailable")
return SimpleNamespace(results=[])

bot._run_startup_diagnostics = diagnostics
monkeypatch.setattr(client_module, "INITIAL_EXTENSIONS", ())
monkeypatch.setattr(client_module, "start_mcp", AsyncMock())

await bot.start_application()

bot.codex_quota_check.start.assert_awaited_once()
assert bot._application_started is True
if diagnostics_fail:
assert not hasattr(bot, "startup_report")


def test_startup_config_logging_covers_nonempty_host_and_option_flags():
Expand Down
48 changes: 47 additions & 1 deletion tests/test_native_media.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,34 @@
from unittest.mock import AsyncMock, MagicMock, patch

import discord
from src.discord.native_tools.media import MediaTools
from src.discord.native_tools.media import MediaTools, _safe_discord_attachment_url
from src.tools.hosts import HostRegistry

PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 32


class TestSafeDiscordAttachmentUrl:
def test_accepts_only_https_discord_attachment_hosts(self):
url = "https://cdn.discordapp.com/attachments/1/2/image.png?ex=abc"
assert _safe_discord_attachment_url(url) == url
assert _safe_discord_attachment_url(
"https://media.discordapp.net/attachments/1/2/image.png"
) == "https://media.discordapp.net/attachments/1/2/image.png"

def test_rejects_invalid_values_and_unsafe_urls(self):
assert _safe_discord_attachment_url(None) is None
assert _safe_discord_attachment_url("x" * 4097) is None
for url in (
"http://cdn.discordapp.com/attachments/1/2/image.png",
"https://discordapp.com/attachments/1/2/image.png",
"https://cdn.discordapp.com.evil.example/image.png",
"https://user@cdn.discordapp.com/image.png",
"https://user:secret@cdn.discordapp.com/image.png",
"https://[invalid-ipv6/image.png",
):
assert _safe_discord_attachment_url(url) is None


def _http_exc(status=500):
return discord.HTTPException(
SimpleNamespace(status=status, reason="err"), "msg") # type: ignore[arg-type]
Expand Down Expand Up @@ -349,6 +371,9 @@ async def test_no_selector_and_no_prompt(self):
async def test_success_posts_attachment(self):
sel = self._selector(result=self._result(backend="openai"))
msg = _message()
msg.channel.send.return_value = SimpleNamespace(attachments=[
SimpleNamespace(url="https://cdn.discordapp.com/attachments/123/456/generated.png?ex=abc")
])
out = await _tools(image_selector=sel)._handle_generate_image(msg, {"prompt": "a cat"})
# Generic user-facing string — the backend name is NOT surfaced there...
assert "Image generated (1024x1024" in str(out)
Expand All @@ -357,6 +382,27 @@ async def test_success_posts_attachment(self):
# ...but IS recorded in the (non-model-facing) audit metadata.
assert out.audit_metadata["backend"] == "openai"
assert out.audit_metadata["delivery_status"] == "posted"
assert "https://cdn.discordapp.com/attachments/123/456/generated.png?ex=abc" in str(out)
assert out.audit_metadata["attachment_url_available"] is True

async def test_success_does_not_expose_non_discord_attachment_url(self):
sel = self._selector(result=self._result())
msg = _message()
msg.channel.send.return_value = SimpleNamespace(attachments=[
SimpleNamespace(url="https://attacker.example/image.png?token=secret")
])
out = await _tools(image_selector=sel)._handle_generate_image(msg, {"prompt": "x"})
assert "attacker.example" not in str(out)
assert "token=secret" not in str(out)
assert out.audit_metadata["attachment_url_available"] is False

async def test_attachment_url_guard_rejects_lookalikes_and_credentials(self):
from src.discord.native_tools.media import _safe_discord_attachment_url

assert _safe_discord_attachment_url("https://cdn.discordapp.com/a.png")
assert _safe_discord_attachment_url("https://cdn.discordapp.com.evil/a.png") is None
assert _safe_discord_attachment_url("https://user@cdn.discordapp.com/a.png") is None
assert _safe_discord_attachment_url("http://cdn.discordapp.com/a.png") is None

async def test_backend_failure_and_http_error(self):
from src.tools.image import ImageGenError
Expand Down
1 change: 1 addition & 0 deletions tests/test_post_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ def test_service(self):
def test_log_absent_unit(self):
cmd = _build_command(Check(type="log_absent", target="unit=nginx:ERROR"))
assert "journalctl" in cmd
assert "-o cat" in cmd
assert "grep" in cmd
assert shlex.split(cmd)[-3:] == ["nginx", "120", "ERROR"]

Expand Down
Loading
Loading