EnvKvStore: find/get split, typed int accessors, nest DotenvParser.

Rename optional lookup to find(), add throwing get(), and provide
getInt/getPositiveInt/getPositiveNonZeroInt with optional defaults so
callers own missing-key policy without baking domain period semantics
into spinscale.

Co-authored-by: Cursor <cursoragent@cursor.com>

include/spinscale/envKvStore.h

@@ -20,9 +20,66 @@ public:
 	explicit EnvKvStore(
 		const std::vector<std::filesystem::path> &envFilePaths);
 
-	std::optional<std::string> get(std::string_view name) const;
+	/**	EXPLANATION:
+	 * Precedence: process getenv wins over compiled file-store values unless
+	 * bypassProcessEnvironment is true (file store only).
+	 */
+	std::optional<std::string> find(
+		std::string_view name,
+		bool bypassProcessEnvironment = false) const;
+
+	/** Throws if find() returns nullopt. */
+	std::string get(
+		std::string_view name,
+		bool bypassProcessEnvironment = false) const;
+
+	/**	EXPLANATION:
+	 * Typed int accessors. defaultValue applies only when find() is nullopt;
+	 * nullopt defaultValue with a missing key throws. A present value that fails
+	 * to parse or fails the positivity constraint always throws.
+	 */
+	int getInt(
+		std::string_view name,
+		std::optional<int> defaultValue = std::nullopt) const
+	{
+		return getIntWithConstraint(name, defaultValue, IntConstraint::Any);
+	}
+
+	/** Parsed value must be >= 0. */
+	int getPositiveInt(
+		std::string_view name,
+		std::optional<int> defaultValue = std::nullopt) const
+	{
+		return getIntWithConstraint(
+			name, defaultValue, IntConstraint::NonNegative);
+	}
+
+	/** Parsed value must be > 0. */
+	int getPositiveNonZeroInt(
+		std::string_view name,
+		std::optional<int> defaultValue = std::nullopt) const
+	{
+		return getIntWithConstraint(
+			name, defaultValue, IntConstraint::PositiveNonZero);
+	}
 
 private:
+	/** dotenv line parsing owned by EnvKvStore (definition in .cpp). */
+	class DotenvParser;
+
+	enum class IntConstraint
+	{
+		Any,
+		NonNegative,
+		PositiveNonZero,
+	};
+
+	static int parseInt(std::string_view name, const std::string &raw);
+	int getIntWithConstraint(
+		std::string_view name,
+		std::optional<int> defaultValue,
+		IntConstraint constraint) const;
+
 	void loadFiles(
 		const std::vector<std::filesystem::path> &envFilePaths,
 		std::ostream &warningStream);

src/envKvStore.cpp

@@ -3,197 +3,186 @@
 #include <cstdlib>
 #include <fstream>
 #include <iostream>
+#include <limits>
 #include <ranges>
 #include <sstream>
 #include <stdexcept>
+#include <utility>
 
 #include <spinscale/envKvStore.h>
 
 namespace sscl {
-namespace {
 
-std::string trim(std::string_view value)
+class EnvKvStore::DotenvParser
 {
-	auto begin = std::ranges::find_if_not(
+public:
-		value, [](unsigned char c) { return std::isspace(c); });
+	static bool lineIsBlankOrComment(const std::string &line)
-	auto rbegin = std::ranges::find_if_not(
-		value | std::views::reverse,
-		[](unsigned char c) { return std::isspace(c); });
-	auto end = rbegin.base();
-	if (begin >= end)
 	{
-		return {};
+		std::string trimmed = trim(line);
+		return trimmed.empty() || trimmed.front() == '#';
 	}
-	return std::string(begin, end);
-}
 
-bool characterIsValidNameStart(char c)
+	static std::pair<std::string, std::string> parseAssignment(
-{
+		const std::filesystem::path &envFilePath,
-	return std::isalpha(static_cast<unsigned char>(c)) || c == '_';
+		std::size_t lineNumber,
-}
+		const std::string &line)
-
-bool characterIsValidNameBody(char c)
-{
-	return std::isalnum(static_cast<unsigned char>(c)) || c == '_';
-}
-
-bool nameIsValid(std::string_view name)
-{
-	if (name.empty() || !characterIsValidNameStart(name.front()))
 	{
-		return false;
+		std::size_t separator = line.find('=');
+		if (separator == std::string::npos)
+		{
+			throw makeParseError(
+				envFilePath, lineNumber, "Expected KEY=value.");
+		}
+
+		std::string name = trim(std::string_view(line).substr(0, separator));
+		if (!nameIsValid(name))
+		{
+			throw makeParseError(
+				envFilePath, lineNumber, "Invalid variable name.");
+		}
+
+		return {
+			std::move(name),
+			parseValue(
+				envFilePath,
+				lineNumber,
+				std::string_view(line).substr(separator + 1))};
 	}
-	return std::ranges::all_of(name.substr(1), characterIsValidNameBody);
-}
 
-std::runtime_error makeParseError(
+private:
-	const std::filesystem::path &envFilePath,
+	static std::string trim(std::string_view value)
-	std::size_t lineNumber,
-	const std::string &message)
-{
-	std::ostringstream stream;
-	stream << envFilePath << ":" << lineNumber << ": " << message;
-	return std::runtime_error(stream.str());
-}
-
-bool lineIsBlankOrComment(const std::string &line)
-{
-	std::string trimmed = trim(line);
-	return trimmed.empty() || trimmed.front() == '#';
-}
-
-std::size_t findClosingQuote(
-	const std::filesystem::path &envFilePath,
-	std::size_t lineNumber,
-	std::string_view value)
-{
-	char quote = value.front();
-	bool escapeNext = false;
-	for (std::size_t i = 1; i < value.size(); ++i)
 	{
-		if (escapeNext)
+		auto begin = std::ranges::find_if_not(
-		{
+			value, [](unsigned char c) { return std::isspace(c); });
-			escapeNext = false;
+		auto rbegin = std::ranges::find_if_not(
-			continue;
+			value | std::views::reverse,
-		}
+			[](unsigned char c) { return std::isspace(c); });
-		if (quote == '"' && value[i] == '\\')
+		auto end = rbegin.base();
-		{
+		if (begin >= end) { return {}; }
-			escapeNext = true;
+		return std::string(begin, end);
-			continue;
-		}
-		if (value[i] == quote)
-		{
-			return i;
-		}
 	}
-	throw makeParseError(envFilePath, lineNumber, "Unterminated quoted value.");
-}
 
-std::string decodeDoubleQuotedValue(std::string_view value)
+	static bool characterIsValidNameStart(char c)
-{
-	std::string decoded;
-	decoded.reserve(value.size());
-	bool escapeNext = false;
-	for (char c : value)
 	{
-		if (!escapeNext && c == '\\')
+		return std::isalpha(static_cast<unsigned char>(c)) || c == '_';
+	}
+
+	static bool characterIsValidNameBody(char c)
+	{
+		return std::isalnum(static_cast<unsigned char>(c)) || c == '_';
+	}
+
+	static bool nameIsValid(std::string_view name)
+	{
+		if (name.empty() || !characterIsValidNameStart(name.front())) { return false; }
+		return std::ranges::all_of(name.substr(1), characterIsValidNameBody);
+	}
+
+	static std::runtime_error makeParseError(
+		const std::filesystem::path &envFilePath,
+		std::size_t lineNumber,
+		const std::string &message)
+	{
+		std::ostringstream stream;
+		stream << envFilePath << ":" << lineNumber << ": " << message;
+		return std::runtime_error(stream.str());
+	}
+
+	static std::size_t findClosingQuote(
+		const std::filesystem::path &envFilePath,
+		std::size_t lineNumber,
+		std::string_view value)
+	{
+		char quote = value.front();
+		bool escapeNext = false;
+		for (std::size_t i = 1; i < value.size(); ++i)
 		{
-			escapeNext = true;
+			if (escapeNext)
-			continue;
-		}
-		if (escapeNext)
-		{
-			switch (c)
 			{
-				case 'n':
+				escapeNext = false;
-					decoded.push_back('\n');
+				continue;
-					break;
-				case 'r':
-					decoded.push_back('\r');
-					break;
-				case 't':
-					decoded.push_back('\t');
-					break;
-				default:
-					decoded.push_back(c);
-					break;
 			}
-			escapeNext = false;
+			if (quote == '"' && value[i] == '\\')
-			continue;
+			{
+				escapeNext = true;
+				continue;
+			}
+			if (value[i] == quote) { return i; }
 		}
-		decoded.push_back(c);
-	}
-	if (escapeNext)
-	{
-		decoded.push_back('\\');
-	}
-	return decoded;
-}
-
-std::string parseQuotedValue(
-	const std::filesystem::path &envFilePath,
-	std::size_t lineNumber,
-	std::string_view value)
-{
-	char quote = value.front();
-	std::size_t closingQuote = findClosingQuote(envFilePath, lineNumber, value);
-	std::string trailing = trim(value.substr(closingQuote + 1));
-	if (!trailing.empty() && trailing.front() != '#')
-	{
 		throw makeParseError(
-			envFilePath, lineNumber, "Unexpected text after quoted value.");
+			envFilePath, lineNumber, "Unterminated quoted value.");
-	}
-	std::string_view quotedBody = value.substr(1, closingQuote - 1);
-	if (quote == '"')
-	{
-		return decodeDoubleQuotedValue(quotedBody);
-	}
-	return std::string(quotedBody);
-}
-
-std::string parseValue(
-	const std::filesystem::path &envFilePath,
-	std::size_t lineNumber,
-	std::string_view rawValue)
-{
-	std::string value = trim(rawValue);
-	if (value.empty())
-	{
-		return {};
-	}
-	if (value.front() == '\'' || value.front() == '"')
-	{
-		return parseQuotedValue(envFilePath, lineNumber, value);
-	}
-	return trim(value.substr(0, value.find('#')));
-}
-
-std::pair<std::string, std::string> parseAssignment(
-	const std::filesystem::path &envFilePath,
-	std::size_t lineNumber,
-	const std::string &line)
-{
-	std::size_t separator = line.find('=');
-	if (separator == std::string::npos)
-	{
-		throw makeParseError(envFilePath, lineNumber, "Expected KEY=value.");
 	}
 
-	std::string name = trim(std::string_view(line).substr(0, separator));
+	static std::string decodeDoubleQuotedValue(std::string_view value)
-	if (!nameIsValid(name))
 	{
-		throw makeParseError(envFilePath, lineNumber, "Invalid variable name.");
+		std::string decoded;
+		decoded.reserve(value.size());
+		bool escapeNext = false;
+		for (char c : value)
+		{
+			if (!escapeNext && c == '\\')
+			{
+				escapeNext = true;
+				continue;
+			}
+			if (escapeNext)
+			{
+				switch (c)
+				{
+					case 'n':
+						decoded.push_back('\n');
+						break;
+					case 'r':
+						decoded.push_back('\r');
+						break;
+					case 't':
+						decoded.push_back('\t');
+						break;
+					default:
+						decoded.push_back(c);
+						break;
+				}
+				escapeNext = false;
+				continue;
+			}
+			decoded.push_back(c);
+		}
+		if (escapeNext) { decoded.push_back('\\'); }
+		return decoded;
 	}
 
-	return {
+	static std::string parseQuotedValue(
-		std::move(name),
+		const std::filesystem::path &envFilePath,
-		parseValue(
+		std::size_t lineNumber,
-			envFilePath,
+		std::string_view value)
-			lineNumber,
+	{
-			std::string_view(line).substr(separator + 1))};
+		char quote = value.front();
-}
+		std::size_t closingQuote =
+			findClosingQuote(envFilePath, lineNumber, value);
+		std::string trailing = trim(value.substr(closingQuote + 1));
+		if (!trailing.empty() && trailing.front() != '#')
+		{
+			throw makeParseError(
+				envFilePath,
+				lineNumber,
+				"Unexpected text after quoted value.");
+		}
+		std::string_view quotedBody = value.substr(1, closingQuote - 1);
+		if (quote == '"') { return decodeDoubleQuotedValue(quotedBody); }
+		return std::string(quotedBody);
+	}
 
-} // namespace
+	static std::string parseValue(
+		const std::filesystem::path &envFilePath,
+		std::size_t lineNumber,
+		std::string_view rawValue)
+	{
+		std::string value = trim(rawValue);
+		if (value.empty()) { return {}; }
+		if (value.front() == '\'' || value.front() == '"') { return parseQuotedValue(envFilePath, lineNumber, value); }
+		return trim(value.substr(0, value.find('#')));
+	}
+};
 
 EnvKvStore::EnvKvStore(
 	const std::vector<std::filesystem::path> &envFilePaths,
@@ -205,8 +194,7 @@ EnvKvStore::EnvKvStore(
 EnvKvStore::EnvKvStore(
 	const std::vector<std::filesystem::path> &envFilePaths)
 :	EnvKvStore(envFilePaths, std::cerr)
-{
+{}
-}
 
 void EnvKvStore::loadFiles(
 	const std::vector<std::filesystem::path> &envFilePaths,
@@ -218,21 +206,115 @@ void EnvKvStore::loadFiles(
 	}
 }
 
-std::optional<std::string> EnvKvStore::get(std::string_view name) const
+std::optional<std::string> EnvKvStore::find(
+	std::string_view name,
+	bool bypassProcessEnvironment) const
 {
-	std::string ownedName(name);
+	if (!bypassProcessEnvironment)
-	if (const char *value = std::getenv(ownedName.c_str()))
 	{
-		return std::string(value);
+		std::string ownedName(name);
+		if (const char *value = std::getenv(ownedName.c_str())) {
+			return std::string(value);
+		}
 	}
+
 	auto value = values.find(std::string(name));
-	if (value == values.end())
+	if (value == values.end()) { return std::nullopt; }
-	{
-		return std::nullopt;
-	}
 	return value->second;
 }
 
+std::string EnvKvStore::get(
+	std::string_view name,
+	bool bypassProcessEnvironment) const
+{
+	std::optional<std::string> value = find(name, bypassProcessEnvironment);
+	if (!value.has_value())
+	{
+		throw std::runtime_error(
+			std::string("EnvKvStore: missing key '")
+			+ std::string(name)
+			+ "'");
+	}
+	return *value;
+}
+
+int EnvKvStore::parseInt(std::string_view name, const std::string &raw)
+{
+	try
+	{
+		std::size_t consumed = 0;
+		const long parsed = std::stol(raw, &consumed);
+		if (consumed != raw.size())
+		{
+			throw std::runtime_error(
+				std::string("EnvKvStore: '")
+				+ std::string(name)
+				+ "' must be an integer, got: "
+				+ raw);
+		}
+		if (parsed < std::numeric_limits<int>::min()
+			|| parsed > std::numeric_limits<int>::max())
+		{
+			throw std::runtime_error(
+				std::string("EnvKvStore: '")
+				+ std::string(name)
+				+ "' is out of int range, got: "
+				+ raw);
+		}
+		return static_cast<int>(parsed);
+	}
+	catch (const std::runtime_error &)
+	{
+		throw;
+	}
+	catch (const std::exception &)
+	{
+		throw std::runtime_error(
+			std::string("EnvKvStore: failed to parse '")
+			+ std::string(name)
+			+ "' as an integer, got: "
+			+ raw);
+	}
+}
+
+int EnvKvStore::getIntWithConstraint(
+	std::string_view name,
+	std::optional<int> defaultValue,
+	IntConstraint constraint) const
+{
+	const std::optional<std::string> raw = find(name);
+	if (!raw.has_value())
+	{
+		if (!defaultValue.has_value())
+		{
+			throw std::runtime_error(
+				std::string("EnvKvStore: missing key '")
+				+ std::string(name)
+				+ "'");
+		}
+		return *defaultValue;
+	}
+
+	const int parsed = parseInt(name, *raw);
+	if (constraint == IntConstraint::NonNegative && parsed < 0)
+	{
+		throw std::runtime_error(
+			std::string("EnvKvStore: '")
+			+ std::string(name)
+			+ "' must be a non-negative integer, got: "
+			+ *raw);
+	}
+	if (constraint == IntConstraint::PositiveNonZero && parsed <= 0)
+	{
+		throw std::runtime_error(
+			std::string("EnvKvStore: '")
+			+ std::string(name)
+			+ "' must be a positive non-zero integer, got: "
+			+ *raw);
+	}
+	return parsed;
+}
+
 void EnvKvStore::loadFile(
 	const std::filesystem::path &envFilePath,
 	std::ostream &warningStream)
@@ -249,11 +331,9 @@ void EnvKvStore::loadFile(
 	while (std::getline(file, line))
 	{
 		++lineNumber;
-		if (lineIsBlankOrComment(line))
+		if (DotenvParser::lineIsBlankOrComment(line)) { continue; }
-		{
+		auto [name, value] =
-			continue;
+			DotenvParser::parseAssignment(envFilePath, lineNumber, line);
-		}
-		auto [name, value] = parseAssignment(envFilePath, lineNumber, line);
 		storeValue(envFilePath, name, value, warningStream);
 	}
 }

tests/env_kv_store_test.cpp

@@ -1,8 +1,11 @@
+#include <algorithm>
 #include <cstdlib>
 #include <chrono>
 #include <filesystem>
 #include <fstream>
+#include <optional>
 #include <sstream>
+#include <stdexcept>
 #include <string>
 #include <vector>
 
@@ -12,6 +15,16 @@
 
 namespace {
 
+constexpr const char *kTestEnvName = "SSCL_ENV_TEST_VALUE";
+constexpr const char *kPositiveIntMsEnvName = "SSCL_POSITIVE_INT_MS";
+constexpr int kPositiveIntMsDefault = 33;
+
+void unsetTestEnvVars()
+{
+	unsetenv(kTestEnvName);
+	unsetenv(kPositiveIntMsEnvName);
+}
+
 class EnvKvStoreTest
 :	public testing::Test
 {
@@ -24,12 +37,12 @@ protected:
 					.time_since_epoch().count())
 				+ "-" + std::to_string(testCounter++));
 		std::filesystem::create_directories(root);
-		unsetenv("SSCL_ENV_TEST_VALUE");
+		unsetTestEnvVars();
 	}
 
 	void TearDown() override
 	{
-		unsetenv("SSCL_ENV_TEST_VALUE");
+		unsetTestEnvVars();
 		std::filesystem::remove_all(root);
 	}
 
@@ -43,6 +56,26 @@ protected:
 		return path;
 	}
 
+	void expectParseErrorContaining(
+		const std::filesystem::path &envFile,
+		const std::string &expectedFragment)
+	{
+		std::ostringstream warnings;
+		try
+		{
+			sscl::EnvKvStore store({envFile}, warnings);
+			FAIL() << "Expected parse of " << envFile << " to throw.";
+		}
+		catch (const std::runtime_error &e)
+		{
+			std::string message = e.what();
+			EXPECT_NE(message.find(envFile.string()), std::string::npos)
+				<< message;
+			EXPECT_NE(message.find(expectedFragment), std::string::npos)
+				<< message;
+		}
+	}
+
 	std::filesystem::path root;
 	static inline int testCounter = 0;
 };
@@ -65,69 +98,196 @@ TEST_F(EnvKvStoreTest, ParsesSupportedDotenvForms)
 	std::ostringstream warnings;
 	sscl::EnvKvStore store({envFile}, warnings);
 
-	EXPECT_EQ(store.get("PLAIN"), "value");
+	EXPECT_EQ(store.find("PLAIN"), "value");
-	EXPECT_EQ(store.get("TRIMMED"), "value with spaces");
+	EXPECT_EQ(store.find("TRIMMED"), "value with spaces");
-	EXPECT_EQ(store.get("SINGLE"), " preserved value ");
+	EXPECT_EQ(store.find("SINGLE"), " preserved value ");
-	EXPECT_EQ(store.get("DOUBLE"), "another preserved value");
+	EXPECT_EQ(store.find("DOUBLE"), "another preserved value");
-	EXPECT_EQ(store.get("ESCAPED"), "quote: \" slash: \\ tab: \t");
+	EXPECT_EQ(store.find("ESCAPED"), "quote: \" slash: \\ tab: \t");
-	EXPECT_EQ(store.get("COMMENTED"), "value");
+	EXPECT_EQ(store.find("COMMENTED"), "value");
 	EXPECT_TRUE(warnings.str().empty());
 }
 
-TEST_F(EnvKvStoreTest, LaterFilesOverwriteEarlierFilesAndWarn)
+TEST_F(EnvKvStoreTest, EmptyPathListYieldsEmptyStore)
 {
-	std::filesystem::path first = writeFile("first.env", "VALUE=first\n");
-	std::filesystem::path second = writeFile("second.env", "VALUE=second\n");
-
 	std::ostringstream warnings;
-	sscl::EnvKvStore store({first, second}, warnings);
+	sscl::EnvKvStore store({}, warnings);
 
-	EXPECT_EQ(store.get("VALUE"), "second");
+	EXPECT_EQ(store.find("ANY"), std::nullopt);
-	EXPECT_NE(warnings.str().find("VALUE"), std::string::npos);
+	EXPECT_TRUE(warnings.str().empty());
-	EXPECT_NE(warnings.str().find("first"), std::string::npos);
-	EXPECT_NE(warnings.str().find("second"), std::string::npos);
-	EXPECT_NE(warnings.str().find(second.string()), std::string::npos);
 }
 
-TEST_F(EnvKvStoreTest, DuplicateKeysInsideSameFileOverwriteAndWarn)
+TEST_F(EnvKvStoreTest, EmptyFileYieldsEmptyStore)
 {
-	std::filesystem::path envFile =
+	std::filesystem::path envFile = writeFile("empty.env", "");
-		writeFile("one.env", "VALUE=first\nVALUE=second\n");
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("ANY"), std::nullopt);
+	EXPECT_TRUE(warnings.str().empty());
+}
+
+TEST_F(EnvKvStoreTest, CommentOnlyAndWhitespaceOnlyLinesAreIgnored)
+{
+	std::filesystem::path envFile = writeFile(
+		"comments.env",
+		"   \n"
+		"\t\n"
+		"# only comment\n"
+		"  # indented comment\n"
+		"KEEP=yes\n"
+		"\n");
 
 	std::ostringstream warnings;
 	sscl::EnvKvStore store({envFile}, warnings);
 
-	EXPECT_EQ(store.get("VALUE"), "second");
+	EXPECT_EQ(store.find("KEEP"), "yes");
-	EXPECT_NE(warnings.str().find("VALUE"), std::string::npos);
+	EXPECT_TRUE(warnings.str().empty());
-	EXPECT_NE(warnings.str().find("first"), std::string::npos);
-	EXPECT_NE(warnings.str().find("second"), std::string::npos);
-	EXPECT_NE(warnings.str().find(envFile.string()), std::string::npos);
 }
 
-TEST_F(EnvKvStoreTest, ProcessEnvironmentOverridesStoreSilently)
+TEST_F(EnvKvStoreTest, EmptyUnquotedValueIsAccepted)
 {
-	std::filesystem::path envFile =
+	std::filesystem::path envFile = writeFile("empty-value.env", "EMPTY=\n");
-		writeFile("one.env", "SSCL_ENV_TEST_VALUE=file\n");
+	std::ostringstream warnings;
-	setenv("SSCL_ENV_TEST_VALUE", "process", 1);
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("EMPTY"), "");
+}
+
+TEST_F(EnvKvStoreTest, UnquotedHashStartsInlineComment)
+{
+	std::filesystem::path envFile = writeFile(
+		"hash.env",
+		"A=before#after\n"
+		"B= # leading comment after equals\n");
 
 	std::ostringstream warnings;
 	sscl::EnvKvStore store({envFile}, warnings);
 
-	EXPECT_EQ(store.get("SSCL_ENV_TEST_VALUE"), "process");
+	EXPECT_EQ(store.find("A"), "before");
-	EXPECT_TRUE(warnings.str().empty());
+	EXPECT_EQ(store.find("B"), "");
 }
 
-TEST_F(EnvKvStoreTest, EmptyProcessEnvironmentValueOverridesStoreSilently)
+TEST_F(EnvKvStoreTest, HashInsideQuotesIsLiteral)
 {
-	std::filesystem::path envFile =
+	std::filesystem::path envFile = writeFile(
-		writeFile("one.env", "SSCL_ENV_TEST_VALUE=file\n");
+		"hash-quoted.env",
-	setenv("SSCL_ENV_TEST_VALUE", "", 1);
+		"SINGLE='#not-comment'\n"
+		"DOUBLE=\"#not-comment\"\n"
+		"DOUBLE_TRAIL=\"kept\" # trailing comment ok\n");
 
 	std::ostringstream warnings;
 	sscl::EnvKvStore store({envFile}, warnings);
 
-	EXPECT_EQ(store.get("SSCL_ENV_TEST_VALUE"), "");
+	EXPECT_EQ(store.find("SINGLE"), "#not-comment");
-	EXPECT_TRUE(warnings.str().empty());
+	EXPECT_EQ(store.find("DOUBLE"), "#not-comment");
+	EXPECT_EQ(store.find("DOUBLE_TRAIL"), "kept");
+}
+
+TEST_F(EnvKvStoreTest, DoubleQuotedEscapeSequences)
+{
+	std::filesystem::path envFile = writeFile(
+		"escapes.env",
+		"NL=\"line\\nbreak\"\n"
+		"CR=\"ret\\rurn\"\n"
+		"TAB=\"a\\tb\"\n"
+		"UNKNOWN=\"\\q\"\n"
+		"TRAILING=\"end\\\\\"\n"
+		"ESCAPED_QUOTE_MID=\"a\\\"b\"\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("NL"), "line\nbreak");
+	EXPECT_EQ(store.find("CR"), "ret\rurn");
+	EXPECT_EQ(store.find("TAB"), "a\tb");
+	EXPECT_EQ(store.find("UNKNOWN"), "q");
+	EXPECT_EQ(store.find("TRAILING"), "end\\");
+	EXPECT_EQ(store.find("ESCAPED_QUOTE_MID"), "a\"b");
+}
+
+TEST_F(EnvKvStoreTest, SingleQuotedValuesDoNotDecodeEscapes)
+{
+	std::filesystem::path envFile = writeFile(
+		"single-escapes.env",
+		"LITERAL='\\n\\t\\\"'\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("LITERAL"), "\\n\\t\\\"");
+}
+
+TEST_F(EnvKvStoreTest, ValidNamesAcceptUnderscoreAndAlnum)
+{
+	std::filesystem::path envFile = writeFile(
+		"names.env",
+		"_LEADING=1\n"
+		"A1B2=2\n"
+		"mixed_Case99=3\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("_LEADING"), "1");
+	EXPECT_EQ(store.find("A1B2"), "2");
+	EXPECT_EQ(store.find("mixed_Case99"), "3");
+}
+
+TEST_F(EnvKvStoreTest, InvalidNamesThrow)
+{
+	expectParseErrorContaining(
+		writeFile("digit.env", "1BAD=x\n"),
+		"Invalid variable name.");
+	expectParseErrorContaining(
+		writeFile("hyphen.env", "BAD-NAME=x\n"),
+		"Invalid variable name.");
+	expectParseErrorContaining(
+		writeFile("dot.env", "BAD.NAME=x\n"),
+		"Invalid variable name.");
+	expectParseErrorContaining(
+		writeFile("empty-name.env", "=value\n"),
+		"Invalid variable name.");
+}
+
+TEST_F(EnvKvStoreTest, UnterminatedQuotedValueThrows)
+{
+	expectParseErrorContaining(
+		writeFile("unterminated-double.env", "X=\"no close\n"),
+		"Unterminated quoted value.");
+	expectParseErrorContaining(
+		writeFile("unterminated-single.env", "X='no close\n"),
+		"Unterminated quoted value.");
+}
+
+TEST_F(EnvKvStoreTest, UnexpectedTextAfterQuotedValueThrows)
+{
+	expectParseErrorContaining(
+		writeFile("trail.env", "X=\"ok\" trailing\n"),
+		"Unexpected text after quoted value.");
+}
+
+TEST_F(EnvKvStoreTest, MalformedLineThrowsWithLineNumber)
+{
+	std::filesystem::path envFile = writeFile(
+		"bad.env",
+		"# header\n"
+		"GOOD=1\n"
+		"NOT AN ASSIGNMENT\n");
+
+	std::ostringstream warnings;
+	try
+	{
+		sscl::EnvKvStore store({envFile}, warnings);
+		FAIL() << "Expected malformed env file to throw.";
+	}
+	catch (const std::runtime_error &e)
+	{
+		std::string message = e.what();
+		EXPECT_NE(message.find(envFile.string()), std::string::npos);
+		EXPECT_NE(message.find(":3:"), std::string::npos) << message;
+		EXPECT_NE(message.find("Expected KEY=value."), std::string::npos)
+			<< message;
+	}
 }
 
 TEST_F(EnvKvStoreTest, MissingFileThrows)
@@ -138,20 +298,320 @@ TEST_F(EnvKvStoreTest, MissingFileThrows)
 		std::runtime_error);
 }
 
-TEST_F(EnvKvStoreTest, MalformedLineThrows)
+TEST_F(EnvKvStoreTest, UnreadableFileThrowsOpenFailure)
 {
-	std::filesystem::path envFile = writeFile("bad.env", "NOT AN ASSIGNMENT\n");
+	std::filesystem::path envFile = writeFile("noread.env", "X=1\n");
-	std::ostringstream warnings;
+	std::filesystem::permissions(envFile, std::filesystem::perms::none);
 
+	std::ostringstream warnings;
 	try
 	{
 		sscl::EnvKvStore store({envFile}, warnings);
-		FAIL() << "Expected malformed env file to throw.";
+		FAIL() << "Expected unreadable file open to throw.";
 	}
 	catch (const std::runtime_error &e)
 	{
-		std::string message = e.what();
+		EXPECT_NE(
-		EXPECT_NE(message.find(envFile.string()), std::string::npos);
+			std::string(e.what()).find("Failed to open env file:"),
-		EXPECT_NE(message.find(":1:"), std::string::npos);
+			std::string::npos);
+	}
+
+	std::filesystem::permissions(
+		envFile,
+		std::filesystem::perms::owner_read
+			| std::filesystem::perms::owner_write);
+}
+
+TEST_F(EnvKvStoreTest, LaterFilesOverwriteEarlierFilesAndWarn)
+{
+	std::filesystem::path first = writeFile("first.env", "VALUE=first\n");
+	std::filesystem::path second = writeFile("second.env", "VALUE=second\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({first, second}, warnings);
+
+	EXPECT_EQ(store.find("VALUE"), "second");
+	EXPECT_NE(warnings.str().find("VALUE"), std::string::npos);
+	EXPECT_NE(warnings.str().find("first"), std::string::npos);
+	EXPECT_NE(warnings.str().find("second"), std::string::npos);
+	EXPECT_NE(warnings.str().find(second.string()), std::string::npos);
+}
+
+TEST_F(EnvKvStoreTest, ThreeFileOverwriteKeepsLastValue)
+{
+	std::filesystem::path a = writeFile("a.env", "K=a\nSHARED=1\n");
+	std::filesystem::path b = writeFile("b.env", "SHARED=2\n");
+	std::filesystem::path c = writeFile("c.env", "SHARED=3\nK=c\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({a, b, c}, warnings);
+
+	EXPECT_EQ(store.find("K"), "c");
+	EXPECT_EQ(store.find("SHARED"), "3");
+	const std::string warningText = warnings.str();
+	EXPECT_GE(
+		std::count(warningText.begin(), warningText.end(), '\n'),
+		2);
+}
+
+TEST_F(EnvKvStoreTest, DuplicateKeysInsideSameFileOverwriteAndWarn)
+{
+	std::filesystem::path envFile =
+		writeFile("one.env", "VALUE=first\nVALUE=second\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("VALUE"), "second");
+	EXPECT_NE(warnings.str().find("VALUE"), std::string::npos);
+	EXPECT_NE(warnings.str().find("first"), std::string::npos);
+	EXPECT_NE(warnings.str().find("second"), std::string::npos);
+	EXPECT_NE(warnings.str().find(envFile.string()), std::string::npos);
+}
+
+TEST_F(EnvKvStoreTest, ProcessEnvironmentOverridesStoreSilently)
+{
+	std::filesystem::path envFile =
+		writeFile("one.env", "SSCL_ENV_TEST_VALUE=file\n");
+	setenv(kTestEnvName, "process", 1);
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find(kTestEnvName), "process");
+	EXPECT_TRUE(warnings.str().empty());
+}
+
+TEST_F(EnvKvStoreTest, EmptyProcessEnvironmentValueOverridesStoreSilently)
+{
+	std::filesystem::path envFile =
+		writeFile("one.env", "SSCL_ENV_TEST_VALUE=file\n");
+	setenv(kTestEnvName, "", 1);
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find(kTestEnvName), "");
+	EXPECT_TRUE(warnings.str().empty());
+}
+
+TEST_F(EnvKvStoreTest, ProcessEnvironmentReturnedWhenKeyAbsentFromStore)
+{
+	setenv(kTestEnvName, "only-process", 1);
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({}, warnings);
+
+	EXPECT_EQ(store.find(kTestEnvName), "only-process");
+}
+
+TEST_F(EnvKvStoreTest, BypassProcessEnvironmentUsesFileStoreOnly)
+{
+	std::filesystem::path envFile =
+		writeFile("one.env", "SSCL_ENV_TEST_VALUE=file\n");
+	setenv(kTestEnvName, "process", 1);
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find(kTestEnvName, true), "file");
+	EXPECT_EQ(store.find(kTestEnvName, false), "process");
+}
+
+TEST_F(EnvKvStoreTest, BypassIgnoresProcessEnvWhenKeyOnlyInProcess)
+{
+	setenv(kTestEnvName, "process-only", 1);
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({}, warnings);
+
+	EXPECT_EQ(store.find(kTestEnvName, true), std::nullopt);
+	EXPECT_EQ(store.find(kTestEnvName, false), "process-only");
+}
+
+TEST_F(EnvKvStoreTest, MissingKeyReturnsNullopt)
+{
+	std::filesystem::path envFile = writeFile("one.env", "PRESENT=1\n");
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(store.find("PRESENT"), "1");
+	EXPECT_EQ(store.find("ABSENT"), std::nullopt);
+	EXPECT_EQ(store.find("ABSENT", true), std::nullopt);
+}
+
+TEST_F(EnvKvStoreTest, DefaultWarningCtorLoadsWithoutThrowing)
+{
+	std::filesystem::path envFile = writeFile("one.env", "OK=1\n");
+	sscl::EnvKvStore store({envFile});
+	EXPECT_EQ(store.find("OK"), "1");
+}
+
+TEST_F(EnvKvStoreTest, GetThrowsWhenKeyMissing)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({}, warnings);
+	EXPECT_THROW(store.get("ABSENT"), std::runtime_error);
+}
+
+TEST_F(EnvKvStoreTest, GetReturnsPresentValue)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store(
+		{writeFile("one.env", "PRESENT=1\n")},
+		warnings);
+	EXPECT_EQ(store.get("PRESENT"), "1");
+}
+
+TEST_F(EnvKvStoreTest, GetIntUsesDefaultWhenUnset)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({}, warnings);
+
+	EXPECT_EQ(store.getInt(kPositiveIntMsEnvName, kPositiveIntMsDefault), kPositiveIntMsDefault);
+	EXPECT_EQ(store.getInt("CUSTOM", 42), 42);
+	EXPECT_THROW(store.getInt("MISSING"), std::runtime_error);
+}
+
+TEST_F(EnvKvStoreTest, GetIntParsesSignedValues)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store(
+		{writeFile("one.env", "NEG=-7\nZERO=0\n")},
+		warnings);
+	EXPECT_EQ(store.getInt("NEG"), -7);
+	EXPECT_EQ(store.getInt("ZERO"), 0);
+	EXPECT_EQ(store.getPositiveInt("ZERO"), 0);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntUsesDefaultWhenUnset)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({}, warnings);
+
+	EXPECT_EQ(
+		store.getPositiveNonZeroInt(kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		kPositiveIntMsDefault);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntParsesFileValue)
+{
+	std::filesystem::path envFile = writeFile(
+		"one.env",
+		std::string(kPositiveIntMsEnvName) + "=50\n");
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(
+		store.getPositiveNonZeroInt(kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		50);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntHonorsProcessEnvironmentOverFile)
+{
+	std::filesystem::path envFile = writeFile(
+		"one.env",
+		std::string(kPositiveIntMsEnvName) + "=50\n");
+	setenv(kPositiveIntMsEnvName, "77", 1);
+
+	std::ostringstream warnings;
+	sscl::EnvKvStore store({envFile}, warnings);
+
+	EXPECT_EQ(
+		store.getPositiveNonZeroInt(kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		77);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntRejectsNonPositive)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore zeroStore(
+		{writeFile(
+			"zero.env",
+			std::string(kPositiveIntMsEnvName) + "=0\n")},
+		warnings);
+	EXPECT_THROW(
+		zeroStore.getPositiveNonZeroInt(
+			kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		std::runtime_error);
+
+	sscl::EnvKvStore negativeStore(
+		{writeFile(
+			"neg.env",
+			std::string(kPositiveIntMsEnvName) + "=-3\n")},
+		warnings);
+	EXPECT_THROW(
+		negativeStore.getPositiveNonZeroInt(
+			kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		std::runtime_error);
+	EXPECT_THROW(
+		negativeStore.getPositiveInt(
+			kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		std::runtime_error);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntRejectsNonNumericAndTrailingJunk)
+{
+	std::ostringstream warnings;
+
+	sscl::EnvKvStore letters(
+		{writeFile(
+			"letters.env",
+			std::string(kPositiveIntMsEnvName) + "=abc\n")},
+		warnings);
+	try
+	{
+		(void)letters.getPositiveNonZeroInt(
+			kPositiveIntMsEnvName, kPositiveIntMsDefault);
+		FAIL() << "Expected non-numeric parse to throw.";
+	}
+	catch (const std::runtime_error &e)
+	{
+		EXPECT_NE(
+			std::string(e.what()).find("failed to parse"),
+			std::string::npos)
+			<< e.what();
+	}
+
+	sscl::EnvKvStore trailing(
+		{writeFile(
+			"trailing.env",
+			std::string(kPositiveIntMsEnvName) + "=50ms\n")},
+		warnings);
+	try
+	{
+		(void)trailing.getPositiveNonZeroInt(
+			kPositiveIntMsEnvName, kPositiveIntMsDefault);
+		FAIL() << "Expected trailing junk to throw.";
+	}
+	catch (const std::runtime_error &e)
+	{
+		EXPECT_NE(
+			std::string(e.what()).find("must be an integer"),
+			std::string::npos)
+			<< e.what();
 	}
 }
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntRejectsEmptyStringValue)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store(
+		{writeFile(
+			"empty.env",
+			std::string(kPositiveIntMsEnvName) + "=\n")},
+		warnings);
+
+	EXPECT_THROW(
+		store.getPositiveNonZeroInt(kPositiveIntMsEnvName, kPositiveIntMsDefault),
+		std::runtime_error);
+}
+
+TEST_F(EnvKvStoreTest, GetPositiveNonZeroIntAcceptsOne)
+{
+	std::ostringstream warnings;
+	sscl::EnvKvStore store(
+		{writeFile("one.env", "CUSTOM=1\n")},
+		warnings);
+
+	EXPECT_EQ(store.getPositiveNonZeroInt("CUSTOM", 99), 1);
+}